multitenancy

[constantine-v]

The goal of this PR is to introduce multitenancy (Tenant per database). It does not take into account existing code that assumes sharing existing database between all Tenants. Multitenancy can be configured through settings, here's example for appsettings.json

"Multitenancy": true,
"Tenants": {
    "Default": {
        "Name": "Default Tenant",
        "Prefix": "default",
        "DatabaseConnectionString": "Data Source = elsa.sqlite.db;Cache = Shared"
        ...any other relevant settings
    }
}

Multitenancy (true/false) setting is needed for backwards compatibility purposes. When it is not set to true, default tenant will be created with DatabaseConnectionSttring taken from regular Elsa settings. Prefix setting is needed to distinguish between different tenants on frontend or when dealing with cached data (which is accessed by hardcoded key). Since user authentication is out of Elsa's scope, detecting current tenant and redirecting to appropriate URL also should be handled outside of Elsa. When dashboard loads for the first time, it gets tenant prefix from the URL and compares it with all tenant prefixes loaded from API. If there is a match, all links are adjusted accordingly by adding tenant prefix to them. For example, if initially user is redirected to https://elsa.mydomain.com/mytenant/, all subsequent requests will follow the same route e.g. https:\elsa.mydomain.com\mytenant\v1\workflow-definitions.

On backend, current tenant is read from request URL prefix and stored in scoped service TenantProvider. For the scenarios when there is no request URL (for instance when running startup tasks during application start), tenant is set explicitly using CreateScopeForTenant(ITenant tenant) extension method for ServiceScopeFactory.

[sfmskywalker]

I noticed the addition of a new model called Tenant to Elsa.Core, which contains ConnectionString and Prefix properties. It seems too opinionated to me that a tenant is always associated with a connection string, or at least too narrowly-scoped.

Having tenant-specific configuration makes sense, so perhaps we can replace Prefix and ConnectionString with a more general-purpose IConfigurationSection kind of thing?

Only some modules care about a tenant-specific DB connection string; others might want to have a tenant-specific Azure SB connection string, etc.

[constantine-v]

I've added TenantConfiguration class that contains dictionary of settings (initially pulled from tenant configuration section). I've replaced Prefix and ConnectionString with TenantConfiguration and added couple of extension methods to retrieve prefix and database connection string. Modules can add their own extension methods to retrieve settings specific for them.

[vargaendre]

Hi, this feature seems to be a really great thing, I would be really interested in it. Is there an issue describing the features / functionality of this PR? Thanks

[sfmskywalker]

@vargaendre there is an issue about this topic, but it doesn't provide a complete description (far from it). Perhaps @constantine-v can update the description of this PR with basic instructions on how to use it?

[tomy2105]

I would definitely say it would be nice to have a place in documentation where multitenancy (what is supported and what is not is described). A comment on this PR could be a good starting point, yes, but somewhere inside documentation would be far better :).

[steviej08]

This is looking really good. I'm also interested in this PR. I've been implementing something similar to this for our product, as we need this fairly soon. I'm wondering, is this targeted for a particular release? Are there any rough timings on said release?

Also, I'm wondering if there is scope to provide asynchronous access to tenants. That is the ITenantStore and ITenantProvider having async functions. This would make it easy to switch out the current TenantStore where we may be accessing an external service to grab tenant information. I also wonder whether we could have an interface for Tenant. Then in the ITenantProvider we can do what we need in when setting the current tenant, like load a bunch of settings for that account (I haven't gone through the PR in a lot of detail btw and appreciate there may be some sort of specific model you guys are aiming for).

[sfmskywalker]

Just as a quick reminder putting this here: need to update this question about multi-tenancy support once this feature goes in: https://github.com/elsa-workflows/elsa-core/discussions/140

[sfmskywalker]

@steviej08 You're right, the ITenantStore and ITenantProvider services' methods should be made async (I assume they are not currently so based on your comment).

Same goed for introducing ITenant for Tenant.

@constantine-v This should be straightforward to do, right?

[constantine-v]

@sfmskywalker, @steviej08 yes, that should be quite straightforward, I'll make sure to push it with next commit. I'll also put some details into description so it could be used as a base for documentation.

[tomy2105]

Hi @constantine-v and @sfmskywalker . First of all excellent work! Was very glad to see this happening :).

One question based on the code changes I've briefly examined (sorry haven't tried yet). Your plan seems to determine tenant based on path prefix, e.g. http://example.com/tenant1/ for first tenant and http://example.com/tenant2/ for second. Is that correct?

How "flexible" do you plan to make your code for tenant "detection". On of use cases I have is to be able "detect" tenant based on hostname, e.g.: http://tenant1.example.com/ for first tenant and http://tenant2.example.com/tenant2/. I see the your classes TenantStore/Provider and those would be easy to customize and implement other logic. But I'm "afraid" of endpoint routes that seem to "hardcode" tenant as first path prefix (maybe I've read the code wrong, sorry about that). So will your endpoint routes be able to handle "my" usecase?

And one question more regarding endpoint routes? What will happen with them if multitenancy is not enabled? Will they have some "default" prefix before version or.....?

[constantine-v]

Hi @tomy2105, I'm afraid at this point the plan is to support hardcoded url prefix scenario (http://example.com/tenant1/, http://example.com/tenant2/, etc..).

However, tenant detection logic could be moved to a middleware, so it could be swapped with another middleware with a different tenant detection logic if needed. I need to process it a bit, but it could be just enough to support more flexible urls.

For backwards-compatibility purposes, all endpoints support urls both with and without tenant prefix, so if multitenancy is not enabled, it would work just as it was before.

[tomy2105]

@constantine-v thanks for considering my use case. If it is "relatively" easy to modify your changes to include my usecase that would be great, if not I'll find some way to work around it :).

One question more. Is it going to be one tenant -> one database or is multiple tenants -> one database or multiple tenants -> muiltiple databases (some of tenants can be in one, some of them in other)?

[constantine-v]

@tomy2105 it is going to be database per tenant. I'm planning to update this PR description with all the details soon.

[tomy2105]

@constantine-v is the door for having one database -> multiple tenants and/or multiple database -> multiple tenants still open or not? I ask because I know there was a tenantid inside db schema so far so wondering if this can be used to achieve these scenarios by pointing configuration of multiple tenants to same db?

[tomy2105]

@constantine-v in light of my last question(s) how are your changes be related to the existing stuff with ITenantAccessor? Please include this relation (or lack of if none exist) in the documentation too :).

[ricksearcy]

What about scenarios where Tenants can't be statically configured in appsettings? In our use case we use a reverse proxy to communicate with Elsa and send in tenant and user information using jwt. Tenants could be added at any time. Using the existing ITenantAccessor almost worked for our scenario, but there seems to be a bug that is keeping us from fully going down that path. https://github.com/elsa-workflows/elsa-core/issues/2861

[tomy2105]

One more question :). If one database per tenant is needed (for workflow, instance, etc.... storage), do we need per tenant configuration for Service Bus Broker, Distributed Lock Provider and Distributed Cache Signal Provider or those stay as is?

[sfmskywalker]

What about scenarios where Tenants can't be statically configured in appsettings? In our use case we use a reverse proxy to communicate with Elsa and send in tenant and user information using jwt. Tenants could be added at any time. Using the existing ITenantAccessor almost worked for our scenario, but there seems to be a bug that is keeping us from fully going down that path. #2861

Sounds like what is needed for this is DB-level multitenancy (where tenant ID is used as a column on various DB tables). This is partially implemented (and you need #2861 to be fixed)

[ricksearcy]

What about scenarios where Tenants can't be statically configured in appsettings? In our use case we use a reverse proxy to communicate with Elsa and send in tenant and user information using jwt. Tenants could be added at any time. Using the existing ITenantAccessor almost worked for our scenario, but there seems to be a bug that is keeping us from fully going down that path. #2861

Sounds like what is needed for this is DB-level multitenancy (where tenant ID is used as a column on various DB tables). This is partially implemented (and you need #2861 to be fixed)

Yes, if #2861 was resolved that multi tenancy implementation would work for our use case.

[constantine-v]

Closing this PR, as it was agreed that multitenancy will be implemented using a better approach with Autofac library.