add explanatory comment and extract out to private function

broadinstitute · Nov 13, 2020 · 8ece56b · 8ece56b
1 parent 8c4127d
commit 8ece56b
Showing 1 changed file with 30 additions and 3 deletions.
diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/openam/PostgresAccessPolicyDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/openam/PostgresAccessPolicyDAO.scala
@@ -968,7 +968,7 @@ class PostgresAccessPolicyDAO(protected val dbRef: DbReference,
         select ${userResourcePolicy.result.baseResourceName}, ${resourceRole.result.role}, ${policyAction.result.action}, ${userResourcePolicy.result.public}, ${userResourcePolicy.result.inherited}
           from ${userResourcePolicyTable as userResourcePolicy}
           left join ${PolicyRoleTable as policyRole} on ${userResourcePolicy.policyId} = ${policyRole.resourcePolicyId}
-          left join ${FlattenedRoleMaterializedView as flattenedRole} on ${policyRole.resourceRoleId} = ${flattenedRole.baseRoleId} and ((${userResourcePolicy.inherited} and (${policyRole.descendantsOnly} or ${flattenedRole.descendantsOnly})) or not (${userResourcePolicy.inherited} or ${policyRole.descendantsOnly} or ${flattenedRole.descendantsOnly}))
+          left join ${FlattenedRoleMaterializedView as flattenedRole} on ${policyRole.resourceRoleId} = ${flattenedRole.baseRoleId} and ${roleAppliesToResource(userResourcePolicy, policyRole, flattenedRole)}
           left join ${ResourceRoleTable as resourceRole} on ${flattenedRole.nestedRoleId} = ${resourceRole.id} and ${userResourcePolicy.baseResourceTypeId} = ${resourceRole.resourceTypeId}
           left join ${PolicyActionTable as policyActionJoin} on ${userResourcePolicy.policyId} = ${policyActionJoin.resourcePolicyId} and ${userResourcePolicy.inherited} = ${policyActionJoin.descendantsOnly}
           left join ${ResourceActionTable as policyAction} on ${policyActionJoin.resourceActionId} = ${policyAction.id} and ${userResourcePolicy.baseResourceTypeId} = ${policyAction.resourceTypeId}
@@ -1048,6 +1048,33 @@ class PostgresAccessPolicyDAO(protected val dbRef: DbReference,
     })
   }
 
+
+  /**
+    *
+    * Determining whether a role should or should not apply to a resource is a bit more complicated than it initially
+    * appears. This logic is shared across the three queries (listUserResourceActions, listUserResourceRoles,
+    * listUserResourcesWithRolesAndActions) that search a resource's hierarchy for all of the relevant roles and actions
+    * that a user has on said resource. The following truth table shows the desired behavior of this SQL fragment where
+    * result indicates whether a given role does or does not apply to the resource
+    *
+    * userResourcePolicy.inherited  |  policyRole.descendantsOnly  |  flattenedRole.descendantsOnly  |  result
+    *            T                  |             T                |                T                |    T
+    *            T                  |             T                |                F                |    T
+    *            T                  |             F                |                T                |    T
+    *            T                  |             F                |                F                |    F
+    *            F                  |             T                |                T                |    F
+    *            F                  |             T                |                F                |    F
+    *            F                  |             F                |                T                |    F
+    *            F                  |             F                |                F                |    T
+    *
+    */
+  private def roleAppliesToResource(userResourcePolicy: QuerySQLSyntaxProvider[SQLSyntaxSupport[UserResourcePolicyRecord], UserResourcePolicyRecord],
+                                    policyRole: QuerySQLSyntaxProvider[SQLSyntaxSupport[PolicyRoleRecord], PolicyRoleRecord],
+                                    flattenedRole: QuerySQLSyntaxProvider[SQLSyntaxSupport[FlattenedRoleRecord], FlattenedRoleRecord]): SQLSyntax = {
+    samsqls"""((${userResourcePolicy.inherited} and (${policyRole.descendantsOnly} or ${flattenedRole.descendantsOnly}))
+             or not (${userResourcePolicy.inherited} or ${policyRole.descendantsOnly} or ${flattenedRole.descendantsOnly}))"""
+  }
+
   override def listUserResourceActions(resourceId: FullyQualifiedResourceId, user: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Set[ResourceAction]] = {
     runInTransaction("listUserResourceActions", samRequestContext)({ implicit session =>
       val userPoliciesCommonTableExpression = userPoliciesOnResourceCommonTableExpressions(resourceId, user)
@@ -1072,7 +1099,7 @@ class PostgresAccessPolicyDAO(protected val dbRef: DbReference,
           join ${ResourceRoleTable as resourceRole} on ${flattenedRole.nestedRoleId} = ${resourceRole.id} and ${userResourcePolicy.baseResourceTypeId} = ${resourceRole.resourceTypeId}
           join ${RoleActionTable as roleActionJoin} on ${resourceRole.id} = ${roleActionJoin.resourceRoleId}
           join ${ResourceActionTable as roleAction} on ${roleActionJoin.resourceActionId} = ${roleAction.id}
-          where (${userResourcePolicy.inherited} and (${policyRole.descendantsOnly} or ${flattenedRole.descendantsOnly})) or not (${userResourcePolicy.inherited} or ${policyRole.descendantsOnly} or ${flattenedRole.descendantsOnly})
+          where ${roleAppliesToResource(userResourcePolicy, policyRole, flattenedRole)}
         union
         select ${policyAction.action} as action
           from ${userResourcePolicyTable as userResourcePolicy}
@@ -1100,7 +1127,7 @@ class PostgresAccessPolicyDAO(protected val dbRef: DbReference,
           join ${PolicyRoleTable as policyRole} on ${userResourcePolicy.policyId} = ${policyRole.resourcePolicyId}
           join ${FlattenedRoleMaterializedView as flattenedRole} on ${policyRole.resourceRoleId} = ${flattenedRole.baseRoleId}
           join ${ResourceRoleTable as resourceRole} on ${flattenedRole.nestedRoleId} = ${resourceRole.id} and ${userResourcePolicy.baseResourceTypeId} = ${resourceRole.resourceTypeId}
-          where (${userResourcePolicy.inherited} and (${policyRole.descendantsOnly} or ${flattenedRole.descendantsOnly})) or not (${userResourcePolicy.inherited} or ${policyRole.descendantsOnly} or ${flattenedRole.descendantsOnly})"""
+          where ${roleAppliesToResource(userResourcePolicy, policyRole, flattenedRole)}"""
 
       listUserResourceRolesQuery.map(rs => ResourceRoleName(rs.string(resourceRole.resultName.role))).list().apply().toSet
     })