-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add support for Basic authentication on SonarScanner mirrors #57
Conversation
Kudos, SonarCloud Quality Gate passed! |
@victor-diez-sonarsource regarding your documentation change, the password protection in this PR is for the mirror itself (i.e. the Nexus instance), not the proxy - proxy auth is separate, as part of the proxy URL. |
Thanks @interone-ms. Yes, I understand that. But you mentioned:
but there was nothing after that. So I understood you were referring to the authentication for proxies, which was already documented in the first PR. can you please clarify? |
config.httpOptions.httpsRequestOptions = { agent: proxyAgent }; | ||
} | ||
|
||
if (finalUrl.username !== '' || finalUrl.password !== '') { |
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.
What would happen if one of them is missing? Should the two of them be required?
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.
should be fine. http basic auth allows for empty usernames or passwords
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.
👍
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.
LGTM
@interone-ms Please let me know if it's all clear or you meant to say something else in the README. Would like to merge this before next release. |
@victor-diez-sonarsource you're right, good catch. Looks like I reordered stuff during writing and didn't update the wording. |
Cherry-picked from #44