Finbuckle / Finbuckle.MultiTenant

Finbuckle.MultiTenant is an open-source multitenancy middleware library for .NET. It enables tenant resolution, per-tenant app behavior, and per-tenant data isolation.
https://www.finbuckle.com/multitenant
Apache License 2.0
1.31k stars 267 forks source link

Issuer validation #785

Open charlieH1 opened 8 months ago

charlieH1 commented 8 months ago

Hi

With the update to the packages I noticed that issuer validation is being enforced differently, previously I didnt have to put in a Valid issuer or anything in the AddOpenIdConnect defaults now I have to, here's some sample code to demo it, below is what I have to do now

services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme)
        .AddCookie("Cookies", options =>
        {
            options.Cookie.Name = "mycookie";
            options.Cookie.SameSite = SameSiteMode.None;

        })
        .AddOpenIdConnect(options =>
        {
            options.ClientId = "__invalid"; // Needed for validation, will be overwritten per-tenant.
            options.Authority = "https://__invalid"; // Needed for validation, will be overwritten per-tenant.
            options.Prompt = "login consent"; // For sample purposes.
            options.ResponseType = "code";
            options.RequireHttpsMetadata = false;//remove when switch to https
            options.GetClaimsFromUserInfoEndpoint = true;
            options.ClaimActions.MapAll();
            options.ClaimActions.Add(new RoleClaimAction());
            options.TokenValidationParameters.ValidIssuers= new List<string>() {"https:/myissuer.dev.com", "http://myissuer2.dev.com/" };

        });

and before

services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme)
        .AddCookie("Cookies", options =>
        {
            options.Cookie.Name = "mycookie";
            options.Cookie.SameSite = SameSiteMode.None;

        })
        .AddOpenIdConnect(options =>
        {
            options.ClientId = "__invalid"; // Needed for validation, will be overwritten per-tenant.
            options.Authority = "https://__invalid"; // Needed for validation, will be overwritten per-tenant.
            options.Prompt = "login consent"; // For sample purposes.
            options.ResponseType = "code";
            options.RequireHttpsMetadata = false;//remove when switch to https
            options.GetClaimsFromUserInfoEndpoint = true;
            options.ClaimActions.MapAll();
            options.ClaimActions.Add(new RoleClaimAction());

        });

Is there a way that hasnt been documented to have per tenant issuer validation or not and if not I'd suggest this might be worth a look into?

AndrewTriesToCode commented 7 months ago

Hi can you please post the error message? I’ve checked OpenIdConnectOptions and they don’t have this validation. I will check a few more places in the aspnetcore source code.

charlieH1 commented 7 months ago

Hi Andrew

Sorry for the slow reply, see below

Main Error:

An unhandled exception occurred while processing the request.
SecurityTokenInvalidIssuerException: IDX10205: Issuer validation failed. Issuer: '<removed for security>'. Did not match: validationParameters.ValidIssuer: 'null' or validationParameters.ValidIssuers: 'null' or validationParameters.ConfigurationManager.CurrentConfiguration.Issuer: 'localhost:5000'. For more details, see https://aka.ms/IdentityModel/issuer-validation.
Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectHandler.ValidateTokenUsingHandlerAsync(string idToken, AuthenticationProperties properties, TokenValidationParameters validationParameters)

AuthenticationFailureException: An error was encountered while handling the remote login.
Microsoft.AspNetCore.Authentication.RemoteAuthenticationHandler<TOptions>.HandleRequestAsync()

Stack:

Microsoft.IdentityModel.Tokens.SecurityTokenInvalidIssuerException: IDX10205: Issuer validation failed. Issuer: '<removed for security>'. Did not match: validationParameters.ValidIssuer: 'null' or validationParameters.ValidIssuers: 'null' or validationParameters.ConfigurationManager.CurrentConfiguration.Issuer: 'localhost:5000'. For more details, see https://aka.ms/IdentityModel/issuer-validation. 
   at Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectHandler.ValidateTokenUsingHandlerAsync(String idToken, AuthenticationProperties properties, TokenValidationParameters validationParameters)
   at Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectHandler.HandleRemoteAuthenticateAsync()

Hope that helps

AndrewTriesToCode commented 7 months ago

Ok I found that it was this commit that added this: https://github.com/dotnet/aspnetcore/commit/a56e968c19be1275db6dc462310d723615b006a7

Per tenant issuer should be possible. You would just use per-tenant options for OpenIdConnectOptions and override that property for the options class using something on the tenant info. Likely you'd want to add a specific property to your tenant info type to store that. I will add a convention to the per-tenant authentication functionality that will look for a property with that name and use it by convention.

AndrewTriesToCode commented 7 months ago

Also I’ll add that in most of my dev and samples I disable issuer validation so I wouldn’t have seen this for a while. Thanks for bringing it to the community’s attention.