Skip to content

.NET 7 Blazor WASM authentication back button history behavior still problematic with 3rd party IDP #45097

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

Closed
mikequ-taggysoft opened this issue Nov 14, 2022 · 8 comments
Labels
area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly feature-blazor-wasm-auth good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue
Milestone

Comments

@mikequ-taggysoft
Copy link

mikequ-taggysoft commented Nov 14, 2022

Summary

.NET 7 improved Blazor WASM's authentication back history behavior in pull #43954. The underlying issue was described in issue #43063 @javiercn

However, after testing the new implementation, I'm still not getting the desirable behavior: back button returns to previous page that does not require authentication.

Motivation and goals

Consider the following typical scenario:

  1. User is currently on contoso.com, an unprotected Blazor home page.
  2. User clicks a link on the page, contoso.com/protected, which is a protected Blazor page via @attribute [Authorize]
  3. The app.razor sees that this is a protected route, and redirects user to contoso.com/Authentication/login via the RedirectToLogin Blazor component.
  4. The authentication component in turn redirects the user to a 3rd party IDP login page, such as AAD B2C's contoso.b2clogin.com.
  5. Now user realizes this is a protected area, but decides not to sign up or login, and presses the back button, with the intent to go back to the unprotected contoso.com home page.

However, in reality, the back button brings the user to contoso.com/protected, which then redirects the user to /Authentication/login and finally the IDP login page again, still resulting in a loop.

I believe this is a bit better than the previous version, where the back button would bring the user to /Authentication/login which would trigger a failure message.

But I think it would be a much better experience if the new history state manager can store the URL the user was on before the protected page was requested that triggered authentication, and bring the user to that page accordingly when back button is clicked.

@mikequ-taggysoft mikequ-taggysoft added the design-proposal This issue represents a design proposal for a different issue, linked in the description label Nov 14, 2022
@TanayParikh TanayParikh added area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly feature-blazor-wasm-auth labels Nov 14, 2022
@javiercn
Copy link
Member

javiercn commented Nov 15, 2022

@mikequ-taggysoft thanks for contacting us.

The code to do this is in your app. Essentially inside the RedirectToLogin component, you can replace the navigation entry that goes to /Authentication/login. and that should do what you want.

manager.NavigateTo(loginPath, new NavigationOptions
{
    ReplaceHistoryEntry = true,
    HistoryEntryState = new InteractiveRequestOptions
    {
        Interaction = InteractionType.SignIn,
        ReturnUrl = manager.Uri
    }.ToState()
});

@mkArtakMSFT mkArtakMSFT added the ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. label Nov 15, 2022
@ghost ghost added the Status: Resolved label Nov 15, 2022
@mikequ-taggysoft
Copy link
Author

mikequ-taggysoft commented Nov 15, 2022

@javiercn You meant to replace the NavigateToLogin call with the one you wrote with custom options right? However, the ToState() method is internal and cannot be called as is.

@ghost
Copy link

ghost commented Nov 16, 2022

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

@ghost ghost closed this as completed Nov 16, 2022
@javiercn javiercn reopened this Nov 16, 2022
@javiercn javiercn added Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. and removed ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved design-proposal This issue represents a design proposal for a different issue, linked in the description labels Nov 16, 2022
@ghost
Copy link

ghost commented Nov 16, 2022

Hi @mikequ-taggysoft. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@mikequ-taggysoft
Copy link
Author

mikequ-taggysoft commented Nov 16, 2022

@javiercn Thanks so much for re-opening and helping me out with this! So yeah after I serialized the HistoryEntryState with HistoryEntryState = JsonSerializer.Serialize(interactiveOptions), it did seem to work!

Now yesterday when I realized that ToState() was internal I did look into the source code and saw the JsonSerializer call, but it called this:

internal string ToState() => JsonSerializer.Serialize(this, InteractiveRequestOptionsSerializerContext.Default.InteractiveRequestOptions);

And InteractiveRequestOptionsSerializerContext happens to be internal too and I couldn't find the source code for the .Default.InteractiveRequestOptions

So I guess not passing this JsonTypeInfo in this case didn't matter.

P.S, when I looked at the source codes for the NavigateToLogin:

 public static void NavigateToLogin(this NavigationManager manager, string loginPath, InteractiveRequestOptions request)
    {
        manager.NavigateTo(loginPath, new NavigationOptions
        {
            HistoryEntryState = request.ToState(),
        });
    }

    public static void NavigateToLogin(this NavigationManager manager, string loginPath)
    {
        manager.NavigateToLogin(
            loginPath,
            new InteractiveRequestOptions
            {
                Interaction = InteractionType.SignIn,
                ReturnUrl = manager.Uri
            });
    }

The only difference with the code you showed me was to add the ReplaceHistoryEntry = true, right? This part confused me because I thought #43954 mentioned history is replaced by default in the new version, but now I guess that was referring to to the internal implementation of the authentication JS?

I personally think the final behavior I proposed and you helped me achieve here should be the "ideal" default, out-of-the-box behavior for most use cases (avoiding authentication loop via back button press), that could warrant a documentation/default code update. What do you think?

@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 Nov 16, 2022
@javiercn
Copy link
Member

The only difference with the code you showed me was to add the ReplaceHistoryEntry = true, right? This part confused me because I thought #43954 mentioned history is replaced by default in the new version, but now I guess that was referring to to the internal implementation of the authentication JS?

Yep.

I personally think the final behavior I proposed and you helped me achieve here should be the "ideal" default, out-of-the-box behavior for most use cases (avoiding authentication loop via back button press), that could warrant a documentation/default code update. What do you think?

I would be ok if we add this option to the helper methods, but I do not think is the correct default. That works when some part of your app does not require auth, but it does not when all your app requires authorization.

@javiercn javiercn added enhancement This issue represents an ask for new feature or an enhancement to an existing one and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Nov 17, 2022
@javiercn
Copy link
Member

The remaining work here is to add an overload that takes a boolean we apply to ReplaceHistoryEntry

@javiercn javiercn added this to the Backlog milestone Nov 17, 2022
@ghost
Copy link

ghost commented Nov 17, 2022

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@javiercn javiercn added help wanted Up for grabs. We would accept a PR to help resolve this issue good first issue Good for newcomers. labels Nov 17, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly feature-blazor-wasm-auth good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue
Projects
None yet
Development

No branches or pull requests

4 participants