-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Will Tenant access be added? #16
Comments
Hi @gil0109 , it's not immediately on the roadmap. But I've already thought about it. Maybe a simple functionality using a selected small number of groups to identify the tenant is sufficient for now. It is clear that multi-tenancy would be a clear advantage in larger distributed scenarios. Also to manage access rights e.g. in the cockpit much more effectively. I can't promise you a timeline, but I will think about a possibility to realize it. |
Would be great to have top-level groups as tenants. |
@VonDerBeck Any updates on this? I'd be happy to contribute a PR towards this but probably need some guidance on what your thought process is... |
@VonDerBeck Any updates on this? |
Is there a way perhaps we could plugin our own implementation for tenants? I think this might be better than trying to build a generic solution... |
Hi @aminmc, you're free to fork the repository at any time. What you have to do is to implement the tenant parts of the IdentityProvider (you'll find this when you look at e.g. https://github.com/camunda/camunda-bpm-identity-keycloak/blob/master/extension/src/main/java/org/camunda/bpm/extension/keycloak/KeycloakUserQuery.java - there is a UnsupportedOperationException for the tenant specific query). You can then implement this in any specific way you think is appropriate for your use case. Just try it out and go for it. |
Hi @VonDerBeck , you mentioned you have a plan in mind. If it's ok, can you please share what you've thought of ? We just integrated this library in one of our projects and we were planning to implement our own experimental tenant support. Just want to make sure we don't digress too much from your approach so that we can upgrade easily. We were thinking of using keycloak realms to represent tenants. Our thought process was that we would ideally want to keep individual tenants as separate as possible and a one-to-one mapping between keycloak realms and camunda tenants feels like a good way of achieving this. Could you perhaps share your thoughts on this ? |
Hi @juja0, using keycloak realms to represent tenants might seem intuitiv from the Keycloak side. A few questions:
Connecting to several realms would likely require changing queries per tenant and different tokens per realm. Currently I don't think that it is the way to go for the Keycloak Identity Provider plugin, because it will blow up things quite a lot. Please let me know, if you think that I'm wrong with that opinion. My simple and basic approach for tenants would be to define a e.g. a group prefix (or a list of predifened tenant names), treat all groups starting with that prefix as tenant and filter them out within group queries. As there a typically only a handful of tenants this is only very light overhead when querying groups. |
To be honest I never considered this scenario. The way I understood the terminology of 'tenant' is that each tenant would be a completely isolated entity (different organizations for example) and if users are using LDAP/AD, I assumed they would have separate LDAP/AD setup for each tenant. I am not sure how accurate my assumption is or if it reflects the majority of usecases so it's worth waiting for others to chime in with their usecases and thoughts on this. The group based tenant representation may be ok for most users I imagine, but for companies with regulatory requirements to keep tenants in separate realms, this might be a no-go. This is because if a user has access to add someone to a group within a realm, they probably have access to add them to any group in the same realm. So a malicious user can add themselves to any group and have access to any data in camunda. I might be overthinking this a bit but I can definitely see this becoming a hard requirement in our company (not sure about other similar companies).
That was not my original thought but now that you've mentioned it, I am beginning to think that it might be more secure and cleaner but as you highlighted, it might bloat the implementation on the plugin side. The approach that I had in mind was to use something similar to keycloak-admin-client to make the queries to keycloak (the client is just an example. we could just make the same REST APIs that the client makes using the existing KeycloakRestTemplate in the plugin). The advantage of using this is that we don't have to retrieve separate tokens for each realm. Keycloak installation usually comes with a default admin-cli in the master realm which can perform actions on any realm, so I imagine we can easily create a more restrictive version of it which only has read access to all groups/users across realms. But to be honest I haven't fully thought through this yet so I might be missing something here.
Tenant independent queries can behave the same way they do now thus making them backward compatible. The realm parameter that is specified in the configuration currently can serve as a sort of default/fallback realm for non-tenant based queries. I don't imagine users will expect to be able query users across any non-default realm using a non-tenant based query. And since camunda queries have the tenant field in them, we can just direct our REST APIs to the realm(tenant) mentioned in the query and if not present, use the default realm. That being said, I do agree that whatever approach we take, it will increase the scope of this plugin so I'm open to thoughts. |
we have any update for this feature? |
Hi Réberth, sorry, but currently there are no updates on this. This is due to the fact that multi-tenancy is a thing which is very specifically handled in each organisation. So it's still open - and a longterm feature. |
Hi ,
Thank you for the great plugin. Will Tenant functionality be supported on the roadmap?
The text was updated successfully, but these errors were encountered: