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

Make handling of DefaultCredentials in NegotiateAuthentication/SocketsHttpHandler more consistent #91138

Closed
wants to merge 1 commit into from

Conversation

filipnavara
Copy link
Member

@filipnavara filipnavara commented Aug 25, 2023

See PR #91160 for alternative approach.

Throw PNSE on Unix/Managed NegotiateAuthenticationPal implementation for NTLM w/ default credentials

  • This was handled inconsistently between the managed NTLM implementation and the GSSAPI one. Let's standardize on throwing PlatformNotSupportedException with a useful message that explains which parameters are unsupported.
  • Adjust managed SPNEGO implementation to handle PlatformNotSupportedException correctly.
  • Add test for the behavior.

SocketsHttpHandler: Handle PNSE from NegotiateAuthentication as unsupported authentication and return server's response instead of passing the exception

  • Treat PNSE from NegotiateAuthentication the same way as if the authentication scheme was unsupported, and return the Unauthorized response from server.
  • Add test for the behavior.

Fixes #91131

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Aug 25, 2023
@ghost
Copy link

ghost commented Aug 25, 2023

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #91131

Author: filipnavara
Assignees: -
Labels:

area-System.Net.Security

Milestone: -

@@ -184,7 +180,7 @@ public UnixNegotiateAuthenticationPal(NegotiateAuthenticationClientOptions clien

if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, $"Peer SPN-> '{_spn}'");

if (clientOptions.Credential == CredentialCache.DefaultCredentials ||
if (clientOptions.Credential == CredentialCache.DefaultNetworkCredentials ||
Copy link
Member Author

@filipnavara filipnavara Aug 25, 2023

Choose a reason for hiding this comment

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

Note: The equality check never worked because the objects are of different type.

…cationPal implementation for NTLM w/ default credentials.

This was handled inconsistently between the managed NTLM implementation and the GSSAPI one.
Add test for the behavior.
Handle PlatformNotSupportedException in SocketsHttpHandler and Managed SPNEGO implementation.
Add test to ensure SocketsHttpHandler using CredentialCache.DefaultCredentials with NTLM doesn't throw PNSE exception and returns the Unauthorized HTTP response instead.
@karelz karelz added this to the 9.0.0 milestone Sep 14, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Oct 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Security 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.

Android NTLM: Empty Credentials now throws PlatformNotSupportedException
2 participants