Skip to content
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

Dynamic product features according to tenants #18100

Closed
9 tasks done
lpichler opened this issue Oct 15, 2018 · 13 comments
Closed
9 tasks done

Dynamic product features according to tenants #18100

lpichler opened this issue Oct 15, 2018 · 13 comments
Assignees

Comments

@lpichler
Copy link
Contributor

lpichler commented Oct 15, 2018

Goal ⚽

We need to be able accomplish this request: "Enable this concrete product feature only for tenants X,Y in any user"

NOTE: product feature - is permission to allow user perform any operation like a add, edit, copy, delete, ...

New names

Tenant Root Feature
Dynamic Tenant Product Feature

How 💡

We have user's roles and we have defined tree of product features independently on tenants.

This work is introducing dynamically defined product features according to tenants.
Let's assume that we have this ability for product feature Edit Dialog - so it will dynamically generate subtree for this product feature:
47439135-20d55a00-d7ac-11e8-84e5-9c3e77a49a0d

Dynamic subtree is fully synchronised with tree of tenants for all stated tenant product features, when tenant is:

  • added
  • created
  • deleted
  • seeded (this operation is done under MiqProductFeature.seed)

Implementation

Scenario

Users's role
47439135-20d55a00-d7ac-11e8-84e5-9c3e77a49a0d

user 1 cannot edit service dialog
screenshot 2018-11-06 at 15 43 15

user 2 can edit service dialog
screenshot 2018-11-06 at 15 46 18

How to use this feature

  1. Choose existing product feature(from now tenant feature root identifier) which you want to use as parent for dynamic product tenant features
  2. Add this feature to array in MiqProductFeature#TENANT_FEATURE_ROOT_IDENTIFIERS
  3. Restart app

Links

https://bugzilla.redhat.com/show_bug.cgi?id=1297415 (service dialogs)
https://bugzilla.redhat.com/show_bug.cgi?id=1468795 (tenant quotas)

PRs - sequence for merge

other usage

Additional Fixes

@kbrock
Copy link
Member

kbrock commented Dec 21, 2018

Little concerned about relying upon seed to populate a table.
If possible, I'd like to get rid of seeds except on upgrades.

Is there an alternative way to populating these tables that does not rely upon seed?

@kbrock
Copy link
Member

kbrock commented Jan 2, 2019

@lpichler This is a really tricky problem Libor

Is it possible to come up with a different RBAC rule to say you have privileges only below you (and not including you?)

Although, our privileges tend to be per model, and I'm not sure how this will integrate here

@lpichler
Copy link
Contributor Author

lpichler commented Jan 3, 2019

@kbrock

Is there an alternative way to populating these tables that does not rely upon seed?

I need to think about it but maybe it is already accomplished now -(1a) seeding is done here just for populating data for existing tenants (what it is needed for upgrade) other populating data are tied with tenant creation.

(1a) so as you are saying except on upgrades. does it mean that the code should be somewhere else ?

This is a really tricky problem Libor
Is it possible to come up with a different RBAC rule to say you have privileges only below you (and not including you?)

Although, our privileges tend to be per model, and I'm not sure how this will integrate here

2a) This comment is related to your comment 1) or are your referring to something new ?

please clarify to me this first to avoid misunderstanding and we can continue with discussion

@kbrock
Copy link
Member

kbrock commented Jan 9, 2019

@lpichler

  1. Yes. except on upgrades. tenant creation is great and what I was suggestion - DONE
  2. This is totally separate (sorry).

I wonder if it makes sense to come up TENANT_ACCESS_STRATEGY = :child_ids which is like descendants, but only from children down.

Not sure if this would work (from a business perspective) but doesn't seem too hard from a technological perspective.

And it would simplify the ui / remove the need to add any of the per tenant records/privileges.

@miq-bot miq-bot added the stale label Jul 15, 2019
@miq-bot
Copy link
Member

miq-bot commented Jul 15, 2019

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

@kbrock kbrock removed the stale label Jul 17, 2019
@kbrock
Copy link
Member

kbrock commented Jul 17, 2019

@lpichler this is a little stale.

If we provided the child down access - would that resolve the need to have each provider in the features access list?

@chessbyte
Copy link
Member

@lpichler @kbrock @Fryguy bump

@kbrock
Copy link
Member

kbrock commented Oct 17, 2019

I was never thrilled with the idea of having dynamic filters. Especially when it looked like a pure tenant scoping/nesting pattern

@lpichler
Copy link
Contributor Author

If we provided the child down access - would that resolve the need to have each provider in the features access list?

I was never thrilled with the idea of having dynamic filters. Especially when it looked like a pure tenant scoping/nesting pattern

Is it possible to come up with a different RBAC rule to say you have privileges only below you (and not including you?)

It doesn't look to me like that we can do it with using any scoping because the request is to have possibility to set product feature for selected tenants.
so it is connection between tenancy and product features.
it adds another type of rule for RBAC and maybe the product features for this is not optimal but doable.

@kbrock does this cover your questions ?

@kbrock
Copy link
Member

kbrock commented Oct 27, 2019

@lpichler I don't fully understand why it wouldn't work, but I'll trust your expertise.

What more do we need to do with this PR?

@miq-bot
Copy link
Member

miq-bot commented Jun 11, 2020

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the traige process documentation.

@gtanzillo
Copy link
Member

@lpichler Can this be closed at this point? If not, please open a separate issue for the remaining work. Thx

@gtanzillo gtanzillo removed the stale label Jun 29, 2020
@lpichler
Copy link
Contributor Author

lpichler commented Jul 8, 2020

Yes it can be closed, I will review it and I will open issue if needed, maybe it could be simplified.

@lpichler lpichler closed this as completed Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants