DR-000014 - Migration of Static Permissions Upon Upgrade

DR-000014 - Migration of Static Permissions Upon Upgrade

Submitted Date

Dec 2, 2020 

Approved Date

Dec 2, 2020 

Status

ACCEPTED

Impact

MEDIUM

 

Overrides/Supersedes 

This decision was migrated from the Tech Leads Decision Log as part of a consolidation process.  The original decision record can be found here.

RFC 

N/A

Stakeholders

@Jakub Skoczen  @Julian Ladisch @Adam Dickmeiss @Hongwei Ji

Contributors

@Craig McNally 

Approvers

This decision was made by the Tech Leads group prior to the adoption of current decision making processes within the FOLIO project.

Background/Context

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.

Assumptions

N/A

Constraints

N/A

Rationale

The following statements apply to permissions defined in module descriptors.  NOT user-defined permissions.

  • Permissions can now be renamed via a new "replaces" property.  Perm-users (Assignments) will be updated automatically.

  • When permissions that once appeared in a module descriptor are removed in a newer version of the module descriptor, they will be marked deprecated. 

    • Initially the displayName of these permissions will be prefixed with "(deprecated)", but the permissions will not be filtered out of any API calls.

    • Eventually these permissions remain assigned to users, but will be filtered out of perms API results unless a new query argument (includeDeprecated=true is specified).  This will be handled in a separate story which may not make it into R1 2021.

  • A new API will be introduced to allow an operator to purge deprecated permissions.  Once this is done a module downgrade will not result in those permissions being re-assigned.

  • If a permission name collides with an user-defined permission with the same name,

    • Initially the call to enable the module will fail with an appropriate error message.  

    • Eventually we may do something like rename the user-defined permissions and adjust assignments as needed.

  • Going forward, all system-defined permissions will include context about the module that defined them (new permission fields moduleName/moduleVersion).

  • When upgrading mod-permissions, OKAPI will "refresh" all the existing permissions, this is already done when first enabling mod-permissions, but now will be done on upgrades as well.  

    • In most cases this will be a no-op, but this allows us to update permissions w/ the module context.

Decision

Static permission removal will use a soft delete to accommodate module downgrades

Implications

  • Pros

    • N/A

  • Cons

    • N/A

Other Related Resources

Spikes:

  • OKAPI-839 - SPIKE: consider migration of static permission and permissionSets CLOSED

  • OKAPI-952 - SPIKE: overwrite existing permission sets CLOSED

  • OKAPI-953 - SPIKE: soft delete and permission migration during downgrade CLOSED

Implementation Stories:

Changes to OKAPI

ModuleDescriptor

  • Add a new field to the permission.json schema

    • replaces

      • Array of string

      • Each item is a name of a permission that this permission replaces.

      • Optional

During module upgrade

  1. OKAPI Makes a call to mod-permissions' _tenantPermissions endpoint, providing a lists of permissions (details below).

Upon mod-permissions Install/Upgrade

  1. Send all permissions to mod-permissions to "refresh" them.  See "Determination of which permissions are new/updated/removed" below.  

Changes to mod-permissions

Tenant Permissions API

_tenantPermissions is updated to determine which permissions are new, updated, removed.  That information is that used to update not only the permissions table but also the user-permissions table.   So if a user has a permission which is being renamed, updated with fewer/additional sub-permissions, or removed, the user's permissions are updated accordingly.  

Determination of which permissions are new/updated/removed

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

See https://github.com/folio-org/mod-permissions/blob/master/ramls/okapiPermissionSet.json

This is the payload of the _tenantPermissions endpoint remains the same, aside from "moduleId" now being a required field.

Example:

{
  "moduleId": "mod-foo-2.0.0",
  "perms": [ <list of permission objects> ]
}

Permission.json Schema

See https://github.com/folio-org/mod-permissions/blob/master/ramls/permission.json

This is the stored permission schema.  It needs to be expanded to include some additional context about who/what defined it.

  • moduleName

    • String

    • Optional

    • The name of the module (not including version) that defined this permission

    • Read only

  • moduleVersion

    • String

    • Optional

    • The version of the module that defined this permission

    • Read only


Example 1 - Module-defined permission Expand source

Or, for a user-defined permission these fields would simply be omitted.  Permissions created by the _tenantPermissions API will always have moduleName populated, which Permissions created by the Perms API will never populate moduleName or moduleVersion. 

Removal of Static Module Permissions

In order to support the ability to downgrade a module, the decision has been made to implement a soft delete when permissions are removed from a module descriptor.  Here, the OKAPI part stays the same, but on the mod-permissions side, the _tenantPermissions API will not actually delete permissions, nor will it remove the permission assignments.  Instead, the permission will be marked as inactive.  A separate process for cleaning up inactive permissions will be introduced.

Inactive permissions should be filtered out of responses from the various perms APIs:

GET /perms/permissions

  • Query the DB for the permissions - automatically/transparently add a clause to filter out inactive permissions 

  • Query the DB for all of the inactive permissions - here I'm assuming there aren't going to be many of them.  we can probably cache this result.

  • Filter out the inactive permissions from the subPermissions array in each of the permissions in the results.

  • Filter out the inactive permissions from the childOf array in each of the permissions in the results.

  • Return the adjusted results

  • Add a new  query argument - includeInactive

    • Boolean

    • Default:  false

    • Indicates whether inactive permissions should be returned or not (including in subPermissions/childOf arrays)

NOTE:  In the case where we query for a permission by name... e.g. GET /perms/permissions?query=permissionName=some.inactive.permission ?  We automatically add a clause to not return inactive permissions, so this would return an empty result set, even though the permission does exist in the system.  Or course, if includeInactive=true the results would include the permission specified.

GET /perms/users/<id>/permissions (and similar GET /perms/users/<id>)

  • Query the DB for permission users (assignments)

  • Query the DB for all inactive permissions (or consult and use a cached copy if one is available)

  • Filter out the inactive permissions from the results

  • Update the totalRecord count in the results

  • Return the adjusted results

  • Add a new  query argument - includeInactive

    • Boolean

    • Default:  false

    • Indicates whether inactive permissions should be returned or not (including in subPermissions/childOf arrays)

GET /perms/permissions/<id> 

Leave that as-is. 

Notes

Several ideas have been briefly discussed for extending this API's functionality

  • Allow finer granularity control, e.g. specify which modules to purge the inactive permissions for

  • Introduce a way to purge permissions which have been inactive for longer than a specified amount of time.

Another idea which was discussed (and considered an implementation detail) was to store inactive permissions off to the side, e.g. in a separate table.  The idea being that it might help simplify/minimize the amount of filtering needed, specifically we wouldn't need to add clauses to provided CQL queries to omit inactive permissions.  We would however still need to filter out permissions from the subPermissions/childOf fields.

Schema Changes

Permission.json Schema

See https://github.com/folio-org/mod-permissions/blob/master/ramls/permission.json

This is the stored permission schema.  It needs to be expanded to include some additional context about who/what defined it.

  • inactive

    • Boolean

    • Optional

    • Default: false

    • Indicates whether this permissions has been marked for deletion (soft deleted)

API Changes

A new API will be introduced for removing inactive permissions

Interface: permissions

Endpoint: POST /perms/permissions/purge-inactive

Request: No body

Responses:

  • 200 - application/json with the following fields:

    • "removed": array of permission names which were removed - may be empty

    • "totalRemoved": integer indicating the number of permissions that were removed, may be zero if there are no inactive permissions

  • 500 - text/plain with an appropriate message

Required Permissions: perms.permissions.purge-inactive.post

Behavior:  

  • Start transaction

  • Query the permissions table for all permissions where inactive == true

  • For each

    • Query the perm-users table for all records w/ permissions contains the inactive permission

    • Remove the inactive permission assignment and update the record

  • Delete from the permissions table where inactive == true

  • End transaction

  • Form a response containing the list of removed permission's names and the total count of removed permissions

  • Upon error, rollback transaction and return a 500 and plain text error w/ appropriate message

NOTE:  A call to GET /perms/permissions?query=inactive==true could be used to retrieve the list of permissions that will be purged prior to using this new endpoint.

Permission Name Conflict Resolution

OKAPI-952 - SPIKE: overwrite existing permission sets CLOSED explores options for handling conflicting permission names.  Three potential solutions were considered:

Fail the install/upgrade

This it probably the safest thing to do, but is undesirable given separate conversations about reducing the number of things that would cause an upgrade to completely stop.

Overwrite the existing permission

While this is the current behavior, it constitutes a security vulnerability in that it could lead to permission escalation.  For this reason, this solution is essentially off the table.

Rename the existing user-defined permission

For example by adding a numeric suffix.  Permissions defined by the system (modules) take precedence.

  1. Renaming a user-defined permission and replacing it with a system-defined permission could get confusing.

  2. This might be the least disruptive solution being considered.

Prefix/scope the permissions

Here, permissions would be scoped.  Module-defined permissions would get a prefix of 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".  

The main concern with this solution is that it's disruptive and more complicated than the other solutions.

  • 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.

End-to-End Examples

Example 1 - New, modified, and removed permissions

Given the following permissions:

{
  "permissionName": "foo",
  "subPermissions": [ ],
  "grantedTo": [ "c183b277-9d16-4687-b22a-2f181529d018" ],
  ...
}, {
  "permissionName": "bar",
  "subPermissions": [ "bar.get", "bar.post", "bar.delete" ],
  "grantedTo": [ "c183b277-9d16-4687-b22a-2f181529d018" ],
  ...
}, {
  "permissionName": "baz",
  "subPermissions": [ ],
  "grantedTo": [ "c183b277-9d16-4687-b22a-2f181529d018" ],
  ...
}

And following user:

{
  "id": "c183b277-9d16-4687-b22a-2f181529d018",
  "username": "bob",
  "permissions": [ "foo", "bar", "baz", "bar.get", "bar.post", "bar.delete" ]
  ...
}

If _tenantPermissions was called with:

{
  "ModuleId": "mod-foo-2.0.0",
  "fromModuleId": "mod-foo-1.2.3",
  "toPerms": [
    {
      "permissionName": "zip",
      ...
    }, {
      "permissionName": "zap",
      "subPermissions": [ "zap.get", "zap.post", "zap.delete" ],
      ...
    }, {
      "permissionName": "foo.config",
      "replaces": [ "foo" ],
      ...
    }, {
      "permissionName": "bar",
      "subPermissions": [ "bar.get", "bar.put", "bar.post", "bar.delete" ],
      ...
    }
  ],
  "fromPerms": [
    {
      "permissionName": "foo",
      ...
    }, {
      "permissionName": "bar",
      "subPermissions": [ "bar.get", "bar.post", "bar.delete" ],
      ...
    }, {
      "permissionName": "baz",
      ...
    }
  ]
}

We'd end up with:

{
  "permissionName": "foo.config",
  "subPermissions": [ ],
  "grantedTo": [ "c183b277-9d16-4687-b22a-2f181529d018" ],
  ...
}, {
  "permissionName": "bar",
  "subPermissions": [ "bar.get", "bar.put", "bar.post", "bar.delete" ],
  "grantedTo": [ "c183b277-9d16-4687-b22a-2f181529d018" ],
  ...
}, {
  "permissionName": "zip",
  "subPermissions": [ ],
  "grantedTo": [ "c183b277-9d16-4687-b22a-2f181529d018" ],
  ...
}, {
  "permissionName": "zap",
  "subPermissions": [ "zap.get", "zap.post", "zap.delete" ],
  "grantedTo": [ "c183b277-9d16-4687-b22a-2f181529d018" ],
  ...
}

And