aspnet / Security

[Archived] Middleware for security and authorization of web apps. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
1.27k stars 598 forks source link

Add Multi-Tenant support #1718

Closed arnd27 closed 6 years ago

arnd27 commented 6 years ago

We want to migrate an multi-tenant asp.net core 1.1 app to asp.net core 2.x. But we haven't find any possibility to implement this behaviour in asp.net core 2.x.

How is it possible to register 'CookieAuthenticationOptions/OpenIdConnectOptions' per tenant!

blowdart commented 6 years ago

Would you like to share how you did it before?

arnd27 commented 6 years ago

For multi tenancy, we use the "SaasKit", but there is no working solution for asp.net core 2.x and authentication. For each tenant, we need an extra clientId.

The minimum what we need, is the possibility to register CookieAuthenticationOptions and OpenIDConnectOptions as transient.

Example:

`

        app.UsePerTenant<Tenant>((ctx, builder) =>
        {
            //Auth
            builder.UseCookieAuthentication(new CookieAuthenticationOptions
            {
                AuthenticationScheme = "Cookies",
                //To work with tenant subdomains
                CookieName = $"{ctx.Tenant.Id}.Auth.Cookie"
            });

            builder.UseOpenIdConnectAuthentication(new OpenIdConnectOptions
            {
                AuthenticationScheme = "oidc",
                SignInScheme = "Cookies",
                Authority = $"{appSettings.Value.Auth.Endpoint}",
                RequireHttpsMetadata = false,

                ClientId = $"{ctx.Tenant.Id}:{appSettings.Value.Auth.ClientId}",
                ClientSecret = $"{appSettings.Value.Auth.ClientSecret}",

                ...
            });`
blowdart commented 6 years ago

SaasKit being a third party product? How are you directing people to login? What distinguishes a tenant here?

arnd27 commented 6 years ago

Hi

We directing people to login with the attribute "Authorize" -> Normal behavior.

In our case, a tenant is the same app with an seperate database, accessible with a subdomain (e.g. customer1.myapp.com). My problem didn't depend on SaasKit. For testing you can hardcode a AppTenant class which deliveres an tenant id.

We want to register the follwoing class as transient (scoped will be better), or how can we solve this problem with asp.net core 2.x.

public class ConfigureMyOpenIdInternal : IConfigureNamedOptions<OpenIdConnectOptions>
    {
        private readonly HttpContext _httpContext;

        public ConfigureMyOpenIdInternal(IHttpContextAccessor contextAccessor)
        {
            _httpContext = contextAccessor.HttpContext;
        }

        public void Configure(string name, OpenIdConnectOptions options)
        {
            var tenantContext = _httpContext.GetTenantContext<AppTenant>();

            if (tenantContext == null)
            {
                options.ClientId = "AspNet.Client";
            }
            else
            {
                options.ClientId = $"{tenantContext.Tenant.Id}.AspNet.Client";
            }
        }

        public void Configure(OpenIdConnectOptions options)
             => Configure(Options.DefaultName, options);
    }
blowdart commented 6 years ago

@davidfowl for visibility. Again this ought to be something we consider not just as a security/identity problem but framework wide.

@sebastienros does the Orchard pieces have magic for this?

Tratcher commented 6 years ago

@HaoK already has magic for this in 2.0 but it doesn't appear to be captured in our docs or samples.

sebastienros commented 6 years ago

Like SaaSKit Orchard has a middleware pipeline and a container per tenant. It also has a DataProtector root for each tenant. And cookies are defined assigned to the domain and prefix of each tenant.

/cc @PinpointTownes

HaoK commented 6 years ago

Yeah, there's some new sugar in 2.1 for this:

services.AddOptions<OpenIdConnectOptions>("name").Configure<IHttpContextAccessor>(o,c => {
            var tenantContext = c.GetTenantContext<AppTenant>();
            if (tenantContext == null)
            {
                o.ClientId = "AspNet.Client";
            }
            else
            {
                o.ClientId = $"{tenantContext.Tenant.Id}.AspNet.Client";
            }
}

https://github.com/aspnet/Options/blob/dev/src/Microsoft.Extensions.Options/OptionsBuilder.cs#L57

kevinchalet commented 6 years ago

@HaoK have you tried your snippet? I'm extremely surprised it works, since options are cached. The first call will produce the correct result, but subsequent calls will return the options associated with the initial tenant.

Here's an approach using IOptionsMonitor: https://stackoverflow.com/questions/49596938/openid-connect-identifying-tenant-during-login

HaoK commented 6 years ago

The sugar is just registering a transient IConfigureOptions underneath the covers, its up to the caller to determine the lifetime of the options instance, so if you use IOptions(singleton)/IOptionsSnapshot(request)/IOptionsMonitor(cache lifetime) that's actually orthogonal to how you wire up how each instance is configured.

HaoK commented 6 years ago

AddOptions<TOptions> is a new 2.1 thing that returns a OptionsBuilder which constrains the operations to a specific named instance so you don't have to keep passing around names.

kevinchalet commented 6 years ago

So your snippet alone is not enough to support multi tenancy?

Tratcher commented 6 years ago

@PinpointTownes is right, the AuthHandlerProvider caches handlers, which cache options. https://github.com/aspnet/HttpAbstractions/blob/a6bdb9b1ec6ed99978a508e71a7f131be7e4d9fb/src/Microsoft.AspNetCore.Authentication.Core/AuthenticationHandlerProvider.cs#L40

HaoK commented 6 years ago

I was only really commenting on the thread about new 2.1 sugar to make what @arndklocker 's code easier. And not anything about multi-tenancy per say

HaoK commented 6 years ago

@PinpointTownes answer on stack overflow with a custom IOptionsMonitor looks like a good approach to me

HaoK commented 6 years ago

@blowdart @davidfowl should I file an issue to write an official 'blessed' multitenant/configure auth options per tenant sample in our AuthSamples repo? We do get asked this a lot....

blowdart commented 6 years ago

I believe this needs addressing all up rather than just limiting to auth. @DamianEdwards your thoughts please?

kevinchalet commented 6 years ago

@HaoK it's a bit ugly, tho'.

That said, if you can afford doing the tenant name resolution twice (or have some caching to avoid it), you can probably mix your new syntactic sugar (that registers an IConfigureOptions<T>) plus a custom IOptionsMonitor<T> that uses IOptionsFactory<T> like the default implementation.

DamianEdwards commented 6 years ago

@blowdart that's always been our position and I've not seen anything that changes that IMO.

blowdart commented 6 years ago

OK limited ugly sample it is then. Post 2.1 work @HaoK ?

HaoK commented 6 years ago

Yeah I filed https://github.com/aspnet/AuthSamples/issues/33

arnd27 commented 6 years ago

@HaoK When can I expect the sample?

My problem was not to register OpenIdConnectOptions, but the method Configure(string name, OpenIdConnectOptions options) was called only once. So something in the aspnet security framework must be wrong (refer @PinpointTownes) or was missunderstood by me.

I need an sample, how to configure or replace the extension methods AddCookie and AddOpenIdConnect for a multi tenant app with asp.net core 2.x.

e.g. customer1.myapp.com -> ClientId: 1:AspNet.Client customer2.myapp.com -> ClientId: 2:AspNet.Client

Notice: In development mode, we use different ports for different tenants. localhost:3001 -> ClientId: 1:AspNet.Client localhost:3002 -> ClientId: 2:AspNet.Client

I add a simple multi-tenant sample (asp.net core 1.1) for porting to asp.net core 2.x. https://github.com/sArchitects/TenantSample

HaoK commented 6 years ago

I'd start with @PinpointTownes 's SO approach for now, since the sample likely won't be ready until after 2.1 is done, and it will likely be based on what will be available in 2.1

arnd27 commented 6 years ago

@HaoK: which SO approach

kevinchalet commented 6 years ago

@arndklocker this one: https://stackoverflow.com/a/49682427/542757. It should work with any authentication handler like OIDC, Google, Twitter or any aspnet-contrib social provider.

AndrewTriesToCode commented 6 years ago

I have a pretty decent solution to this based on using a custom IOptionsCache. I'm still working on the documentation, but the samples are pretty clear: https://github.com/Finbuckle/Finbuckle.MultiTenant

Check out the AuthenticationOptions sample. The nuget pacakge to add is Finbuckle.MultiTenant.AspNetCore v1.0.0. The config code looks like this:

services.AddMultiTenant()
  .WithInMemoryStore(Configuration.GetSection("Finbuckle:MultiTenant:InMemoryMultiTenantStore"))
  .WithRouteStrategy()
  .WithPerTenantOptionsConfig<CookieAuthenticationOptions>((o, tc) => o.Cookie.Name += tc.Id);

You could easily add another "WithPerTenantOptionsConfig" for OpenIdConnectOptions.