Skip to content
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

feat: add hostedSuppressionsAuthHeader option to provide credentials for the hosted suppression file #7268

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

ftiercelin
Copy link
Contributor

Description of Change

add hostedSuppressionsAuthHeader option to provide credentials for the hosted suppression file in the form of the auth header. e.g. 'Basic dXNlcjpwYXNzd29yZA==' or 'Bearer xxxxxxxxxxxxxxxxxxxxxxxx'

Related issues

Have test cases been added to cover the new functionality?

yes: see org.owasp.dependencycheck.utils.CredentialHelperTest

@boring-cyborg boring-cyborg bot added ant changes to ant cli changes to the cli core changes to core documentation site documentation maven changes to the maven plugin tests test cases utils changes to utils labels Dec 23, 2024
@ftiercelin
Copy link
Contributor Author

Testing with a server requiring either no authentication, a basic or a bearer auth:
mvn dependency-check:check
no access to server, going to default location

mvn dependency-check:check -DhostedSuppressionsUrl=http://<ip>:8080/bearer/suppressions.xml -DhostedSuppressionsAuthHeader="Bearer THE_TOKEN_PLACEHOLDER"
server logs:

# INFO - HTTP-401 UNAUTHORIZED
# INFO - authorization: 'Bearer THE_TOKEN_PLACEHOLDER'
# INFO - HTTP-200 OK
  • plugin execution success

mvn dependency-check:check -DhostedSuppressionsUrl=http://<ip>:8080/unauthenticated/suppressions.xml
server logs:

# INFO - HTTP-200 OK
  • plugin execution success

mvn dependency-check:check -DhostedSuppressionsUrl=http://<ip>:8080/basic/suppressions.xml -DhostedSuppressionsAuthHeader="Basic dXNlcjpwYXNzd29yZA=="
server logs:

# INFO - HTTP-401 UNAUTHORIZED
# INFO - authorization: 'Basic dXNlcjpwYXNzd29yZA=='
# INFO - HTTP-200 OK
  • plugin execution success

mvn dependency-check:check -DhostedSuppressionsUrl=http://<ip>:8080/basic/suppressions.xml -DhostedSuppressionsAuthHeader="Something else"

  • plugin execution failure Invalid configuration for Hosted suppressions: unknown authentication scheme. Supported authentication schemes are Basic and Bearer

@jeremylong jeremylong requested a review from aikebah December 23, 2024 12:07
@aikebah
Copy link
Collaborator

aikebah commented Dec 23, 2024

@ftiercelin Looks like our efforts overlapped. I've implemented authentication for the hosted suppressions in the context of #5387 and never spotted #5783 otherwise I would've linked that as well.

Configuration is currently possible only via dependency-check properties file (since the merge of #6949), but addition of user/password configuration options were foreseen for the various integrations in the near future (I had started work on updating (and meanwhile sanitizing) the Ant tasks to add in the missing authentication settings).

Do you see a strong need for adding Bearer authentications in the mix?

Currently a bit short in time, but hope to do a more in-depth review towards the end of the year or very early in the new year.

Regarding the general approach, I think that (contrary to the suggestion by @jeremylong in ticket #5783) it is better to have explicit configuration for user/password (Basic) or bearer token by separate configuration properties in line with what was already there for the Artifactory Analyzer.

No sense in adding logic to reverse-parse the token in order to supply the appropriate credentials to the apache HTTPClient (#7255 is pending merge which is making authentication pre-emptive to avoid the round-trip of a HTTP 401 for a known-protected server and solve the HTTP-302 based authentication flow that appears to happen in Azure DevOps)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ant changes to ant cli changes to the cli core changes to core documentation site documentation maven changes to the maven plugin tests test cases utils changes to utils
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Download of suppression files with authorization header
2 participants