elsa-workflows / elsa-core

A .NET workflows library
https://v3.elsaworkflows.io/
MIT License
6.59k stars 1.22k forks source link

Multitenancy #764

Closed craigfowler closed 5 months ago

craigfowler commented 3 years ago

An area of functionality which has been requested a few times across various mediums (issues, discussion, chat) is support for multi-tenancy.

This is a quite wide subject with numerous potential interpretations. Before it can be conclusively implemented we should answer such questions as:

Once we have a better understanding of the requirements, we can formulate a strategy for support and schedule appropriate enhancements.

Roundup of related issues

The following are issues which relate to requests for multi-tenancy. It should be added-to as further issues are raised/found.

tomy2105 commented 3 years ago

First thing about multi-tenancy that comes into my mind is user authorization (for dashboard and designer). How else do you know which workflow belongs to which tenant?

craigfowler commented 3 years ago

That's an interesting point.

Elsa & authorization

I don't personally think that Elsa should get too deeply involved in auth. The main reason for that is that we can't predict every app's authentication/authorization model. I think that at absolute most what we should do is provide an extension point/hook which is something like:

public interface IGetsPermissionToEditWorkflow
{
  bool CanEditWorkflow(string workflowCorrelationId);
}

Our default impl of that class would be to hard-code a true result every time. But then a consumer of Elsa may supply their own impl via DI and add whatever logic in there that they want.

One multi-tenancy example

As it happens, "my day job customer" is using Elsa in an app which is already multi-tenant. We achieve this by:

Now, we've already got this implemented just fine, actually using Elsa 1.4 (we'll upgrade probably once 2.0 is ready for stable release). Elsa itself knows nothing about our users, or our multi-tenancy setup. All we needed to do (in our closed source app) was to extend the registration mechanisms for Elsa. That was partly replayed back into Elsa as open source contribs in #748 (actually some of we had done had already been done as part of Elsa 2.0!).

We are kind of lucky though in that we don't use the dashboard or designer UIs yet. The user-specific configuration/customisation of our workflow(s) is already built into the app (we used to use a different workflow product) and so we already have our own UI for doing all of that, and it already respects our own auth rules.

tomy2105 commented 3 years ago

Thanks for the explanation of your use case. I agree Elsa should have an interface for multi-tenancy/authorization and implement true by default. But would definitely like to see this interface include "all" aspects of multi-tenancy in elsa (for example connection string fetching through that interface). Or at least ones that you described and know of (since you already did it) combined with dashboard and designer.

craigfowler commented 3 years ago

Exactly, I think that's a good starting point for the discussion which needs to happen. Indeed, one possible strategy to supporting multi-tenancy might just be to support all of the right extension points to do all of this stuff (part of the work we'd need to do is to identify what those are).

There's lots of possibilities available and at some point I think we're all going to have to get together and make our minds up. Right now priority no. 1 is getting Elsa 2.0 out of the door, and so we're not focussing too much on multi-tenancy. That doesn't mean we're not accepting ideas and talking points though.

tomy2105 commented 3 years ago

So would it be possible to make a place/hook in Elsa 2.0 dashboard/designer to include custom JS code which could be fired and executed in cases where API calls fail due to the fact that they weren't authorized? And of course ability to add custom headers and or cookies to the API calls (namely authorization header one I guess).

sfmskywalker commented 3 years ago

We can definitely add that, it makes sense.

ghost commented 3 years ago

Elsa is good enough workflow engine, multi tenant should be something handled outside wrapped to Elsa and set the required DB connection to it

instead of making Elsa jack of all its better Elsa becomes just master of workflow

tomy2105 commented 3 years ago

That depends on the how you define multi-tenancy @khan-mail-com Your approach assumes (or at least that is how I read your comment, sorry if I'm mistaken) that each tenant has its own database (so all you need to do is point Elsa to different db). However what about when you want multiple tenants inside each database?

Moreover even if they were inside same database, when (and how) would you point Elsa to different database? At each inbound HTTP request? Yes this is possible probably. But since workflow can continue on timers/schedules without inbound requests that leaves two places where it should be possible, and probably many more....

I agreee that some stuff should be left outside of Elsa (like user auth, etc....). But elsa definitely needs to be "internally" multi-tenant and have hooks for all the cases where you want to somehow "tell" it which tenant to use (either by completely switching database or by changing some internal id of a tenant when multiple tenants are inside same db).

ghost commented 3 years ago

Yes, multi-tenancy for separate DB, all you need to do is point to separate DB but i think HangFire services may require to configured to point each of DB to run schedules

for same DB, we need to apply the context filter with the tenant ID, which is again a top layer thing

On Tue, Nov 23, 2021 at 5:08 PM tomy2105 @.***> wrote:

That depends on the how you define multi-tenancy @khan-mail-com https://github.com/khan-mail-com Your approach assumes (or at least that is how I read your comment, sorry if I'm mistaken) that each tenant has its own database (so all you need to do is point Elsa to different db). However what about when you want multiple tenants inside each database?

Moreover even if they were inside same database, when (and how) would you point Elsa to different database? At each inbound HTTP request? Yes this is possible probably. But since workflow can continue on timers/schedules without inbound requests that leaves two places where it should be possible, and probably many more....

I agreee that some stuff should be left outside of Elsa (like user auth, etc....). But elsa definitely needs to be "internally" multi-tenant and have hooks for all the cases where you want to somehow "tell" it which tenant to use (either by completely switching database or by changing some internal id of a tenant when multiple tenants are inside same db).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/elsa-workflows/elsa-core/issues/764#issuecomment-976588105, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKECRB7DBK43UJIMY5RV5CTUNON7PANCNFSM4ZHLREBA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

vgb1993 commented 2 years ago

I have to support both multi-tencancy modes. This would be super usefull.

gentledepp commented 1 year ago

Well being able to have multi-tenancy by using a separate database for each tenant is gread. We use abp however, which also allows to keep tenants in a single database. In that case, we have a "int TenantId" property on every entity which denotes the tenant. The EF DbContext then automatically applies the filter to get only the current tenant data.

I read that for ELsa 3 the storage api is revised to make it easier for devs to create their own storage provider. Does this then also allow us to add such a "tenant filtering" mechanism?

jspitman commented 1 year ago

I see in the code that there is a reference to TenantId, so it looks like it has been implemented in Elsa to a certain extent for single database approaches.

It might be interesting to look at a library like FinBuckle and the patterns it uses to introduce multitenancy into .NET projects. That could provide an essential guide for opening Elsa up to multitenancy through other frameworks. Aspects of better access to DBContext in Elsa and implementation of options builders.

sfmskywalker commented 1 year ago

@jspitman I had not heard of FinBuckle before, will look into it. Thanks for pointing it out!

vgb1993 commented 1 year ago

I’m not a 100% sure but I think it had some issues with Blazor. Maybe I’m wrong. Anyway we’re doing multitenancy in our new Blazor app without it and it works just fine, it’s simpler. Maybe it’s not needed. Just saying.

Gryhyphen commented 1 year ago

Re: MultiTenancy,

Am investigating different workflow automation tools for work. Elsa looks pretty good but part of my evaluation is looking at aspects like multitenancy and project separation, which is an area Elsa performs poorly in.

Another product I'm comparing it to is Dolphin Scheduler, which allows for both multitenancy and project separation. It is however, excessively complex and unintuitive for our intended audience (operators/tier 2), which is something that Elsa does really, REALLY, well.

Ultimately, cause elsa is .net, we can probably roll our own nodes to ensure that data for one organisation never hits an endpoint or accesses data outside of what it is allowed, and edit the dashboard to allow us to filter on orgs, but ya know, that is a pain. Or we would need to spin up multiple elsa instances (like logic apps), and put some sort of proxy service in front of it, which is still, a pain.

Definitely something I think elsa should support, but I'm biased because it would make my life easier haha.

sfmskywalker commented 1 year ago

Hi @Gryhyphen,

Thank you for your detailed feedback on Elsa's multi-tenancy capabilities compared to other tools like Dolphin Scheduler.

I completely understand where you're coming from. Multi-tenancy is indeed an essential feature, and we are committed to improving Elsa in this aspect. While Elsa 3 offers features like Events and the Webhooks module, and also ensures extensibility for cross-app communication via service bus messaging, I acknowledge that there might be areas where it falls short in terms of project separation and multi-tenancy.

Could you perhaps provide specific instances or features you found lacking or any suggestions for improvement? This would greatly help us refine and potentially update our roadmap to better cater to your scenarios.

Our 2024 focus is on enhancing the maturity of the Elsa engine for scalable production use, which encompasses multi-tenancy and security.

Again, I appreciate your insights and look forward to hearing more from you!

Gryhyphen commented 4 months ago

@sfmskywalker Sorry for not having time to respond. Work gets hectic, you know how it is. We are still rolling our own internal workflow management stuff which is a pain in the neck (one of those, designed for a specific purpose, then grew into a mess) so haven't had much luck progressing this. We have been looking more into logic apps because that is what some larger parts of our organisation are trained on, and leveraging existing resources obviously is a good linchpin to pivot and advocate for a better workflow management solution.

I have noticed that Elsa has recently added multitenancy support as part of this PR https://github.com/elsa-workflows/elsa-core/pull/5159

However, it doesn't seem to have been documented yet, and it would be hard to argue a use case in my organisation if I can't evaluate if this multitenancy functionality suits our needs.

We do document intelligence processing, so taking in human-readable documents like purchase orders, and integrating them into erps. As a multitenant app, we have multiple organisations using our services, and thus need mechanisms to restrict workflows to specific tenants at the levels of the:

All I can provide is our use case that I'm evaluating for, which might help your team / inform your design. Hope this feedback helps!

sfmskywalker commented 1 week ago

Hi @Gryhyphen , not worries, I know exactly how it is and all the more reason for me to appreciate the time you took to share this valuable bit of feedback - thank you!

The current state of multitenancy covers some aspects, but not all:

Implemented

Not implemented

Thanks again for the feedback, it's much appreciated!