Middleware Extensibility: Support "Multi-tenancy" In AuthenticationOptions (and more)

Copied from https://katanaproject.codeplex.com/discussions/561673 as I was advised this project is moving from CodePlex to GitHub.

Hello Katana team,

Problem

I've been unable to cleanly extend the "built-in" authentication middleware because of its dependency on a concrete instance of AuthenticationOptions (and subclasses per implementation).

Presently the auth middleware is initialized at start-up and any configuration (API KEY, API SECRET, etc) cannot be changed on a per-request or per-tenant basis. I've seen some suggestion of multiple middleware instances per tenant as a solution but this is not very efficient or (feasible) when you have N tenants.

In my case each of my customers has their own OAUTH api credentials for each service and I need to be able to set them per-request based on the tenant scope. My problem, although seemingly common, is not the only reason to consider my proposed change.

The current dependency on a concrete implementation of [Middleware]AuthenticationOptions and AuthenticationOptions limits extensibility to the point that many are forced to "roll their own" copies of existing middleware with minor changes for their individual needs.

Proposed Design Change

I propose a design change, of which I'd like to contribute code, to address this limitation and allow for an optional alternative way to load middleware AuthenticationOptions. To achieve this we would depend on an interface IAuthenticationOptions and I[MiddlewareName]AuthenticationOptions for each specific middleware implementation.

These interfaces would have default implementations for each middleware. For example, for Facebook there would be a DefaultFacebookAuthenticationOptions which would implement the interface and use the current (static) approach to loading AppId and AppSecret.

A second scenario-specific implementation (which would be left to someone else to implement) could be MultitenantFacebookAuthenticationOptions which also implements the interface and identifies the tenant based on the request and then uses a datastore to load the AppId and AppSecret.

It would probably make most sense to have a FacebookAuthenticationOptionsBase abstract class which contains most of the needed functionality. We would also need to modify the existing FacebookAuthenticationOptions class to support the new interface but without breaking backward compatibility.

I have already proven this concept in my own project. In order to implement this we would need to change several authentication middleware classes which have dependencies on AuthenticationOptions as well as existing middleware which has dependencies on concretes of their respective implementations (e.g. FacebookAuthenticationOptions). Likewise, Extension Methods related to adding middleware to the pipeline would be modified to depend on the aforementioned interfaces instead of concretes.

Backwards Compatibility

This design change would have no impact on existing implementations. Developers would still be able to use app.UseFacebookAuthentication (et al) as well as the existing FacebookAuthenticationOptions class.

A new option for developers will be to provide/inject an IAuthenticationOptions when adding the middleware to the pipeline. Typically that would be done as follows:

// Old approach still supported
app.UseFacebookAuthentication(appId, appSecret);

// A new approach to inject an IFacebookAuthenticationOptions implementation
app.UseFacebookAuthentication(new MultitenantFacebookAuthenticationOptions());

Both approaches would be supported under the hood as such:

public static class FacebookAuthenticationExtensions
{
    public static IAppBuilder UseFacebookAuthentication(this IAppBuilder app, IFacebookAuthenticationOptions options)
    {
        if(app == null) throw new ArgumentNullException("app");
        if(options == null) throw new ArgumentNullException("options");

        app.Use(typeof (FacebookAuthenticationMiddleware), app, options);

        return app;
    }

    public static IAppBuilder UseFacebookAuthentication(this IAppBuilder app, string appId, string appSecret)
    {
        return UseFacebookAuthentication(
            app,
            new DefaultFacebookAuthenticationOptions
            {
                AppId = appId,
                AppSecret = appSecret,
            });
    }
}

Existing custom middleware, such as those provided by the owin-middleware/OwinOAuthProviders project should continue to work but will need to update their Microsoft.Owin.Security dependencies if they want to stay current. I believe this is the only burden added by my proposed design.

Finally...

I'm eager to see this implemented as I believe it is a design improvement which makes authentication middleware much more extensible without making it more complex. I wanted to get buy-in before I started the code changes required to implement the change.

So, Katana team, what do you think? :)

This is definitely a show stopper for many of us with many customers requiring us to support multiple websites using shared code bases.

I have already create a similar issue (before seeing this one) at dotnet/aspnetcore#743

Moved from aspnet-contrib/AspNet.Security.OpenIdConnect.Server#107:

Multi-tenancy has always been a thorny topic, even back to the Katana era 😄
Sadly, it's still not implemented in ASP.NET 5: #35

In theory, you should be able to create a custom IOptions<OpenIdConnectServerOptions> and return a different instance for each request, depending on the hostname, the path, a query string parameter, a header... :

public void ConfigureServices(IServiceCollection services) {
    services.AddScoped<IOptions<OpenIdConnectServerOptions>, MyOptions>();
}

public class MyOptions : IOptions<OpenIdConnectServerOptions> {
    private readonly IHttpContextAccessor context;

    public MyOptions(IHttpContextAccessor context) {
        this.context = context;
    }

    public OpenIdConnectServerOptions Options => GetNamedOptions(name: null);

    public OpenIdConnectServerOptions GetNamedOptions(string name) {
        // Extract the tenant identifier and use it to return different options.
        // You could extract it from a header, from the query string,
        // from the path and of course, from the hostname.
        var tenant = context.HttpContext.Request.Headers["Tenant-ID"];

        return new OpenIdConnectServerOptions {
            Issuer = new Uri("http://www.google.com/")
        };
    }
}

Sadly, this won't work, as AuthenticationMiddleware calls IOptions<>.Options directly in its constructor (where you can't access the current request, since there's simply none 😄) and stores it as a singleton: https://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNet.Authentication/AuthenticationMiddleware.cs#L26-L34

To support multi-tenancy, AuthenticationMiddleware's inheritors would also have to update their constructor, to avoid accessing the options instance too early. I guess we'd need a ValidateOptions directly in AuthenticationMiddleware to delay options validation.

@blowdart

/cc @blowdart @brentschmaltz @HaoK @Tratcher

It seems that this issue has been marked as an "enhancement". I cannot agree with that. I would say that for the new ASP.NET to rock, it is a "requirement" and a "show stopper". The issue is open for almost a year now.

I am an architect/developer with more or less 16 years of web development background and have been awarded as Microsoft MVP for ASP.NET for 4 times. With that experience in my past, I can clearly state that even though not every client (and project as a result) needs it, there are many enterprise clients that like to manage multiple websites from one instance.

Look at the CMS frameworks for instance. One of the best selling features of enterprise level CMS frameworks is that one can manage multiple websites on a single installation. Name it Umbraco, WordPress, EPiServer, Drupal, Django, Sitecore whatever. The market needs that technical ability.

Now that we have to go to clients and say "well, we could do that with the previous stack of .NET but not anymore with the new .NET release because the AuthenticationMiddleware does not support it". That does not make sense to me.

I could name you (which I would not of course as per the NDAs I signed) clients that host more than 70 websites on a single CMS installation. When I say single, I don't mean to say on a single server. If it is a webfarm of 5 servers, then they of course have 5 installations, instead of 70x5=350 installations.

If someone tells me how to explain a client that we have to charge them for maintenance of 350 installations instead of 5, I could be happily developing with a framework that does not support multi-tenant applications. Even if they are fine to pay for that, let's say that we need to make an update to the application, how would you update 350 installations in a timely manner? It would take days, if not weeks.

I don't really know what is the reason behind security modules not supporting multi tenant applications but I would really like to hear whatever it might be from the team that develops it.

It's an enhancement simply because it's never been like this. It's a new feature, an enhancement to the existing feature set.

I did not literally mean "it is not an enhancement" and imho, it was not the most important thing in my previous comment.

Thank you for your explanation but I am rather looking for feedback regarding at which milestone you are planning to release the support for multi tenant applications if you plan to support it at all and if not, how do you propose us to solve the maintenance issues I shared previously.

There is currently no milestone attached to it.

I can clearly see that on the right side of the original post where it says "Milestone: Backlog".

Does not really answer any of my concerns I shared or questions I asked above.

@blowdart

@blowdart's secret name is "Captain Obvious" 😄

Admittedly I haven't tried anything code-wise, but why can't you build a concrete implementation that doesn't STORE the credentials it is given, rather it proxies the properties as context-specific values that get computed using the in-flight context? I did something like this for view models in a multi-tenant app a while back.

I would like to add my voice and my vote in support of this issue. I need to be able to implement multi-tenancy configurable as either per hostname or based on the first folder segment of the url and I need to be able to configure different authentication settings for each tenant. If it is going to be impossible or very very difficult to do that with asp.net 5 that will be a big problem and certainly indicative of a design flaw in the framework IMHO.

I have to back-pedal my previous comment. While it would be nice if the middleware supported multi-tenancy by default, I have to admit I did not find it that difficult implementing my own multi tenant cookie middleware especially since the single tenant cookie middleware is open source. It was wrong of me to say it is indicative of a flaw in the framework, I really like the framework.

I'm currently working on creating a multi-tenant app in DNX as well. And apparently, (according to @PinpointTownes :P), this is one of the problems WRT that. So +1 from me.

@joeaudette

I'd like to see this prioritised too. It was confirmed some time ago that multi-tenancy wouldn't be supported out of the box in v1. However, it should at least be considered so that we have the extensibility points necessary to implement it ourselves.

I recently released SaasKit for ASP.NET Core that provides lightweight middleware to perform tenant identification and makes the current tenant available via DI.

When it came to ASP.NET Identity (cookie based auth) it was fairly trivial to support multi-tenancy since we could just swap out the underlying DbContext for each tenant (see http://benfoster.io/blog/aspnet-core-multi-tenancy-data-isolation-with-entity-framework). I know @joeaudette does have some concerns about the cookie middleware but since each of our tenants have separate domains, we don't need to set the cookie path per tenant so it was less of an issue for us.

However, when it comes to the external auth middleware this is a bit of a showstopper for us. Whilst it is possible to register multiple instances of the same auth provider by specifying a unique AuthenticationType, this does not work if you're provisioning tenants on-demand or if you have n number of tenants.

The main issue as a others have pointed out is that the AuthenticationOptions instance is created once per application rather than being resolved per request. This makes it virtually impossible to use for multi-tenant applications without essentially re-writing the underlying middleware and handlers.

The most logical solution seems to be to leverage DI and have the options resolved per request. Given that ASP.NET Core supports parameter injection into a middleware's Invoke method and as far as I can tell (please correct me if I'm wrong) the options are not actually used until the handler is created (on each request) would this not be a viable option?

It would then be easy to customise the options per tenant:

        services.AddScoped<GoogleOptions>(x => {
          var tenant = x.GetRequiredService<AppTenant>();

          return new GoogleOptions {
            ClientId = tenant.GoogleCliendId,
            ClientSecret = tenant.GoogleClientSecret
          };
        });

It would be great to get some feedback on this.

We realise this is wanted but we're at the point now where we are not doing any more feature work, hence this being in the backlog.

We will look at this once we get v1 out the door.

We have a pretty good workaround in the latest release of SaasKit.

This allows the auth middleware to be configured per tenant by creating forked middleware pipelines per tenant:

    app.UsePerTenant<AppTenant>((ctx, builder) =>
    {
        builder.UseCookieAuthentication(options =>
        {
            options.AuthenticationScheme = "Cookies";
            options.LoginPath = new PathString("/account/login");
            options.AccessDeniedPath = new PathString("/account/forbidden");
            options.AutomaticAuthenticate = true;
            options.AutomaticChallenge = true;

            options.CookieName = $"{ctx.Tenant.Id}.AspNet.Cookies";
        });

        builder.UseGoogleAuthentication(options =>
        {
            options.AuthenticationScheme = "Google";
            options.SignInScheme = "Cookies";

            options.ClientId = Configuration[$"{ctx.Tenant.Id}:GoogleClientId"];
            options.ClientSecret = Configuration[$"{ctx.Tenant.Id}:GoogleClientSecret"];
        });
    });

More info here

@blowdart

@blowdart any update now v1 is out the door 🤔 I also have a need of this type of feature

This is in progress for 2.0: #1170

Tracked via #1179

Navigation Menu

Repository navigation

Middleware Extensibility: Support "Multi-tenancy" In AuthenticationOptions (and more) #35

Problem

Proposed Design Change

Backwards Compatibility

Finally...

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Middleware Extensibility: Support "Multi-tenancy" In AuthenticationOptions (and more) #35

Description

Problem

Proposed Design Change

Backwards Compatibility

Finally...

Activity

CoskunSunali commented on Jul 16, 2015

kevinchalet commented on Jul 16, 2015

kevinchalet commented on Jul 16, 2015

CoskunSunali commented on Jul 16, 2015

blowdart commented on Jul 16, 2015

CoskunSunali commented on Jul 17, 2015

blowdart commented on Jul 17, 2015

CoskunSunali commented on Jul 17, 2015

kevinchalet commented on Jul 17, 2015

IDisposable commented on Jul 23, 2015

joeaudette commented on Jul 26, 2015

joeaudette commented on Aug 8, 2015

Alxandr commented on Oct 6, 2015

benfoster commented on Feb 19, 2016

blowdart commented on Feb 19, 2016

benfoster commented on Mar 1, 2016

grahamehorner commented on Apr 6, 2017

Tratcher commented on Apr 7, 2017

HaoK commented on Apr 13, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions