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

AuthContext.IsPeerAuthenticated should return true if User.Identity.IsAuthenticated does #2537

Open
Eagle3386 opened this issue Sep 17, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@Eagle3386
Copy link

Is your feature request related to a problem? Please describe.

After loads of trial & error (no offense, but some docs are confusing for beginners), it seems my Blazor WASM standalone app authenticates Azure B2C users via MSAL correctly - except for 1 thing:
context.AuthContext.IsPeerAuthenticated always returns false, even if context.GetHttpContext().User.Identity.IsAuthenticated returns true.

Since the docs for AuthContext.PeerIdentityPropertyName state:

Gets the name of the property that indicates the peer identity

… I looked at the code which clarified that IsPeerAuthenticated returns true as soon as the former just isn't null - but that doesn't always seem to get set properly.
From what I could find (mainly 2 unit tests in this repo), the C# implementation solely focuses on authentication via certificates, because if the underlying HttpContext contains a ClaimsPrincipal with an IIdentity whose IsAuthenticated is true, gRPC seems to "simply not care".

Describe the solution you'd like

Please, for Padawan-like developers like me, i.e., those struggling with authentication & authorization, enable that authenticated users (= those with an HttpContext containing a ClaimsPrincipal whose IIdentity.IsAuthenticated evaluates to true), can be easily checked/identified via context.AuthContext.IsPeerAuthenticated, too.
Because otherwise context.GetHttpContext() needs to be called on each & every method call.

Describe alternatives you've considered

Continuing to use either workaround:

  • Calling context.GetHttpContext().User.Identity.IsAuthenticated inside each overwritten method.
  • Using a custom AuthenticationInterceptor which overrides 9 (!) methods to evaluate the IIdentity property & in the event of it being true sort of "modifies" the existing ServerCallContext by cloning everything from it, except for the AuthContext which is replaced with one that has IsPeerAuthenticated return true.

Additional context

The docs regarding AuthContext should be improved to make its content as well as usage clearer - maybe even by adding a usage example for the scenario I just described.

@Eagle3386 Eagle3386 added the enhancement New feature or request label Sep 17, 2024
@JamesNK
Copy link
Member

JamesNK commented Sep 17, 2024

AuthContext is legacy from the older Grpc.Core implementation. The docs don't talk about it because getting the identity from HttpContext is better.

It wouldn't be difficult to change the IsPeerAuthenticated flag to be true based on HttpContext.User.Identity.IsAuthenticated, but all other info on the identity wouldn't be available.

It's probably best to have docs to say to not worry about the auth context and get that information from HttpContext.User. It's properly integrated into the rest of ASP.NET Core.

@Eagle3386
Copy link
Author

At the very least, update the docs rather sooner than later, because after reading

/// AuthContext is the only reliable source of truth when it comes to authenticating calls.
/// Using any other call/context properties for authentication purposes is wrong and inherently unsafe.

I felt almost "forced" to make IsPeerAuthenticated return true, if the user got authenticated via MSAL.. 🙈😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants