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

AuthorizationMessageHandler fails to set authenticationstate on error #30927

Closed
davhdavh opened this issue Mar 15, 2021 · 6 comments
Closed

AuthorizationMessageHandler fails to set authenticationstate on error #30927

davhdavh opened this issue Mar 15, 2021 · 6 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly feature-blazor-wasm-auth ✔️ Resolution: By Design Resolved because the behavior in this issue is the intended design. investigate Status: Resolved
Milestone

Comments

@davhdavh
Copy link
Contributor

Describe the bug

If _provider.RequestAccessToken is unable to provide a valid token, the IAccessTokenProvider API assumes that the RedirectUri is followed and the authentication flow reapplied. However, the AuthorizationMessageHandler class throws an exception because it cannot handle that flow.
It should however also inform IAccessTokenProvider that the current AuthenticationState is no longer valid.

Further technical details

.Net 6.0 preview 2

@javiercn javiercn added area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly labels Mar 15, 2021
@javiercn javiercn added this to the Next sprint planning milestone Mar 16, 2021
@ghost
Copy link

ghost commented Mar 16, 2021

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@TanayParikh
Copy link
Contributor

Hi @davhdavh. We made some changes in the RC2 .NET 6 release which may resolve this. Could you see if you're still encountering this issue with the Release/6.0.1XX-rc2 in https://github.com/dotnet/installer#installers-and-binaries.

If so, do you happen to have repro steps, or a basic repro repository I can try out?

However, the AuthorizationMessageHandler class throws an exception because it cannot handle that flow.

What's the exception?

@TanayParikh TanayParikh added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Sep 16, 2021
@davhdavh
Copy link
Contributor Author

Hi @TanayParikh
That change solves the reverse problem (That AuthorizationMessageHandler didnt know the token was invalidated), whereas this bug is that AuthorizationMessageHandler doesn't inform _provider that the token is invalid (when it throws AccessTokenNotAvailableException) and that _provider doesn't tell itself that the token is invalid if RequestAccessToken fails to return a valid token.
Like I was trying to say in #30929 it occurs when the token expired AND the token refresh time expired, because the AuthorizationMessageHandler is the only check that the token is still valid and that is only called when it is used and doesn't refresh based on the expiration datetime. For example, if someone left your site open overnight without doing anything.

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Sep 17, 2021
@ghost ghost added the Working label Sep 29, 2021
@mkArtakMSFT mkArtakMSFT removed the Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. label Sep 29, 2021
@TanayParikh
Copy link
Contributor

It should however also inform IAccessTokenProvider that the current AuthenticationState is no longer valid.

whereas this bug is that AuthorizationMessageHandler doesn't inform _provider that the token is invalid

Not sure what you mean by this. The IAccessTokenProvider doesn't explicitly track the authentication state.

The AccessTokenNotAvailableException exception thrown in this case needs to be caught by user code, the current state saved, and the user subsequently redirected to re-authenticate. Closing this as by design.

@ghost ghost added Done This issue has been fixed and removed Working labels Oct 4, 2021
@TanayParikh TanayParikh added the ✔️ Resolution: By Design Resolved because the behavior in this issue is the intended design. label Oct 4, 2021
@ghost ghost added the Status: Resolved label Oct 4, 2021
@davhdavh
Copy link
Contributor Author

davhdavh commented Oct 5, 2021

The IAccessTokenProvider doesn't explicitly track the authentication state.

Correct, the problem is not inside AuthorizationMessageHandler. However, it does call _provider.RequestAccessToken. This is actually implemented in a class that DOES track authentication state, namely the AuthenticationStateProvider.

The bug is that when RequestAccessToken fails inside AuthenticationStateProvider, it itself should detect that the authentication state is no longer valid so that code that depends on the AuthenticationStateProvider (which would be ALL default code in everyones projects) can inform the user that they were logged out.

@ghost ghost added the Working label Oct 6, 2021
@TanayParikh TanayParikh removed the Done This issue has been fixed label Oct 6, 2021
@TanayParikh
Copy link
Contributor

TanayParikh commented Oct 15, 2021

so that code that depends on the AuthenticationStateProvider (which would be ALL default code in everyones projects) can inform the user that they were logged out.

As I mentioned above:

The AccessTokenNotAvailableException exception thrown in this case needs to be caught by user code, the current state saved, and the user subsequently redirected to re-authenticate.

Hence, by catching the AccessTokenNotAvailableException, you can inform the user they were logged out.

If you don't feel that this sufficiently resolves the issue you're facing, feel free to put up a PR with your suggested change (I'm assuming this would be a minor change, let me know if not), and we'll evaluate accordingly whether the change would be appropriate for the framework.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly feature-blazor-wasm-auth ✔️ Resolution: By Design Resolved because the behavior in this issue is the intended design. investigate Status: Resolved
Projects
None yet
Development

No branches or pull requests

4 participants