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.32k stars 267 forks source link

IdentityServer4 sample with Multitenancy #374

Closed Blackleones closed 3 years ago

Blackleones commented 3 years ago

I am learning how to use Finbuckle to create a multitenant project with the following infrastructure:

I've already created the javascript client, the web API with Finbuckle, and the Identity Provider using ID4. Unfortunately, I am struggling with how to set ID4 to multitenant.

I've googled a lot about ID4 with multitenancy and I've read a lot of solutions: who use acr_values, who customize ID4 to read tenant identifier from the route, and so on. I would like to know if there is a sample of how to set ID4 to multitenancy using Finbuckle.

AndrewTriesToCode commented 3 years ago

@Blackleones I apologize for the late reply.

I'm working on a sample and blog post for this so stay tuned.

xPathin commented 3 years ago

Hey @AndrewTriesToCode, any updates on this? I'm craving for that sample and post :grin:

natelaff commented 3 years ago

I'm pretty curious, too. I have a working IS4 implementation, but I'm eager to see another. I think what Andrew does based on what he's mentioned in a past issue is better than my solution.

VeeEng commented 3 years ago

Hey @natelaff , could you please share your example. I haven't been able to figure this out yet

natelaff commented 3 years ago

I replace the default EndpointRouter service with a custom IEndpointRouter that puts the tenant name in the base path. It's not super complicated but ultimately I'd like to get away from this method.

I think the better solution is to utilize acr_values tenant and write a custom Finbuckle resolution strategy which pulls this out using the IdentityInteractionService or whatever it's called. In theory, this should work, but in practice I haven't been able to get it working when I try it and give up because I have bigger fish to fry at the time. When ASP.NET Core Identity attempts to connect to the database, I don't have the tenant for some reason (I use db per tenant, so I need my EF Core context to know which tenant), that acr_values doesn't flow though to /connect/token. I think it might have to be a two part approach where you register two resolution strategies, one (custom) strategy that is looking to AcrValues, then a second one (ClaimStrategy) that looks to a claim that you issue somewhere between the initial request and the token issuance. This was going to be my next approach, but I haven't had time.

AndrewTriesToCode commented 3 years ago

Hi @xPathin, @VeeEng , and @natelaff

Sorry for the slow reply. What's funny is that after really digging into this I've come to the conclusion that basepath + remapping the basepath via the standard Map middleware to rebase the request after the multitenant middleware so that IS is basically unaware of the original basepath. Or alternatively use the domain strategy.

The other piece is the databases that IS4 uses -- I am working to make sure they will still work well with adjustments for multi tenant.

I'm working on an example of this but there some other developments with the library going on that I'm trying to improve at the same time.

VeeEng commented 3 years ago

Hey @natelaff . Thanks for your explanation. I am not using ASP.NET Core Identity for my current project but your strategy my just work for me.

Looking forward to the example @AndrewTriesToCode

rlmartins76 commented 3 years ago

@AndrewTriesToCode

Any ETA when this sample will be available?

AndrewTriesToCode commented 3 years ago

@rlmartins76 @VeeEng @natelaff @xPathin @Blackleones

Took me a while and this is still a little dirty but the fundamentals are there: https://github.com/AndrewTriesToCode/MultiTenantIdentityServer4

The Identity Server project in there is MultiTenant using the basepath strategy and adjusts the PathBase (see the inline middleware) so that Identity Server basically doesn't see it. The server maintains separate login sessions per-tenant and uses aeparate configuration store per-tenant (i.e. each tenant can register its own clients). Things like APIs and whatnot are currently not multitenant but could be added easily. The operational store is not yet multitenant but that can be added without too much more work and I don't believe causes any problems as-is.

Still needed:

Here are some embarrassingly bad steps on how I put this together:

natelaff commented 3 years ago

So one thing I do is replace the IEndpointRouter with a custom one. Your middleware is essentially doing this same thing by throwing the path in, but this is the "IdentityServer way" to do it.

    public class TenantEndpointRouter : IEndpointRouter
    {
        private readonly IEnumerable<Endpoint> _endpoints;
        private readonly IdentityServerOptions _options;
        private readonly ILogger _logger;

        public TenantEndpointRouter(IEnumerable<Endpoint> endpoints,
            IdentityServerOptions options, ILogger<TenantEndpointRouter> logger)
        {
            _endpoints = endpoints;
            _options = options;
            _logger = logger;
        }

        private static bool IsMatch(HttpContext context, string path)
        {
            TenantInfo tenant = context.GetMultiTenantContext<TenantInfo>()?.TenantInfo;

            if (context.Request.Path.StartsWithSegments(path))
                return true;

            if (tenant == null)
                return false;

            if (!string.IsNullOrEmpty(tenant.Name))
            {
                var folderPath = $"/{tenant.Name}{path}";
                if (context.Request.Path.StartsWithSegments(folderPath))
                {
                    var idBasePath = context.GetIdentityServerBasePath().EnsureTrailingSlash();
                    context.SetIdentityServerBasePath($"{idBasePath}{tenant.Name}/");
                    return true;
                }
            }

            return false;
        }

        public IEndpointHandler Find(HttpContext context)
        {
            if (context == null)
            {
                throw new ArgumentNullException(nameof(context));
            }

            foreach (var endpoint in _endpoints)
            {
                var path = endpoint.Path;
                if (IsMatch(context, path))
                {
                    var endpointName = endpoint.Name;
                    _logger.LogDebug("Request path {path} matched to endpoint type {endpoint}", context.Request.Path, endpointName);

                    return GetEndpointHandler(endpoint, context);
                }
            }

            this._logger.LogTrace("No endpoint entry found for request path: {path}", context.Request.Path);

            return null;
        }

        private IEndpointHandler GetEndpointHandler(Endpoint endpoint, HttpContext context)
        {
            if (_options.Endpoints.IsEndpointEnabled(endpoint))
            {
                if (context.RequestServices.GetService(endpoint.Handler) is IEndpointHandler handler)
                {
                    _logger.LogDebug("Endpoint enabled: {endpoint}, successfully created handler: {endpointHandler}", endpoint.Name, endpoint.Handler.FullName);
                    return handler;
                }
                else
                {
                    _logger.LogDebug("Endpoint enabled: {endpoint}, failed to create handler: {endpointHandler}", endpoint.Name, endpoint.Handler.FullName);
                }
            }
            else
            {
                _logger.LogWarning("Endpoint disabled: {endpoint}", endpoint.Name);
            }

            return null;
        }
    }

You then also have to register this service and change your route to have the tenant in the path.

michelevirgilio commented 3 years ago

@AndrewTriesToCode https://github.com/AndrewTriesToCode/MultiTenantIdentityServer4 Hi, thank you for your awesome work. I'm trying to run your solution, but for Tenant 1 i get a

Sorry, there was an error : invalid_request
Invalid redirect_uri

instead for Tenant 2 i get a

Sorry, there was an error : unauthorized_client
Unknown client or client not enabled

Any idea? Thanks

EDIT: PR https://github.com/AndrewTriesToCode/MultiTenantIdentityServer4/pull/1

michelevirgilio commented 3 years ago

@AndrewTriesToCode Trying your sample https://github.com/AndrewTriesToCode/MultiTenantIdentityServer4 By logging out from mvc client seems not logging out from IS: indeed IS shows the login page, but in the upper bar i can see the name of the logged user (bob/alice)

AndrewTriesToCode commented 3 years ago

@michelevirgilio in general OpenID Connect logout is kind of a mess. I see what you are saying. the IS4 sample by default keeps the user logged in so on the client I added the prompt = "login" line to always force a login for demo purposes. I woudl expect the IS4 app to also not show the previously logged in user. I'll take a closer look and see if this can be cleaned up a little bit.

I also think this issue is worse in the dev environment because in real world users will probably not be sharing the same browser/cookies.

natelaff commented 3 years ago

I remember dealing with this about a year ago. I jumped through a lot of hoops to allow the log out of this too so that it would actually display the right way. Ultimately, at least for my use case, it was pointless as if you're doing SSO you're generally hiding that bar anyways because you don't want them to be able to navigate away in native clients.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.