dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.16k stars 9.92k forks source link

Consider re-designing the Blazor WASM authentication stack #40764

Open kevinchalet opened 2 years ago

kevinchalet commented 2 years ago

👋🏻

The Blazor WASM OIDC authentication stack was built around the oidc-client-js library. Sadly, this library is no longer supported and the GitHub repository was archived last year.

As I suspect the ASP.NET team will consider opting for a different solution at some point, I guess it's a good opportunity to discuss the design of the authentication stack and suggest potential improvements.

Last month, I unveiled the OpenIddict client, a new OAuth 2.0/OIDC client designed with extreme flexibility in mind (so it can be used with the worst non-standard server providers the world can offer :smile:). As part of the effort, I'd love to provide a native Blazor integration. I worked on a prototype based on the existing Blazor 6.0 authentication APIs and it's promising, but I believe there's room for improvement.

One of the main points that could be improved is how things are currently layered: unlike ASP.NET Core's authentication stack that offers specialized authentication handlers (cookies, OIDC, etc.), things are tightly coupled in the Blazor WASM world. More specifically, it would be great if the user persistence part (using local or session storage) was independent from the components handling the external authentication dance (in my case, OIDC). Something modeled after ASP.NET Core's IAuthenticationHandler/IAuthenticationService abstractions would be excellent.

Is it something that could be done as part of 7.0?

javiercn commented 2 years ago

@kevinchalet thanks for contacting us.

Replacing oidc-client.js is in our roadmap, that said, we are not looking at revamping the auth system with something as sophisticated as the auth system in ASP.NET Core.

Originally our auth system was implemented as a wrapper over JS libraries because they offered out of the box support for things like silent sign-in, etc. Given that the cookie changes have effectively killed most of those features, we are reconsidering our approach on this area.

That said, we are considering what we do in this area, however in general we want to minimize as much as possible the number of primitives/complexity we ship in-box and enable people to integrate with our abstractions to tune auth to their needs.

If you have concrete code snippets of what you are proposing I'm happy to move the conversation forward.

ghost commented 2 years ago

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

kevinchalet commented 2 years ago

Originally our auth system was implemented as a wrapper over JS libraries because they offered out of the box support for things like silent sign-in, etc. Given that the cookie changes have effectively killed most of those features, we are reconsidering our approach on this area.

Sadly, this led to an odd design: RemoteAuthenticationService aims at being an open/generic base implementation but it's actually tied to oidc-client-js under the hood as AuthenticationService.ts is just a wrapper around it. Even things like storing or getting the current user identity are simple wrappers around oidc-client-js.

Replacing oidc-client.js is in our roadmap, that said, we are not looking at revamping the auth system with something as sophisticated as the auth system in ASP.NET Core.

I suggested that for one reason : ASP.NET Core's authentication abstractions are battle-proven, extremely extensible and yet not insanely complex (they were inherited from Katana and got an overhaul as part of ASP.NET Core 2.0). So why would you reinvent the wheel? 😃

That said, we are considering what we do in this area, however in general we want to minimize as much as possible the number of primitives/complexity we ship in-box and enable people to integrate with our abstractions to tune auth to their needs.

Unlike ASP.NET Core - for which devs have written thousands of authentication handlers of all types - I personally haven't seen many actual derived implementations of Microsoft.AspNetCore.Components.WebAssembly.Authentication, which makes me think the "enable people to integrate with our abstractions" part of the contract is not exactly fulfilled 😄

If you have concrete code snippets of what you are proposing I'm happy to move the conversation forward.

I don't have specific snippets to share, but as I said in my OP, the main issue is how things are coupled. If at least the user persistence part was decoupled from the oidc-client-js, it would help a lot. Think of it as an equivalent of CookieAuthenticationHandler for Blazor (should I dare call that LocalStorageAuthenticationHandler or SessionStorageAuthenticationHandler?)

kevinchalet commented 2 years ago

(since the first post got already 5 👍🏻, I guess I'm not the only one interested in seeing improvements in this area 😃)

ghost commented 2 years ago

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.

kevinchalet commented 2 years ago

Given the number of 👍🏻 under my OP, I suspect the interest is strong. Is it on your radar, @javiercn? If so, can you share some details about your plan?

javiercn commented 2 years ago

@kevinchalet I am starting to look back at this area. We were in the middle of releasing Blazor Hybrid and this fell of my radar a bit.

For the time being, I am interested in understanding what concrete scenarios are not possible to implement with the current implementation to get a better view of the landscape as a first step.

I haven't looked at the client that you wrote, but I will take a look in the following weeks. I will also be interested in understanding what problems you faced when you tried to integrate with the current abstractions, maybe together we can identify what is missing. If you can provide concrete details on problems, maybe I can give some ideas.

Whatever we do here, this is going to be a .NET 8.0 feature/work.

kevinchalet commented 2 years ago

@kevinchalet I am starting to look back at this area. We were in the middle of releasing Blazor Hybrid and this fell of my radar a bit.

I gave the MAUI flavor a try when creating the OpenIddict client MAUI integration prototype and it's really fun! (amusingly, when the first Blazor bits were released, I suggested that Blazor should be in its own Microsoft.Blazor.* namespace to account for potential evolutions like this one 🤣).

For the time being, I am interested in understanding what concrete scenarios are not possible to implement with the current implementation to get a better view of the landscape as a first step.

As I mentioned in my previous messages, the blocking part is the fact the external authentication handling and the user persistence parts are tightly coupled in the current implementation: Blazor exposes an IRemoteAuthenticationService that you can implement to handle the OIDC dance (which is fine), but it can't work alone without also creating a subclass of AuthenticationStateProvider as the default implementation - RemoteAuthenticationProvider - is a simple wrapper around oidc-client-js.

The two things should be decoupled so that you can create IRemoteAuthenticationService integrations without also implementing the local/session storage parts (that should be ideally provided OOTB by Blazor WASM).

I haven't looked at the client that you wrote, but I will take a look in the following weeks.

An ASP.NET Core sample using the OpenIddict client stack can be found here: https://github.com/openiddict/openiddict-core/blob/dev/sandbox/OpenIddict.Sandbox.AspNetCore.Client/Controllers/AuthenticationController.cs

As you can see in that sample, OpenIddict itself only handles the external authentication dance: the persistence part is handled by ASP.NET Core using the cookie middleware, which is not something we can (easily?) replicate with Blazor due to the tight coupling I mentioned.

A MAUI prototype with a sample targeting both Twitter and a local OIDC server (currently WinUI-only, unlikely to ship as part of OpenIddict 4.0 due to the low demand) can also be found here: https://github.com/kevinchalet/openiddict-core/tree/maui_winui_sample/sandbox/OpenIddict.Sandbox.Maui.Client

The Blazor WASM prototype is not currently public as it's not fully working, but if you're interested in taking a look, let me know and I'll make it public 😃

Whatever we do here, this is going to be a .NET 8.0 feature/work.

That's fine: crypto support in 7.0 is still extremely lacking so I wasn't expecting anything concrete for 7.0 🤣

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

sturlath commented 1 year ago

My interest in this is the limitation to impersonate my tenants as host https://github.com/abpframework/abp/issues/9997

adrianwright109 commented 1 year ago

Would be good to consider baking token generation back into ASP.NET Core, so that we're not reliant on third party packages / cloud solutions.

See blog post for more details: https://developer.okta.com/blog/2018/03/23/token-authentication-aspnetcore-complete-guide#generate-tokens-for-authentication-in-aspnet-core

Lepollo3000 commented 1 year ago

I don´t know if this go here, but I'm trying to implement OpenIddict in a self hosted Blazor Webassembly project and I suceeded, but I don´t like to use the scaffolded views of identity to login, register and that kind of things. Is there a way to keep using the default authentication state provider but take the authentication views from client and not from server?

I don't know if I'm explaining very well, the project I'm doing is on Github: https://github.com/Lepollo3000/WebAssemblyWithOpenIddict

Lepollo3000 commented 1 year ago

I don´t know if this go here, but I'm trying to implement OpenIddict in a self hosted Blazor Webassembly project and I suceeded, but I don´t like to use the scaffolded views of identity to login, register and that kind of things. Is there a way to keep using the default authentication state provider but take the authentication views from client and not from server?

I don't know if I'm explaining very well, the project I'm doing is on Github: https://github.com/Lepollo3000/WebAssemblyWithOpenIddict

By the way, my project is in .NET 6

Herdo commented 1 year ago

Is the iframe solution implemented in oidc-client.js the de facto standard for SPAs, that a new solution in .NET 8 Blazor WASM would implement as well? Or is there a better solution to this nowadays?

marinasundstrom commented 1 year ago

I want to port my Blazor WASM app (that uses auth) to Blazor MAUI Hybrid. I wish there was a common abstraction.

mrentier commented 1 year ago

It is rather painful that returning a token from the identity server triggers a reload of Blazor WASM. There might be a path with the new Blazor United to perform the authentication server side and then transition to client side with the received token?

Herdo commented 1 year ago

@mkArtakMSFT The current implementation with iframes and silent signin based on oidc-client-js is causing some timeout issues with third-party IdPs. I've described the cause of this issue in this Stackoverflow question: Blazor WASM - Spending a long time initially in Authorizing component. I'll add my analysis results below.

IMHO, the new solution should allow more freedom for cases like this, where you cannot influence IdP configuration like X-Frame-Options header.

Source of issue

The issue is caused by a timeout in the underlying implementation of the authentication services. I traced down the source, but there's no easy solution to this issue.

If you enable Debug tracing for your WASM client, you should see this log message in the console:

dbug: Microsoft.AspNetCore.Components.WebAssembly.Authentication.RemoteAuthenticationService[0] Initial silent sign in failed 'Frame window timed out'

For me - using Keycloak (instead of Auth0), and Discord as IdP behind Keycloak - the Discord login cannot be framed in the hidden iframe:

Refused to frame 'https://discord.com/' because it violates the following Content Security Policy directive: "frame-src 'self' my.domain.com".

Of course this policy can be modified to include discord.com, but Discord denies being embedded that way with X-Frame-Options header.

What's happening

  1. The app gets loaded
  2. AuthorizeViewCore is being rendered, entering OnParametersSetAsync:

    // Clear the previous result of authorization
    // This will cause the Authorizing state to be displayed until the authorization has been completed
    isAuthorized = null;
    
    currentAuthenticationState = await AuthenticationState;
    isAuthorized = await IsAuthorizedAsync(currentAuthenticationState.User);
  3. The AuthenticationState is initialized by RemoteAuthenticationService.GetAuthenticationStateAsync:
    new AuthenticationState(await GetUser(useCache: true));
  4. This will invoke GetAuthenticatedUser:

    /// <summary>
    /// Gets the current authenticated used using JavaScript interop.
    /// </summary>
    /// <returns>A <see cref="Task{ClaimsPrincipal}"/>that will return the current authenticated user when completes.</returns>
    protected internal virtual async ValueTask<ClaimsPrincipal> GetAuthenticatedUser()
    {
        await EnsureAuthService();
        var account = await JsRuntime.InvokeAsync<TAccount>("AuthenticationService.getUser");
        var user = await AccountClaimsPrincipalFactory.CreateUserAsync(account, Options.UserOptions);
    
        return user;
    }
  5. AuthenticationService.getUser will invoke trySilentSignIn:

        async trySilentSignIn() {
        if (!this._intialSilentSignIn) {
            this._intialSilentSignIn = (async () => {
                try {
                    this.debug('Beginning initial silent sign in.');
                    await this._userManager.signinSilent();
                    this.debug('Initial silent sign in succeeded.');
                } catch (e) {
                    if (e instanceof Error) {
                        this.debug(`Initial silent sign in failed '${e.message}'`);
                    }
                    // It is ok to swallow the exception here.
                    // The user might not be logged in and in that case it
                    // is expected for signinSilent to fail and throw
                }
            })();
        }
    
        return this._intialSilentSignIn;
    }
  6. The await this._userManager.signinSilent(); will invoke the oidc-client-js UserManager signinSilent and then _signinSilentIframe:

    _signinSilentIframe(args = {}) {
        let url = args.redirect_uri || this.settings.silent_redirect_uri || this.settings.redirect_uri;
        if (!url) {
            Log.error("UserManager.signinSilent: No silent_redirect_uri configured");
            return Promise.reject(new Error("No silent_redirect_uri configured"));
        }
    
        args.redirect_uri = url;
        args.prompt = args.prompt || "none";
    
        return this._signin(args, this._iframeNavigator, {
            startUrl: url,
            silentRequestTimeout: args.silentRequestTimeout || this.settings.silentRequestTimeout
        }).then(user => {
            if (user) {
                if (user.profile && user.profile.sub) {
                    Log.info("UserManager.signinSilent: successful, signed in sub: ", user.profile.sub);
                }
                else {
                    Log.info("UserManager.signinSilent: no sub");
                }
            }
    
            return user;
        });
    }
  7. Finally, this will end up at IFrameWindow.js, which has a timeout of 10000 ms configured:
    const DefaultTimeout = 10000;
  8. The initially logged timeout error is thrown:
    _timeout() {
        Log.debug("IFrameWindow.timeout");
        this._error("Frame window timed out");
    }
CesarD commented 1 year ago

I'm wondering if there's not going to be a revamp of this like the author suggested, if at least there could be a refactor to base this implementation on a more modern and better maintained library like https://github.com/authts/oidc-client-ts, which would help reduce the hassle of migrating and revamping everything entirely, and will at least provide a more up-to-date library that fixes some of the problems with the old oidc-client.js that is no longer maintained since a couple years ago already (which I think it's a nasty thing for security purposes on a platform as young as Blazor).

ascott18 commented 10 months ago

Some other issues I have with the current implementation that I'd love to see addressed in a future rework:

  1. When the token refresh fails, the only thing propagated from JS back to C# is "RequiresRedirect" (link). We cannot get any details of the failure, like the OAuth response object from the server (with error and error_description properties), or if the failure was a network error (i.e. "no internet").
  2. There doesn't seem to be a good place to configure a global handler for token refresh failures. The best I've managed was subclassing RemoteAuthenticationService and overriding RequestAccessToken, but this circles back to the first point where in the event of a failure in token acquisition, we cannot determine what the failure mode was. Example use case: token refresh fails because the user's account was disabled on the server. We want to immediately kick the user out of the app no matter what they were doing at the time.
  3. The HttpClient handler that injects tokens has a hardcoded refresh buffer of 5 minutes for attempting a refresh, but AuthenticationService.ts will continue serving the token from the javascript side until it is fully expired (effectively a hardcoded refresh buffer of zero seconds). A buffer of 5 minutes causes problems if your server issues 5-minute access tokens, though, which is the lowest possible access token duration for many services (Okta/Auth0, AWS Cognito, and probably many others)
  4. Blazor Hybrid (MAUI) is basically not supported. It works if you write your own AuthenticationStateProvider/IRemoteAuthenticationService<>/IAccessTokenProvider implementation, but that takes quite a bit of work and also feels "dirty" to be using classes and components namespaced in Microsoft.AspNetCore.Components.WebAssembly in a non-WebAssembly app.
  5. As Kevin noted in the original issue, there's no control over the token persistence mechanism - they are only stored in sessionStorage.
ascott18 commented 10 months ago

Another problem I just ran into:

ghost commented 9 months ago

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.