Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Add Multi-Tenant support #1718

Closed
arnd27 opened this issue Apr 5, 2018 · 26 comments
Closed

Add Multi-Tenant support #1718

arnd27 opened this issue Apr 5, 2018 · 26 comments

Comments

@arnd27
Copy link

arnd27 commented Apr 5, 2018

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

blowdart commented Apr 5, 2018

Would you like to share how you did it before?

@arnd27
Copy link
Author

arnd27 commented Apr 6, 2018

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

blowdart commented Apr 6, 2018

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

@arnd27
Copy link
Author

arnd27 commented Apr 9, 2018

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

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

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

@sebastienros
Copy link
Member

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

HaoK commented Apr 10, 2018

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

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

HaoK commented Apr 10, 2018

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

HaoK commented Apr 10, 2018

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

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

@Tratcher
Copy link
Member

@HaoK
Copy link
Member

HaoK commented Apr 10, 2018

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

HaoK commented Apr 10, 2018

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

@HaoK
Copy link
Member

HaoK commented Apr 10, 2018

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

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

@kevinchalet
Copy link
Contributor

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

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

@blowdart
Copy link
Member

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

@HaoK
Copy link
Member

HaoK commented Apr 11, 2018

@HaoK HaoK closed this as completed Apr 11, 2018
@arnd27
Copy link
Author

arnd27 commented Apr 11, 2018

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

HaoK commented Apr 11, 2018

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

arnd27 commented Apr 11, 2018

@HaoK: which SO approach

@kevinchalet
Copy link
Contributor

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

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants