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

Add restriction to permissions #8518

Closed
hikalkan opened this issue Apr 8, 2021 · 5 comments
Closed

Add restriction to permissions #8518

hikalkan opened this issue Apr 8, 2021 · 5 comments

Comments

@hikalkan
Copy link
Member

hikalkan commented Apr 8, 2021

Add FeatureName and GlobalFeatureName properties to PermissionDefinition.

  • Check feature name from IFeatureChecker. If it returns false, PermissionChecker should return false when we check IsGranted for this permission. Also, PermissionAppService.GetAsync should not return this permission in the returning list.
  • Check the global feature name from GlobalFeatureManager. If it returns false, PermissionChecker should return false when we check IsGranted for this permission. Also, PermissionAppService.GetAsync should not return this permission in the returning list.

You should also care this on PermissionManager.

Hint: Find usages of PermissionDefinition.IsEnabled. You typically mostly changes is same places for checking.

@maliming
Copy link
Member

maliming commented Apr 8, 2021

What if I want to add other checks in the future? Add more properties to the PermissionDefinition?

We can abstract it, and developers can customize a service to deciding whether this permission is enabled.

Feature-management provides a check service, and GlobalFeatures also provides it.

Use case:
Users with the manager role can only manage some permissions but not all. I can disable some permissions based on the current user role.

@NecatiMeral
Copy link
Contributor

The use case @maliming is describing is kinda equal to #8309.

@hikalkan
Copy link
Member Author

hikalkan commented Apr 8, 2021

@maliming PermissionDefinition has a Properties dictionary as an extension point. Instead of adding string RequiredFeature property to the PermissionDefinition, we can create an extension point like PermissionDefinition.RequireFeature(string featureName) which internally adds the feature name to the Properties dictionary.

Then you can define an interface, like IPermissionFilterProvider* that has a bool FilterAsync(PermissionDefinition permission) method. We can then implement RequireFeaturePermissionFilterProvider that gets the feature name from the Properties dictionary and returns false if current tenant has no feature.
We accept multiple IPermissionFilterProvider implementations, like other contribution systems.

What about such a design?

*name should be thought again :)

@maliming
Copy link
Member

maliming commented Apr 8, 2021

I will give it a try.

@muratyuceer
Copy link
Contributor

muratyuceer commented Apr 8, 2021

@hikalkan also if you add CanManageFeature property to User/Roles (for decide which users/roles can modify which feature permissions) in this way i guess there may be a solution about that problem #8309
Or maybe you may have a different approach

--Update--
Or maybe you can add admin/manager property to Feature object (feature based admin)
Now admin is like god mood we need to separate that

@maliming maliming changed the title Add Feature restriction to permissions Add restriction to permissions Apr 8, 2021
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

4 participants