Merge branch 'develop' into update/grpc-core-1.68.1

.github/workflows/auto-approve-broadbot-prs.yml

@@ -5,9 +5,9 @@ permissions:
   pull-requests: write
 
 jobs:
-  dependabot:
+  broadbot:
     runs-on: ubuntu-latest
-    if: github.actor == 'broadbot[bot]'
+    if: github.actor == 'broadbot'
     steps:
       - name: Approve a PR
         run: gh pr review --approve "$PR_URL"

pact4s/README.md

@@ -13,8 +13,8 @@ On the command line, you can try the following:
 source env/local.env
 source src/main/resources/rendered/secrets.env
 export PACT_BROKER_URL="https://pact-broker.dsp-eng-tools.broadinstitute.org/"
-export PACT_BROKER_USERNAME="$(gcloud secrets versions access latest --project 'broad-dsp-eng-tools' --secret 'pact-broker-users-read-only' | jq -r '.basic_auth_read_only_username')"
-export PACT_BROKER_PASSWORD="$(gcloud secrets versions access latest --project 'broad-dsp-eng-tools' --secret 'pact-broker-users-read-only' | jq -r '.basic_auth_read_only_password')"
+export PACT_BROKER_USERNAME=$(vault read -field=basic_auth_read_only_username secret/dsp/pact-broker/users/read-only)
+export PACT_BROKER_PASSWORD=$(vault read -field=basic_auth_read_only_password secret/dsp/pact-broker/users/read-only)
 ```
 
 In IntelliJ, you can create a Run Configuration for `SamProviderSpec.scala` and save `Environment Variables` for:

pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala

@@ -158,7 +158,7 @@ object MockTestSupport extends MockTestSupport {
       policyDAO,
       googleExt,
       FakeOpenIDConnectConfiguration,
-      azureService: Option[AzureService]
+      azureService:Option[AzureService]
     )
   }
 

project/Dependencies.scala

@@ -171,8 +171,6 @@ object Dependencies {
   // was included transitively before, now explicit
   val commonsCodec: ModuleID = "commons-codec" % "commons-codec" % "1.17.1"
 
-  val caffeine: ModuleID = "com.github.ben-manes.caffeine" % "caffeine" % "3.1.8"
-
   val rootDependencies = Seq(
     // proactively pull in latest versions of Jackson libs, instead of relying on the versions
     // specified as transitive dependencies, due to OWASP DependencyCheck warnings for earlier versions.
@@ -224,8 +222,7 @@ object Dependencies {
     sentry,
     sentryLogback,
     okio,
-    terraCommonLib,
-    caffeine
+    terraCommonLib
   )
 
   // Needed because it looks like the dependency overrides of wb-libs doesn't propagate to the importing project...

src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/AccessPolicyDAO.scala

@@ -125,6 +125,7 @@ trait AccessPolicyDAO {
       resourceTypeNames: Set[ResourceTypeName],
       policies: Set[AccessPolicyName],
       roles: Set[ResourceRoleName],
+      actions: Set[ResourceAction],
       includePublic: Boolean,
       samRequestContext: SamRequestContext
   ): IO[Seq[FilterResourcesResult]]

src/main/scala/org/broadinstitute/dsde/workbench/sam/model/FilterResourcesResult.scala

@@ -1,10 +1,15 @@
 package org.broadinstitute.dsde.workbench.sam.model
 
+import org.broadinstitute.dsde.workbench.model.WorkbenchGroupName
+
 case class FilterResourcesResult(
     resourceId: ResourceId,
     resourceTypeName: ResourceTypeName,
-    policy: AccessPolicyName,
-    roleOrAction: Option[Either[ResourceRoleName, ResourceAction]],
+    policy: Option[AccessPolicyName],
+    role: Option[ResourceRoleName],
+    action: Option[ResourceAction],
     isPublic: Boolean,
+    authDomain: Option[WorkbenchGroupName],
+    inAuthDomain: Boolean,
     inherited: Boolean
 )

src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceService.scala

@@ -116,10 +116,7 @@ class ResourceService(
                 samRequestContext
               )
             }
-        } yield {
-          logger.info(s"Upserted policy $fullyQualifiedPolicyId")
-          upsertedPolicy
-        }
+        } yield upsertedPolicy
         upsertIO.attempt.map(fullyQualifiedPolicyId -> _)
       }
       .map(_.toMap)
@@ -1031,91 +1028,62 @@ class ResourceService(
       authDomainGroups: Map[WorkbenchGroupName, Boolean] = Map.empty
   )
 
-  private def groupFlat(
-      dbResult: Seq[FilterResourcesResult],
-      filterActions: Set[ResourceAction],
-      resourceAuthDomains: Map[FullyQualifiedResourceId, Set[WorkbenchGroupName]],
-      userGroups: Set[WorkbenchGroupName]
-  ): FilteredResourcesFlat = {
+  private def groupFlat(dbResult: Seq[FilterResourcesResult]): FilteredResourcesFlat = {
     val groupedFilteredResource = dbResult
       .groupBy(_.resourceId)
       .map { tuple =>
         val (k, v) = tuple
-        val grouped = v.foldLeft(GroupedDbRows()) { (acc: GroupedDbRows, r: FilterResourcesResult) =>
-          val role = r.roleOrAction.flatMap(_.left.toOption)
-          val roleActions = role.flatMap(roleName => getRoleActions(r.resourceTypeName, roleName, filterActions)).getOrElse(Set.empty)
-          // if filterActions is not emtpy, we only want to include roles that still have actions
-          val filteredRole = if (filterActions.isEmpty) role else role.filter(_ => roleActions.nonEmpty)
-          val policyActions = r.roleOrAction.flatMap(_.toOption).toSet
-          // if filterActions is not emtpy, we only want to include actions that are in the filterActions set
-          val filteredPolicyActions = if (filterActions.isEmpty) policyActions else policyActions.intersect(filterActions)
+        val grouped = v.foldLeft(GroupedDbRows())((acc: GroupedDbRows, r: FilterResourcesResult) =>
           acc.copy(
-            policies = acc.policies + FilteredResourceFlatPolicy(r.policy, r.isPublic, r.inherited),
-            roles = acc.roles ++ filteredRole,
-            actions = acc.actions ++ filteredPolicyActions ++ roleActions
+            policies = acc.policies ++ r.policy.map(p => FilteredResourceFlatPolicy(p, r.isPublic, r.inherited)),
+            roles = acc.roles ++ r.role,
+            actions = acc.actions ++ r.action,
+            authDomainGroups = acc.authDomainGroups ++ r.authDomain.map(_ -> r.inAuthDomain)
           )
-        }
+        )
 
-        val authDomainGroups = resourceAuthDomains.getOrElse(FullyQualifiedResourceId(v.head.resourceTypeName, k), Set.empty)
         FilteredResourceFlat(
           resourceId = k,
           resourceType = v.head.resourceTypeName,
           policies = grouped.policies,
           roles = grouped.roles,
           actions = grouped.actions,
-          authDomainGroups = authDomainGroups,
-          missingAuthDomainGroups = authDomainGroups -- userGroups
+          authDomainGroups = grouped.authDomainGroups.keySet,
+          missingAuthDomainGroups = grouped.authDomainGroups.filter(!_._2).keySet // Get only the auth domains where the user is not a member.
         )
       }
       .toSet
     FilteredResourcesFlat(resources = groupedFilteredResource)
   }
 
-  private def getRoleActions(resourceTypeName: ResourceTypeName, roleName: ResourceRoleName, filterActions: Set[ResourceAction]) =
-    // if filterActions is not emtpy, we only want to include actions that are in the filterActions set
-    resourceTypes(resourceTypeName).roles
-      .find(_.roleName == roleName)
-      .map(actions => if (filterActions.isEmpty) actions.actions else actions.actions.intersect(filterActions))
-
-  private def groupHierarchical(
-      dbResult: Seq[FilterResourcesResult],
-      filterActions: Set[ResourceAction],
-      resourceAuthDomains: Map[FullyQualifiedResourceId, Set[WorkbenchGroupName]],
-      userGroups: Set[WorkbenchGroupName]
-  ): FilteredResourcesHierarchical = {
+  private def groupHierarchical(dbResult: Seq[FilterResourcesResult]): FilteredResourcesHierarchical = {
     val groupedFilteredResources = dbResult
       .groupBy(_.resourceId)
       .map { tuple =>
         val (resourceId, resourceRows) = tuple
         val policies = resourceRows
-          .groupBy(_.policy)
+          .groupBy(_.policy.get)
           .map { policyTuple =>
             val (policyName, policyRows) = policyTuple
-            // if filterActions is not emtpy, we only want to include actions that are in the filterActions set
-            val policyActions = policyRows.flatMap(_.roleOrAction.flatMap(_.toOption)).toSet
-            val filteredPolicyActions = if (filterActions.isEmpty) policyActions else policyActions.intersect(filterActions)
-            val roles = policyRows
-              .flatMap(_.roleOrAction.flatMap(_.left.toOption))
-              .map { roleName =>
-                FilteredResourceHierarchicalRole(
-                  roleName,
-                  getRoleActions(policyRows.head.resourceTypeName, roleName, filterActions).getOrElse(Set.empty)
-                )
+            val actionsWithoutRoles = policyRows.filter(_.role.isEmpty).flatMap(_.action).toSet
+            val actionsWithRoles = policyRows.filter(_.role.nonEmpty)
+            val roles = actionsWithRoles
+              .groupBy(_.role.get)
+              .map { roleTuple =>
+                val (roleName, roleRows) = roleTuple
+                FilteredResourceHierarchicalRole(roleName, roleRows.flatMap(_.action).toSet)
               }
               .toSet
-            // if filterActions is not emtpy, we only want to include roles that still have actions
-            val filteredRoles = if (filterActions.isEmpty) roles else roles.filter(_.actions.nonEmpty)
-            FilteredResourceHierarchicalPolicy(policyName, filteredRoles, filteredPolicyActions, policyRows.head.isPublic, policyRows.head.inherited)
+            FilteredResourceHierarchicalPolicy(policyName, roles, actionsWithoutRoles, policyRows.head.isPublic, policyRows.head.inherited)
           }
           .toSet
-        val fullyQualifiedResourceId = FullyQualifiedResourceId(resourceRows.head.resourceTypeName, resourceId)
-        val authDomainGroups = resourceAuthDomains.getOrElse(fullyQualifiedResourceId, Set.empty)
+        val authDomainGroupMemberships = resourceRows.flatMap(r => r.authDomain.map(_ -> r.inAuthDomain)).toMap
         FilteredResourceHierarchical(
           resourceId = resourceId,
           resourceType = resourceRows.head.resourceTypeName,
           policies = policies,
-          authDomainGroups = authDomainGroups,
-          missingAuthDomainGroups = authDomainGroups -- userGroups
+          authDomainGroups = authDomainGroupMemberships.keySet,
+          missingAuthDomainGroups = authDomainGroupMemberships.filter(!_._2).keySet // Get only the auth domains where the user is not a member.
         )
       }
       .toSet
@@ -1157,16 +1125,7 @@ class ResourceService(
       includePublic: Boolean,
       samRequestContext: SamRequestContext
   ): IO[FilteredResourcesFlat] =
-    listResourcesAndTransform(samUserId, resourceTypeNames, policies, roles, includePublic, samRequestContext) {
-      case (filterResults, resourceAuthDomains, userGroups) =>
-        val groupedResults = groupFlat(filterResults, actions, resourceAuthDomains, userGroups)
-        // If we are filtering by actions, remove any resources that don't have the actions either directly or through a role
-        if (actions.nonEmpty) {
-          groupedResults.copy(resources = groupedResults.resources.filter(resource => resource.roles.nonEmpty || resource.actions.nonEmpty))
-        } else {
-          groupedResults
-        }
-    }
+    accessPolicyDAO.filterResources(samUserId, resourceTypeNames, policies, roles, actions, includePublic, samRequestContext).map(groupFlat)
 
   def listResourcesHierarchical(
       samUserId: WorkbenchUserId,
@@ -1177,72 +1136,7 @@ class ResourceService(
       includePublic: Boolean,
       samRequestContext: SamRequestContext
   ): IO[FilteredResourcesHierarchical] =
-    listResourcesAndTransform(samUserId, resourceTypeNames, policies, roles, includePublic, samRequestContext) {
-      case (filterResults, resourceAuthDomains, userGroups) =>
-        val groupedResults = groupHierarchical(filterResults, actions, resourceAuthDomains, userGroups)
-        // If we are filtering by actions, remove any resources that don't have the actions either directly or through a role
-        if (actions.nonEmpty) {
-          groupedResults.copy(resources =
-            groupedResults.resources.filter(resource => resource.policies.exists(policy => policy.actions.nonEmpty || policy.roles.nonEmpty))
-          )
-        } else {
-          groupedResults
-        }
-    }
-
-  private def listResourcesAndTransform[T](
-      samUserId: WorkbenchUserId,
-      resourceTypeNames: Set[ResourceTypeName],
-      policies: Set[AccessPolicyName],
-      roles: Set[ResourceRoleName],
-      includePublic: Boolean,
-      samRequestContext: SamRequestContext
-  )(transform: (Seq[FilterResourcesResult], Map[FullyQualifiedResourceId, Set[WorkbenchGroupName]], Set[WorkbenchGroupName]) => T): IO[T] =
-    // note that filtering by actions is implemented by application logic, not by the database query
-    // adding actions to the query would make return many more rows than necessary because roles can have many actions
-    // and those actions are static and already in memory
-    for {
-      filterResults <- accessPolicyDAO.filterResources(samUserId, resourceTypeNames, policies, roles, includePublic, samRequestContext)
-      resourceAuthDomains <- listResourceAuthDomains(filterResults, samRequestContext)
-      // only load user groups is there are auth domains
-      userGroups <-
-        if (resourceAuthDomains.values.exists(_.nonEmpty)) {
-          listUserManagedGroups(samUserId, samRequestContext)
-        } else {
-          IO.pure(Set.empty[WorkbenchGroupName])
-        }
-    } yield transform(filterResults, resourceAuthDomains, userGroups)
-
-  private def listResourceAuthDomains(
-      filteredResources: Seq[FilterResourcesResult],
-      samRequestContext: SamRequestContext
-  ): IO[Map[FullyQualifiedResourceId, Set[WorkbenchGroupName]]] =
-    filteredResources
-      .groupBy(_.resourceTypeName)
-      .toList
-      .traverse { case (resourceTypeName, resources) =>
-        for {
-          resourceTypeOpt <- getResourceType(resourceTypeName)
-          authDomainsByResourceId <- resourceTypeOpt match {
-            case Some(resourceType) if resourceType.isAuthDomainConstrainable =>
-              accessPolicyDAO.listResourcesWithAuthdomains(resourceTypeName, resources.map(_.resourceId).toSet, samRequestContext).map { authDomainResults =>
-                authDomainResults.map { authDomainResult =>
-                  authDomainResult.fullyQualifiedId -> authDomainResult.authDomain
-                }.toMap
-              }
-            case _ =>
-              IO.pure(Map.empty[FullyQualifiedResourceId, Set[WorkbenchGroupName]])
-          }
-        } yield authDomainsByResourceId
-      }
-      .map(_.flatten.toMap)
-
-  private def listUserManagedGroups(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Set[WorkbenchGroupName]] =
-    for {
-      groupPolicies <- accessPolicyDAO.listAccessPolicies(ManagedGroupService.managedGroupTypeName, userId, samRequestContext)
-    } yield {
-      val membershipPolicies =
-        groupPolicies.filter(p => ManagedGroupService.userMembershipRoleNames.contains(ManagedGroupService.getRoleName(p.accessPolicyName.value)))
-      membershipPolicies.map(policy => WorkbenchGroupName(policy.resourceId.value))
-    }
+    accessPolicyDAO
+      .filterResources(samUserId, resourceTypeNames, policies, roles, actions, includePublic, samRequestContext)
+      .map(groupHierarchical)
 }

src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockAccessPolicyDAO.scala

@@ -361,6 +361,7 @@ class MockAccessPolicyDAO(private val resourceTypes: mutable.Map[ResourceTypeNam
       resourceTypeNames: Set[ResourceTypeName],
       policies: Set[AccessPolicyName],
       roles: Set[ResourceRoleName],
+      actions: Set[ResourceAction],
       includePublic: Boolean,
       samRequestContext: SamRequestContext
   ): IO[Seq[FilterResourcesResult]] = IO {
@@ -369,28 +370,38 @@ class MockAccessPolicyDAO(private val resourceTypes: mutable.Map[ResourceTypeNam
         this.policies.collect {
           case (fqPolicyId @ FullyQualifiedPolicyId(FullyQualifiedResourceId(`resourceTypeName`, _), _), accessPolicy: AccessPolicy)
               if accessPolicy.members.contains(samUserId) || accessPolicy.public =>
-            val roleResults = accessPolicy.roles.map { role =>
-              FilterResourcesResult(
-                fqPolicyId.resource.resourceId,
-                fqPolicyId.resource.resourceTypeName,
-                fqPolicyId.accessPolicyName,
-                Option(Left(role)),
-                accessPolicy.public,
-                false
-              )
+            val rolesAndActions = RolesAndActions.fromPolicy(accessPolicy)
+            rolesAndActions.roles.flatMap { role =>
+              if (actions.isEmpty) {
+                Set(
+                  FilterResourcesResult(
+                    fqPolicyId.resource.resourceId,
+                    fqPolicyId.resource.resourceTypeName,
+                    Some(fqPolicyId.accessPolicyName),
+                    Some(role),
+                    None,
+                    accessPolicy.public,
+                    None,
+                    false,
+                    false
+                  )
+                )
+              } else {
+                rolesAndActions.actions.map { action =>
+                  FilterResourcesResult(
+                    fqPolicyId.resource.resourceId,
+                    fqPolicyId.resource.resourceTypeName,
+                    Some(fqPolicyId.accessPolicyName),
+                    Some(role),
+                    Some(action),
+                    accessPolicy.public,
+                    None,
+                    false,
+                    false
+                  )
+                }
+              }
             }
-
-            val actionResults = accessPolicy.actions.map { action =>
-              FilterResourcesResult(
-                fqPolicyId.resource.resourceId,
-                fqPolicyId.resource.resourceTypeName,
-                fqPolicyId.accessPolicyName,
-                Option(Right(action)),
-                accessPolicy.public,
-                false
-              )
-            }
-            roleResults ++ actionResults
         }
       }
       .flatten