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

Permissions and Roles API #158

Closed
C0rby opened this issue Nov 16, 2021 · 11 comments
Closed

Permissions and Roles API #158

C0rby opened this issue Nov 16, 2021 · 11 comments

Comments

@C0rby
Copy link
Contributor

C0rby commented Nov 16, 2021

Currently in Reva the permissions of a user are stored closely to the file as grants.
Now we want to introduce global permissions which do not belong to a certain resource but allow the user to do things like Create a new space or user has role "admin" (roles are just a collection of permissions).
I want to propose a new simple API to add and list (global) permissions of a user. Although this API could be used to also store resource specific permissions, we would only use it for global permissions for now.

The service would store role and permissions assignments and could answer queries like has user x the permissions 'create-space'.

@ishank011 @labkode
Did you already think of something like that? Are you against such a service? If not I will create a PR.

/cc @refs

@C0rby
Copy link
Contributor Author

C0rby commented Nov 16, 2021

I have a rough draft of the API on my local machine. The consuming side could look like this:

pc, _ := pool.GetPermissionssClient("")

checkRes, _ := pc.CheckPermission(ctx, &permissionsv1beta1.CheckPermissionRequest{
	Permission: "list-all-spaces",
	SubjectRef: &permissionsv1beta1.SubjectReference{
		Spec: &permissionsv1beta1.SubjectReference_UserId{
			UserId: currentUser.Id,
		},
	},
})

@ishank011
Copy link
Contributor

@C0rby can you elaborate on the list of such permissions?

If they are only spaces-related, this essentially translates to whether they have resource permissions on the reference they're passing in the request. For example, for the create-space permissions, I would rather have it specified for a particular path or resource ID, not a global permission.

@C0rby
Copy link
Contributor Author

C0rby commented Nov 18, 2021

It's not only about spaces-related permissions.
But let's start with those.

So our current use case for example is we have a user with the permission list-all-spaces. This person should be able to see all spaces (not the content of the spaces) and then could do things like increase the quota of a space or delete a space etc (those actions require yet other permissions).

Sure we could add this user to all spaces with a special grant but that has some disadvantages. Like if another user or group should also have that permission, we would need to add the other user or group to all existing spaces. Or to revoke the permission from the user we would need to update the grants of all spaces. And when creating new spaces, the grant needs to be added to those too.

What this permissions service essentially allows us to do is to implement a role based system where we can create new roles as we like with any collection of permissions and assign it to users or groups. Then we can use this service to check if a user is permitted to do any action without having to change the code for new roles or when roles change.

The first role which comes to mind is the admin role which has every permission. But there will be other roles too which only have a subset of the permissions.
This service can also restrict the permission to certain resources, i.e. you can make someone an admin only in space ID: xyz.

For example, for the create-space permissions, I would rather have it specified for a particular path or resource ID, not a global permission.

Could you describe how you would implement that? By setting grants on the storage's root node?

@labkode
Copy link
Member

labkode commented Dec 2, 2021

@C0rby @ishank011 will be back next Tuesday to discuss over Zoom as we discussed.

@C0rby
Copy link
Contributor Author

C0rby commented Dec 2, 2021

Great! 👍

@C0rby
Copy link
Contributor Author

C0rby commented Dec 8, 2021

Here is some code with the proposed changes:

CS3 API: #161
Reva: cs3org/reva#2341
oCIS: owncloud/ocis#2852

@refs and I also wrote down some things which are the result of our zoom meeting and some brainstorming afterwards.

tl;dr

Our proposed changes do not touch the existing Permissions/Grants model. It is meant as an additional system to be able to use an RBAC system.

Storage drivers which just abstract other systems like the nextcloud driver do not need to use the permissions API since they communicate to their underlying system using API which already should have permissions checks implemented. (But they can implement and use it if it makes sense) /cc @michielbdejong

Storage drivers which will be used as oCIS storage backends MUST use this API in order to behave the same.

Proposed solution knowns and gotchas:

  • Extend the cs3 api RPC and Messages to define permissions. This is done here

  • Add business logic in those parts where permission enforcement is needed. See here

    • in the case of list-spaces permission, this MUST be done in the storage, because technically all users can list their own space, but only admins can list all spaces apart from their own.
    • The enforcement has to be done in the driver, otherwise we would need to wrap the driver around an abstraction that would take care of the permission-checking (or any other related business logic) so that every driver behaves the same way.
  • Following this approach (where we do not build an abstraction around the drivers) we would introduce inconsistencies across drivers. For example ListSpaces will behave differently on the DecomposedFS and on the EOS driver, but these drivers are meant to be used as storage backends for oCIS as opposed to the Nextcloud storage driver, which is meant to abstract a Nextcloud service. This means that actions like permission checking is actually done within the Nextcloud service, and it should not be responsibility of Reva.

  • When wrapping a service (i.e Nextcloud) and using it as a storage driver, then there are issues with the CS3 API Grants concept and that of Nextcloud permissions, in the sense that they don't map 1:1, but instead they have to be inferred from the Grants, so the Nextcloud service can set permissions. So there is some glue code involved in translating Grants into Nextcloud's permissions. But this is not the problem that we're trying to solve with this solution.

  • These changes have have nothing to do with the existing Grants, and there are no expected side effects with them.

@michielbdejong
Copy link
Contributor

Cool! As discussed in the Zoom call, just bear in mind that revad can be (and is) used in two ways:

  • from scratch, where it makes sense if all permission checks happen at the revad level, and the storage backend is unaware of permissions.
  • as an addition to a pre-existing EFSS system, where revad is just one of several ways to access it.

In this latter case, the EFSS will need to know about the permissions too, and hence not all constructs invented at the revad level can be (easily) translated to whatever permission model the existing EFSS system is already using.

So the mapping will never be perfect in that case. But we'll find a way to make it come as close as possible! :)

@micbar
Copy link
Member

micbar commented Dec 9, 2021

@michielbdejong In the future we will also have External Permission systems or "policy engines" which will already evaluate permissions. So not only the permissions of existing EFSS is relevant but also external ones. Enterprise EFSS should be able to integrate into existing infrastructure and business workflows of an organisation .

@micbar
Copy link
Member

micbar commented Jan 28, 2022

@michielbdejong let us close this issue here. Feel free to follow-up in a new ticket.

@micbar
Copy link
Member

micbar commented Jan 28, 2022

@labkode I have no permission to close this issue here.

@C0rby
Copy link
Contributor Author

C0rby commented Jan 28, 2022

Closing it since the proposed solution was implemented as a proof of concept.

@C0rby C0rby closed this as completed Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants