Improve performance of permission checks in UI

[chrisknoll]

WebAPI Security Implementation (Authorization)

Introduction

The purpose of this document is to describe the technology stack and implementation involved in the authorization layer of WebAPI. Note the difference between authentication and authorization: authentication is the mechanism to accept user credentials and validate the user's identity with a security store (example: openAuth, database, google) while authorization is about making the determination that a user is allowed to perform an action. This article will focus on the authorization implementation.

Background

There is existing documentation containing some technical details in the WebAPI wiki: Security Configuration. It is recommended for the reader to review the information linked in that article so to not repeat that information in this document. Information about the Java libraries, Authentication, Authorization and other techical details about security-related tables and Access Control. This wiki article also describes how Atlas interacts wth WebAPI to perform permission checks and how authentication information is passed between the client and server.

For purposes of this document, a closer look at the implementation details of the authorization implementation will be reviewed, with an eye towards issues with scalability.

Permissions

Permissions are loaded in PermissionManager.getAuthorizationInfo() with the following steps:

1. Fetch UserEntity from UserRepository
2. Fetch UserRoleEntity from UserEntity
3. Fetch user permissions (Set) from UserEntity by getting each permission associated to the user's assigned roles. Note: when fetching permissions, there is a status check on the role that if it excludes non-approved roles from being returned.
4. Create a Set<String> of permissionNames (which is actually a permission value like cohortdefinition:*:get)
5. Return the roles, permission names, userId and login as a UserSimpleAuthorizationInfo object.

The permissions are relayed back to the client via a series of Filters: UpdateAccessTokenFilter fetches the list of permissions via getStringPermissions() and creates a string as a |-concatenated string. The subsequent filter SendTokenInHeaderFilter serializes the PermissionDTO wrapper into a JSON object to the client.

Because of this, the JavaScript UI must interpret these security strings (which closely resemble WildcardPermissions from Shiro), which means all permissions for a user are sent to the client, causing a large payload to be parsed when a permission is checked and the Shiro implementation must be re-created on the JS side in order to handle wild-card permission checks.

Implementation Changes

Simple / Less Intrusive

The simple approach to addressing the performance problem is to leave the general approach (where UI receives all permissions for the user) and focus on the structure of the response to reduce size (remove dupes?) or structure the response so that is faster to navigate and find the requested permissions (a tree-structure result?).

Permissions as a tree

Consider the set of permissions assigned to a user:

"*:cohortresults:*:breakdown:get"
"*:person:*:get:dates"
"*:vocabulary:lookup:identifiers:post"
"annotation:*:get"
"annotation:answers:post"
"annotation:deleteset:*:get"
"annotation:fullquestion:get"
"annotation:get"
"annotation:getsets:get"
"annotation:navigation:get"
"annotation:post"
"annotation:questions:get"
"annotation:questions:post"
"annotation:results:*:get"
"annotation:results:get"
"annotation:sample:post"
"annotation:sets:get"
"annotation:sets:post"
"cache:clear:get"
"cdmresults:*:get"
"cdmresults:EUNOMIA:*:*:get"
"cdmresults:EUNOMIA:*:get"
"cdmresults:EUNOMIA:conceptRecordCount:post"
"cdmresults:EUNOMIA:multidrilldown:post"

As a tree, this can be represented as:

*
 |- cohortresults
   |- *
     |- breakdown
       |- get
  |- person
    |- *
      |- get
        |- dates
  |- vocabulary
    |- lookup
      |- identifiers
        |- post
annotation
  |- answers
    |- post
  |- deleteset
    |- *
      |- get
  |- fullquestion
    |- get
  |- get
  |- getsets
    |- get
  |- navigation
    |- get
  |- post
  |- questions
    |- get
    |- post
  |- results
    |- *
      |-get
    |- get
  |- sample
    |- post
  |- sets
    |- get
    |- post
  |- cache
    |- clear
      |- get
  |- cdmresults
    |- *
      |- get
    |- EUNOMIA
      |- *
        |- *
          |- get
        |- get
      |- conceptRecordCount
        |- get
      |- multidrilldown
        |- post

While this 'renders' longer, this uses less space in memory for any element that is duplicated:

"cdmresults:EUNOMIA:*:*:get"
"cdmresults:EUNOMIA:*:get"
"cdmresults:EUNOMIA:conceptRecordCount:post"
"cdmresults:EUNOMIA:multidrilldown:post"

becomes:

  |- cdmresults
    |- *
      |- get
    |- EUNOMIA
      |- *
        |- *
          |- get
        |- get
      |- conceptRecordCount
        |- get
      |- multidrilldown
        |- post

And you can quickly find the cdmresults permissions via key-lookup (permission["cdmresults"]) vs scanning through a list of permissions.

While this could reduce the payload size, there is a problem:

You can be granted a permission of *, making the first rows:

"*"
"*:cohortresults:*:breakdown:get"

yielding this tree:

*
 |- cohortresults
   |- *
     |- breakdown
       |- get

But there's no way to know that one of the permissions you were granted was simply *, from the tree it looks like the only permission the user is assigned was:

*:cohortresults:*:breakdown:get

(by walking the tree and creating the :-separated permission strings) which means very different things than "*" which is global access. Other permissions that could be inferred from the tree structure would be:

*:cohortresults
*:cohortresults:breakdown
*:cohortresults:breakdown:get

but none of these came from the initial permission set. We'd need some special token embedded into the tree to indicate a 'end' at a node indicating that a permission expression ended at the tree (while another permission exists that built the rest of the tree's branch)

Storing permissions in a hash on first permission term

Alternatively, we could build a 'first-term' index where permissions are stored in a hash indexed on the first term of the permission. From our origional example, this can look like:

{
  "*": [
    "*:cohortresults:*:breakdown:get",
    "*:person:*:get:dates",
    "*:vocabulary:lookup:identifiers:post",
  ],
  "annotation" : [
    "annotation:*:get"
    "annotation:answers:post"
    "annotation:deleteset:*:get"
    "annotation:fullquestion:get"
    "annotation:get"
    "annotation:getsets:get"
    "annotation:navigation:get"
    "annotation:post"
    "annotation:questions:get"
    "annotation:questions:post"
    "annotation:results:*:get"
    "annotation:results:get"
    "annotation:sample:post"
    "annotation:sets:get"
    "annotation:sets:post"
  ],
  "cache": [
    "cache:clear:get"
  ],
  "cdmresults" : [
    "cdmresults:*:get"
    "cdmresults:EUNOMIA:*:*:get"
    "cdmresults:EUNOMIA:*:get"
    "cdmresults:EUNOMIA:conceptRecordCount:post"
    "cdmresults:EUNOMIA:multidrilldown:post"
  ]
}

This adds a little overhead to payloads, but you can quickly find specific permissions that start with a prefix in order to find the applicable user permissions that should check against a permission assignment. Testing will be required to know if the slowdown in UI permission checks is scanning through permissions or the initial download.

Implementation of checkPermission

The AuthAPI.js defines a function checkPermission that is used to determine if a granted permission (the etalon) satisfies the requested permission (the permission to check).

There seems to be a bug in the implementation: According to WildCard permissions, an etalon of * should match any permission. However this logic in checkPermission:

        if (etalonLevels.length != permissionLevels.length) {
            return false;
        }

This requires that a permission check will automatically fail if the etalonLevels (the number of : to split) doesn't match the desired permission. But, the case where a etalon of * which matches any permission check is not = to the number of levels if you have a permission to check of something like cdmresults:*:get.

The Java implementation of implies(Permission p) can be used to facilitate the JS implementation.

[chrisknoll]

This issue was opened to collect feedback from the developer community about ideas on how to improve the security implementation. As more feedback is collected, I will update the initial post with modifications based on general consensus.

[chrisknoll]

Closed in #2912