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

Consider re-designing the Blazor WASM authentication stack #40764

Open
kevinchalet opened this issue Mar 17, 2022 · 21 comments
Open

Consider re-designing the Blazor WASM authentication stack #40764

kevinchalet opened this issue Mar 17, 2022 · 21 comments
Assignees
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 Pillar: Technical Debt Priority:1 Work that is critical for the release, but we could probably ship without

Comments

@kevinchalet
Copy link
Contributor

kevinchalet commented Mar 17, 2022

👋🏻

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 😄). 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?

@Pilchie Pilchie added the area-blazor Includes: Blazor, Razor Components label Mar 17, 2022
@javiercn javiercn added feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly feature-blazor-wasm-auth labels Mar 18, 2022
@javiercn
Copy link
Member

@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.

@javiercn javiercn added this to the .NET 7 Planning milestone Mar 18, 2022
@ghost
Copy link

ghost commented Mar 18, 2022

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@mkArtakMSFT mkArtakMSFT modified the milestones: .NET 7 Planning, Backlog Mar 23, 2022
@mkArtakMSFT mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Mar 23, 2022
@ghost
Copy link

ghost commented Mar 23, 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.

@kevinchalet
Copy link
Contributor Author

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
Copy link
Member

@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
Copy link
Contributor Author

kevinchalet commented Jul 26, 2022

@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
Copy link

ghost commented Sep 28, 2022

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
Copy link

My interest in this is the limitation to impersonate my tenants as host abpframework/abp#9997

@mkArtakMSFT mkArtakMSFT added the Priority:1 Work that is critical for the release, but we could probably ship without label Nov 10, 2022
@adrianwright109
Copy link
Contributor

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

@marinasundstrom
Copy link

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

@mrentier
Copy link

mrentier commented Apr 22, 2023

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
Copy link

Herdo commented May 29, 2023

@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
Copy link

CesarD commented Jun 12, 2023

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
Copy link
Contributor

ascott18 commented Oct 26, 2023

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
Copy link
Contributor

Another problem I just ran into:

  • RemoteAuthenticationService is a scoped service that implements IAccessTokenProvider.
  • IAccessTokenProvider is injected into a transient service AuthorizationMessageHandler (and its derivatives like BaseAddressAuthorizationMessageHandler).
  • AuthorizationMessageHandler instances are requested dynamically from a singleton service (DefaultHttpClientFactory). Because this is dynamic, it is not caught by static scope validation via ValidateScopes when building the service collection.
  • Ergo, a scoped service (RemoteAuthenticationService) is being injected into a singleton (DefaultHttpClientFactory), which is causing multiple instances of RemoteAuthenticationService to be created. In this case, a new instance is made every 2 minutes.

@ghost
Copy link

ghost commented Dec 12, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 Pillar: Technical Debt Priority:1 Work that is critical for the release, but we could probably ship without
Projects
None yet
Development

No branches or pull requests