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

Build in AuthenticationStateProviders from Blazor Web templates #52769

Closed
halter73 opened this issue Dec 12, 2023 · 12 comments · Fixed by #55821 or #56878
Closed

Build in AuthenticationStateProviders from Blazor Web templates #52769

halter73 opened this issue Dec 12, 2023 · 12 comments · Fixed by #55821 or #56878
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-authentication Pillar: Complete Blazor Web Priority:1 Work that is critical for the release, but we could probably ship without
Milestone

Comments

@halter73
Copy link
Member

halter73 commented Dec 12, 2023

In .NET 9, we should automatically register both server and client AuthenticationStateProvider's similar to the ones added by the "Individual Auth" option in the Blazor Web templates.

This should make it a lot easier to add auth to a Blazor project that is created without the "Individual Auth" template. As part of this, we will want to figure out which claims are safe to transmit to the clients, and potentially add a callback to configure custom claims.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Dec 12, 2023
@mkArtakMSFT mkArtakMSFT added enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-authentication Pillar: Complete Blazor Web labels Dec 12, 2023
@mkArtakMSFT mkArtakMSFT added this to the .NET 9 Planning milestone Dec 12, 2023
@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.

@halter73 halter73 changed the title Build in AuthenticationStateProviders from Blazor Web Templates Build in AuthenticationStateProviders from Blazor Web templates Dec 12, 2023
@Deineris
Copy link

Is there any guides on how to have a workaround untill this is finished? Wanted to suggest the newly released Blazor United as a base for an upcoming project, but since I can't figure out how the authentication and authorization against B2C with custom user flows would work, that a nonstarter, sadly. I assume in might work OK with server-side rendering, but the whole idea is to have them mixed. And since there is no template for Microsoft Identity (only individual accounts), then I assume that is not guaranteed to work, right?

@halter73
Copy link
Member Author

Is there any guides on how to have a workaround untill this is finished?

You can take a look at https://learn.microsoft.com/en-us/aspnet/core/blazor/security/blazor-web-app-with-oidc. The sample include PersistingServerAuthenticationStateProvider.cs and PersistentAuthenticationStateProvider.cs that are similar to the ones in the individual auth template.

@halter73
Copy link
Member Author

halter73 commented May 13, 2024

Background and Motivation

In .NET 9, we should automatically register both server and client AuthenticationStateProvider's similar to the ones added by the "Individual Auth" option in the Blazor Web templates.

This should make it a lot easier to add auth to a Blazor project that is created without the "Individual Auth" template. As part of this, we will want to figure out which claims are safe to transmit to the clients, and potentially add a callback to configure custom claims.

Proposed API

// Microsoft.AspNetCore.Components.Endpoints.dll

namespace Microsoft.AspNetCore.Components.Endpoints;

/// <summary>
/// Provides options for configuring server-side rendering of Razor Components.
/// </summary>
public sealed class RazorComponentsServiceOptions
{
+    public bool SerializeAuthenticationStateToClient { get; set; } = false;
+
+    public Func<AuthenticationState, Task<IEnumerable<KeyValuePair<string, string>>>> SerializeAuthenticationState { get; set; } = SerializeClaimsDefault;

    private static async Task<IEnumerable<KeyValuePair<string, string>>> SerializeClaimsDefault(AuthenticationState authenticationState)
    {
        foreach (var claim in authenticationState.User.Claims)
        {
            yield return new KeyValuePair<string, string>(claim.Type, claim.Value);
        }
    }
}
// namespace Microsoft.AspNetCore.Components.WebAssembly.dll

namespace Microsoft.AspNetCore.Components.WebAssembly.Hosting;

+ public sealed class AuthenticationStateDeserializationOptions
+ {
+    public Func<IEnumerable<KeyValuePair<string, string>>, Task<AuthenticationState>> DeserializeAuthenticationState { get; set; } = DeserializeAuthenticationState;
+
+    private static Task<AuthenticationState> DeserializeClaimsDefault(IEnumerable<KeyValuePair<string, string>> claims)
+    {
+        return Task.FromResult(
+            new AuthenticationState(new ClaimsPrincipal(new ClaimsIdentity(claims.Select(c => new(c.Type, c.Value),
+                authenticationType: "DeserializedAuthenticationState"))));
+    }
+ }
+

namespace Microsoft.Extensions.DependencyInjection;

+ public static class DeserializeAuthentciationStateServiceCollectionExtensions
+ {
+     public static void ConfigureAuthenticationStateDeserialization(this IServiceCollection services, Action<AuthenticationStateDeserializationOptions> configureOptions);
+ }

Usage Examples

The simplest way to serialize/deserialize all claims is just to set SerializeAuthenticationStateToClient to true.

// Program.cs for server project

builder.Services.AddRazorComponents(o => o.SerializeAuthenticationStateToClient = true)
                .AddInteractiveWebAssemblyComponents()

Alternatively, you could include only specific claims:

// Program.cs for server project

async Task<IEnumerable<KeyValuePair<string, string>>> MySerializeClaims(AuthenticationState authenticationState)
{
    foreach (var claim in authenticationState.User.Claims)
    {
        if (myAllowedClaims.Contains(claim.Type))
        {
            yield return new KeyValuePair<string, string>(claim.Type, claim.Value);
        }
    }
}

builder.Services.AddRazorComponents(o =>
{
    o.SerializeAuthenticationStateToClient = true;
    o.SerializeAuthenticationState = MySerializeClaims
})
    .AddInteractiveWebAssemblyComponents()
    .AddInteractiveServerComponents();

Alternative Designs

We could change all references to "AuthenticationState" to "ClaimsPrincipal". We could come up with better naming for "serialize" and "deserialize"? Maybe something involving "transmit" or "initialize".

Instead of exposing KeyValuePair<string, string> we could create a ClaimItem type or something. We don't want to use Claim because it includes a lot of detail we're usually uninterested in.

Risks

We must always register a default AuthenticationStateProvider for calls to AddRazorComponents, because we enable authentication state serialization using options which can be configured after registered services are finalized. If someone tries to TryAdd a custom AuthenticationStateProvider, this might break them. I think this is unlikely though.

@halter73 halter73 added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label May 13, 2024
@amcasey
Copy link
Member

amcasey commented May 13, 2024

  • Typo: Authentciation
  • Task<IEnumerable> vs IAsyncEnumerable?
  • Can we use an extension method instead on AddRazorComponents instead of adding a settable boolean?
    • That wouldn't be as clean - this is more consistent with the ownership
  • Perf concern: if this happens automatically, it might cost apps that aren't using it
    • It's lazy though

@halter73
Copy link
Member Author

halter73 commented May 21, 2024

Updated API Proposal

// Microsoft.AspNetCore.Components.Server.dll
+ [assembly: TypeForwardedTo(typeof(Microsoft.AspNetCore.Components.Server.ServerAuthenticationStateProvider))]

// Microsoft.AspNetCore.Components.Endpoints.dll
+ namespace Microsoft.AspNetCore.Components.Server;

+ public class ServerAuthenticationStateProvider : AuthenticationStateProvider, IHostEnvironmentAuthenticationStateProvider
// Microsoft.AspNetCore.Components.Authorization.dll
namespace Microsoft.AspNetCore.Components.Authorization;

+ public class AuthenticationStateData
+ {
+     public IList<KeyValuePair<string, string>> Claims { get; set; } = [];
+     public string NameClaimType { get; set; } = ClaimsIdentity.DefaultNameClaimType;
+     public string RoleClaimType { get; set; } = ClaimsIdentity.DefaultRoleClaimType;
+ }
// Microsoft.AspNetCore.Components.WebAssembly.Server.dll
namespace Microsoft.AspNetCore.Components.WebAssembly.Server;

+ public class AuthenticationStateSerializationOptions
+ {
+     public bool SerializeAllClaims { get; set; }
+     public Func<AuthenticationState, ValueTask<AuthenticationStateData?>> SerializationCallback { get; set; } = SerializeAuthenticationStateAsync;
+ }

namespace Microsoft.Extensions.DependencyInjection;

public static class WebAssemblyRazorComponentsBuilderExtensions
{
    public static IRazorComponentsBuilder AddInteractiveWebAssemblyComponents(this IRazorComponentsBuilder builder);

+     public static IRazorComponentsBuilder AddAuthenticationStateSerialization(this IRazorComponentsBuilder builder, Action<AuthenticationStateSerializationOptions>? configure = null)
}
// Microsoft.AspNetCore.Components.WebAssembly.Authentication.dll
namespace Microsoft.AspNetCore.Components.WebAssembly.Authentication;

+ public sealed class AuthenticationStateDeserializationOptions
+ {
+     public Func<AuthenticationStateData?, Task<AuthenticationState>> DeserializationCallback { get; set; } = DeserializeAuthenticationStateAsync;
+ }

namespace Microsoft.Extensions.DependencyInjection;

public static class WebAssemblyAuthenticationServiceCollectionExtensions
{
    // public static IRemoteAuthenticationBuilder<TRemoteAuthenticationState, TAccount> AddRemoteAuthentication<...
+    public static IServiceCollection AddAuthenticationStateDeserialization(this IServiceCollection services, Action<AuthenticationStateDeserializationOptions>? configure = null);
}

Usage Examples

The simplest way to serialize/deserialize all claims is to call AddAuthenticationStateSerialization() on the server's IRazorComponentsBuilder and AddAuthenticationStateDeserialization() on the client's IServiceCollection.

This will only serialize only the name and role claims:

// Server Program.cs
builder.Services.AddRazorComponents()
    .AddInteractiveWebAssemblyComponents()
    .AddAuthenticationStateSerialization();
// Client Program.cs
builder.Services.AddAuthorizationCore();
builder.Services.AddCascadingAuthenticationState();
builder.Services.AddAuthenticationStateDeserialization();

Alternatively, you could include all claims:

// Server Program.cs
builder.Services.AddRazorComponents()
    .AddInteractiveWebAssemblyComponents()
    .AddAuthenticationStateSerialization(options => options.SerializeAllClaims = true);

SerializeAllClaims only has an impact if you use the default SerializationCallback. You can provide custom serialization and deserialization callbacks as follows:

// Server Program.cs
builder.Services.AddRazorComponents()
    .AddInteractiveWebAssemblyComponents()
    .AddAuthenticationStateSerialization(options =>
    {
        options.SerializationCallback = authenticationState =>
        {
            AuthenticationStateData authenticationStateData = null;
            var identity = authenticationState.User.Identities.First();
            if (identity.IsAuthenticated)
            {
                authenticationStateData = new AuthenticationStateData();
                authenticationStateData.NameClaimType = identity.NameClaimType;
                authenticationStateData.RoleClaimType = identity.RoleClaimType;
                foreach (var claim in identity.Claims)
                {
                    if (_allowedClaims.Contains(claim.Type))
                    {
                        authenticationStateData.Claims.Add(new(claim.Type, claim.Value));
                    }
                }
            }

            return ValueTask.FromResult(authenticationStateData);
        };
    });
// Client Program.cs
// ...
builder.Services.AddAuthorizationCore();
builder.Services.AddCascadingAuthenticationState();
builder.Services.AddAuthenticationStateDeserialization(options =>
{
    options.DeserializationCallback = authenticationStateData =>
    {
        if (authenticationStateData is null)
        {
            return Task.FromResult(new AuthenticationState(new ClaimsPrincipal(new ClaimsIdentity())));
        }

        var claims = new List<Claim>();
        foreach (var kvp in authenticationStateData.Claims)
        {
            claims.Add(new(kvp.Key, kvp.Value));
        }

        claims.Add(new("client_only_claim", "custom"));

        return Task.FromResult(
            new AuthenticationState(new ClaimsPrincipal(
                new ClaimsIdentity(claims,
                    authenticationType: "DeserializationCallback",
                    nameType: authenticationStateData.NameClaimType,
                    roleType: authenticationStateData.RoleClaimType))));
    };
});
// ...

Alternative Designs

We could come up with better naming for "Serialization" and "Deserialization"? Maybe something involving "transmit" or "initialize". This feature relies on PersistentComponentState to do the serialization and deserialization, but I think the word "persistent" does more to confuse matters than help unless people are already very familiar with PersistentComponentState which I'm not sure is the case.

Initially, I wanted to make it possible to turn on this feature with only one line of code on the server. It would have been possible to achieve this by making the call to AddAuthenticationStateSerialization on the server implicitly enable AddAuthenticationStateDeserialization on the client if Microsoft.AspNetCore.Components.WebAssembly had a dependency on Microsoft.AspNetCore.Components.Authorization, but it does not currently. I'm not sure if it's worth adding a dependency to make this feature work with just one line of code instead of two, but I'm willing to try it if other people think it is.

We could change all references to "AuthenticationState" to "ClaimsPrincipal", but I think referencing AuthenticationState makes it clear this feature is specific to Blazor, and we might be helpful reference the entire AuthenticationState if it gets more properties in the future.

Instead of exposing KeyValuePair<string, string> we could create a ClaimItem type or something. I don't want to use Claim because it includes a lot of detail, we're usually uninterested in. We could also consider a ValueTuple<string, string> with explicit field names (string Type, string Value), but I don't think it adds much. I think it's clear that the key should be the type.

In the previous API review, the possibility of using IAsyncEnumerable was suggested. This is less relevant now that the claims are nested inside of AuthenticationStateData, but I think even if the callbacks returned the claims directly, Task<IEnumerable> makes more sense. PersistentComponentState, which does the JSON-serialization, serializes everything in memory before transmitting the payload to the client.

I did decide to use ValueTask<AuthenticationStateData?> for the SerializationCallback and Task<AuthenticationState> for the DeserializationCallback. Both callbacks complete inline by default, but they are both called from asynchronous code, so I saw no harm in giving developers the flexibility to do async work in the callbacks. The only reason I did not use ValueTask<AuthenticationState> for deserialization is because the client AuthenticationStateProvider needs to return a Task<AuthenticationState> anyway, so I didn't see the point in going through the extra step of converting it. We could try to consistently use Task or ValueTask in both places though.

I did address comment about using an extension method instead of adding a settable boolean to enable the feature. Calling AddAuthenticationStateSerialization does feel a lot cleaner than setting SerializeAuthenticationStateToClient = true.

Risks

People might be surprised that SerializeAllClaims isn't the default behavior. We're erroring on the side of more secure defaults since not every claim is always readable by the client given a cookie or JWE token. However, it is fairly common for clients to be able to read all claims from a JWS, and it's not clear if there are any common claims it would be dangerous to send to the client.

People might also be surprised that if they override the SerializationCallback to always return a non-null AuthenticationStateData the client will always deserialize it to AuthenticationState with an authenticated user unless they also update the DeserializationCallback. It might be better to make the AuthenticationStateData parameter and return types non-nullable and treat any AuthenticationStateData with no claims as unauthenticated. However, this would deviate from ClaimsPrincipal which can be authenticated without any claims.

@halter73
Copy link
Member Author

The PR has been merged in preview5, but I'm reopening this to finish the API review discussion. We can still make changes in preview6.

@halter73 halter73 reopened this May 23, 2024
@amcasey
Copy link
Member

amcasey commented May 23, 2024

[API Review]

  • Why KeyValuePair<string, string> instead of Claim?
    • Claim contains a bunch of extra stuff that seems superfluous (e.g. when serializing)
    • What if I already have a Claim in hand?
      • Yes, you have to pull out the fields
    • It is sufficiently clear to an auth user what the two strings are and what order they're in
  • Claim types are usually URLs and that's why they're typed as strings
  • AuthenticationStateData is basically just the object model for the JSON going back and forth
  • "All" is in contrast to "only name and role"
  • Seal all the things
  • Name and Role claims are special and that's why they get dedicated properties in AuthenticationStateData (but the values are in the regular list of pairs)
  • Let's go with a custom type in place of KeyValuePair<string, string> - it can also make conversions from Claim easy
    • It will be called ClaimData
    public readonly struct ClaimData(string type, string value)
    {
        public ClaimData(Claim claim) { }
        public string Type => type;
        public string Value => value;
    }
  • We like Propagation, but we're worried it might be ambiguous if both ends were called that
  • We kind of like Transmission because it conveys something will be leaving your box
  • Serialization sort of ambiguously suggests local persistence
  • AddAuthenticationStateSender/Receiver conveys the relationship but doesn't accurately convey the action
  • We're not big fans of SerializeAllClaims because of the way it introduces interaction between option properties, but it does work nicely with configuration binding, which is why an overload or a void-method isn't a good substitute
    • If we get permission from the security SMEs, we'll remove it entirely

@amcasey
Copy link
Member

amcasey commented May 23, 2024

// Microsoft.AspNetCore.Components.Server.dll
+ [assembly: TypeForwardedTo(typeof(Microsoft.AspNetCore.Components.Server.ServerAuthenticationStateProvider))]

// Microsoft.AspNetCore.Components.Endpoints.dll
+ namespace Microsoft.AspNetCore.Components.Server;

public sealed class ServerAuthenticationStateProvider : AuthenticationStateProvider, IHostEnvironmentAuthenticationStateProvider
// Microsoft.AspNetCore.Components.Authorization.dll
namespace Microsoft.AspNetCore.Components.Authorization;

+ public sealed class AuthenticationStateData
+ {
+     public IList<ClaimData> Claims { get; set; } = [];
+     public string NameClaimType { get; set; } = ClaimsIdentity.DefaultNameClaimType;
+     public string RoleClaimType { get; set; } = ClaimsIdentity.DefaultRoleClaimType;
+ }
+ 
+ public readonly struct ClaimData(string type, string value)
+ {
+     public ClaimData(Claim claim) { }
+     public string Type => type;
+     public string Value => value;
+ }
// Microsoft.AspNetCore.Components.WebAssembly.Server.dll
namespace Microsoft.AspNetCore.Components.WebAssembly.Server;

+ public sealed class AuthenticationStateSerializationOptions
+ {
+     public bool SerializeAllClaims { get; set; }
+     public Func<AuthenticationState, ValueTask<AuthenticationStateData?>> SerializationCallback { get; set; } = SerializeAuthenticationStateAsync;
+ }

namespace Microsoft.Extensions.DependencyInjection;

public static class WebAssemblyRazorComponentsBuilderExtensions
{
    public static IRazorComponentsBuilder AddInteractiveWebAssemblyComponents(this IRazorComponentsBuilder builder);

+     public static IRazorComponentsBuilder AddAuthenticationStateSerialization(this IRazorComponentsBuilder builder, Action<AuthenticationStateSerializationOptions>? configure = null)
}
// Microsoft.AspNetCore.Components.WebAssembly.Authentication.dll
namespace Microsoft.AspNetCore.Components.WebAssembly.Authentication;

+ public sealed class AuthenticationStateDeserializationOptions
+ {
+     public Func<AuthenticationStateData?, Task<AuthenticationState>> DeserializationCallback { get; set; } = DeserializeAuthenticationStateAsync;
+ }

namespace Microsoft.Extensions.DependencyInjection;

public static class WebAssemblyAuthenticationServiceCollectionExtensions
{
    // public static IRemoteAuthenticationBuilder<TRemoteAuthenticationState, TAccount> AddRemoteAuthentication<...
+    public static IServiceCollection AddAuthenticationStateDeserialization(this IServiceCollection services, Action<AuthenticationStateDeserializationOptions>? configure = null);
}

We would really like to drop SerializeAllClaims, security permitting.

API approved

@amcasey amcasey added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels May 23, 2024
@amcasey
Copy link
Member

amcasey commented Jul 18, 2024

@halter73 What happened with this?

We would really like to drop SerializeAllClaims, security permitting.

@halter73
Copy link
Member Author

The idea was to make SerializeAllClaims the default, right? I don't think anything has changed since we decide to remove InfoResponse.Claims from MapIdentityApi. Both were new APIs that someone would need to opt into calling, but where it might not be clear that all server-side claims would be visible to the client. I didn't really push it though.

If @blowdart thinks someone calling AddAuthenticationStateSerialization is a clear enough opt-in to serializing all claims, I'm more than happy to remove the SerializeAllClaims option and make that the default behavior.

@blowdart
Copy link
Contributor

I think being specific is better here.

And I wish I'd read the design before because I disagree that some of the information in the claims class is superfluous 😕

halter73 added a commit to halter73/AspNetCore that referenced this issue Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-authentication Pillar: Complete Blazor Web Priority:1 Work that is critical for the release, but we could probably ship without
Projects
None yet
6 participants
@halter73 @blowdart @Deineris @amcasey @mkArtakMSFT and others