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

Ignore DefaultCredentials when doing Basic & Digest auth in SocketsHttpHandler #113728

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

Conversation

makazeu
Copy link
Contributor

@makazeu makazeu commented Mar 20, 2025

Fixes #113145.

As described in #113145 (comment)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@Copilot Copilot bot review requested due to automatic review settings March 20, 2025 13:13
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR prevents Basic authentication from mistakenly using DefaultCredentials in SocketsHttpHandler.

  • Adds a conditional check in the Basic authentication case to skip processing when DefaultCredentials is used.
  • Enhances the authentication handling by ensuring only supported credential types are processed in Basic auth.
Comments suppressed due to low confidence (1)

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.cs:269

  • [nitpick] Consider verifying that the variable name 'credentials' is consistent with other parts of the code where a similar credential is referred to as 'credential'. If applicable, renaming it for consistency may help reduce confusion.
if (CredentialCache.DefaultCredentials == credentials)
@makazeu
Copy link
Contributor Author

makazeu commented Mar 20, 2025

cc @MihaZupan

makazeu added 2 commits March 20, 2025 21:44

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@MihaZupan
Copy link
Member

Can you please add a test that validates the new behavior?
Default credentials are set, 401 response, only 1 request is made

makazeu added 6 commits March 21, 2025 12:05

Verified

This commit was signed with the committer’s verified signature.
jolheiser John Olheiser
Add unit test
@makazeu makazeu changed the title Ignore DefaultCredentials when doing BasicAuth in SocketsHttpHandler Ignore DefaultCredentials when doing Basic & Digest auth in SocketsHttpHandler Mar 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net.Http community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SocketsHttpHandler retries with useless "Authorization" header when SystemNetworkCredential is in use
3 participants