From 012a524c2e0d476ca56567c7bf5fd06cddcaf989 Mon Sep 17 00:00:00 2001 From: Broad Bot <61600560+broadbot@users.noreply.github.com> Date: Thu, 7 Nov 2024 12:58:11 -0800 Subject: [PATCH 01/15] CORE-69: Update azure-resourcemanager-managedapplications from 1.0.0-beta.1 to 1.0.0-beta.4 (#1578) Co-authored-by: Bria Morgan --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 7ed1f629f..823e51275 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -137,7 +137,7 @@ object Dependencies { val cloudResourceLib: ModuleID = "bio.terra" % "terra-cloud-resource-lib" % crlVersion excludeAll (excludeGoogleServiceUsage, excludeGoogleCloudResourceManager, excludeJerseyCore, excludeJerseyMedia, excludeSLF4J, excludeAwsSdk) val azureManagedApplications: ModuleID = - "com.azure.resourcemanager" % "azure-resourcemanager-managedapplications" % "1.0.0-beta.1" + "com.azure.resourcemanager" % "azure-resourcemanager-managedapplications" % "1.0.0-beta.4" def excludeSpringBoot = ExclusionRule("org.springframework.boot") def excludeSpringAop = ExclusionRule("org.springframework.spring-aop") From af56ca05c333b97dc1cf050814d01e603b9cff4a Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Thu, 7 Nov 2024 18:21:49 -0500 Subject: [PATCH 02/15] [CORE-105] Enable Single Reviewer Approval for Broadbot PRs (#1583) * Auto-approve action * Use broadbot token --- .github/workflows/auto-approve-broadbot-prs.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .github/workflows/auto-approve-broadbot-prs.yml diff --git a/.github/workflows/auto-approve-broadbot-prs.yml b/.github/workflows/auto-approve-broadbot-prs.yml new file mode 100644 index 000000000..c93fcc047 --- /dev/null +++ b/.github/workflows/auto-approve-broadbot-prs.yml @@ -0,0 +1,16 @@ +name: Broadbot auto-approve +on: pull_request + +permissions: + pull-requests: write + +jobs: + dependabot: + runs-on: ubuntu-latest + if: github.actor == 'broadbot[bot]' + steps: + - name: Approve a PR + run: gh pr review --approve "$PR_URL" + env: + PR_URL: ${{ github.event.pull_request.html_url }} + GH_TOKEN: ${{ secrets.BROADBOT_TOKEN }} From 0b76884fd06d4a1bc0155d374f085491047cfdee Mon Sep 17 00:00:00 2001 From: Broad Bot <61600560+broadbot@users.noreply.github.com> Date: Fri, 8 Nov 2024 07:05:04 -0800 Subject: [PATCH 03/15] CORE-69: Update janino from 3.1.7 to 3.1.12 (#1334) --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 823e51275..d1eeacce6 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -53,7 +53,7 @@ object Dependencies { val ficus: ModuleID = "com.iheart" %% "ficus" % "1.5.2" // val stackdriverLogging: ModuleID = "org.springframework.cloud" % "spring-cloud-gcp-logging" % "1.2.8.RELEASE" excludeAll(excludeSpring, excludeSpringBoot) val stackdriverLogging: ModuleID = "com.google.cloud" % "google-cloud-logging-logback" % "0.127.11-alpha" - val janino: ModuleID = "org.codehaus.janino" % "janino" % "3.1.7" // For if-else logic in logging config + val janino: ModuleID = "org.codehaus.janino" % "janino" % "3.1.12" // For if-else logic in logging config val akkaActor: ModuleID = "com.typesafe.akka" %% "akka-actor" % akkaV val akkaSlf4j: ModuleID = "com.typesafe.akka" %% "akka-slf4j" % akkaV From 66dbfa083828d699cc611de72ff604d7f81d9ea4 Mon Sep 17 00:00:00 2001 From: Broad Bot <61600560+broadbot@users.noreply.github.com> Date: Fri, 8 Nov 2024 07:42:02 -0800 Subject: [PATCH 04/15] CORE-69: Update commons-codec from 1.15 to 1.17.1 (#1494) Co-authored-by: Bria Morgan --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index d1eeacce6..4ad256675 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -169,7 +169,7 @@ object Dependencies { val terraCommonLib = tclExclusions("bio.terra" % "terra-common-lib" % tclVersion classifier "plain") // was included transitively before, now explicit - val commonsCodec: ModuleID = "commons-codec" % "commons-codec" % "1.15" + val commonsCodec: ModuleID = "commons-codec" % "commons-codec" % "1.17.1" val rootDependencies = Seq( // proactively pull in latest versions of Jackson libs, instead of relying on the versions From 50e89e7a1847ab22b4c0e6423d9f53f79539493d Mon Sep 17 00:00:00 2001 From: Broad Bot <61600560+broadbot@users.noreply.github.com> Date: Fri, 8 Nov 2024 08:16:38 -0800 Subject: [PATCH 05/15] CORE-69: Update postgresql from 42.7.2 to 42.7.4 (#1522) Co-authored-by: Bria Morgan --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 4ad256675..26522d0ef 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -8,7 +8,7 @@ object Dependencies { val scalaTestV = "3.2.19" val scalaCheckV = "1.18.1" val scalikejdbcVersion = "3.4.2" - val postgresDriverVersion = "42.7.2" + val postgresDriverVersion = "42.7.4" val sentryVersion = "6.15.0" val workbenchLibV = "fa46370" // If updating this, make sure googleStorageLocal in test dependencies is up-to-date From 5d5d606c6c465e5073a6b761ddec36ea6a5df400 Mon Sep 17 00:00:00 2001 From: Broad Bot <61600560+broadbot@users.noreply.github.com> Date: Fri, 8 Nov 2024 10:04:46 -0800 Subject: [PATCH 06/15] CORE-69: Update google-api-services-storage from v1-rev20220401-1.32.1 to v1-rev20241008-2.0.0 (#1562) Co-authored-by: Bria Morgan --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 26522d0ef..af4f2c14c 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -71,7 +71,7 @@ object Dependencies { val ioGrpc: ModuleID = "io.grpc" % "grpc-core" % "1.34.1" val googleOAuth2: ModuleID = "com.google.auth" % "google-auth-library-oauth2-http" % "0.18.0" excludeAll excludIoGrpc - val googleStorage: ModuleID = "com.google.apis" % "google-api-services-storage" % "v1-rev20220401-1.32.1" excludeAll excludIoGrpc // force this version + val googleStorage: ModuleID = "com.google.apis" % "google-api-services-storage" % "v1-rev20241008-2.0.0" excludeAll excludIoGrpc // force this version val monocle: ModuleID = "com.github.julien-truffaut" %% "monocle-core" % monocleVersion val monocleMacro: ModuleID = "com.github.julien-truffaut" %% "monocle-macro" % monocleVersion From 1b49c1aa7b34b2335a55c04504e853ddf305b82d Mon Sep 17 00:00:00 2001 From: Broad Bot <61600560+broadbot@users.noreply.github.com> Date: Fri, 8 Nov 2024 10:40:45 -0800 Subject: [PATCH 07/15] CORE-69: Update circe-yaml from 0.14.2 to 0.16.0 (#1506) Co-authored-by: Bria Morgan --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index af4f2c14c..8dc277824 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -107,7 +107,7 @@ object Dependencies { val liquibaseCore: ModuleID = "org.liquibase" % "liquibase-core" % "4.2.2" - val circeYAML: ModuleID = "io.circe" %% "circe-yaml" % "0.14.2" + val circeYAML: ModuleID = "io.circe" %% "circe-yaml" % "0.16.0" val snakeYAML: ModuleID = "org.yaml" % "snakeyaml" % "2.3" val scalikeCore = "org.scalikejdbc" %% "scalikejdbc" % scalikejdbcVersion From ab2829dba3c0a62f9f1f2b29426a9a8798fd874c Mon Sep 17 00:00:00 2001 From: dvoet Date: Fri, 8 Nov 2024 16:34:09 -0500 Subject: [PATCH 08/15] CORE-137 Optimize filter resources (#1577) --- pact4s/README.md | 4 +- .../dsde/workbench/sam/MockTestSupport.scala | 2 +- project/Dependencies.scala | 5 +- .../sam/dataAccess/AccessPolicyDAO.scala | 1 - .../dataAccess/PostgresAccessPolicyDAO.scala | 145 ++++++++-------- .../sam/model/FilterResourcesResult.scala | 9 +- .../sam/service/ResourceService.scala | 160 +++++++++++++++--- .../sam/dataAccess/MockAccessPolicyDAO.scala | 53 +++--- .../PostgresAccessPolicyDAOSpec.scala | 129 ++++---------- .../StatefulMockAccessPolicyDaoBuilder.scala | 80 +++------ .../sam/service/ResourceServiceUnitSpec.scala | 104 +++++------- 11 files changed, 328 insertions(+), 364 deletions(-) diff --git a/pact4s/README.md b/pact4s/README.md index eb8053df8..55daf9a24 100644 --- a/pact4s/README.md +++ b/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=$(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) +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')" ``` In IntelliJ, you can create a Run Configuration for `SamProviderSpec.scala` and save `Environment Variables` for: diff --git a/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala b/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala index 5e3d9e603..e7ed9ae97 100644 --- a/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala +++ b/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] ) } diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 8dc277824..8051115a5 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -171,6 +171,8 @@ 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. @@ -222,7 +224,8 @@ object Dependencies { sentry, sentryLogback, okio, - terraCommonLib + terraCommonLib, + caffeine ) // Needed because it looks like the dependency overrides of wb-libs doesn't propagate to the importing project... 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 index 755e027cd..191e4329a 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/AccessPolicyDAO.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/AccessPolicyDAO.scala @@ -125,7 +125,6 @@ trait AccessPolicyDAO { resourceTypeNames: Set[ResourceTypeName], policies: Set[AccessPolicyName], roles: Set[ResourceRoleName], - actions: Set[ResourceAction], includePublic: Boolean, samRequestContext: SamRequestContext ): IO[Seq[FilterResourcesResult]] diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala index 68d7e508d..b53735eea 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala @@ -22,10 +22,8 @@ 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 +import com.github.benmanes.caffeine.cache.Caffeine +import java.util.concurrent.{ConcurrentMap, TimeUnit} class PostgresAccessPolicyDAO( protected val writeDbRef: DbReference, @@ -1779,8 +1777,8 @@ from ${GroupMemberTable as groupMemberTable} } } - private val publicResourcesCache: java.util.Map[ResourceTypeName, Seq[FilterResourcesResult]] = - Collections.synchronizedMap(new PassiveExpiringMap(1, TimeUnit.HOURS)) + private val publicResourcesCache: ConcurrentMap[ResourceTypeName, Seq[FilterResourcesResult]] = + Caffeine.newBuilder().expireAfterWrite(1, TimeUnit.HOURS).build[ResourceTypeName, Seq[FilterResourcesResult]]().asMap() private def getPublicResourcesOfType(resourceTypeName: ResourceTypeName, samRequestContext: SamRequestContext): IO[Seq[FilterResourcesResult]] = { val resourcePolicy = PolicyTable.syntax("resourcePolicy") @@ -1794,61 +1792,63 @@ from ${GroupMemberTable as groupMemberTable} 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 + samsql""" + select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, ${resourceRole.result.role}, ${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} + 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 + 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 + samsql""" + select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, ${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} + 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 + join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id} $resourceTypeConstraint where ${resourcePolicy.public} $resourceTypeConstraint - $notNullConstraintPolicyAction """ - val includePublicPolicyActionQuery = samsqls"union all $publicPolicyActionQuery" - val publicResourcesQuery = samsql"$publicRoleActionQuery $includePublicPolicyActionQuery" - readOnlyTransaction("filterResourcesPublic", samRequestContext) { implicit session => publicResourcesCache.computeIfAbsent( resourceTypeName, - resourceTypeName => - publicResourcesQuery + resourceTypeName => { + val roles = publicRoleActionQuery .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[AccessPolicyName](resourcePolicy.resultName.name), + rs.stringOpt(resourceRole.resultName.role).map(r => Left[ResourceRoleName, ResourceAction](ResourceRoleName(r))), rs.get[Boolean](resourcePolicy.resultName.public), - None, - false, rs.booleanOpt("inherited").getOrElse(false) ) ) .list() .apply() + val actions = publicPolicyActionQuery + .map(rs => + FilterResourcesResult( + rs.get[ResourceId](resource.resultName.name), + resourceTypeNamesByPK(rs.get[ResourceTypePK](resource.resultName.resourceTypeId)), + rs.get[AccessPolicyName](resourcePolicy.resultName.name), + rs.stringOpt(resourceAction.resultName.action).map(a => Right[ResourceRoleName, ResourceAction](ResourceAction(a))), + rs.get[Boolean](resourcePolicy.resultName.public), + rs.booleanOpt("inherited").getOrElse(false) + ) + ) + .list() + .apply() + roles ++ actions + } ) } } @@ -1857,7 +1857,6 @@ from ${GroupMemberTable as groupMemberTable} resourceTypeNames: Set[ResourceTypeName], policies: Set[AccessPolicyName], roles: Set[ResourceRoleName], - actions: Set[ResourceAction], samRequestContext: SamRequestContext ): IO[Seq[FilterResourcesResult]] = { val groupMemberFlat = GroupMemberFlatTable.syntax("groupMemberFlat") @@ -1869,83 +1868,77 @@ from ${GroupMemberTable as groupMemberTable} val roleAction = RoleActionTable.syntax("roleAction") val resourceAction = ResourceActionTable.syntax("resourceAction") val resource = ResourceTable.syntax("resource") - val authDomain = AuthDomainTable.syntax("authDomain") - val authDomainGroup = GroupTable.syntax("authDomainGroup") - val authDomainGroupMemberFlat = GroupMemberFlatTable.syntax("authDomainGroupMemberFlat") val resourceTypeConstraint = if (resourceTypeNames.nonEmpty) samsqls"and ${resource.resourceTypeId} in (${resourceTypeNames.flatMap(resourceTypePKsByName.get)})" else samsqls"and ${resource.resourceTypeId} in (${resourceTypeNamesByPK.keys.map(_.value)})" 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""" - select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, ${resourceRole.result.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 + samsql""" + select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, ${resourceRole.result.role}, ${resourcePolicy.result.public}, ${resourcePolicy.resourceId} != ${resource.id} as inherited from ${GroupMemberFlatTable as groupMemberFlat} - left join ${PolicyTable as resourcePolicy} on ${groupMemberFlat.groupId} = ${resourcePolicy.groupId} - left join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} + join ${PolicyTable as resourcePolicy} on ${groupMemberFlat.groupId} = ${resourcePolicy.groupId} + join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} + join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id} 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} - 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} = ${samUserId} where ${groupMemberFlat.memberUserId} = ${samUserId} $resourceTypeConstraint $policyConstraint $roleConstraint - $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 + samsql""" + select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, ${resourceAction.result.action}, ${resourcePolicy.result.public}, ${resourcePolicy.resourceId} != ${resource.id} as inherited from ${GroupMemberFlatTable as groupMemberFlat} - left join ${PolicyTable as resourcePolicy} on ${groupMemberFlat.groupId} = ${resourcePolicy.groupId} - left join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} + join ${PolicyTable as resourcePolicy} on ${groupMemberFlat.groupId} = ${resourcePolicy.groupId} + join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} + join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id} 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} - 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} = ${samUserId} where ${groupMemberFlat.memberUserId} = ${samUserId} $resourceTypeConstraint $policyConstraint - $actionConstraint - $notNullConstraintPolicyAction """ - val includePolicyActionQuery = if (roles.isEmpty) samsqls"union all $policyActionQuery" else samsqls"" - val query = - samsqls"""$policyRoleActionQuery - $includePolicyActionQuery""" - readOnlyTransaction("filterResources", samRequestContext) { implicit session => - samsql"$query" + val rolesResult = policyRoleActionQuery .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[AccessPolicyName](resourcePolicy.resultName.name), + rs.stringOpt(resourceRole.resultName.role).map(r => Left[ResourceRoleName, ResourceAction](ResourceRoleName(r))), rs.get[Boolean](resourcePolicy.resultName.public), - rs.stringOpt(authDomainGroup.resultName.name).map(WorkbenchGroupName(_)), - rs.booleanOpt("in_auth_domain").getOrElse(false), rs.booleanOpt("inherited").getOrElse(false) ) ) .list() .apply() + + // if roles is not empty that means we are filtering by roles and we don't need to query for actions directly on a policy since they have no roles + val actionsResult = if (roles.nonEmpty) { + List.empty + } else { + policyActionQuery + .map(rs => + FilterResourcesResult( + rs.get[ResourceId](resource.resultName.name), + resourceTypeNamesByPK(rs.get[ResourceTypePK](resource.resultName.resourceTypeId)), + rs.get[AccessPolicyName](resourcePolicy.resultName.name), + rs.stringOpt(resourceAction.resultName.action).map(a => Right[ResourceRoleName, ResourceAction](ResourceAction(a))), + rs.get[Boolean](resourcePolicy.resultName.public), + rs.booleanOpt("inherited").getOrElse(false) + ) + ) + .list() + .apply() + } + rolesResult ++ actionsResult + } } @@ -1954,7 +1947,6 @@ from ${GroupMemberTable as groupMemberTable} resourceTypeNames: Set[ResourceTypeName], policies: Set[AccessPolicyName], roles: Set[ResourceRoleName], - actions: Set[ResourceAction], includePublic: Boolean, samRequestContext: SamRequestContext ): IO[Seq[FilterResourcesResult]] = @@ -1966,11 +1958,10 @@ from ${GroupMemberTable as groupMemberTable} .sequence .map(_.flatten) } else IO.pure(List.empty) - privateResources <- filterPrivateResources(samUserId, resourceTypeNames, policies, roles, actions, samRequestContext) + privateResources <- filterPrivateResources(samUserId, resourceTypeNames, policies, roles, 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 + .filter(r => policies.isEmpty || policies.contains(r.policy)) + .filter(r => roles.isEmpty || r.roleOrAction.exists(_.left.exists(role => roles.contains(role)))) ++ privateResources private def recreateEffectivePolicyRolesTableEntry(resourceTypeNames: Set[ResourceTypeName])(implicit session: DBSession): Int = { val resource = ResourceTable.syntax("resource") diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/FilterResourcesResult.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/FilterResourcesResult.scala index 2a6cdcc5a..04d729632 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/FilterResourcesResult.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/FilterResourcesResult.scala @@ -1,15 +1,10 @@ package org.broadinstitute.dsde.workbench.sam.model -import org.broadinstitute.dsde.workbench.model.WorkbenchGroupName - case class FilterResourcesResult( resourceId: ResourceId, resourceTypeName: ResourceTypeName, - policy: Option[AccessPolicyName], - role: Option[ResourceRoleName], - action: Option[ResourceAction], + policy: AccessPolicyName, + roleOrAction: Option[Either[ResourceRoleName, ResourceAction]], isPublic: Boolean, - authDomain: Option[WorkbenchGroupName], - inAuthDomain: Boolean, inherited: Boolean ) 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 index 5ba6767af..ebf1c33f4 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceService.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceService.scala @@ -116,7 +116,10 @@ class ResourceService( samRequestContext ) } - } yield upsertedPolicy + } yield { + logger.info(s"Upserted policy $fullyQualifiedPolicyId") + upsertedPolicy + } upsertIO.attempt.map(fullyQualifiedPolicyId -> _) } .map(_.toMap) @@ -1028,62 +1031,91 @@ class ResourceService( authDomainGroups: Map[WorkbenchGroupName, Boolean] = Map.empty ) - private def groupFlat(dbResult: Seq[FilterResourcesResult]): FilteredResourcesFlat = { + private def groupFlat( + dbResult: Seq[FilterResourcesResult], + filterActions: Set[ResourceAction], + resourceAuthDomains: Map[FullyQualifiedResourceId, Set[WorkbenchGroupName]], + userGroups: Set[WorkbenchGroupName] + ): FilteredResourcesFlat = { val groupedFilteredResource = dbResult .groupBy(_.resourceId) .map { tuple => val (k, v) = tuple - val grouped = v.foldLeft(GroupedDbRows())((acc: GroupedDbRows, r: FilterResourcesResult) => + 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) acc.copy( - 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) + policies = acc.policies + FilteredResourceFlatPolicy(r.policy, r.isPublic, r.inherited), + roles = acc.roles ++ filteredRole, + actions = acc.actions ++ filteredPolicyActions ++ roleActions ) - ) + } + 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 = grouped.authDomainGroups.keySet, - missingAuthDomainGroups = grouped.authDomainGroups.filter(!_._2).keySet // Get only the auth domains where the user is not a member. + authDomainGroups = authDomainGroups, + missingAuthDomainGroups = authDomainGroups -- userGroups ) } .toSet FilteredResourcesFlat(resources = groupedFilteredResource) } - private def groupHierarchical(dbResult: Seq[FilterResourcesResult]): FilteredResourcesHierarchical = { + 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 = { val groupedFilteredResources = dbResult .groupBy(_.resourceId) .map { tuple => val (resourceId, resourceRows) = tuple val policies = resourceRows - .groupBy(_.policy.get) + .groupBy(_.policy) .map { policyTuple => val (policyName, policyRows) = policyTuple - 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) + // 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) + ) } .toSet - FilteredResourceHierarchicalPolicy(policyName, roles, actionsWithoutRoles, policyRows.head.isPublic, policyRows.head.inherited) + // 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) } .toSet - val authDomainGroupMemberships = resourceRows.flatMap(r => r.authDomain.map(_ -> r.inAuthDomain)).toMap + val fullyQualifiedResourceId = FullyQualifiedResourceId(resourceRows.head.resourceTypeName, resourceId) + val authDomainGroups = resourceAuthDomains.getOrElse(fullyQualifiedResourceId, Set.empty) FilteredResourceHierarchical( resourceId = resourceId, resourceType = resourceRows.head.resourceTypeName, policies = policies, - authDomainGroups = authDomainGroupMemberships.keySet, - missingAuthDomainGroups = authDomainGroupMemberships.filter(!_._2).keySet // Get only the auth domains where the user is not a member. + authDomainGroups = authDomainGroups, + missingAuthDomainGroups = authDomainGroups -- userGroups ) } .toSet @@ -1125,7 +1157,16 @@ class ResourceService( includePublic: Boolean, samRequestContext: SamRequestContext ): IO[FilteredResourcesFlat] = - accessPolicyDAO.filterResources(samUserId, resourceTypeNames, policies, roles, actions, includePublic, samRequestContext).map(groupFlat) + 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 + } + } def listResourcesHierarchical( samUserId: WorkbenchUserId, @@ -1136,7 +1177,72 @@ class ResourceService( includePublic: Boolean, samRequestContext: SamRequestContext ): IO[FilteredResourcesHierarchical] = - accessPolicyDAO - .filterResources(samUserId, resourceTypeNames, policies, roles, actions, includePublic, samRequestContext) - .map(groupHierarchical) + 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)) + } } diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockAccessPolicyDAO.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockAccessPolicyDAO.scala index b9a00c3e4..e7e76b737 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockAccessPolicyDAO.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockAccessPolicyDAO.scala @@ -361,7 +361,6 @@ 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 { @@ -370,38 +369,28 @@ 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 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 roleResults = accessPolicy.roles.map { role => + FilterResourcesResult( + fqPolicyId.resource.resourceId, + fqPolicyId.resource.resourceTypeName, + fqPolicyId.accessPolicyName, + Option(Left(role)), + accessPolicy.public, + 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 diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAOSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAOSpec.scala index 6c85d9c52..80d4dc530 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAOSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAOSpec.scala @@ -3277,18 +3277,11 @@ class PostgresAccessPolicyDAOSpec extends AnyFreeSpec with Matchers with BeforeA "filterResources" - { - def verify( - dbResultRows: Seq[FilterResourcesResult], - roles: Set[ResourceRoleName], - roleActions: Set[ResourceAction], - policyActions: Set[ResourceAction] - ) = { - val testRoles: Set[ResourceRoleName] = dbResultRows.flatMap(_.role).toSet - val testRoleActions: Set[ResourceAction] = dbResultRows.filter(_.role.isDefined).flatMap(_.action).toSet - val testPolicyActions: Set[ResourceAction] = dbResultRows.filter(_.role.isEmpty).flatMap(_.action).toSet + def verify(dbResultRows: Seq[FilterResourcesResult], roles: Set[ResourceRoleName], policyActions: Set[ResourceAction]): Any = { + val testRoles: Set[ResourceRoleName] = dbResultRows.flatMap(_.roleOrAction.flatMap(_.left.toOption)).toSet + val testPolicyActions: Set[ResourceAction] = dbResultRows.flatMap(_.roleOrAction.flatMap(_.toOption)).toSet testRoles should be(roles) - testRoleActions should be(roleActions) testPolicyActions should be(policyActions) } @@ -3335,84 +3328,67 @@ class PostgresAccessPolicyDAOSpec extends AnyFreeSpec with Matchers with BeforeA false ) // No roles available - val publicProbePolicy = AccessPolicy( + val publicProbeEmptyPolicy = AccessPolicy( FullyQualifiedPolicyId(kitchenSink.fullyQualifiedId, AccessPolicyName(uuid)), Set.empty, WorkbenchEmail(s"${uuid}@policy.com"), - Set(actionlessRole.roleName), + Set.empty, Set.empty, Set.empty, true ) + val privateProbeEmptyPolicy = AccessPolicy( + FullyQualifiedPolicyId(kitchenSink.fullyQualifiedId, AccessPolicyName(uuid)), + Set(user.id), + WorkbenchEmail(s"${uuid}@policy.com"), + Set.empty, + Set.empty, + Set.empty, + false + ) dao.createPolicy(directProbePolicy, samRequestContext).unsafeRunSync() - dao.createPolicy(publicProbePolicy, samRequestContext).unsafeRunSync() - - val writeActions = - dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set.empty, Set(writeAction), false, samRequestContext).unsafeRunSync() - writeActions.length should be(5) - writeActions.map(_.action).forall(a => a.exists(_.equals(writeAction))) should be(true) - writeActions.map(_.isPublic).forall(ip => !ip) should be(true) - - val writerViaOwner = writeActions.filter(r => r.role.exists(_.equals(ownerRole.roleName))) - writerViaOwner.size should be(1) - - val readActions = dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set.empty, Set(readAction), false, samRequestContext).unsafeRunSync() - readActions.length should be(6) - - val readerViaOwner = readActions.filter(r => r.role.exists(_.equals(ownerRole.roleName))) - readerViaOwner.size should be(1) + dao.createPolicy(publicProbeEmptyPolicy, samRequestContext).unsafeRunSync() + dao.createPolicy(privateProbeEmptyPolicy, samRequestContext).unsafeRunSync() val readerRoles = - dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set(readerRole.roleName), Set.empty, false, samRequestContext).unsafeRunSync() + dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set(readerRole.roleName), false, samRequestContext).unsafeRunSync() readerRoles.size should be(5) val policies = dao - .filterResources(user.id, Set(resourceTypeName), Set(directProbePolicy.id.accessPolicyName), Set.empty, Set.empty, false, samRequestContext) + .filterResources(user.id, Set(resourceTypeName), Set(directProbePolicy.id.accessPolicyName), Set.empty, false, samRequestContext) .unsafeRunSync() - val foundPolicies = policies.flatMap(_.policy).toSet + val foundPolicies = policies.map(_.policy).toSet foundPolicies.size should be(1) foundPolicies.head should be(directProbePolicy.id.accessPolicyName) - val writeActionsIncludingPublic = - dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set.empty, Set(writeAction), true, samRequestContext).unsafeRunSync() - writeActionsIncludingPublic.length should be(7) - writeActionsIncludingPublic.filter(_.isPublic).map(_.resourceId).toSet should be(Set(publicResource.resourceId, publicChildResource.resourceId)) - val readerRolesIncludingPublic = - dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set(readerRole.roleName), Set.empty, true, samRequestContext).unsafeRunSync() + dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set(readerRole.roleName), true, samRequestContext).unsafeRunSync() readerRolesIncludingPublic.size should be(7) readerRolesIncludingPublic.filter(_.isPublic).map(_.resourceId).toSet should be(Set(publicResource.resourceId, publicChildResource.resourceId)) - println(dao.filterResources(user.id, Set(resourceTypeName), Set.empty, Set.empty, Set.empty, true, samRequestContext).unsafeRunSync()) - val inheritedReaderRoles = - dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set(readerRole.roleName), Set.empty, true, samRequestContext).unsafeRunSync() + dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set(readerRole.roleName), true, samRequestContext).unsafeRunSync() val inheritedPolicies = inheritedReaderRoles.filter(_.inherited) inheritedPolicies.map(_.resourceId).toSet should be( Set(childResource1.resourceId, childResource2.resourceId, publicChildResource.resourceId, kitchenSink.resourceId) ) - val filtered = dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set.empty, Set.empty, true, samRequestContext).unsafeRunSync() + val filtered = dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set.empty, true, samRequestContext).unsafeRunSync() + + val emptyPublicResults = filtered.filter(_.policy == publicProbeEmptyPolicy.id.accessPolicyName) + emptyPublicResults.map(_.roleOrAction) should contain only None + val emptyPrivateResults = filtered.filter(_.policy == privateProbeEmptyPolicy.id.accessPolicyName) + emptyPrivateResults.map(_.roleOrAction) should contain only None val readRoleWriteActionResources = Seq(userReadRoleWriteAction, groupReadRoleWriteAction, childResource1, childResource2, publicResource, publicChildResource) readRoleWriteActionResources .map(resource => - verify( - filtered.filter(_.resourceId.equals(resource.resourceId)), - roles = Set(readerRole.roleName), - roleActions = Set(readAction), - policyActions = Set(writeAction) - ) + verify(filtered.filter(_.resourceId.equals(resource.resourceId)), roles = Set(readerRole.roleName), policyActions = Set(writeAction)) ) - verify( - filtered.filter(_.resourceId.equals(kitchenSink.resourceId)), - roles = Set(readerRole.roleName, ownerRole.roleName, actionlessRole.roleName), - roleActions = Set(readAction, writeAction), - policyActions = Set.empty - ) + verify(filtered.filter(_.resourceId.equals(kitchenSink.resourceId)), roles = Set(readerRole.roleName, ownerRole.roleName), policyActions = Set.empty) } "filters on the user's policies, roles, and actions when using nested roles" in { @@ -3449,60 +3425,17 @@ class PostgresAccessPolicyDAOSpec extends AnyFreeSpec with Matchers with BeforeA createResourceHierarchy(Option(parentGroup.id), Set(writeAction), Set(includesRole.roleName, descendsRole.roleName), false, nestedResourceType.name) val nestedReaderRoles = - dao.filterResources(user.id, Set(nestedResourceType.name), Set.empty, Set(readerRole.roleName), Set.empty, false, samRequestContext).unsafeRunSync() + dao.filterResources(user.id, Set(nestedResourceType.name), Set.empty, Set(readerRole.roleName), false, samRequestContext).unsafeRunSync() nestedReaderRoles.size should be(4) nestedReaderRoles.map(_.resourceId).toSet should be( Set(directAccessResource.resourceId, groupAccessResource.resourceId, directAccessChildResource.resourceId, groupAccessChildResource.resourceId) ) val publicNestedReaderRoles = - dao.filterResources(user.id, Set(nestedResourceType.name), Set.empty, Set(readerRole.roleName), Set.empty, true, samRequestContext).unsafeRunSync() + dao.filterResources(user.id, Set(nestedResourceType.name), Set.empty, Set(readerRole.roleName), true, samRequestContext).unsafeRunSync() publicNestedReaderRoles.size should be(6) publicNestedReaderRoles.filter(_.isPublic).map(_.resourceId).toSet should be(Set(publicResource.resourceId, publicChildResource.resourceId)) } - - "includes Authorization Domain information in its queries" in { - assume(databaseEnabled, databaseEnabledClue) - - val user = Generator.genWorkbenchUserGoogle.sample.get - - val subGroup = BasicWorkbenchGroup(WorkbenchGroupName("subGroup"), Set(user.id), WorkbenchEmail("sub@groups.com")) - val parentGroup = BasicWorkbenchGroup(WorkbenchGroupName("parent"), Set(subGroup.id), WorkbenchEmail("parent@groups.com")) - val authDomainGroup1 = BasicWorkbenchGroup(WorkbenchGroupName("authDomainGroup1"), Set(parentGroup.id), WorkbenchEmail("authDomainGroup1@groups.com")) - val authDomainGroup2 = BasicWorkbenchGroup(WorkbenchGroupName("authDomainGroup2"), Set.empty, WorkbenchEmail("authDomainGroup2@groups.com")) - - dirDao.createUser(user, samRequestContext).unsafeRunSync() - dirDao.createGroup(subGroup, samRequestContext = samRequestContext).unsafeRunSync() - dirDao.createGroup(parentGroup, samRequestContext = samRequestContext).unsafeRunSync() - dirDao.createGroup(authDomainGroup1, samRequestContext = samRequestContext).unsafeRunSync() - dirDao.createGroup(authDomainGroup2, samRequestContext = samRequestContext).unsafeRunSync() - dirDao.addGroupMember(subGroup.id, user.id, samRequestContext).unsafeRunSync() - dirDao.addGroupMember(parentGroup.id, subGroup.id, samRequestContext).unsafeRunSync() - dao.createResourceType(resourceType, samRequestContext).unsafeRunSync() - dao.createResourceType(otherResourceType, samRequestContext).unsafeRunSync() - - // can access via auth domain - val resource1 = createResource(Option(user.id), Set(writeAction), Set(readerRole.roleName), public = false, authDomainGroups = Set(authDomainGroup1)) - // cannot access via auth domain - val resource2 = - createResource(Option(user.id), Set(writeAction), Set(readerRole.roleName), false, authDomainGroups = Set(authDomainGroup1, authDomainGroup2)) - - val writeActions = - dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set.empty, Set(writeAction), false, samRequestContext).unsafeRunSync() - val byResource = writeActions.groupBy(_.resourceId) - - val resource1Results = byResource(resource1.resourceId) - resource1Results.length should be(1) - resource1Results.head.authDomain should be(Some(authDomainGroup1.id)) - resource1Results.head.inAuthDomain should be(true) - - val resource2Results = byResource(resource2.resourceId) - resource2Results.length should be(2) - resource2Results.flatMap(_.authDomain).toSet should be(Set(authDomainGroup1.id, authDomainGroup2.id)) - resource2Results.map(_.inAuthDomain).toSet should be(Set(true, false)) - resource2Results.filter(_.inAuthDomain).head.authDomain should be(Some(authDomainGroup1.id)) - resource2Results.filter(!_.inAuthDomain).head.authDomain should be(Some(authDomainGroup2.id)) - } } "listResourcesUsingAuthDomain" - { diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/StatefulMockAccessPolicyDaoBuilder.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/StatefulMockAccessPolicyDaoBuilder.scala index 9acb40e3a..c9c3f1fa0 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/StatefulMockAccessPolicyDaoBuilder.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/StatefulMockAccessPolicyDaoBuilder.scala @@ -92,7 +92,6 @@ case class StatefulMockAccessPolicyDaoBuilder() extends MockitoSugar { ArgumentMatchers.eq(Set(policy.id.resource.resourceTypeName)), ArgumentMatchers.eq(Set.empty), ArgumentMatchers.eq(Set.empty), - ArgumentMatchers.eq(Set.empty), ArgumentMatchers.eq(true), any[SamRequestContext] ) @@ -115,67 +114,42 @@ case class StatefulMockAccessPolicyDaoBuilder() extends MockitoSugar { ) } - private def constructFilterResourcesResult(accessPolicy: AccessPolicy): Seq[FilterResourcesResult] = - if (accessPolicy.roles.isEmpty) { + private def constructFilterResourcesResult(accessPolicy: AccessPolicy): Seq[FilterResourcesResult] = { + val results = accessPolicy.roles.map { role => + FilterResourcesResult( + accessPolicy.id.resource.resourceId, + accessPolicy.id.resource.resourceTypeName, + accessPolicy.id.accessPolicyName, + Option(Left(role)), + accessPolicy.public, + false + ) + }.toSeq ++ accessPolicy.actions.map { action => + FilterResourcesResult( + accessPolicy.id.resource.resourceId, + accessPolicy.id.resource.resourceTypeName, + accessPolicy.id.accessPolicyName, + Option(Right(action)), + accessPolicy.public, + false + ) + + }.toSeq + if (results.isEmpty) { Seq( FilterResourcesResult( accessPolicy.id.resource.resourceId, accessPolicy.id.resource.resourceTypeName, - Some(accessPolicy.id.accessPolicyName), - None, + accessPolicy.id.accessPolicyName, None, accessPolicy.public, - None, - false, false ) ) - } else - { - accessPolicy.roles.map { role => - FilterResourcesResult( - accessPolicy.id.resource.resourceId, - accessPolicy.id.resource.resourceTypeName, - Some(accessPolicy.id.accessPolicyName), - Some(role), - None, - accessPolicy.public, - None, - false, - false - ) - }.toSeq - } ++ - (if (accessPolicy.actions.isEmpty) { - Seq( - FilterResourcesResult( - accessPolicy.id.resource.resourceId, - accessPolicy.id.resource.resourceTypeName, - Some(accessPolicy.id.accessPolicyName), - None, - None, - accessPolicy.public, - None, - false, - false - ) - ) - } else { - accessPolicy.actions.map { action => - FilterResourcesResult( - accessPolicy.id.resource.resourceId, - accessPolicy.id.resource.resourceTypeName, - Some(accessPolicy.id.accessPolicyName), - None, - Some(action), - accessPolicy.public, - None, - false, - false - ) - - }.toSeq - }) + } else { + results + } + } def withRandomAccessPolicy(resourceTypeName: ResourceTypeName, members: Set[WorkbenchSubject]): StatefulMockAccessPolicyDaoBuilder = { val policy = AccessPolicy( diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceUnitSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceUnitSpec.scala index 2487bc3d6..91938664b 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceUnitSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceUnitSpec.scala @@ -40,107 +40,64 @@ class ResourceServiceUnitSpec extends AnyFlatSpec with Matchers with ScalaFuture FilterResourcesResult( ResourceId(UUID.randomUUID().toString), resourceTypeName, - Some(AccessPolicyName(UUID.randomUUID().toString)), - Some(readerRoleName), - Some(readAction), + AccessPolicyName(UUID.randomUUID().toString), + Option(Left(readerRoleName)), true, - None, - false, - false - ), - FilterResourcesResult( - ResourceId(UUID.randomUUID().toString), - resourceTypeName, - Some(AccessPolicyName(UUID.randomUUID().toString)), - None, - None, - true, - None, - false, - false - ), - FilterResourcesResult( - ResourceId(UUID.randomUUID().toString), - resourceTypeName, - Some(AccessPolicyName(UUID.randomUUID().toString)), - Some(readerRoleName), - Some(readAction), - false, - None, - false, false ), FilterResourcesResult( ResourceId(UUID.randomUUID().toString), resourceTypeName, - Some(AccessPolicyName(UUID.randomUUID().toString)), - None, - None, - false, - None, + AccessPolicyName(UUID.randomUUID().toString), + Option(Left(readerRoleName)), false, false ), // Testable DB Results - FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy1), Some(readerRoleName), Some(readAction), false, None, false, false), + FilterResourcesResult(testResourceId, resourceTypeName, testPolicy1, Option(Left(readerRoleName)), false, false), FilterResourcesResult( testResourceId, resourceTypeName, - Some(testPolicy1), - Some(readerRoleName), - Some(readAction), - false, - None, + testPolicy1, + Option(Left(readerRoleName)), false, false ), // testing duplicate row results FilterResourcesResult( testResourceId, resourceTypeName, - Some(testPolicy1), - Some(readerRoleName), - Some(readAction), - false, - None, + testPolicy1, + Option(Left(readerRoleName)), false, false ), // testing duplicate row results - FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy2), Some(nothingRoleName), None, true, None, false, false), - FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy3), None, None, false, None, false, false), - FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy4), Some(ownerRoleName), Some(readAction), false, None, false, false), - FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy4), Some(ownerRoleName), Some(writeAction), false, None, false, false), - FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy5), None, Some(readAction), true, None, false, false), + FilterResourcesResult(testResourceId, resourceTypeName, testPolicy2, Option(Left(nothingRoleName)), true, false), + FilterResourcesResult(testResourceId, resourceTypeName, testPolicy3, None, false, false), + FilterResourcesResult(testResourceId, resourceTypeName, testPolicy4, Option(Left(ownerRoleName)), false, false), + FilterResourcesResult(testResourceId, resourceTypeName, testPolicy4, Option(Left(ownerRoleName)), false, false), + FilterResourcesResult(testResourceId, resourceTypeName, testPolicy5, Option(Right(readAction)), true, false), FilterResourcesResult( testResourceId, resourceTypeName, - Some(testPolicy5), - None, - Some(readAction), + testPolicy5, + Option(Right(readAction)), true, - None, - false, false ), // testing duplicate row results // Auth Domain Results FilterResourcesResult( testResourceId2, resourceTypeName, - Some(testPolicy6), - Some(readerRoleName), - Some(readAction), + testPolicy6, + Option(Left(readerRoleName)), false, - Some(authDomainGroup1), - true, false ), FilterResourcesResult( testResourceId2, resourceTypeName, - Some(testPolicy6), - Some(readerRoleName), - Some(readAction), - false, - Some(authDomainGroup2), + testPolicy6, + Option(Left(readerRoleName)), false, false ) @@ -153,15 +110,29 @@ class ResourceServiceUnitSpec extends AnyFlatSpec with Matchers with ScalaFuture any[Set[ResourceTypeName]], any[Set[AccessPolicyName]], any[Set[ResourceRoleName]], - any[Set[ResourceAction]], any[Boolean], any[SamRequestContext] ) ) .thenReturn(IO.pure(dbResult)) + when(mockAccessPolicyDAO.listResourcesWithAuthdomains(any[ResourceTypeName], any[Set[ResourceId]], any[SamRequestContext])) + .thenReturn(IO.pure(Set(Resource(resourceTypeName, testResourceId2, Set(authDomainGroup1, authDomainGroup2), Set.empty)))) + when(mockAccessPolicyDAO.listAccessPolicies(eqTo(ManagedGroupService.managedGroupTypeName), any[WorkbenchUserId], any[SamRequestContext])) + .thenReturn(IO.pure(Set(ResourceIdAndPolicyName(ResourceId(authDomainGroup1.value), ManagedGroupService.memberPolicyName)))) val resourceService = new ResourceService( - Map.empty, + Map( + resourceTypeName -> ResourceType( + resourceTypeName, + Set(ResourceActionPattern(readAction.value, "", true)), + Set( + ResourceRole(readerRoleName, Set(readAction)), + ResourceRole(ownerRoleName, Set(readAction, writeAction)), + ResourceRole(nothingRoleName, Set.empty, Set.empty) + ), + ownerRoleName + ) + ), mock[PolicyEvaluatorService], mockAccessPolicyDAO, mock[DirectoryDAO], @@ -215,6 +186,9 @@ class ResourceServiceUnitSpec extends AnyFlatSpec with Matchers with ScalaFuture role.role should be(ownerRoleName) role.actions should be(Set(readAction, writeAction)) + policies.filter(_.policy.equals(testPolicy3)).flatMap(_.roles) should be(empty) + policies.filter(_.policy.equals(testPolicy3)).flatMap(_.actions) should be(empty) + val authDomainResource = filteredResources.resources.filter(_.resourceId.equals(testResourceId2)).head authDomainResource.resourceType should be(resourceTypeName) authDomainResource.policies.map(_.policy) should be(Set(testPolicy6)) From 236171624eee13225df31b6a308ee42b9b9bb9b8 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Tue, 12 Nov 2024 12:03:43 -0500 Subject: [PATCH 09/15] Update auto-approve-broadbot-prs.yml (#1584) --- .github/workflows/auto-approve-broadbot-prs.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/auto-approve-broadbot-prs.yml b/.github/workflows/auto-approve-broadbot-prs.yml index c93fcc047..7a5f2eb54 100644 --- a/.github/workflows/auto-approve-broadbot-prs.yml +++ b/.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" From 57b278fdcd4b061bef696aefe6b0800f82416aab Mon Sep 17 00:00:00 2001 From: dvoet Date: Tue, 12 Nov 2024 13:41:48 -0500 Subject: [PATCH 10/15] Revert "CORE-137 Optimize filter resources (#1577)" (#1588) This reverts commit ab2829dba3c0a62f9f1f2b29426a9a8798fd874c. --- pact4s/README.md | 4 +- .../dsde/workbench/sam/MockTestSupport.scala | 2 +- project/Dependencies.scala | 5 +- .../sam/dataAccess/AccessPolicyDAO.scala | 1 + .../dataAccess/PostgresAccessPolicyDAO.scala | 145 ++++++++-------- .../sam/model/FilterResourcesResult.scala | 9 +- .../sam/service/ResourceService.scala | 160 +++--------------- .../sam/dataAccess/MockAccessPolicyDAO.scala | 53 +++--- .../PostgresAccessPolicyDAOSpec.scala | 129 ++++++++++---- .../StatefulMockAccessPolicyDaoBuilder.scala | 80 ++++++--- .../sam/service/ResourceServiceUnitSpec.scala | 104 +++++++----- 11 files changed, 364 insertions(+), 328 deletions(-) diff --git a/pact4s/README.md b/pact4s/README.md index 55daf9a24..eb8053df8 100644 --- a/pact4s/README.md +++ b/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: diff --git a/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala b/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala index e7ed9ae97..5e3d9e603 100644 --- a/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala +++ b/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] ) } diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 8051115a5..8dc277824 100644 --- a/project/Dependencies.scala +++ b/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... 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 index 191e4329a..755e027cd 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/AccessPolicyDAO.scala +++ b/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]] diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala index b53735eea..68d7e508d 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala @@ -22,8 +22,10 @@ import scalikejdbc._ import scala.collection.concurrent.TrieMap import scala.util.{Failure, Try} import cats.effect.Temporal -import com.github.benmanes.caffeine.cache.Caffeine -import java.util.concurrent.{ConcurrentMap, TimeUnit} +import org.apache.commons.collections4.map.PassiveExpiringMap + +import java.util.Collections +import java.util.concurrent.TimeUnit class PostgresAccessPolicyDAO( protected val writeDbRef: DbReference, @@ -1777,8 +1779,8 @@ from ${GroupMemberTable as groupMemberTable} } } - private val publicResourcesCache: ConcurrentMap[ResourceTypeName, Seq[FilterResourcesResult]] = - Caffeine.newBuilder().expireAfterWrite(1, TimeUnit.HOURS).build[ResourceTypeName, Seq[FilterResourcesResult]]().asMap() + 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") @@ -1792,63 +1794,61 @@ from ${GroupMemberTable as groupMemberTable} 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 = - samsql""" - select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, ${resourceRole.result.role}, ${resourcePolicy.result.public}, ${resourcePolicy.resourceId} != ${resource.id} as inherited + 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} - join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} and ${resourcePolicy.public} + 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} - join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id} $resourceTypeConstraint + 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 = - samsql""" - select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, ${resourceAction.result.action}, ${resourcePolicy.result.public}, ${resourcePolicy.resourceId} != ${resource.id} as inherited + 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} - join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} and ${resourcePolicy.public} + 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} - join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id} $resourceTypeConstraint + left join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id} $resourceTypeConstraint where ${resourcePolicy.public} $resourceTypeConstraint + $notNullConstraintPolicyAction """ + val includePublicPolicyActionQuery = samsqls"union all $publicPolicyActionQuery" + val publicResourcesQuery = samsql"$publicRoleActionQuery $includePublicPolicyActionQuery" + readOnlyTransaction("filterResourcesPublic", samRequestContext) { implicit session => publicResourcesCache.computeIfAbsent( resourceTypeName, - resourceTypeName => { - val roles = publicRoleActionQuery + resourceTypeName => + publicResourcesQuery .map(rs => FilterResourcesResult( rs.get[ResourceId](resource.resultName.name), resourceTypeNamesByPK(rs.get[ResourceTypePK](resource.resultName.resourceTypeId)), - rs.get[AccessPolicyName](resourcePolicy.resultName.name), - rs.stringOpt(resourceRole.resultName.role).map(r => Left[ResourceRoleName, ResourceAction](ResourceRoleName(r))), + 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() - val actions = publicPolicyActionQuery - .map(rs => - FilterResourcesResult( - rs.get[ResourceId](resource.resultName.name), - resourceTypeNamesByPK(rs.get[ResourceTypePK](resource.resultName.resourceTypeId)), - rs.get[AccessPolicyName](resourcePolicy.resultName.name), - rs.stringOpt(resourceAction.resultName.action).map(a => Right[ResourceRoleName, ResourceAction](ResourceAction(a))), - rs.get[Boolean](resourcePolicy.resultName.public), - rs.booleanOpt("inherited").getOrElse(false) - ) - ) - .list() - .apply() - roles ++ actions - } ) } } @@ -1857,6 +1857,7 @@ from ${GroupMemberTable as groupMemberTable} resourceTypeNames: Set[ResourceTypeName], policies: Set[AccessPolicyName], roles: Set[ResourceRoleName], + actions: Set[ResourceAction], samRequestContext: SamRequestContext ): IO[Seq[FilterResourcesResult]] = { val groupMemberFlat = GroupMemberFlatTable.syntax("groupMemberFlat") @@ -1868,77 +1869,83 @@ from ${GroupMemberTable as groupMemberTable} val roleAction = RoleActionTable.syntax("roleAction") val resourceAction = ResourceActionTable.syntax("resourceAction") val resource = ResourceTable.syntax("resource") + val authDomain = AuthDomainTable.syntax("authDomain") + val authDomainGroup = GroupTable.syntax("authDomainGroup") + val authDomainGroupMemberFlat = GroupMemberFlatTable.syntax("authDomainGroupMemberFlat") val resourceTypeConstraint = if (resourceTypeNames.nonEmpty) samsqls"and ${resource.resourceTypeId} in (${resourceTypeNames.flatMap(resourceTypePKsByName.get)})" else samsqls"and ${resource.resourceTypeId} in (${resourceTypeNamesByPK.keys.map(_.value)})" 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 = - samsql""" - select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, ${resourceRole.result.role}, ${resourcePolicy.result.public}, ${resourcePolicy.resourceId} != ${resource.id} as inherited + samsqls""" + select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, ${resourceRole.result.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 from ${GroupMemberFlatTable as groupMemberFlat} - join ${PolicyTable as resourcePolicy} on ${groupMemberFlat.groupId} = ${resourcePolicy.groupId} - join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} - join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id} + left join ${PolicyTable as resourcePolicy} on ${groupMemberFlat.groupId} = ${resourcePolicy.groupId} + left join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} 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} + 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} = ${samUserId} where ${groupMemberFlat.memberUserId} = ${samUserId} $resourceTypeConstraint $policyConstraint $roleConstraint + $actionConstraint + $notNullConstraintRoleAction """ val policyActionQuery = - samsql""" - select ${resource.result.name}, ${resource.result.resourceTypeId}, ${resourcePolicy.result.name}, ${resourceAction.result.action}, ${resourcePolicy.result.public}, ${resourcePolicy.resourceId} != ${resource.id} as inherited + 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 from ${GroupMemberFlatTable as groupMemberFlat} - join ${PolicyTable as resourcePolicy} on ${groupMemberFlat.groupId} = ${resourcePolicy.groupId} - join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} - join ${ResourceTable as resource} on ${effectiveResourcePolicy.resourceId} = ${resource.id} + left join ${PolicyTable as resourcePolicy} on ${groupMemberFlat.groupId} = ${resourcePolicy.groupId} + left join ${EffectiveResourcePolicyTable as effectiveResourcePolicy} on ${resourcePolicy.id} = ${effectiveResourcePolicy.sourcePolicyId} 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} + 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} = ${samUserId} where ${groupMemberFlat.memberUserId} = ${samUserId} $resourceTypeConstraint $policyConstraint + $actionConstraint + $notNullConstraintPolicyAction """ + val includePolicyActionQuery = if (roles.isEmpty) samsqls"union all $policyActionQuery" else samsqls"" + val query = + samsqls"""$policyRoleActionQuery + $includePolicyActionQuery""" + readOnlyTransaction("filterResources", samRequestContext) { implicit session => - val rolesResult = policyRoleActionQuery + samsql"$query" .map(rs => FilterResourcesResult( rs.get[ResourceId](resource.resultName.name), resourceTypeNamesByPK(rs.get[ResourceTypePK](resource.resultName.resourceTypeId)), - rs.get[AccessPolicyName](resourcePolicy.resultName.name), - rs.stringOpt(resourceRole.resultName.role).map(r => Left[ResourceRoleName, ResourceAction](ResourceRoleName(r))), + 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), + rs.stringOpt(authDomainGroup.resultName.name).map(WorkbenchGroupName(_)), + rs.booleanOpt("in_auth_domain").getOrElse(false), rs.booleanOpt("inherited").getOrElse(false) ) ) .list() .apply() - - // if roles is not empty that means we are filtering by roles and we don't need to query for actions directly on a policy since they have no roles - val actionsResult = if (roles.nonEmpty) { - List.empty - } else { - policyActionQuery - .map(rs => - FilterResourcesResult( - rs.get[ResourceId](resource.resultName.name), - resourceTypeNamesByPK(rs.get[ResourceTypePK](resource.resultName.resourceTypeId)), - rs.get[AccessPolicyName](resourcePolicy.resultName.name), - rs.stringOpt(resourceAction.resultName.action).map(a => Right[ResourceRoleName, ResourceAction](ResourceAction(a))), - rs.get[Boolean](resourcePolicy.resultName.public), - rs.booleanOpt("inherited").getOrElse(false) - ) - ) - .list() - .apply() - } - rolesResult ++ actionsResult - } } @@ -1947,6 +1954,7 @@ from ${GroupMemberTable as groupMemberTable} resourceTypeNames: Set[ResourceTypeName], policies: Set[AccessPolicyName], roles: Set[ResourceRoleName], + actions: Set[ResourceAction], includePublic: Boolean, samRequestContext: SamRequestContext ): IO[Seq[FilterResourcesResult]] = @@ -1958,10 +1966,11 @@ from ${GroupMemberTable as groupMemberTable} .sequence .map(_.flatten) } else IO.pure(List.empty) - privateResources <- filterPrivateResources(samUserId, resourceTypeNames, policies, roles, samRequestContext) + privateResources <- filterPrivateResources(samUserId, resourceTypeNames, policies, roles, actions, samRequestContext) } yield publicResources - .filter(r => policies.isEmpty || policies.contains(r.policy)) - .filter(r => roles.isEmpty || r.roleOrAction.exists(_.left.exists(role => roles.contains(role)))) ++ privateResources + .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") diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/FilterResourcesResult.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/FilterResourcesResult.scala index 04d729632..2a6cdcc5a 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/FilterResourcesResult.scala +++ b/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 ) 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 index ebf1c33f4..5ba6767af 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceService.scala +++ b/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) } diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockAccessPolicyDAO.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockAccessPolicyDAO.scala index e7e76b737..b9a00c3e4 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockAccessPolicyDAO.scala +++ b/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 diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAOSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAOSpec.scala index 80d4dc530..6c85d9c52 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAOSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAOSpec.scala @@ -3277,11 +3277,18 @@ class PostgresAccessPolicyDAOSpec extends AnyFreeSpec with Matchers with BeforeA "filterResources" - { - def verify(dbResultRows: Seq[FilterResourcesResult], roles: Set[ResourceRoleName], policyActions: Set[ResourceAction]): Any = { - val testRoles: Set[ResourceRoleName] = dbResultRows.flatMap(_.roleOrAction.flatMap(_.left.toOption)).toSet - val testPolicyActions: Set[ResourceAction] = dbResultRows.flatMap(_.roleOrAction.flatMap(_.toOption)).toSet + def verify( + dbResultRows: Seq[FilterResourcesResult], + roles: Set[ResourceRoleName], + roleActions: Set[ResourceAction], + policyActions: Set[ResourceAction] + ) = { + val testRoles: Set[ResourceRoleName] = dbResultRows.flatMap(_.role).toSet + val testRoleActions: Set[ResourceAction] = dbResultRows.filter(_.role.isDefined).flatMap(_.action).toSet + val testPolicyActions: Set[ResourceAction] = dbResultRows.filter(_.role.isEmpty).flatMap(_.action).toSet testRoles should be(roles) + testRoleActions should be(roleActions) testPolicyActions should be(policyActions) } @@ -3328,67 +3335,84 @@ class PostgresAccessPolicyDAOSpec extends AnyFreeSpec with Matchers with BeforeA false ) // No roles available - val publicProbeEmptyPolicy = AccessPolicy( + val publicProbePolicy = AccessPolicy( FullyQualifiedPolicyId(kitchenSink.fullyQualifiedId, AccessPolicyName(uuid)), Set.empty, WorkbenchEmail(s"${uuid}@policy.com"), - Set.empty, + Set(actionlessRole.roleName), Set.empty, Set.empty, true ) - val privateProbeEmptyPolicy = AccessPolicy( - FullyQualifiedPolicyId(kitchenSink.fullyQualifiedId, AccessPolicyName(uuid)), - Set(user.id), - WorkbenchEmail(s"${uuid}@policy.com"), - Set.empty, - Set.empty, - Set.empty, - false - ) dao.createPolicy(directProbePolicy, samRequestContext).unsafeRunSync() - dao.createPolicy(publicProbeEmptyPolicy, samRequestContext).unsafeRunSync() - dao.createPolicy(privateProbeEmptyPolicy, samRequestContext).unsafeRunSync() + dao.createPolicy(publicProbePolicy, samRequestContext).unsafeRunSync() + + val writeActions = + dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set.empty, Set(writeAction), false, samRequestContext).unsafeRunSync() + writeActions.length should be(5) + writeActions.map(_.action).forall(a => a.exists(_.equals(writeAction))) should be(true) + writeActions.map(_.isPublic).forall(ip => !ip) should be(true) + + val writerViaOwner = writeActions.filter(r => r.role.exists(_.equals(ownerRole.roleName))) + writerViaOwner.size should be(1) + + val readActions = dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set.empty, Set(readAction), false, samRequestContext).unsafeRunSync() + readActions.length should be(6) + + val readerViaOwner = readActions.filter(r => r.role.exists(_.equals(ownerRole.roleName))) + readerViaOwner.size should be(1) val readerRoles = - dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set(readerRole.roleName), false, samRequestContext).unsafeRunSync() + dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set(readerRole.roleName), Set.empty, false, samRequestContext).unsafeRunSync() readerRoles.size should be(5) val policies = dao - .filterResources(user.id, Set(resourceTypeName), Set(directProbePolicy.id.accessPolicyName), Set.empty, false, samRequestContext) + .filterResources(user.id, Set(resourceTypeName), Set(directProbePolicy.id.accessPolicyName), Set.empty, Set.empty, false, samRequestContext) .unsafeRunSync() - val foundPolicies = policies.map(_.policy).toSet + val foundPolicies = policies.flatMap(_.policy).toSet foundPolicies.size should be(1) foundPolicies.head should be(directProbePolicy.id.accessPolicyName) + val writeActionsIncludingPublic = + dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set.empty, Set(writeAction), true, samRequestContext).unsafeRunSync() + writeActionsIncludingPublic.length should be(7) + writeActionsIncludingPublic.filter(_.isPublic).map(_.resourceId).toSet should be(Set(publicResource.resourceId, publicChildResource.resourceId)) + val readerRolesIncludingPublic = - dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set(readerRole.roleName), true, samRequestContext).unsafeRunSync() + dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set(readerRole.roleName), Set.empty, true, samRequestContext).unsafeRunSync() readerRolesIncludingPublic.size should be(7) readerRolesIncludingPublic.filter(_.isPublic).map(_.resourceId).toSet should be(Set(publicResource.resourceId, publicChildResource.resourceId)) + println(dao.filterResources(user.id, Set(resourceTypeName), Set.empty, Set.empty, Set.empty, true, samRequestContext).unsafeRunSync()) + val inheritedReaderRoles = - dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set(readerRole.roleName), true, samRequestContext).unsafeRunSync() + dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set(readerRole.roleName), Set.empty, true, samRequestContext).unsafeRunSync() val inheritedPolicies = inheritedReaderRoles.filter(_.inherited) inheritedPolicies.map(_.resourceId).toSet should be( Set(childResource1.resourceId, childResource2.resourceId, publicChildResource.resourceId, kitchenSink.resourceId) ) - val filtered = dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set.empty, true, samRequestContext).unsafeRunSync() - - val emptyPublicResults = filtered.filter(_.policy == publicProbeEmptyPolicy.id.accessPolicyName) - emptyPublicResults.map(_.roleOrAction) should contain only None - val emptyPrivateResults = filtered.filter(_.policy == privateProbeEmptyPolicy.id.accessPolicyName) - emptyPrivateResults.map(_.roleOrAction) should contain only None + val filtered = dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set.empty, Set.empty, true, samRequestContext).unsafeRunSync() val readRoleWriteActionResources = Seq(userReadRoleWriteAction, groupReadRoleWriteAction, childResource1, childResource2, publicResource, publicChildResource) readRoleWriteActionResources .map(resource => - verify(filtered.filter(_.resourceId.equals(resource.resourceId)), roles = Set(readerRole.roleName), policyActions = Set(writeAction)) + verify( + filtered.filter(_.resourceId.equals(resource.resourceId)), + roles = Set(readerRole.roleName), + roleActions = Set(readAction), + policyActions = Set(writeAction) + ) ) - verify(filtered.filter(_.resourceId.equals(kitchenSink.resourceId)), roles = Set(readerRole.roleName, ownerRole.roleName), policyActions = Set.empty) + verify( + filtered.filter(_.resourceId.equals(kitchenSink.resourceId)), + roles = Set(readerRole.roleName, ownerRole.roleName, actionlessRole.roleName), + roleActions = Set(readAction, writeAction), + policyActions = Set.empty + ) } "filters on the user's policies, roles, and actions when using nested roles" in { @@ -3425,17 +3449,60 @@ class PostgresAccessPolicyDAOSpec extends AnyFreeSpec with Matchers with BeforeA createResourceHierarchy(Option(parentGroup.id), Set(writeAction), Set(includesRole.roleName, descendsRole.roleName), false, nestedResourceType.name) val nestedReaderRoles = - dao.filterResources(user.id, Set(nestedResourceType.name), Set.empty, Set(readerRole.roleName), false, samRequestContext).unsafeRunSync() + dao.filterResources(user.id, Set(nestedResourceType.name), Set.empty, Set(readerRole.roleName), Set.empty, false, samRequestContext).unsafeRunSync() nestedReaderRoles.size should be(4) nestedReaderRoles.map(_.resourceId).toSet should be( Set(directAccessResource.resourceId, groupAccessResource.resourceId, directAccessChildResource.resourceId, groupAccessChildResource.resourceId) ) val publicNestedReaderRoles = - dao.filterResources(user.id, Set(nestedResourceType.name), Set.empty, Set(readerRole.roleName), true, samRequestContext).unsafeRunSync() + dao.filterResources(user.id, Set(nestedResourceType.name), Set.empty, Set(readerRole.roleName), Set.empty, true, samRequestContext).unsafeRunSync() publicNestedReaderRoles.size should be(6) publicNestedReaderRoles.filter(_.isPublic).map(_.resourceId).toSet should be(Set(publicResource.resourceId, publicChildResource.resourceId)) } + + "includes Authorization Domain information in its queries" in { + assume(databaseEnabled, databaseEnabledClue) + + val user = Generator.genWorkbenchUserGoogle.sample.get + + val subGroup = BasicWorkbenchGroup(WorkbenchGroupName("subGroup"), Set(user.id), WorkbenchEmail("sub@groups.com")) + val parentGroup = BasicWorkbenchGroup(WorkbenchGroupName("parent"), Set(subGroup.id), WorkbenchEmail("parent@groups.com")) + val authDomainGroup1 = BasicWorkbenchGroup(WorkbenchGroupName("authDomainGroup1"), Set(parentGroup.id), WorkbenchEmail("authDomainGroup1@groups.com")) + val authDomainGroup2 = BasicWorkbenchGroup(WorkbenchGroupName("authDomainGroup2"), Set.empty, WorkbenchEmail("authDomainGroup2@groups.com")) + + dirDao.createUser(user, samRequestContext).unsafeRunSync() + dirDao.createGroup(subGroup, samRequestContext = samRequestContext).unsafeRunSync() + dirDao.createGroup(parentGroup, samRequestContext = samRequestContext).unsafeRunSync() + dirDao.createGroup(authDomainGroup1, samRequestContext = samRequestContext).unsafeRunSync() + dirDao.createGroup(authDomainGroup2, samRequestContext = samRequestContext).unsafeRunSync() + dirDao.addGroupMember(subGroup.id, user.id, samRequestContext).unsafeRunSync() + dirDao.addGroupMember(parentGroup.id, subGroup.id, samRequestContext).unsafeRunSync() + dao.createResourceType(resourceType, samRequestContext).unsafeRunSync() + dao.createResourceType(otherResourceType, samRequestContext).unsafeRunSync() + + // can access via auth domain + val resource1 = createResource(Option(user.id), Set(writeAction), Set(readerRole.roleName), public = false, authDomainGroups = Set(authDomainGroup1)) + // cannot access via auth domain + val resource2 = + createResource(Option(user.id), Set(writeAction), Set(readerRole.roleName), false, authDomainGroups = Set(authDomainGroup1, authDomainGroup2)) + + val writeActions = + dao.filterResources(user.id, Set(resourceType.name), Set.empty, Set.empty, Set(writeAction), false, samRequestContext).unsafeRunSync() + val byResource = writeActions.groupBy(_.resourceId) + + val resource1Results = byResource(resource1.resourceId) + resource1Results.length should be(1) + resource1Results.head.authDomain should be(Some(authDomainGroup1.id)) + resource1Results.head.inAuthDomain should be(true) + + val resource2Results = byResource(resource2.resourceId) + resource2Results.length should be(2) + resource2Results.flatMap(_.authDomain).toSet should be(Set(authDomainGroup1.id, authDomainGroup2.id)) + resource2Results.map(_.inAuthDomain).toSet should be(Set(true, false)) + resource2Results.filter(_.inAuthDomain).head.authDomain should be(Some(authDomainGroup1.id)) + resource2Results.filter(!_.inAuthDomain).head.authDomain should be(Some(authDomainGroup2.id)) + } } "listResourcesUsingAuthDomain" - { diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/StatefulMockAccessPolicyDaoBuilder.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/StatefulMockAccessPolicyDaoBuilder.scala index c9c3f1fa0..9acb40e3a 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/StatefulMockAccessPolicyDaoBuilder.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/StatefulMockAccessPolicyDaoBuilder.scala @@ -92,6 +92,7 @@ case class StatefulMockAccessPolicyDaoBuilder() extends MockitoSugar { ArgumentMatchers.eq(Set(policy.id.resource.resourceTypeName)), ArgumentMatchers.eq(Set.empty), ArgumentMatchers.eq(Set.empty), + ArgumentMatchers.eq(Set.empty), ArgumentMatchers.eq(true), any[SamRequestContext] ) @@ -114,42 +115,67 @@ case class StatefulMockAccessPolicyDaoBuilder() extends MockitoSugar { ) } - private def constructFilterResourcesResult(accessPolicy: AccessPolicy): Seq[FilterResourcesResult] = { - val results = accessPolicy.roles.map { role => - FilterResourcesResult( - accessPolicy.id.resource.resourceId, - accessPolicy.id.resource.resourceTypeName, - accessPolicy.id.accessPolicyName, - Option(Left(role)), - accessPolicy.public, - false - ) - }.toSeq ++ accessPolicy.actions.map { action => - FilterResourcesResult( - accessPolicy.id.resource.resourceId, - accessPolicy.id.resource.resourceTypeName, - accessPolicy.id.accessPolicyName, - Option(Right(action)), - accessPolicy.public, - false - ) - - }.toSeq - if (results.isEmpty) { + private def constructFilterResourcesResult(accessPolicy: AccessPolicy): Seq[FilterResourcesResult] = + if (accessPolicy.roles.isEmpty) { Seq( FilterResourcesResult( accessPolicy.id.resource.resourceId, accessPolicy.id.resource.resourceTypeName, - accessPolicy.id.accessPolicyName, + Some(accessPolicy.id.accessPolicyName), + None, None, accessPolicy.public, + None, + false, false ) ) - } else { - results - } - } + } else + { + accessPolicy.roles.map { role => + FilterResourcesResult( + accessPolicy.id.resource.resourceId, + accessPolicy.id.resource.resourceTypeName, + Some(accessPolicy.id.accessPolicyName), + Some(role), + None, + accessPolicy.public, + None, + false, + false + ) + }.toSeq + } ++ + (if (accessPolicy.actions.isEmpty) { + Seq( + FilterResourcesResult( + accessPolicy.id.resource.resourceId, + accessPolicy.id.resource.resourceTypeName, + Some(accessPolicy.id.accessPolicyName), + None, + None, + accessPolicy.public, + None, + false, + false + ) + ) + } else { + accessPolicy.actions.map { action => + FilterResourcesResult( + accessPolicy.id.resource.resourceId, + accessPolicy.id.resource.resourceTypeName, + Some(accessPolicy.id.accessPolicyName), + None, + Some(action), + accessPolicy.public, + None, + false, + false + ) + + }.toSeq + }) def withRandomAccessPolicy(resourceTypeName: ResourceTypeName, members: Set[WorkbenchSubject]): StatefulMockAccessPolicyDaoBuilder = { val policy = AccessPolicy( diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceUnitSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceUnitSpec.scala index 91938664b..2487bc3d6 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceUnitSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceUnitSpec.scala @@ -40,64 +40,107 @@ class ResourceServiceUnitSpec extends AnyFlatSpec with Matchers with ScalaFuture FilterResourcesResult( ResourceId(UUID.randomUUID().toString), resourceTypeName, - AccessPolicyName(UUID.randomUUID().toString), - Option(Left(readerRoleName)), + Some(AccessPolicyName(UUID.randomUUID().toString)), + Some(readerRoleName), + Some(readAction), true, + None, + false, + false + ), + FilterResourcesResult( + ResourceId(UUID.randomUUID().toString), + resourceTypeName, + Some(AccessPolicyName(UUID.randomUUID().toString)), + None, + None, + true, + None, + false, + false + ), + FilterResourcesResult( + ResourceId(UUID.randomUUID().toString), + resourceTypeName, + Some(AccessPolicyName(UUID.randomUUID().toString)), + Some(readerRoleName), + Some(readAction), + false, + None, + false, false ), FilterResourcesResult( ResourceId(UUID.randomUUID().toString), resourceTypeName, - AccessPolicyName(UUID.randomUUID().toString), - Option(Left(readerRoleName)), + Some(AccessPolicyName(UUID.randomUUID().toString)), + None, + None, + false, + None, false, false ), // Testable DB Results - FilterResourcesResult(testResourceId, resourceTypeName, testPolicy1, Option(Left(readerRoleName)), false, false), + FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy1), Some(readerRoleName), Some(readAction), false, None, false, false), FilterResourcesResult( testResourceId, resourceTypeName, - testPolicy1, - Option(Left(readerRoleName)), + Some(testPolicy1), + Some(readerRoleName), + Some(readAction), + false, + None, false, false ), // testing duplicate row results FilterResourcesResult( testResourceId, resourceTypeName, - testPolicy1, - Option(Left(readerRoleName)), + Some(testPolicy1), + Some(readerRoleName), + Some(readAction), + false, + None, false, false ), // testing duplicate row results - FilterResourcesResult(testResourceId, resourceTypeName, testPolicy2, Option(Left(nothingRoleName)), true, false), - FilterResourcesResult(testResourceId, resourceTypeName, testPolicy3, None, false, false), - FilterResourcesResult(testResourceId, resourceTypeName, testPolicy4, Option(Left(ownerRoleName)), false, false), - FilterResourcesResult(testResourceId, resourceTypeName, testPolicy4, Option(Left(ownerRoleName)), false, false), - FilterResourcesResult(testResourceId, resourceTypeName, testPolicy5, Option(Right(readAction)), true, false), + FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy2), Some(nothingRoleName), None, true, None, false, false), + FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy3), None, None, false, None, false, false), + FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy4), Some(ownerRoleName), Some(readAction), false, None, false, false), + FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy4), Some(ownerRoleName), Some(writeAction), false, None, false, false), + FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy5), None, Some(readAction), true, None, false, false), FilterResourcesResult( testResourceId, resourceTypeName, - testPolicy5, - Option(Right(readAction)), + Some(testPolicy5), + None, + Some(readAction), true, + None, + false, false ), // testing duplicate row results // Auth Domain Results FilterResourcesResult( testResourceId2, resourceTypeName, - testPolicy6, - Option(Left(readerRoleName)), + Some(testPolicy6), + Some(readerRoleName), + Some(readAction), false, + Some(authDomainGroup1), + true, false ), FilterResourcesResult( testResourceId2, resourceTypeName, - testPolicy6, - Option(Left(readerRoleName)), + Some(testPolicy6), + Some(readerRoleName), + Some(readAction), + false, + Some(authDomainGroup2), false, false ) @@ -110,29 +153,15 @@ class ResourceServiceUnitSpec extends AnyFlatSpec with Matchers with ScalaFuture any[Set[ResourceTypeName]], any[Set[AccessPolicyName]], any[Set[ResourceRoleName]], + any[Set[ResourceAction]], any[Boolean], any[SamRequestContext] ) ) .thenReturn(IO.pure(dbResult)) - when(mockAccessPolicyDAO.listResourcesWithAuthdomains(any[ResourceTypeName], any[Set[ResourceId]], any[SamRequestContext])) - .thenReturn(IO.pure(Set(Resource(resourceTypeName, testResourceId2, Set(authDomainGroup1, authDomainGroup2), Set.empty)))) - when(mockAccessPolicyDAO.listAccessPolicies(eqTo(ManagedGroupService.managedGroupTypeName), any[WorkbenchUserId], any[SamRequestContext])) - .thenReturn(IO.pure(Set(ResourceIdAndPolicyName(ResourceId(authDomainGroup1.value), ManagedGroupService.memberPolicyName)))) val resourceService = new ResourceService( - Map( - resourceTypeName -> ResourceType( - resourceTypeName, - Set(ResourceActionPattern(readAction.value, "", true)), - Set( - ResourceRole(readerRoleName, Set(readAction)), - ResourceRole(ownerRoleName, Set(readAction, writeAction)), - ResourceRole(nothingRoleName, Set.empty, Set.empty) - ), - ownerRoleName - ) - ), + Map.empty, mock[PolicyEvaluatorService], mockAccessPolicyDAO, mock[DirectoryDAO], @@ -186,9 +215,6 @@ class ResourceServiceUnitSpec extends AnyFlatSpec with Matchers with ScalaFuture role.role should be(ownerRoleName) role.actions should be(Set(readAction, writeAction)) - policies.filter(_.policy.equals(testPolicy3)).flatMap(_.roles) should be(empty) - policies.filter(_.policy.equals(testPolicy3)).flatMap(_.actions) should be(empty) - val authDomainResource = filteredResources.resources.filter(_.resourceId.equals(testResourceId2)).head authDomainResource.resourceType should be(resourceTypeName) authDomainResource.policies.map(_.policy) should be(Set(testPolicy6)) From e5043ddb1d9fcadc979219c6f5d4f4a0e3943a80 Mon Sep 17 00:00:00 2001 From: Broad Bot <61600560+broadbot@users.noreply.github.com> Date: Tue, 12 Nov 2024 11:39:30 -0800 Subject: [PATCH 11/15] CORE-69: Update google-cloud-nio from 0.127.25 to 0.127.26 (#1579) Co-authored-by: Bria Morgan Co-authored-by: Saman Ehsan Co-authored-by: David An --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 8dc277824..00928e248 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -103,7 +103,7 @@ object Dependencies { val workbenchGoogle2Tests: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-google2" % workbenchGoogle2V % "test" classifier "tests" excludeAll (excludeWorkbenchUtil, excludeWorkbenchModel) val googleStorageLocal: ModuleID = - "com.google.cloud" % "google-cloud-nio" % "0.127.25" % "test" // needed for mocking google cloud storage. Should use same version as wb-libs + "com.google.cloud" % "google-cloud-nio" % "0.127.26" % "test" // needed for mocking google cloud storage. Should use same version as wb-libs val liquibaseCore: ModuleID = "org.liquibase" % "liquibase-core" % "4.2.2" From 0c8178b2b95ecb1cd463232cbf712527775ab2dd Mon Sep 17 00:00:00 2001 From: Broad Bot <61600560+broadbot@users.noreply.github.com> Date: Tue, 12 Nov 2024 11:57:37 -0800 Subject: [PATCH 12/15] CORE-69: Update commons-compress from 1.26.0 to 1.26.2 (#1585) Co-authored-by: David An --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 00928e248..8a4dda6d6 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -227,6 +227,6 @@ object Dependencies { // Needed because it looks like the dependency overrides of wb-libs doesn't propagate to the importing project... val rootDependencyOverrides = Seq( - "org.apache.commons" % "commons-compress" % "1.26.0" + "org.apache.commons" % "commons-compress" % "1.26.2" ) } From 10f570064e33004ea236b96489ea762bf6b02bd9 Mon Sep 17 00:00:00 2001 From: Broad Bot <61600560+broadbot@users.noreply.github.com> Date: Tue, 12 Nov 2024 13:18:53 -0800 Subject: [PATCH 13/15] CORE-69: Update scalafmt-core from 3.6.1 to 3.8.3 (#1499) * Update scalafmt-core to 3.8.3 * Reformat with scalafmt 3.8.3 Executed command: scalafmt --non-interactive * Add 'Reformat with scalafmt 3.8.3' to .git-blame-ignore-revs --------- Co-authored-by: David An Co-authored-by: Saman Ehsan --- .git-blame-ignore-revs | 3 +++ .scalafmt.conf | 2 +- automation/project/Dependencies.scala | 6 +++-- .../dsde/workbench/sam/MockTestSupport.scala | 2 +- project/Dependencies.scala | 26 ++++++++++++++++--- .../sam/dataAccess/PostgresGroupDAO.scala | 17 +++++------- 6 files changed, 38 insertions(+), 18 deletions(-) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index 4bd5edcd9..778dc87cb 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -1,2 +1,5 @@ # Scala Steward: Reformat with scalafmt 3.6.1 0c093c52c09d7b12cdbd38008e0bbc58c9d110be + +# Scala Steward: Reformat with scalafmt 3.8.3 +fc6844bda9d3bdf0b5751381ed9e402fdeb577b8 diff --git a/.scalafmt.conf b/.scalafmt.conf index 47e45a8de..b0234070a 100644 --- a/.scalafmt.conf +++ b/.scalafmt.conf @@ -1,4 +1,4 @@ -version=3.6.1 +version=3.8.3 style = default runner.dialect = scala213 diff --git a/automation/project/Dependencies.scala b/automation/project/Dependencies.scala index a9793ad49..5b01ab480 100644 --- a/automation/project/Dependencies.scala +++ b/automation/project/Dependencies.scala @@ -34,8 +34,10 @@ object Dependencies { ExclusionRule("com.google.guava", "guava-jdk5"), ExclusionRule("org.apache.httpcomponents", "httpclient") ), - "com.google.api-client" % "google-api-client" % "1.22.0" excludeAll (ExclusionRule("com.google.guava", "guava-jdk5"), - ExclusionRule("org.apache.httpcomponents", "httpclient")), + "com.google.api-client" % "google-api-client" % "1.22.0" excludeAll ( + ExclusionRule("com.google.guava", "guava-jdk5"), + ExclusionRule("org.apache.httpcomponents", "httpclient") + ), "com.typesafe.akka" %% "akka-http-core" % akkaHttpV, "com.typesafe.akka" %% "akka-stream-testkit" % akkaV, "com.typesafe.akka" %% "akka-http" % akkaHttpV, diff --git a/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala b/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala index 5e3d9e603..e7ed9ae97 100644 --- a/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala +++ b/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] ) } diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 8a4dda6d6..0dbbf165a 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -95,13 +95,24 @@ object Dependencies { val excludeGoogleAutoValue = ExclusionRule(organization = "com.google.auto.value", name = "auto-value") val excludeBouncyCastle = ExclusionRule("org.bouncycastle") val workbenchGoogle2: ModuleID = - "org.broadinstitute.dsde.workbench" %% "workbench-google2" % workbenchGoogle2V excludeAll (excludeWorkbenchModel, excludeWorkbenchUtil, excludeGoogleAutoValue, excludeBouncyCastle) + "org.broadinstitute.dsde.workbench" %% "workbench-google2" % workbenchGoogle2V excludeAll ( + excludeWorkbenchModel, + excludeWorkbenchUtil, + excludeGoogleAutoValue, + excludeBouncyCastle + ) val workbenchNotifications: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-notifications" % workbenchNotificationsV excludeAll (excludeWorkbenchGoogle, excludeWorkbenchModel) val workbenchGoogleTests: ModuleID = - "org.broadinstitute.dsde.workbench" %% "workbench-google" % workbenchGoogleV % "test" classifier "tests" excludeAll (excludeWorkbenchUtil, excludeWorkbenchModel) + "org.broadinstitute.dsde.workbench" %% "workbench-google" % workbenchGoogleV % "test" classifier "tests" excludeAll ( + excludeWorkbenchUtil, + excludeWorkbenchModel + ) val workbenchGoogle2Tests: ModuleID = - "org.broadinstitute.dsde.workbench" %% "workbench-google2" % workbenchGoogle2V % "test" classifier "tests" excludeAll (excludeWorkbenchUtil, excludeWorkbenchModel) + "org.broadinstitute.dsde.workbench" %% "workbench-google2" % workbenchGoogle2V % "test" classifier "tests" excludeAll ( + excludeWorkbenchUtil, + excludeWorkbenchModel + ) val googleStorageLocal: ModuleID = "com.google.cloud" % "google-cloud-nio" % "0.127.26" % "test" // needed for mocking google cloud storage. Should use same version as wb-libs @@ -135,7 +146,14 @@ object Dependencies { ) val cloudResourceLib: ModuleID = - "bio.terra" % "terra-cloud-resource-lib" % crlVersion excludeAll (excludeGoogleServiceUsage, excludeGoogleCloudResourceManager, excludeJerseyCore, excludeJerseyMedia, excludeSLF4J, excludeAwsSdk) + "bio.terra" % "terra-cloud-resource-lib" % crlVersion excludeAll ( + excludeGoogleServiceUsage, + excludeGoogleCloudResourceManager, + excludeJerseyCore, + excludeJerseyMedia, + excludeSLF4J, + excludeAwsSdk + ) val azureManagedApplications: ModuleID = "com.azure.resourcemanager" % "azure-resourcemanager-managedapplications" % "1.0.0-beta.4" diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresGroupDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresGroupDAO.scala index 7507fca4d..265495033 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresGroupDAO.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresGroupDAO.scala @@ -28,8 +28,8 @@ import scala.util.{Failure, Try} * * Example database records: group(7795) contains user(userid) group(7798) contains user(userid) group(7801) contains group(7798) and group(7799) * - * testdb=# select * from sam_group_member; id | group_id | member_group_id | member_user_id - * -------+----------+-----------------+---------------- 15636 | 7795 | | userid 15637 | 7798 | | userid 15638 | 7801 | 7798 | 15639 | 7801 | 7799 | + * testdb=# select * from sam_group_member; id | group_id | member_group_id | member_user_id -------+----------+-----------------+---------------- 15636 | 7795 + * \| | userid 15637 | 7798 | | userid 15638 | 7801 | 7798 | 15639 | 7801 | 7799 | * * testdb=# select * from sam_group_member_flat; id | group_id | member_group_id | member_user_id | group_membership_path | last_group_membership_element * --------+----------+-----------------+----------------+-----------------------+------------------------------ 345985 | 7795 | | userid | {7795} | 7795 @@ -115,19 +115,16 @@ trait PostgresGroupDAO { * the head path + tail path * * Example: Insert group T into group H. H starts empty but is already a member of groups A and B. T already has member groups X and Y which are empty. The - * flat group model starts containing: Group | Member Group | Path - * ------|--------------|------ A | H | {A} B | H | {B} T | X | {T} T | Y | {T} + * flat group model starts containing: Group | Member Group | Path ------|--------------|------ A | H | {A} B | H | {B} T | X | {T} T | Y | {T} * - * step 1 inserts direct membership of T in H Group | Member Group | Path - * ------|--------------|------ H | T | {H} + * step 1 inserts direct membership of T in H Group | Member Group | Path ------|--------------|------ H | T | {H} * - * step 2 inserts indirect memberships T in A and B Group | Member Group | Path - * ------|--------------|------ A | T | {A,H} B | T | {B,H} + * step 2 inserts indirect memberships T in A and B Group | Member Group | Path ------|--------------|------ A | T | {A,H} B | T | {B,H} * * step 3 inserts T's lower group hierarchy so that X and Y are members of H, A and B. The tail records are all of the records above where Group is T: ((T, * X, {T}), (T, Y, {T}) The head records are all of the records above where Member Group is T and the last path element is H: ((H, T, {H}), (A, T, {A,H}), - * (B, T, {B,H})) Group | Member Group | Path - * ------|--------------|------ H | X | {H,T} H | Y | {H,T} A | X | {A,H,T} A | Y | {A,H,T} B | X | {B,H,T} B | Y | {B,H,T} + * (B, T, {B,H})) Group | Member Group | Path ------|--------------|------ H | X | {H,T} H | Y | {H,T} A | X | {A,H,T} A | Y | {A,H,T} B | X | {B,H,T} B | Y + * \| {B,H,T} * * @param groupId * group being added to From 0e81af2ecd6a2f72953d2f7cddf75d6055c92ad4 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Wed, 13 Nov 2024 09:09:03 -0500 Subject: [PATCH 14/15] Update auto-approve-broadbot-prs.yml (#1590) --- .github/workflows/auto-approve-broadbot-prs.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/auto-approve-broadbot-prs.yml b/.github/workflows/auto-approve-broadbot-prs.yml index 7a5f2eb54..189e1a33f 100644 --- a/.github/workflows/auto-approve-broadbot-prs.yml +++ b/.github/workflows/auto-approve-broadbot-prs.yml @@ -5,6 +5,13 @@ permissions: pull-requests: write jobs: + getActor: + runs-on: ubuntu-latest + steps: + - name: "Echo github actor" + env: + GH_ACTOR: ${{ github.actor }} + run: echo "$GH_ACTOR" broadbot: runs-on: ubuntu-latest if: github.actor == 'broadbot' From c6b7df475d5db38ff761f4be2b832a422d09f765 Mon Sep 17 00:00:00 2001 From: Broad Bot <61600560+broadbot@users.noreply.github.com> Date: Wed, 13 Nov 2024 07:44:08 -0800 Subject: [PATCH 15/15] CORE-69: Update logback-classic from 1.4.14 to 1.5.12 (#1574) Co-authored-by: Saman Ehsan --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 0dbbf165a..6a28b069e 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -47,7 +47,7 @@ object Dependencies { val jacksonCore: ModuleID = "com.fasterxml.jackson.core" % "jackson-core" % jacksonV val logstashLogback: ModuleID = "net.logstash.logback" % "logstash-logback-encoder" % "6.6" - val logbackClassic: ModuleID = "ch.qos.logback" % "logback-classic" % "1.4.14" + val logbackClassic: ModuleID = "ch.qos.logback" % "logback-classic" % "1.5.12" val ravenLogback: ModuleID = "com.getsentry.raven" % "raven-logback" % "7.8.6" val scalaLogging: ModuleID = "com.typesafe.scala-logging" %% "scala-logging" % scalaLoggingV val ficus: ModuleID = "com.iheart" %% "ficus" % "1.5.2"