-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: Update to support OSS Index Authentication Requirements #7920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+97
−16
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
5ac11f3
Add information on how to get an API token for OSS Index
framayo b500daa
Disable OSS Index by default
framayo d62a5e2
If OssIndexAnalyzer is enabled, check if credentials are set before a…
framayo fd1719c
Merge branch 'main' of https://github.com/dependency-check/Dependency…
framayo a54f779
fix: improve error message
jeremylong bec7d45
fix: enable the ossindex analyzer
jeremylong 119425b
fix: update documentation
jeremylong File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,14 +8,19 @@ | |
import org.owasp.dependencycheck.dependency.Dependency; | ||
import org.owasp.dependencycheck.dependency.naming.Identifier; | ||
import org.owasp.dependencycheck.dependency.naming.PurlIdentifier; | ||
import org.owasp.dependencycheck.exception.InitializationException; | ||
import org.owasp.dependencycheck.utils.Settings; | ||
import org.owasp.dependencycheck.utils.Settings.KEYS; | ||
|
||
import org.sonatype.goodies.packageurl.PackageUrl; | ||
import org.sonatype.ossindex.service.api.componentreport.ComponentReport; | ||
import org.sonatype.ossindex.service.client.OssindexClient; | ||
import org.sonatype.ossindex.service.client.transport.Transport; | ||
|
||
import java.net.SocketTimeoutException; | ||
import java.net.URI; | ||
import java.util.Collections; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.concurrent.ExecutionException; | ||
|
@@ -25,6 +30,7 @@ | |
|
||
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; | ||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertThrows; | ||
import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
||
class OssIndexAnalyzerTest extends BaseTest { | ||
|
@@ -42,10 +48,12 @@ void should_enrich_be_included_in_mutex_to_prevent_NPE() | |
Dependency dependency = new Dependency(); | ||
dependency.addSoftwareIdentifier(identifier); | ||
Settings settings = getSettings(); | ||
setCredentials(settings); | ||
Engine engine = new Engine(settings); | ||
engine.setDependencies(Collections.singletonList(dependency)); | ||
|
||
analyzer.initialize(settings); | ||
analyzer.prepareAnalyzer(engine); | ||
|
||
String expectedOutput = "https://ossindex.sonatype.org/component/pkg:maven/test/[email protected]"; | ||
|
||
|
@@ -75,6 +83,11 @@ void should_enrich_be_included_in_mutex_to_prevent_NPE() | |
*/ | ||
static final class SproutOssIndexAnalyzer extends OssIndexAnalyzer { | ||
private Future<?> pendingClosureTask; | ||
@Override | ||
OssindexClient newOssIndexClient() { | ||
return new OssIndexClientOk(); | ||
} | ||
|
||
@Override | ||
void enrich(Dependency dependency) { | ||
ExecutorService executor = Executors.newSingleThreadExecutor(); | ||
|
@@ -93,19 +106,46 @@ void awaitPendingClosure() throws ExecutionException, InterruptedException { | |
} | ||
} | ||
|
||
private static final class OssIndexClientOk implements OssindexClient { | ||
|
||
@Override | ||
public Map<PackageUrl, ComponentReport> requestComponentReports(List<PackageUrl> coordinates) throws Exception { | ||
HashMap<PackageUrl, ComponentReport> reports = new HashMap<>(); | ||
ComponentReport report = new ComponentReport(); | ||
PackageUrl packageUrl = coordinates.get(0); | ||
report.setCoordinates(packageUrl); | ||
report.setReference(new URI("https://ossindex.sonatype.org/component/pkg:maven/test/[email protected]?utm_source=dependency-check&utm_medium=integration&utm_content=12.1.4-SNAPSHOT")); | ||
reports.put(packageUrl, report); | ||
return reports; | ||
} | ||
|
||
@Override | ||
public ComponentReport requestComponentReport(PackageUrl coordinates) throws Exception { | ||
return new ComponentReport(); | ||
} | ||
|
||
@Override | ||
public void close() { | ||
|
||
} | ||
} | ||
|
||
@Test | ||
void should_analyzeDependency_return_a_dedicated_error_message_when_403_response_from_sonatype() throws Exception { | ||
// Given | ||
OssIndexAnalyzer analyzer = new OssIndexAnalyzerThrowing403(); | ||
analyzer.initialize(getSettings()); | ||
Settings settings = getSettings(); | ||
setCredentials(settings); | ||
Engine engine = new Engine(settings); | ||
|
||
analyzer.initialize(settings); | ||
analyzer.prepareAnalyzer(engine); | ||
|
||
Identifier identifier = new PurlIdentifier("maven", "test", "test", "1.0", | ||
Confidence.HIGHEST); | ||
|
||
Dependency dependency = new Dependency(); | ||
dependency.addSoftwareIdentifier(identifier); | ||
Settings settings = getSettings(); | ||
Engine engine = new Engine(settings); | ||
engine.setDependencies(Collections.singletonList(dependency)); | ||
|
||
// When | ||
|
@@ -126,17 +166,19 @@ void should_analyzeDependency_return_a_dedicated_error_message_when_403_response | |
void should_analyzeDependency_only_warn_when_transport_error_from_sonatype() throws Exception { | ||
// Given | ||
OssIndexAnalyzer analyzer = new OssIndexAnalyzerThrowing502(); | ||
Settings settings = getSettings(); | ||
setCredentials(settings); | ||
settings.setBoolean(Settings.KEYS.ANALYZER_OSSINDEX_WARN_ONLY_ON_REMOTE_ERRORS, true); | ||
Engine engine = new Engine(settings); | ||
|
||
getSettings().setBoolean(Settings.KEYS.ANALYZER_OSSINDEX_WARN_ONLY_ON_REMOTE_ERRORS, true); | ||
analyzer.initialize(getSettings()); | ||
analyzer.initialize(settings); | ||
analyzer.prepareAnalyzer(engine); | ||
|
||
Identifier identifier = new PurlIdentifier("maven", "test", "test", "1.0", | ||
Confidence.HIGHEST); | ||
|
||
Dependency dependency = new Dependency(); | ||
dependency.addSoftwareIdentifier(identifier); | ||
Settings settings = getSettings(); | ||
Engine engine = new Engine(settings); | ||
|
||
// When | ||
try (engine) { | ||
|
@@ -148,22 +190,23 @@ void should_analyzeDependency_only_warn_when_transport_error_from_sonatype() thr | |
} | ||
} | ||
|
||
|
||
@Test | ||
void should_analyzeDependency_only_warn_when_socket_error_from_sonatype() throws Exception { | ||
// Given | ||
OssIndexAnalyzer analyzer = new OssIndexAnalyzerThrowingSocketTimeout(); | ||
Settings settings = getSettings(); | ||
setCredentials(settings); | ||
settings.setBoolean(Settings.KEYS.ANALYZER_OSSINDEX_WARN_ONLY_ON_REMOTE_ERRORS, true); | ||
analyzer.initialize(settings); | ||
|
||
getSettings().setBoolean(Settings.KEYS.ANALYZER_OSSINDEX_WARN_ONLY_ON_REMOTE_ERRORS, true); | ||
analyzer.initialize(getSettings()); | ||
Engine engine = new Engine(settings); | ||
analyzer.prepareAnalyzer(engine); | ||
|
||
Identifier identifier = new PurlIdentifier("maven", "test", "test", "1.0", | ||
Confidence.HIGHEST); | ||
|
||
Dependency dependency = new Dependency(); | ||
dependency.addSoftwareIdentifier(identifier); | ||
Settings settings = getSettings(); | ||
Engine engine = new Engine(settings); | ||
|
||
// When | ||
try (engine) { | ||
|
@@ -180,17 +223,19 @@ void should_analyzeDependency_only_warn_when_socket_error_from_sonatype() throws | |
void should_analyzeDependency_fail_when_socket_error_from_sonatype() throws Exception { | ||
// Given | ||
OssIndexAnalyzer analyzer = new OssIndexAnalyzerThrowingSocketTimeout(); | ||
Settings settings = getSettings(); | ||
setCredentials(settings); | ||
settings.setBoolean(Settings.KEYS.ANALYZER_OSSINDEX_WARN_ONLY_ON_REMOTE_ERRORS, false); | ||
Engine engine = new Engine(settings); | ||
|
||
getSettings().setBoolean(Settings.KEYS.ANALYZER_OSSINDEX_WARN_ONLY_ON_REMOTE_ERRORS, false); | ||
analyzer.initialize(getSettings()); | ||
analyzer.initialize(settings); | ||
analyzer.prepareAnalyzer(engine); | ||
|
||
Identifier identifier = new PurlIdentifier("maven", "test", "test", "1.0", | ||
Confidence.HIGHEST); | ||
|
||
Dependency dependency = new Dependency(); | ||
dependency.addSoftwareIdentifier(identifier); | ||
Settings settings = getSettings(); | ||
Engine engine = new Engine(settings); | ||
engine.setDependencies(Collections.singletonList(dependency)); | ||
|
||
// When | ||
|
@@ -206,7 +251,25 @@ void should_analyzeDependency_fail_when_socket_error_from_sonatype() throws Exce | |
analyzer.close(); | ||
} | ||
|
||
@Test | ||
void should_prepareAnalyzer_fail_when_credentials_not_set() throws Exception { | ||
OssIndexAnalyzer analyzer = new OssIndexAnalyzer(); | ||
Settings settings = getSettings(); | ||
Engine engine = new Engine(settings); | ||
analyzer.initialize(settings); | ||
try { | ||
analyzer.prepareAnalyzer(engine); | ||
assertThrows(InitializationException.class, () -> analyzer.prepareAnalyzer(engine)); | ||
} catch (InitializationException e) { | ||
analyzer.close(); | ||
engine.close(); | ||
} | ||
} | ||
|
||
private static void setCredentials(final Settings settings) { | ||
settings.setString(KEYS.ANALYZER_OSSINDEX_USER, "user"); | ||
settings.setString(KEYS.ANALYZER_OSSINDEX_PASSWORD, "pass"); | ||
} | ||
|
||
static final class OssIndexAnalyzerThrowing403 extends OssIndexAnalyzer { | ||
@Override | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you intentionally ignore the
ossIndexServerId
property here? Or is this method only called after user/pw were already resolved from thesettings.xml
(via server ID)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only called after this is configured in the mojo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and in hindsight - this probably should have been a breaking change as most users will now get an exception...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't they have gotten an exception anyway? Either from ODC or from Sonatype?
I think this would have been the best course of action (accompanied by a warning in the log).
I didn't consider this very community friendly; received the email on the 16th...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I have enough time I was going to put more work into this tomorrow and possibly one more quick release. Disable by default, enabling by either providing creds or setting enabled=true (and flipping the CLI's disable to enableOssIndex). Just not sure if I will have time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To disable the analyzer by default would be great. I have a lot failed builds on our CI server now. I don't want to update the configuration for all of them. Or is it possible to to that by an environment variable?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'd have to agree with @AndreVirtimo here. Disabling the analyzer if the credentials are empty seems like the way to go. If users want to use OSS Index to analyze their dependencies, they setup an account, otherwise the analyzer is skipped.
We'll wait for your release @jeremylong, so we can update
sbt-dependecy-check
. Please let us know if we can help somehow.Edit: Feel free to take a look at #7963