Overview

The purpose of this spike is to investigate how to implement static permission migration.  That is, how to handle when permissions & permission sets defined in module descriptors are renamed, removed, or updated (e.g. to include new permissions) during a module upgrade.  This does not cover the case of user-defined module permission sets.

...

Spikes:

• Jira Legacy
  server System JiraJIRA serverId 01505d01-b853-3c2e-90f1-ee9b165564fc key OKAPI-839

• Jira Legacy
  server System JiraJIRA serverId 01505d01-b853-3c2e-90f1-ee9b165564fc key OKAPI-952

• Jira Legacy
  server System JiraJIRA columnIds issuekey,summary,issuetype,created,updated,duedate,assignee,reporter,priority,status,resolution columns key,summary,type,created,updated,due,assignee,reporter,priority,status,resolution serverId 01505d01-b853-3c2e-90f1-ee9b165564fc key OKAPI-953

Implementation Stories:

• Jira Legacy
  server System JiraJIRA serverId 01505d01-b853-3c2e-90f1-ee9b165564fc key OKAPI-947

• Jira Legacy
  server System JiraJIRA serverId 01505d01-b853-3c2e-90f1-ee9b165564fc key MODPERMS-115

• Jira Legacy
  server System JiraJIRA serverId 01505d01-b853-3c2e-90f1-ee9b165564fc key MODPERMS-116

...

The first time mod-permissions that implements v2.0 of the _tenantPermissions interface is enabled, OKAPI will call that API with the permissions defined in each of the enabled module descriptors.   Special handling exists in mod-permissions that will allow it to add the necessary module context, enabling it to detect/determine which permissions are new/removed/updated in subsequent _tenantPermissions calls.  

OkapiPermissionSet.json Schema

...

Permission Name Conflict Resolution

...

• If we implement this to be implicit, that is module descriptors still define permissions like:  inventory-storage.items.collection.get, mod-permissions would add the scope for you. 
  • Presumably the other mod-permissions APIs would need to be updated as well to support searching for permissions w/o having to specify the scope.  e.g. ?query=permissionName=inventory-storage.items.collection.get. 
  • Assuming we migrate to scoped permissions when upgrading to a new version of mod-permissions,  the tenant API would need to be extended to add the scope to existing permissions, and to remove the scope upon downgrade.
  • The question then becomes, how does mod-permissions determine the appropriate scope for each of the permissions.
  • I'm not sure if this is a big deal or not, but the UUIDs of the permissions would change.  If system operators aren't expecting this it may lead to problems.
  • This approach is much more work compared to other solutions.
  • There will always be this disconnect between what's defined in the system and what's actually in the mod-permissions database.  This is confusing.
• If we make this explicit, that is force module developers to add the scope to their permissions in module descriptors, and in search queries, etc.  
  • This is clearly much more disruptive in the sense that module developers need to do something (or many things in some cases) in order to keep their modules working as desired/expected.
  • Another problem with this is that you'd need to take precautions to avoid a similar permission escalation vulnerability... i.e. user's shouldn't be able to specify the scope, or at least the scope needs to be validated.

NOTE:  There are currently no restrictions on display name uniqueness... So this means that multiple permissions could have the same "display name", but behind the scenes one might be defined by a module and another might be user-defined.

...

• Status
  colour Green title RESOLOVED

  How should we handle collisions (i.e. A user-defined permission exists in the system when a module renames or adds a permission with the same name)?
  • See

    Jira Legacy
    server System JiraJIRA serverId 01505d01-b853-3c2e-90f1-ee9b165564fc key OKAPI-952

  • See "Permission Name Conflict Resolution" for additional details.
  • Fail the install/upgrade w/ an appropriate message.  
  • Overwrite the existing permission.  Not ideal as this could lead to privilege escalation, and confusion. 
    • This is the current behavior...  if a permission with the same name as one sent to the _tenantPermissions API, it is overwritten.
  • One idea being discussed is to prefix/scope permissions with the module name, and disallow users from creating permissions which follow this pattern. 
    • Maybe mod-permissions automatically adds the scope based on the API being used; e.g. permissions defined via _tenantPermissions would be scoped with something like "system", "static" or "module", whereas permissions defined via the perms API would be scoped with something different, such as "custom", "dynamic" or "user-defined".  
    • NOTE:  There are currently no restrictions on display name uniqueness... So this means that multiple permissions could have the same "display name", but behind the scenes one might be defined by a module and another might be user-defined.
  • Another idea is to automatically rename the existing permission, e.g. if a user-defined permisison named foo.get exists, and a module defines one with the same name, the user-defined permission is renamed to foo.get.1

• Status
  colour Green title RESOLVED

  The PoC uses the same okapiPermission.json schema for the three arrays (newPermissions, modifiedPermissions, removedPermissions).  The current behavior of RMB is to generate one java class for this, named after the first field which references it (newPermissions).  This gets confusing when you're populating the a "RemovedPermissions" array with "NewPermission" objects.  One sub-optimal workaround is to duplicate the okapiPermission.json schema so RMB generates three distinct pojos w/ more apprioriate names.  There's probably a better way to deal with this, but I'm not aware of it.
  • https://github.com/folio-org/raml-module-builder#step-6-design-the-raml-files : Use "javaType": "org.folio.rest.jaxrs.model.MyEntity" to set the class name of the generated Java type to prevent a duplicate name where the second class overwrites the first class or to avoid a misleading name. Otherwise the field name in the .json schema file is used as class name.

• Status
  colour Green title RESOLVED

  How are module downgrades handled?
  • See

    Jira Legacy
    server System JiraJIRA serverId 01505d01-b853-3c2e-90f1-ee9b165564fc key OKAPI-953

  • One challenge here is what happens when a module upgrade removes or renames a permission which had was assigned to users.  If the upgrade process removes those permissions from the perm user record, we won't be able re-assign those permissions to the various users  upon downgrading to the previous version of the module.
  • The prevailing solution ATM is to use a soft delete.  There are a couple ways we could do this:
    • 1) Add a new field to the user permission record - "removedPermissions" - a list of permissions which the user once had, but are now marked for removal.  As long as this list isn't purged, it should be possible to back out the user perm changes upon module downgrade. 
      • It occurred to me that this "removedPermissions" field would need to be protected somehow - that is a user shouldn't be able to modify it, as that could be another path to abuse and permission escalation.  Since this information is solely for system/internal use, it might be as simple as adding a new column to the perm users table, which isn't accessible to API users....  In other words only this can only be changed by the _tenantPermissions API during modules upgrades/downgrades.
    • 2) Instead of removing the permissions, just mark them as inactive/deleted.  The user-perms records (assignments) will not be touched in this case.  A separate mechanism for cleaning up inactive/deleted permissions could then be introduced and manually invoked at some later time once the system operators are confident a module downgrade will never need to happen.
      • Would we want to limit this to only system defined permissions, or would we change the behavior of DELETE /perms/permissions/<id>? 
        • The team decided to keep this limited to _tenantPermissions, at least for now.
  • Julian Ladisch proposed another solution in the comments

• Status
  colour Green title RESOLVED

  Is it enough to just add RenamedFrom replaces in MD or should we also consider adding the actual version change info when the rename happens? For example fromVersion=1.1.1, toVersion=1.12 More precise metadata like this gives us more control how to handle perm changes, and flexibility to reuse an old perm name in later versions (if need to)
  • It was determined that this is not needed.

• Status
  colour Green title Resolved

  Who should determine which permissions are new, updated, deleted, OKAPI or mod-permissions?  From Adam Dickmeiss 's comments below.
  • 1) OKAPI compares modules descriptors - current version and new version being enabled, then tells mod-permissions which perms are new, modified and removed. 
    • This is how the PoC works.  However, in the case where a module is enabled, disabled, and then some time later, a newer version of that module is enabled.  Here mod-permissions would (likely) still have the original permissions in the database, but from OKAPI's perspective it's the first time this module has been enabled for the tenant.  This means it would send everything in the "newPermissions" list.  Given how this is currently implemented, it's not a problem for new and updated permissions (newPermissions is essentially handled as an upsert). However, permissions removed in the new version of the module will remain in the database (and in theory could still be assigned) since OKAPI will not send these in the removedPermissions list.  I think this is minor but worth noting.  We may be able to detect and remove these "zombie" permissions as part of the "purge-inactive" API described above.
    • Idea:  When mod-permissions is upgraded, OKAPI refreshes all the existing module permissions... this would result in the permissions getting the module context.  Mod-permissions would need to match the permissions (by name, and other criteria... they should match exactly).
  • 2) Mod-permissions is responsible for looking at the database and determining which permissions are new, modified, or removed.  
    • The catch here is that there will be a transition period where mod-permissions knows only about permissions, it doesn't have the moduleId context.  This will make it difficult to determine which perms are new/modified/removed.  One idea is to have OKAPI also provide not only the moduleId (the id of the new module version), but also fromModuleId, the id of the current module version.  This would allow mod-permissions to ask OKAPI for the module descriptor and use that to help determine which which perms are new/modified/removed.
    • IMO it feels a little wrong to force mod-permissions to understand module descriptors, and to a lesser extent to have a dependency on an OKAPI interface.  It should only need to know about permissions, possibly with some context about which module the permission was defined in.
  • 3) OKAPI sends both from and to permission lists.  See "Determination of which permissions are new/updated/removed"

Decisions

...