AuthenticationProperties object is not passed to ChallengeAsync / ForbidAsync

[PrefixAM]

AuthenticationProperties object is not passed to ChallengeAsync / ForbidAsync. AuthorizationMiddleware calls policyEvaluator.AuthenticateAsync method that returns AuthenticateResult object. That object has a property AuthenticationProperties that can be populated with the authentication data (AuthenticationProperties.Items, AuthenticationProperties.Parameters, AuthenticationProperties.RedirectUri etc.). When AuthorizationMiddleware receives the AuthenticateResult (AuthorizationMiddleware.cs, line 56), it does not pass AuthenticateResult.AuthenticationProperties to the method ChallengeAsync (AuthorizationMiddleware.cs, line 74 / line 79) as well as to the method ForbidAsync (AuthorizationMiddleware.cs, line 90 / line 95) while the corresponding overrides of these methods exist.

[Tratcher]

What information are you flowing in this scenario? It's not clear that the AuthProperties from a prior sign-in apply to a new challenge.

[PrefixAM]

1. AuthenticationHandler.HandleAuthenticateAsync { //properties.Items or properties.Parameters are populated with authentication errors return AuthenticateResult.Fail( ex, properties ); }
2. AuthorizationHandler.HandleRequirementAsync { // evaluate requirements, if requires authenticated user then return context.Fail() }
3. AuthenticationHandler.HandleChallengeAsync or AuthenticationHandler.HandleForbiddenAsync getting invoked

Both HandleChallengeAsync and HandleForbiddenAsync can accept AuthenticationProperties that AuthenticationMiddleware could've passed from step 1 to step 3, but it does not call the corresponding override, instead it calls ChallengeAsync / ForbidAsync which internally create new blank AuthenticationProperties and pass them to HandleChallengeAsync / HandleForbiddenAsync.

Subjectively, from the technical point of view, just by looking into the code, it looks like a bug. But from the business or "use case" prospective (which might not be the best example) - in my case I can customize Challenge / Forbidden responses by looking at the authentication result, i.e. that way I can return the authentication errors.

You said: "It's not clear that the AuthProperties from a prior sign-in apply to a new challenge." - basically it's an attempt of sign-in, and the state of that attempt is stored in AuthProperties. And if policy tells that it's not authorized, then that state is passed to Challenge / Forbid handlers where it can be processed properly.

[Tratcher]

What kind of auth are you using this with? JwtBearer? That's likely the only scenario where the flow you describe makes sense. For other flows like OpenIdConnect or OAuth the sign-in happens on an independent series of request and the end result is an authenticated user stored in the auth cookie. Any errors there would never reach the Authorization middleware.

[PrefixAM]

OIDC \ OAuth are already implemented protocols, they implement RemoteAuthenticationHandlers that handle responses and throw errors if needed.

In my case it's just a custom token based authentication. I implement an AuthenticationHandler, define my own authentication rules and return AuthResults. So yes, that issue applies to the cases when someone implements their own authentication.

[Tratcher]

Ok. Aside from the auth error scenario we have to consider what impact this would have for users that were successfully authenticated but not authorized. There may be quite a bit of state in there that's not applicable to a new login flow, or worse would conflict with it.

E.g. cookie auth stores timeout information in the auth properties. If you pre-set some of that it may confuse it.

[Tratcher]

@HaoK has a broader issue tracking how to cleanly report authN and authZ errors.

[HaoK]

I don't think it generally makes sense to flow properties like this, as @Tratcher mentions, they are meant as a way to flow additional properties for an individual auth operation. In case of a custom handler that wants this behavior, it seems like a reasonable workaround to require the user to write their own custom AuthorizationMiddleware that flows the AuthenticationProperties if they so desire

[PrefixAM]

@Tratcher If I understand you correct, the flow you described leads straight to Forbid. I personally don't see any issues there. @HaoK So are you saying that the fact that both methods always receive "new AuthenticationProperties()" as an argument: protected virtual Task HandleChallengeAsync( AuthenticationProperties properties ); protected virtual Task HandleForbiddenAsync( AuthenticationProperties properties ); is basically by design?

[Tratcher]

There are specific scenarios for passing auth properties to Challenge such as overriding return url or scopes. Just because Challenge accepts auth properties doesn't mean it's appropriate to pass in the ones from Authenticate.

[PrefixAM]

These specific scenarios are implemented through: OAuthChallengeProperties : AuthenticationProperties and OpenIdConnectChallengeProperties : OAuthChallengeProperties by handling these objects in the corresponding inherited methods. It doesn't change the fact that default implementation does not pass AuthenticationProperties. IMHO. Feel free to close this issue.

[HaoK]

yes this behavior is by design, the semantics of authentication properties is specific to each handler, think of it as a bag of arbitrary metadata that you can pass to each handler, its definitely not something that you can always flow from handler to handler or scheme to scheme in general. It likely to cause bad side effects too, as they are not meant to be reused automatically.