Merge branch 'develop' into better_routes

broadinstitute · Feb 1, 2024 · b3b4d7a · b3b4d7a
2 parents 025eb3c + 3addbe2
commit b3b4d7a
Show file tree

Hide file tree

Showing 12 changed files with 392 additions and 97 deletions.
diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/ResourceRoutes.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/ResourceRoutes.scala
@@ -259,7 +259,7 @@ trait ResourceRoutes extends SamUserDirectives with SecurityDirectives with SamM
 
   def getUserResourcesOfType(resourceType: ResourceType, samUser: SamUser, samRequestContext: SamRequestContext): server.Route =
     get {
-      complete(policyEvaluatorService.listUserResources(resourceType.name, samUser.id, samRequestContext))
+      complete(resourceService.listUserResources(resourceType.name, samUser.id, samRequestContext))
     }
 
   def postResource(resourceType: ResourceType, samUser: SamUser, samRequestContext: SamRequestContext): server.Route =
@@ -590,7 +590,7 @@ trait ResourceRoutes extends SamUserDirectives with SecurityDirectives with SamM
           complete {
             resourceService
               .listResourcesFlat(
-                samUser,
+                samUser.id,
                 resourceTypes.map(_.split(",").map(ResourceTypeName(_)).toSet).getOrElse(Set.empty),
                 policies.map(_.split(",").map(AccessPolicyName(_)).toSet).getOrElse(Set.empty),
                 roles.map(_.split(",").map(ResourceRoleName(_)).toSet).getOrElse(Set.empty),
@@ -604,7 +604,7 @@ trait ResourceRoutes extends SamUserDirectives with SecurityDirectives with SamM
           complete {
             resourceService
               .listResourcesHierarchical(
-                samUser,
+                samUser.id,
                 resourceTypes.map(_.split(",").map(ResourceTypeName(_)).toSet).getOrElse(Set.empty),
                 policies.map(_.split(",").map(AccessPolicyName(_)).toSet).getOrElse(Set.empty),
                 roles.map(_.split(",").map(ResourceRoleName(_)).toSet).getOrElse(Set.empty),

diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/AccessPolicyDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/AccessPolicyDAO.scala
@@ -108,7 +108,7 @@ trait AccessPolicyDAO {
       }
     }
   def filterResources(
-      samUser: SamUser,
+      samUserId: WorkbenchUserId,
       resourceTypeNames: Set[ResourceTypeName],
       policies: Set[AccessPolicyName],
       roles: Set[ResourceRoleName],

diff --git a/...main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala b/...main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala
@@ -22,6 +22,10 @@ import scalikejdbc._
 import scala.collection.concurrent.TrieMap
 import scala.util.{Failure, Try}
 import cats.effect.Temporal
+import org.apache.commons.collections4.map.PassiveExpiringMap
+
+import java.util.Collections
+import java.util.concurrent.TimeUnit
 
 class PostgresAccessPolicyDAO(
     protected val writeDbRef: DbReference,
@@ -1686,13 +1690,86 @@ class PostgresAccessPolicyDAO(
         .toSet
     }
   }
-  override def filterResources(
-      samUser: SamUser,
+
+  private val publicResourcesCache: java.util.Map[ResourceTypeName, Seq[FilterResourcesResult]] =
+    Collections.synchronizedMap(new PassiveExpiringMap(1, TimeUnit.HOURS))
+
+  private def getPublicResourcesOfType(resourceTypeName: ResourceTypeName, samRequestContext: SamRequestContext): IO[Seq[FilterResourcesResult]] = {
+    val resourcePolicy = PolicyTable.syntax("resourcePolicy")
+    val effectiveResourcePolicy = EffectiveResourcePolicyTable.syntax("effectiveResourcePolicy")
+    val effectivePolicyRole = EffectivePolicyRoleTable.syntax("effectivePolicyRole")
+    val effectivePolicyAction = EffectivePolicyActionTable.syntax("effectivePolicyAction")
+    val resourceRole = ResourceRoleTable.syntax("resourceRole")
+    val roleAction = RoleActionTable.syntax("roleAction")
+    val resourceAction = ResourceActionTable.syntax("resourceAction")
+    val resource = ResourceTable.syntax("resource")
+
+    val resourceTypeConstraint =
+      samsqls"and ${resource.resourceTypeId} = ${resourceTypePKsByName.get(resourceTypeName)}"
+    val notNullConstraintRoleAction =
+      samsqls"and not (${resourceRole.role} is null and ${resourceAction.action} is null)"
+    val notNullConstraintPolicyAction = samsqls"and not (${resourceAction.action} is null)"
+
+    val publicRoleActionQuery =
+      samsqls"""
+        select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, ${resourceRole.result.role}, ${resourceAction.result.action}, ${resourcePolicy.result.public}, ${resourcePolicy.resourceId} != ${resource.id} as inherited
+        from ${PolicyTable as resourcePolicy}
+          left join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} and ${resourcePolicy.public}
+          left join ${EffectivePolicyRoleTable as effectivePolicyRole} on ${effectiveResourcePolicy.id} = ${effectivePolicyRole.effectiveResourcePolicyId}
+          left join ${ResourceRoleTable as resourceRole} on ${effectivePolicyRole.resourceRoleId} = ${resourceRole.id}
+          left join ${RoleActionTable as roleAction} on ${effectivePolicyRole.resourceRoleId} = ${roleAction.resourceRoleId}
+          left join ${ResourceActionTable as resourceAction} on ${roleAction.resourceActionId} = ${resourceAction.id}
+          left join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id} $resourceTypeConstraint
+        where ${resourcePolicy.public}
+          $resourceTypeConstraint
+          $notNullConstraintRoleAction
+          """
+
+    val publicPolicyActionQuery =
+      samsqls"""
+        select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, null as ${resourceRole.resultName.role}, ${resourceAction.result.action}, ${resourcePolicy.result.public}, ${resourcePolicy.resourceId} != ${resource.id} as inherited
+        from ${PolicyTable as resourcePolicy}
+          left join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} and ${resourcePolicy.public}
+          left join ${EffectivePolicyActionTable as effectivePolicyAction} on ${effectiveResourcePolicy.id} = ${effectivePolicyAction.effectiveResourcePolicyId}
+          left join ${ResourceActionTable as resourceAction} on ${effectivePolicyAction.resourceActionId} = ${resourceAction.id}
+          left join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id} $resourceTypeConstraint
+        where ${resourcePolicy.public}
+          $resourceTypeConstraint
+          $notNullConstraintPolicyAction
+                """
+
+    val includePublicPolicyActionQuery = samsqls"union $publicPolicyActionQuery"
+    val publicResourcesQuery = samsql"$publicRoleActionQuery $includePublicPolicyActionQuery"
+
+    readOnlyTransaction("filterResourcesPublic", samRequestContext) { implicit session =>
+      publicResourcesCache.computeIfAbsent(
+        resourceTypeName,
+        resourceTypeName =>
+          publicResourcesQuery
+            .map(rs =>
+              FilterResourcesResult(
+                rs.get[ResourceId](resource.resultName.name),
+                resourceTypeNamesByPK(rs.get[ResourceTypePK](resource.resultName.resourceTypeId)),
+                rs.stringOpt(resourcePolicy.resultName.name).map(AccessPolicyName(_)),
+                rs.stringOpt(resourceRole.resultName.role).map(ResourceRoleName(_)),
+                rs.stringOpt(resourceAction.resultName.action).map(ResourceAction(_)),
+                rs.get[Boolean](resourcePolicy.resultName.public),
+                None,
+                false,
+                rs.booleanOpt("inherited").getOrElse(false)
+              )
+            )
+            .list()
+            .apply()
+      )
+    }
+  }
+  private def filterPrivateResources(
+      samUserId: WorkbenchUserId,
       resourceTypeNames: Set[ResourceTypeName],
       policies: Set[AccessPolicyName],
       roles: Set[ResourceRoleName],
       actions: Set[ResourceAction],
-      includePublic: Boolean,
       samRequestContext: SamRequestContext
   ): IO[Seq[FilterResourcesResult]] = {
     val groupMemberFlat = GroupMemberFlatTable.syntax("groupMemberFlat")
@@ -1714,6 +1791,9 @@ class PostgresAccessPolicyDAO(
     val policyConstraint = if (policies.nonEmpty) samsqls"and ${resourcePolicy.name} in (${policies})" else samsqls""
     val roleConstraint = if (roles.nonEmpty) samsqls"and ${resourceRole.role} in (${roles})" else samsqls""
     val actionConstraint = if (actions.nonEmpty) samsqls"and ${resourceAction.action} in (${actions})" else samsqls""
+    val notNullConstraintRoleAction =
+      samsqls"and not (${resourceRole.role} is null and ${resourceAction.action} is null)"
+    val notNullConstraintPolicyAction = samsqls"and not (${resourceAction.action} is null)"
 
     val policyRoleActionQuery =
       samsqls"""
@@ -1728,12 +1808,15 @@ class PostgresAccessPolicyDAO(
             left join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id}
             left join ${AuthDomainTable as authDomain} on ${authDomain.resourceId} = ${resource.id}
             left join ${GroupTable as authDomainGroup} on ${authDomainGroup.id} = ${authDomain.groupId}
-            left join ${GroupMemberFlatTable as authDomainGroupMemberFlat} on ${authDomainGroup.id} = ${authDomainGroupMemberFlat.groupId} and ${authDomainGroupMemberFlat.memberUserId} = ${samUser.id}
-          where ${groupMemberFlat.memberUserId} = ${samUser.id}
+            left join ${GroupMemberFlatTable as authDomainGroupMemberFlat} on ${authDomainGroup.id} = ${authDomainGroupMemberFlat.groupId} and ${authDomainGroupMemberFlat.memberUserId} = ${samUserId}
+          where ${groupMemberFlat.memberUserId} = ${samUserId}
             $resourceTypeConstraint
             $policyConstraint
             $roleConstraint
-            $actionConstraint"""
+            $actionConstraint
+            $notNullConstraintRoleAction
+            """
+
     val policyActionQuery =
       samsqls"""
         select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, null as ${resourceRole.resultName.role}, ${resourceAction.result.action}, ${resourcePolicy.result.public}, ${authDomainGroup.result.name}, ${authDomainGroupMemberFlat.memberUserId} is not null as in_auth_domain, ${resourcePolicy.resourceId} != ${resource.id} as inherited
@@ -1745,48 +1828,18 @@ class PostgresAccessPolicyDAO(
             left join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id}
             left join ${AuthDomainTable as authDomain} on ${authDomain.resourceId} = ${resource.id}
             left join ${GroupTable as authDomainGroup} on ${authDomainGroup.id} = ${authDomain.groupId}
-            left join ${GroupMemberFlatTable as authDomainGroupMemberFlat} on ${authDomainGroup.id} = ${authDomainGroupMemberFlat.groupId} and ${authDomainGroupMemberFlat.memberUserId} = ${samUser.id}
-          where ${groupMemberFlat.memberUserId} = ${samUser.id}
+            left join ${GroupMemberFlatTable as authDomainGroupMemberFlat} on ${authDomainGroup.id} = ${authDomainGroupMemberFlat.groupId} and ${authDomainGroupMemberFlat.memberUserId} = ${samUserId}
+          where ${groupMemberFlat.memberUserId} = ${samUserId}
             $resourceTypeConstraint
             $policyConstraint
-            $actionConstraint"""
-
-    val publicRoleActionQuery =
-      samsqls"""
-        select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, ${resourceRole.result.role}, ${resourceAction.result.action}, ${resourcePolicy.result.public}, null as ${authDomainGroup.resultName.name}, null as in_auth_domain, ${resourcePolicy.resourceId} != ${resource.id} as inherited
-        from ${PolicyTable as resourcePolicy}
-          left join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} and ${resourcePolicy.public}
-          left join ${EffectivePolicyRoleTable as effectivePolicyRole} on ${effectiveResourcePolicy.id} = ${effectivePolicyRole.effectiveResourcePolicyId}
-          left join ${ResourceRoleTable as resourceRole} on ${effectivePolicyRole.resourceRoleId} = ${resourceRole.id}
-          left join ${RoleActionTable as roleAction} on ${effectivePolicyRole.resourceRoleId} = ${roleAction.resourceRoleId}
-          left join ${ResourceActionTable as resourceAction} on ${roleAction.resourceActionId} = ${resourceAction.id}
-          left join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id} $resourceTypeConstraint
-        where ${resourcePolicy.public}
-          $resourceTypeConstraint
-          $policyConstraint
-          $roleConstraint
-          $actionConstraint"""
-    val publicPolicyActionQuery =
-      samsqls"""
-        select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, null as ${resourceRole.resultName.role}, ${resourceAction.result.action}, ${resourcePolicy.result.public}, null as ${authDomainGroup.resultName.name}, null as in_auth_domain, ${resourcePolicy.resourceId} != ${resource.id} as inherited
-        from ${PolicyTable as resourcePolicy}
-          left join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} and ${resourcePolicy.public}
-          left join ${EffectivePolicyActionTable as effectivePolicyAction} on ${effectiveResourcePolicy.id} = ${effectivePolicyAction.effectiveResourcePolicyId}
-          left join ${ResourceActionTable as resourceAction} on ${effectivePolicyAction.resourceActionId} = ${resourceAction.id}
-          left join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id} $resourceTypeConstraint
-        where ${resourcePolicy.public}
-          $resourceTypeConstraint
-          $policyConstraint
-          $actionConstraint"""
-
-    val includePolicyActionQuery = if (policies.nonEmpty || actions.nonEmpty) samsqls"union $policyActionQuery" else samsqls""
-    val includePublicPolicyActionQuery = if ((policies.nonEmpty || actions.nonEmpty) && includePublic) samsqls"union $publicPolicyActionQuery" else samsqls""
-    val includePublicQuery = if (includePublic) samsqls"union $publicRoleActionQuery $includePublicPolicyActionQuery" else samsqls""
+            $actionConstraint
+            $notNullConstraintPolicyAction
+            """
 
+    val includePolicyActionQuery = if (roles.isEmpty) samsqls"union $policyActionQuery" else samsqls""
     val query =
       samsqls"""$policyRoleActionQuery
-                            $includePolicyActionQuery
-                            $includePublicQuery"""
+                            $includePolicyActionQuery"""
 
     readOnlyTransaction("filterResources", samRequestContext) { implicit session =>
       samsql"$query"
@@ -1808,6 +1861,29 @@ class PostgresAccessPolicyDAO(
     }
   }
 
+  override def filterResources(
+      samUserId: WorkbenchUserId,
+      resourceTypeNames: Set[ResourceTypeName],
+      policies: Set[AccessPolicyName],
+      roles: Set[ResourceRoleName],
+      actions: Set[ResourceAction],
+      includePublic: Boolean,
+      samRequestContext: SamRequestContext
+  ): IO[Seq[FilterResourcesResult]] =
+    for {
+      publicResources <-
+        if (includePublic) {
+          (if (resourceTypeNames.isEmpty) resourceTypePKsByName.keys.toList else resourceTypeNames.toList)
+            .map(resourceTypeName => getPublicResourcesOfType(resourceTypeName, samRequestContext))
+            .sequence
+            .map(_.flatten)
+        } else IO.pure(List.empty)
+      privateResources <- filterPrivateResources(samUserId, resourceTypeNames, policies, roles, actions, samRequestContext)
+    } yield publicResources
+      .filter(r => policies.isEmpty || r.policy.exists(p => policies.contains(p)))
+      .filter(r => roles.isEmpty || r.role.exists(role => roles.contains(role)))
+      .filter(r => actions.isEmpty || r.action.exists(action => actions.contains(action))) ++ privateResources
+
   private def recreateEffectivePolicyRolesTableEntry(resourceTypeNames: Set[ResourceTypeName])(implicit session: DBSession): Int = {
     val resource = ResourceTable.syntax("resource")
     val policyResource = ResourceTable.syntax("policyResource")

diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceService.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceService.scala
@@ -288,6 +288,11 @@ class ResourceService(
     for {
       resourceType <- getResourceType(resource.resourceTypeName)
       _ <- validateAuthDomain(resourceType.get, authDomains, userId, samRequestContext)
+      accessPolicies <- accessPolicyDAO.listAccessPolicies(resource, samRequestContext)
+      _ <-
+        if (accessPolicies.exists(_.public)) {
+          IO.raiseError(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.BadRequest, "Cannot add an auth domain group to a public resource")))
+        } else IO.unit
       policies <- listResourcePolicies(resource, samRequestContext)
       _ <- accessPolicyDAO.addResourceAuthDomain(resource, authDomains, samRequestContext)
       _ <- cloudExtensions.onGroupUpdate(policies.map(p => FullyQualifiedPolicyId(resource, p.policyName)), samRequestContext)
@@ -963,25 +968,53 @@ class ResourceService(
     FilteredResourcesHierarchical(resources = groupedFilteredResources)
   }
 
+  private def toUserResourcesResponse(hierarchicalResource: FilteredResourceHierarchical): UserResourcesResponse = {
+    val directPolicies = hierarchicalResource.policies.filter(p => !p.inherited)
+    val inheritedPolicies = hierarchicalResource.policies.filter(p => p.inherited)
+    val publicPolicies = hierarchicalResource.policies.filter(p => p.isPublic)
+
+    def policiesToRolesAndActions(policies: Set[FilteredResourceHierarchicalPolicy]) =
+      RolesAndActions(policies.flatMap(_.roles.map(_.role)), policies.flatMap(_.actions))
+
+    UserResourcesResponse(
+      hierarchicalResource.resourceId,
+      policiesToRolesAndActions(directPolicies),
+      policiesToRolesAndActions(inheritedPolicies),
+      policiesToRolesAndActions(publicPolicies),
+      hierarchicalResource.authDomainGroups,
+      hierarchicalResource.missingAuthDomainGroups
+    )
+  }
+  def listUserResources(
+      resourceTypeName: ResourceTypeName,
+      userId: WorkbenchUserId,
+      samRequestContext: SamRequestContext
+  ): IO[Iterable[UserResourcesResponse]] =
+    for {
+      resources <- listResourcesHierarchical(userId, Set(resourceTypeName), Set.empty, Set.empty, Set.empty, true, samRequestContext)
+    } yield resources.resources.map(toUserResourcesResponse)
+
   def listResourcesFlat(
-      samUser: SamUser,
+      samUserId: WorkbenchUserId,
       resourceTypeNames: Set[ResourceTypeName],
       policies: Set[AccessPolicyName],
       roles: Set[ResourceRoleName],
       actions: Set[ResourceAction],
       includePublic: Boolean,
       samRequestContext: SamRequestContext
   ): IO[FilteredResourcesFlat] =
-    accessPolicyDAO.filterResources(samUser, resourceTypeNames, policies, roles, actions, includePublic, samRequestContext).map(groupFlat)
+    accessPolicyDAO.filterResources(samUserId, resourceTypeNames, policies, roles, actions, includePublic, samRequestContext).map(groupFlat)
 
   def listResourcesHierarchical(
-      samUser: SamUser,
+      samUserId: WorkbenchUserId,
       resourceTypeNames: Set[ResourceTypeName],
       policies: Set[AccessPolicyName],
       roles: Set[ResourceRoleName],
       actions: Set[ResourceAction],
       includePublic: Boolean,
       samRequestContext: SamRequestContext
   ): IO[FilteredResourcesHierarchical] =
-    accessPolicyDAO.filterResources(samUser, resourceTypeNames, policies, roles, actions, includePublic, samRequestContext).map(groupHierarchical)
+    accessPolicyDAO
+      .filterResources(samUserId, resourceTypeNames, policies, roles, actions, includePublic, samRequestContext)
+      .map(groupHierarchical)
 }
diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/TestSupport.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/TestSupport.scala
@@ -229,6 +229,9 @@ object TestSupport extends TestSupport {
           PolicyRoleTable,
           PolicyTable,
           AuthDomainTable,
+          EffectiveResourcePolicyTable,
+          EffectivePolicyRoleTable,
+          EffectivePolicyActionTable,
           ResourceTable,
           RoleActionTable,
           ResourceActionTable,