[FOLIO-526] Guidelines for module permissions Created: 29/Mar/17 Updated: 02/Sep/21 Resolved: 02/Sep/21 |
|
| Status: | Closed |
| Project: | FOLIO |
| Components: | None |
| Affects versions: | None |
| Fix versions: | None |
| Type: | New Feature | Priority: | P2 |
| Reporter: | Heikki Levanto | Assignee: | Heikki Levanto |
| Resolution: | Won't Do | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | 1 day, 2 hours, 30 minutes | ||
| Original estimate: | Not Specified | ||
| Issue links: |
|
||||||||||||||||
| Sprint: | |||||||||||||||||
| Description |
|
We need a common set of guidelines on how to deal with permissions, so that all (most?) modules do the same kind of thing |
| Comments |
| Comment by Heikki Levanto [ 29/Mar/17 ] |
|
This is a break-out of DMOD-180, where I was supposed to set up permissions for the user management complex. I realized that what little permission work there has been done in different places was somewhat divergent, and didn't show much consistency. Once we have these guidelines, they should be documented at some central location. |
| Comment by Heikki Levanto [ 29/Mar/17 ] |
|
To begin with, permissions and permission sets are the same thing, internally. This can lead to some confusing use of those terms. In practice, we have three categories of permission sets:
Permission bits are short strings like "users.delete". These are the most granular, specific permissions in the system, and the only ones that any code should ever look at! The back-end modules can declare in their ModuleDescriptor that they require or desire some of these. Okapi and the auth module complex cooperate in enforcing that all required permissions are present, or the whole request is rejected. The desired permissions are passed to the module, which can then make more complex decisions about its behavior - for example not returning sensitive information, or disallowing forgiving of fines over some configured limit. These permissions are never directly granted to the users (see below about sets), but can be granted to modules, via the ModulePermissions sections of the ModuleDescriptor. For example the loans module, in its function to read loan details, may be granted a permission to read item details and user details, so it can provide a complete record. Module-specific permission sets are declared in the ModuleDescriptor, in the PermissionSets section. These are supposed to be immutable, the users can not change them, because a new version of the module may define something else. These are supposed to contain permission bits that the module itself defines in its PermissionsRequired and -Desired. They may also contain permission bits from other modules, typically those mentioned in ModulePermissions. Furhtermore, they can contain permission bits needed by the UI, as long as those are closely related to the function of the module. For example, a bit saying if the settings button should be visible - although in most cases the UI should be using the same back-end permission bits to control what it displays - if the user has a permission to list users, then the UI can offer to list users. Finally, these module-specific permission sets can contain other permission sets from the same module, so that the permission set for editing users will not have to repeat all permissions for viewing users, but can include the that set. These can be assigned to users, but are usually not - see below. User-defined permission sets (or roles) are defined by the admin user. These reflect the reality at a given library, what kind of functionality makes sense to combine into roles that can be assigned to (staff) users. For example "front desk librarian" could have permissions to edit users addresses, but not forgive fines... |
| Comment by Heikki Levanto [ 29/Mar/17 ] |
|
I am proposing the following: Permission bits should (almost?) never be displayed to (admin) users. Therefore they do not need a display name. That should make it easy to filter them out from the UI. In rare cases where they need to be displayed, we are already so deeply technical that the permission string itself is good enough to display. Especially since that is likely to be seen in the error messages on missing permissions. Permission bits should map directly to the operations the back-end module provides. I suggest naming them as something like "users.delete" or "users.get.list", where the first part is the module, the second is the HTTP verb, and if more distinction is needed, those come at the end (like "users.get.list" and "users.get.record". Maybe even "users.get.record.full"). If the UI needs specific bits, we can use something like "users.ui.shortcut". Module-specific permission sets should follow a similar pattern, except that they can not point to HTTP verbs, but should use a different set. I propose "list", "view", "add", "modify", and "remove" (not delete, that is already used). Since these will be visible in the UI, they of course need a displayName. User-defined permission sets are up to the users. I suspect there will be higher-level verbs like "Can manage users", etc. |
| Comment by Mike Taylor [ 29/Mar/17 ] |
|
Sorry if I am being dense, but all this leaves me with a lot of questions ...
I don't see any of these in mod-users. Where should I look for examples?
Where are the other permissions defined – the ones not declared in a module's PermissionSets?
Why would we do this? I suppose this is part of a broader question of what is actually achieved by listing any permissions in PermissionSets?
I don't understand what it means for one permission set to contain another permission set, given your earlier claim that "permissions and permission sets are the same thing".
I don't understand why you want to use two different sets of names for the CRUD operations. |
| Comment by Heikki Levanto [ 29/Mar/17 ] |
|
I try to explain. Examples are not yet widely available, I am working out a design. The Okapi guide has some stuff - only slightly out of date. I have started to make some permissions in mod-users-bl, not quite up to date with this proposal, but might be helpful. The definitions are a bit messy. Permission bits can technically be used without defining them anywhere, but I would like to specify that any permission the module works with should be defined in the PermissionSets section. Also any module-specific permission set should be defined in the same place. When defining a permission or a set in PermissionSets, you attach a description to it, and (maybe) a displayName. (omitting the displayName from permissionBits is part of my new proposal here, which I am still trying to work out). As to listing other modules permissions in a permissionSet, I can give you a concrete example. If the users-bl module defines a permissionSet called "users-bl.view" that contains "users-bl.get.record" and "users-bl.get.list", we have a problem when the users-bl makes a request to users, we will need permission for "users.get.record" and "users.get.list". We can solve this in two ways, either by giving users-bl ModulePermissions to those. Or by adding them in the "users-bl.view" permissionSet. The difference is that if we do the latter, the user who has the "users-bl.view" permission will automatically be allowed to view user records, even when not doing so through the users-bl module. This has turned out to be necessary as long as the users-bl does not yet implement all the update functionality, and thus can not make use modulePermissions. Permissions and permission sets are internally the same thing, they both can contain subpermissions. As to permission sets containing other sets, a good example is "users-bl.update" which would contain "users-bl.view", since anyone allowed to edit users must also be allowed to view them, and it is easier to refer to the existing view set than to repeat all the individual permissions needed for viewing. As to the two sets, the difference is like explained above. The ones with HTTP verbs are the CRUD operations, and no more. Not very relevant for the UI, but will come in handy for batch jobs etc. The ones with non-HTTP verbs are more related to the UI, where editing a record implies also the permission to view it, and creating a record implies permission to edit and view it. I hope this clarifies things. If you have more questions, ask here, or maybe better on slack. |
| Comment by Mike Taylor [ 29/Mar/17 ] |
When the use is trying to view user records via some other module, how can that be enabled by means of the user-bl module descriptor? Surely that is not of interest to (or available to) other modules?
Then I think we need to review our nomenclature. What about "Permission bits are short strings like "users.delete". These are the most granular, specific permissions in the system"? Are they not permissions? If so, then they are permission sets, which means they can contain subpermssions, so they are not the most granular permission objects. (I am quite sure something in what I just said is wrong; but I said it anyway in the hope that when you see my misunderstanding clearly laid out, you'll better understand what it is that I misunderstand.) I think it's better to keep the discussion here than on Slack, as it will be useful for us and others to refer back to (not least when we come to document all this.) I find Slack pretty ephemeral. |
| Comment by Heikki Levanto [ 29/Mar/17 ] |
|
When the user wants to see user details via another module, he needs the CRUD permission bits related to it. Those he can get from permission sets that he has been assigned. If he has a permission set that contains, directly or indirectly, users.get.record, he is allowed to get that record. No matter if the permission comes from users-bl.view, or from some other source. I admit that this is not going to be very useful when everything works as it should, but it is a necessary workaround while the users-bl module does not do updating, and we need to give permissions for the user to update user records, directly into mod-users. ModulePermissions would be the more natural way to handle this. I agree the terminology is messy here. It comes from things being done in the UI design, auth module, Okapi, ModuleDescriptors, and the permission module without quite enough coordination. Here is how I understand the terms today: Permission bits are strings, and the most granular, lowest level of things. They don't even have a description or displayName. That is the level where Okapi and the auth check module operate. Permissions, as loaded into the permission module can have a desription, displayName, and some flags. They can also have subPermissions, which can refer to other Permissions which in turn can have subPermissions, etc. The permissions module can assign Permissions (with or without subPermissions) to users. When assigned, the user will have the permission bit t hat is the name of the permission, and all the permission bits mentioned in its subpermissions, and what those may mention in their subPermissions, recursively until we can go no further. |
| Comment by Kurt Nordstrom [ 29/Mar/17 ] |
|
So, if I understand our terms here, we've only got one kind of "Permission Object" which looks like the following:
{
"permissionName" : "users.bl-view",
"displayName" : "View Users Business Logic Records",
"mutable" : false,
"subPermissions" : [ "users-bl.get.record", "users-bl.get.list" ]
}
When we say a "permission bit" do we mean "the permission name string used to uniquely identify a permission object"? Or do we mean "a permission object that is at the lowest level on the hierarchy and contains no subpermissions"? We've resisted making separate object types for permissions and sets of permissions...but would it possibly clarify things if permissions were only singular "bit" definitions and we had a separate object defined for aggregating these into sets? |
| Comment by Heikki Levanto [ 29/Mar/17 ] |
|
I have used the term "permission bit" to mean the "string used to identify a permission object", which is the same as "the string used in PermissionsRequired and permissionsDesired". (My Okapi bias showing through here). Yes, we have only (almost!) one Permission Object, like you say. With the minor exception that the ModuleDescriptor's PermissionSets section carries a slightly simplified version of it (no mutable bit, no tags, etc) |
| Comment by Heikki Levanto [ 29/Mar/17 ] |
|
Here is how I would expect a ModuleDescriptor to look like {
"id": "users-bl",
"name": "users business logic",
"provides": [
{
"id": "users-bl",
"version": "0.0.1",
"handlers": [
{
"methods": ["GET"],
"pathPattern": "/bl-users",
"permissionsRequired": [ ],
"permissionsDesired": ["users-bl.get.list"],
"modulePermissions": ["users.get.list"]
}, {
"methods": ["GET"],
"pathPattern": "/bl-users/{id}",
"permissionsRequired": [ ],
"permissionsDesired": ["users-bl.get.record"],
"modulePermissions": ["users.get.record", "perms.users.get", "login.get"]
}, {
"methods": ["POST"],
"pathPattern": "/bl-users",
"permissionsRequired": ["users-bl.post"],
"modulePermissions": ["users.post", "perms.users.post", "login.post"]
}, {
"methods": ["PUT"],
"pathPattern": "/bl-users/{id}",
"permissionsRequired": ["users-bl.put"],
"modulePermissions": ["users.put", "perms.users.put", "login.put"]
}
]
}
],
"permissionSets": [
{
"permissionName": "users-bl.get.list",
"displayName": "",
"description": "get user list",
"subPermissions": ["users.get.list"]
}, {
"permissionName": "users-bl.get.record",
"displayName": "",
"description": "get user record (subPerms are a workaround, will be dropped)",
"subPermissions": ["users.get.record", "perms.users.get", "login.get"]
}, {
"permissionName": "users-bl.post",
"displayName": "",
"description": "post user record",
"subPermissions": ["users.post", "perms.users.post", "login.post"]
}, {
"permissionName": "users-bl.put",
"displayName": "",
"description": "put user record",
"subPermissions": ["users.put", "perms.users.put", "login.put"]
}, {
"permissionName": "users-bl.view",
"displayName": "view user",
"description": "Can view user profile",
"subPermissions": ["users-bl.get.list", "users-bl.get.record"]
}, {
"permissionName": "users-bl.edit",
"displayName": "edit user",
"description": "Can edit user profile",
"subPermissions": ["users-bl.view", "users-bl.put"]
}, {
"permissionName": "users-bl.create",
"displayName": "create user",
"description": "Can create new user",
"subPermissions": ["users-bl.edit", "users-bl.create"]
}
]
}
Noteworthy differences from a current ModuleDescriptor:
There are some other changes I feel this descriptor should have, not directly related to this discussion:
|
| Comment by Mike Taylor [ 29/Mar/17 ] |
Oh, got it now. Thanks!
Yep. I think that is natural in a project like this, unavoidable – even healthy. But now might be a good moment for us all to step back and synchronise our terminology.
Ah, good! I get that, too, now.
... and an actual permission bit, of course – right? (That's what appears as PermissionName in Kurt's example structure above. What are "flags" in this context? I assume Kurt's example's mutable is one such? What does it mean? And what are "tags", which you tossed in casually at the end of your last comment? |
| Comment by Mike Taylor [ 29/Mar/17 ] |
|
I wrote that comment before having seen your example ModuleDescriptor – thanks for adding that. Do we want to explicitly provide empty displayNames? Isn't it better just to leave it undefined? You say "Using HTTP verbs for the CRUD permissions, other verbs for the higher-level sets", but I am still seeing "get", "post" and "put" in the names of BL-level permissions like "users-bl.get.list". Otherwise, I think I am finally on more or less the same page as you |
| Comment by Kurt Nordstrom [ 29/Mar/17 ] |
|
Mike Taylor The permissions object schema defines a field called "tags". The idea was that it could be used for user-assigned tags to permission sets, to assist in organizing sets on the user-end. I didn't put it in the example object because I didn't want to muddy the discussion. |
| Comment by Mike Taylor [ 29/Mar/17 ] |
|
OK. I am not sure I see much value in tags, but they are at least harmless in the context of the present discussion |
| Comment by Kurt Nordstrom [ 29/Mar/17 ] |
|
Tags aren't currently being used by anything. That said, I think I added them in response to something shown in the prototype that seemed to call for something of the sort. I won't cry if we eventually do away with them. |
| Comment by Mike Taylor [ 29/Mar/17 ] |
|
More importantly, we can ignore them right now. But what about flags? |
| Comment by Kurt Nordstrom [ 29/Mar/17 ] |
|
"Do you have a flag...?" -E. Izzard. I believe Heikki is referring to fields such as "mutable" which are primarily used for filtering when getting requests. |
| Comment by Mike Taylor [ 29/Mar/17 ] |
|
And would it be terribly predictable of me to ask what a "mutable" permission is? |
| Comment by Kurt Nordstrom [ 29/Mar/17 ] |
|
It defines whether or not the permission should be modifiable by the UI. The permissions defined in the module descriptors get flagged as immutable: they never change. The user-created permissions sets, OTOH, are subject to modification. |
| Comment by Jakub Skoczen [ 29/Mar/17 ] |
|
Guys, I think quite a few misconceptions arose about the permission model over time. I will take it upon myself to do a short write-up about how they are laid out conceptually. Some of that I have already put here: https://folio-org.atlassian.net/browse/FOLIO-377?focusedCommentId=191475&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel |
| Comment by Mike Taylor [ 29/Mar/17 ] |
|
Kurt Nordstrom I don't understand what it would mean for the UI to modify a permission. Jakub Skoczen That will be helpful! |
| Comment by Heikki Levanto [ 30/Mar/17 ] |
|
Mike - The (admin) user may edit permissionSets in the UI - at least those that I call user-defined permission sets. The ones that come from ModuleDescriptors are immutable, the user may not edit those. They must be protected from user modification, because when a module gets upgraded, those permission(set)s get reloaded, and any changes would be lost. I believe this is what Kurt means. |
| Comment by Heikki Levanto [ 30/Mar/17 ] |
|
After discussing this with Jakub, we concluded that we do not usually need those higher-level module-specific permission sets like users-bl.create - what we need is those permission sets listed in the spreadsheet attached to DMOD-180, or whatever the UX people come up with. Jakub also pointed out that the RAML uses terms "item" and "collection", so we should use those in the CRUD permission bit names too. It might even be possible to create those bits directly from the RAML, in some future version. It might make sense to move the UX-level permission sets ("Can edit users") to be defined in the ModuleDescriptor for the UI module, since only that knows all the parts that are needed for the thing. For example, the User profile app shows loan histories and fines, which are not part of the users-bl. But for now, the UI modules don't load their moduleDescriptors, so that will have to wait. Instead we put those sets in the highest level module that is available, which seems to be users-bl. |
| Comment by Heikki Levanto [ 05/May/17 ] |
|
I think we have reached a consensus on how the permissions should be handled. Jakub Skoczen - you wrote above "I will take it upon myself to do a short write-up about how they are laid out conceptually." Should I assign this issue to you, or simply close it? |