diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 9b63450cf..bc9dba895 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -11,10 +11,10 @@ object Dependencies { val postgresDriverVersion = "42.7.2" val sentryVersion = "6.15.0" - val workbenchLibV = "a6ad7dc" // If updating this, make sure googleStorageLocal in test dependencies is up-to-date + val workbenchLibV = "8d55689" // If updating this, make sure googleStorageLocal in test dependencies is up-to-date val workbenchUtilV = s"0.10-$workbenchLibV" val workbenchUtil2V = s"0.9-$workbenchLibV" - val workbenchModelV = s"0.19-$workbenchLibV" + val workbenchModelV = s"0.20-$workbenchLibV" val workbenchGoogleV = s"0.32-$workbenchLibV" val workbenchGoogle2V = s"0.36-$workbenchLibV" val workbenchNotificationsV = s"0.6-$workbenchLibV" diff --git a/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changelog.xml b/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changelog.xml index c9ba57fb0..578a27770 100644 --- a/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changelog.xml +++ b/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changelog.xml @@ -30,4 +30,5 @@ + diff --git a/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changesets/20240701_add_group_version_and_last_synchronized_version.xml b/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changesets/20240701_add_group_version_and_last_synchronized_version.xml new file mode 100644 index 000000000..562805cb6 --- /dev/null +++ b/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changesets/20240701_add_group_version_and_last_synchronized_version.xml @@ -0,0 +1,25 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/DirectoryDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/DirectoryDAO.scala index 26e596d97..1be569418 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/DirectoryDAO.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/DirectoryDAO.scala @@ -45,7 +45,9 @@ trait DirectoryDAO { def isGroupMember(groupId: WorkbenchGroupIdentity, member: WorkbenchSubject, samRequestContext: SamRequestContext): IO[Boolean] - def updateSynchronizedDate(groupId: WorkbenchGroupIdentity, samRequestContext: SamRequestContext): IO[Unit] + def updateSynchronizedDateAndVersion(group: WorkbenchGroup, samRequestContext: SamRequestContext): IO[Unit] + + def updateGroupUpdatedDateAndVersionWithSession(groupId: WorkbenchGroupIdentity, samRequestContext: SamRequestContext): IO[Unit] def getSynchronizedDate(groupId: WorkbenchGroupIdentity, samRequestContext: SamRequestContext): IO[Option[Date]] 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 48cc58e90..74ec4df86 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 @@ -995,6 +995,7 @@ class PostgresAccessPolicyDAO( } removeAllGroupMembers(groupId) insertGroupMembers(groupId, memberList) + updateGroupUpdatedDateAndVersion(id) } override def overwritePolicy(newPolicy: AccessPolicy, samRequestContext: SamRequestContext): IO[AccessPolicy] = @@ -1084,7 +1085,7 @@ class PostgresAccessPolicyDAO( val ra = ResourceActionTable.syntax("ra") val listPoliciesQuery = - samsql"""select ${p.result.name}, ${r.result.name}, ${rt.result.name}, ${g.result.email}, ${p.result.public}, ${rr.result.role}, ${ra.result.action} + samsql"""select ${p.result.name}, ${r.result.name}, ${rt.result.name}, ${g.result.email}, ${p.result.public}, ${g.result.version}, ${g.result.lastSynchronizedVersion}, ${rr.result.role}, ${ra.result.action} from ${GroupTable as g} join ${PolicyTable as p} on ${g.id} = ${p.groupId} join ${ResourceTable as r} on ${p.resourceId} = ${r.id} @@ -1106,7 +1107,9 @@ class PostgresAccessPolicyDAO( rs.get[ResourceId](r.resultName.name), rs.get[ResourceTypeName](rt.resultName.name), rs.get[WorkbenchEmail](g.resultName.email), - rs.boolean(p.resultName.public) + rs.boolean(p.resultName.public), + rs.get[Int](g.resultName.version), + rs.get[Option[Int]](g.resultName.lastSynchronizedVersion) ), (rs.stringOpt(rr.resultName.role).map(ResourceRoleName(_)), rs.stringOpt(ra.resultName.action).map(ResourceAction(_))) ) @@ -1163,7 +1166,9 @@ class PostgresAccessPolicyDAO( policyRoles, policyActions, policyDescendantPermissions, - policyInfo.public + policyInfo.public, + policyInfo.version, + policyInfo.lastSynchronizedVersion ) } .to(LazyList) @@ -1215,7 +1220,7 @@ class PostgresAccessPolicyDAO( val part = ResourceTypeTable.syntax("part") // policy action resource type val listPoliciesQuery = - samsql"""select ${p.result.name}, ${g.result.email}, ${p.result.public}, ${prrt.result.name}, ${rr.result.role}, ${pr.result.descendantsOnly}, ${part.result.name}, ${ra.result.action}, ${pa.result.descendantsOnly} + samsql"""select ${p.result.name}, ${g.result.email}, ${p.result.public}, ${g.result.version}, ${g.result.lastSynchronizedVersion}, ${prrt.result.name}, ${rr.result.role}, ${pr.result.descendantsOnly}, ${part.result.name}, ${ra.result.action}, ${pa.result.descendantsOnly} from ${GroupTable as g} join ${PolicyTable as p} on ${g.id} = ${p.groupId} left join ${PolicyRoleTable as pr} on ${p.id} = ${pr.resourcePolicyId} @@ -1234,7 +1239,9 @@ class PostgresAccessPolicyDAO( resource.resourceId, resource.resourceTypeName, rs.get[WorkbenchEmail](g.resultName.email), - rs.boolean(p.resultName.public) + rs.boolean(p.resultName.public), + rs.get[Int](g.resultName.version), + rs.get[Option[Int]](g.resultName.lastSynchronizedVersion) ), ( RoleResult( @@ -1469,7 +1476,7 @@ class PostgresAccessPolicyDAO( val ra = ResourceActionTable.syntax("ra") val listPoliciesQuery = - samsql"""select ${p.result.name}, ${r.result.name}, ${rt.result.name}, ${g.result.email}, ${p.result.public}, ${rr.result.role}, ${ra.result.action} + samsql"""select ${p.result.name}, ${r.result.name}, ${rt.result.name}, ${g.result.email}, ${p.result.public}, ${g.result.version}, ${g.result.lastSynchronizedVersion}, ${rr.result.role}, ${ra.result.action} from ${GroupTable as g} join ${GroupMemberFlatTable as f} on ${f.groupId} = ${g.id} join ${PolicyTable as p} on ${g.id} = ${p.groupId} @@ -1490,7 +1497,9 @@ class PostgresAccessPolicyDAO( rs.get[ResourceId](r.resultName.name), rs.get[ResourceTypeName](rt.resultName.name), rs.get[WorkbenchEmail](g.resultName.email), - rs.boolean(p.resultName.public) + rs.boolean(p.resultName.public), + rs.get[Int](g.resultName.version), + rs.get[Option[Int]](g.resultName.lastSynchronizedVersion) ), (rs.stringOpt(rr.resultName.role).map(ResourceRoleName(_)), rs.stringOpt(ra.resultName.action).map(ResourceAction(_))) ) @@ -2057,7 +2066,15 @@ class PostgresAccessPolicyDAO( } } -private final case class PolicyInfo(name: AccessPolicyName, resourceId: ResourceId, resourceTypeName: ResourceTypeName, email: WorkbenchEmail, public: Boolean) +private final case class PolicyInfo( + name: AccessPolicyName, + resourceId: ResourceId, + resourceTypeName: ResourceTypeName, + email: WorkbenchEmail, + public: Boolean, + version: Int, + lastSynchronizedVersion: Option[Int] +) private final case class RoleResult(resourceTypeName: Option[ResourceTypeName], role: Option[ResourceRoleName], descendantsOnly: Option[Boolean]) private final case class ActionResult(resourceTypeName: Option[ResourceTypeName], action: Option[ResourceAction], descendantsOnly: Option[Boolean]) diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAO.scala index 9338bf59d..cd9be4e70 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAO.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAO.scala @@ -85,7 +85,7 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val val r = ResourceTable.syntax("r") val rt = ResourceTypeTable.syntax("rt") - samsql"""select ${g.result.email}, ${gm.result.memberUserId}, ${sg.result.name}, ${p.result.name}, ${r.result.name}, ${rt.result.name} + samsql"""select ${g.result.email}, ${gm.result.memberUserId}, ${sg.result.name}, ${p.result.name}, ${r.result.name}, ${rt.result.name}, ${g.result.version}, ${g.result.lastSynchronizedVersion} from ${GroupTable as g} left join ${GroupMemberTable as gm} on ${g.id} = ${gm.groupId} left join ${GroupTable as sg} on ${gm.memberGroupId} = ${sg.id} @@ -100,7 +100,9 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val rs.stringOpt(sg.resultName.name).map(WorkbenchGroupName), rs.stringOpt(p.resultName.name).map(AccessPolicyName(_)), rs.stringOpt(r.resultName.name).map(ResourceId(_)), - rs.stringOpt(rt.resultName.name).map(ResourceTypeName(_)) + rs.stringOpt(rt.resultName.name).map(ResourceTypeName(_)), + rs.get[Int](g.resultName.version), + rs.get[Option[Int]](g.resultName.lastSynchronizedVersion) ) } .list() @@ -112,13 +114,16 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val } else { val email = results.head._1 val members: Set[WorkbenchSubject] = results.collect { - case (_, Some(userId), None, None, None, None) => userId - case (_, None, Some(subGroupName), None, None, None) => subGroupName - case (_, None, Some(_), Some(policyName), Some(resourceName), Some(resourceTypeName)) => + case (_, Some(userId), None, None, None, None, _, _) => userId + case (_, None, Some(subGroupName), None, None, None, _, _) => subGroupName + case (_, None, Some(_), Some(policyName), Some(resourceName), Some(resourceTypeName), _, _) => FullyQualifiedPolicyId(FullyQualifiedResourceId(resourceTypeName, resourceName), policyName) }.toSet - Option(BasicWorkbenchGroup(groupName, members, email)) + val version = results.head._7 + val lastSynchronized = results.head._8 + + Option(BasicWorkbenchGroup(groupName, members, email, version, lastSynchronized)) } override def loadGroupEmail(groupName: WorkbenchGroupName, samRequestContext: SamRequestContext): IO[Option[WorkbenchEmail]] = @@ -154,7 +159,7 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val serializableWriteTransaction("addGroupMember", samRequestContext) { implicit session => val numberAdded = insertGroupMembers(queryForGroupPKs(Set(groupId)).head, Set(addMember)) if (numberAdded > 0) { - updateGroupUpdatedDate(groupId) + updateGroupUpdatedDateAndVersion(groupId) true } else { false @@ -169,21 +174,34 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val val removed = removeGroupMember(groupId, removeMember) if (removed) { - updateGroupUpdatedDate(groupId) + updateGroupUpdatedDateAndVersion(groupId) } removed } + override def updateGroupUpdatedDateAndVersionWithSession(groupId: WorkbenchGroupIdentity, samRequestContext: SamRequestContext): IO[Unit] = + serializableWriteTransaction("updateGroupUpdatedDateAndVersionWithSession", samRequestContext) { implicit session => + updateGroupUpdatedDateAndVersion(groupId) + } + override def isGroupMember(groupId: WorkbenchGroupIdentity, member: WorkbenchSubject, samRequestContext: SamRequestContext): IO[Boolean] = readOnlyTransaction("isGroupMember", samRequestContext) { implicit session => isGroupMember(groupId, member) } - override def updateSynchronizedDate(groupId: WorkbenchGroupIdentity, samRequestContext: SamRequestContext): IO[Unit] = - serializableWriteTransaction("updateSynchronizedDate", samRequestContext) { implicit session => + /* + Update last synchronized version only when it is less than the current group version. This is to avoid + threads stepping over each other and causing sam to become out of sync with google. The last synchronized version + should only be set to the version of the group that is help in memory from when the sync started. + */ + override def updateSynchronizedDateAndVersion(group: WorkbenchGroup, samRequestContext: SamRequestContext): IO[Unit] = + serializableWriteTransaction("updateSynchronizedDateAndVersion", samRequestContext) { implicit session => val g = GroupTable.column - samsql"update ${GroupTable.table} set ${g.synchronizedDate} = ${Instant.now()} where ${g.id} = (${workbenchGroupIdentityToGroupPK(groupId)})" + samsql"""update ${GroupTable.table} + set ${g.synchronizedDate} = ${Instant.now()}, + ${g.lastSynchronizedVersion} = ${group.version} + where ${g.id} = (${workbenchGroupIdentityToGroupPK(group.id)}) and COALESCE(${g.lastSynchronizedVersion}, 0) < ${group.version}""" .update() .apply() } 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 1fdf1a371..7507fca4d 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 @@ -250,9 +250,12 @@ trait PostgresGroupDAO { query.map(rs => rs.int(1)).single().apply().getOrElse(0) > 0 } - def updateGroupUpdatedDate(groupId: WorkbenchGroupIdentity)(implicit session: DBSession): Int = { + def updateGroupUpdatedDateAndVersion(groupId: WorkbenchGroupIdentity)(implicit session: DBSession): Int = { val g = GroupTable.column - samsql"update ${GroupTable.table} set ${g.updatedDate} = ${Instant.now()} where ${g.id} = (${workbenchGroupIdentityToGroupPK(groupId)})".update().apply() + samsql"""update ${GroupTable.table} + set ${g.updatedDate} = ${Instant.now()}, + ${g.version} = ${g.version} + 1 + where ${g.id} = (${workbenchGroupIdentityToGroupPK(groupId)})""".update().apply() } def deleteGroup(groupName: WorkbenchGroupName)(implicit session: DBSession): Int = { diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/db/tables/GroupTable.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/db/tables/GroupTable.scala index aaba9aad9..1845f7be9 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/db/tables/GroupTable.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/db/tables/GroupTable.scala @@ -7,7 +7,15 @@ import scalikejdbc._ import java.time.Instant final case class GroupPK(value: Long) extends DatabaseKey -final case class GroupRecord(id: GroupPK, name: WorkbenchGroupName, email: WorkbenchEmail, updatedDate: Option[Instant], synchronizedDate: Option[Instant]) +final case class GroupRecord( + id: GroupPK, + name: WorkbenchGroupName, + email: WorkbenchEmail, + version: Int, + lastSynchronizedVersion: Option[Int], + updatedDate: Option[Instant], + synchronizedDate: Option[Instant] +) object GroupTable extends SQLSyntaxSupportWithDefaultSamDB[GroupRecord] { override def tableName: String = "SAM_GROUP" @@ -17,6 +25,8 @@ object GroupTable extends SQLSyntaxSupportWithDefaultSamDB[GroupRecord] { rs.get(e.id), rs.get(e.name), rs.get(e.email), + rs.get(e.version), + rs.get(e.lastSynchronizedVersion), rs.timestampOpt(e.updatedDate).map(_.toInstant), rs.timestampOpt(e.synchronizedDate).map(_.toInstant) ) diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleGroupSyncMessageReceiver.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleGroupSyncMessageReceiver.scala index 2aa6076fc..6f4e1ad3f 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleGroupSyncMessageReceiver.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleGroupSyncMessageReceiver.scala @@ -49,6 +49,12 @@ class GoogleGroupSyncMessageReceiver(groupSynchronizer: GoogleGroupSynchronizer) logger.info(s"group to synchronize not found: ${groupNotFound.errorReport}") consumer.ack() + case groupAlreadySynchronized: GroupAlreadySynchronized => + // this can happen if a group is synchronized multiple times in quick succession + // acknowledge it so we don't have to handle it again + logger.info(s"group already previously synchronized: ${groupAlreadySynchronized.getMessage}") + consumer.ack() + case regrets: Throwable => logger.error("failure synchronizing google group", regrets) consumer.nack() // redeliver message to hopefully rectify the failure diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleGroupSynchronizer.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleGroupSynchronizer.scala index 1a0bbbc9a..2e865f1c7 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleGroupSynchronizer.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleGroupSynchronizer.scala @@ -15,6 +15,9 @@ import org.broadinstitute.dsde.workbench.sam.util.OpenTelemetryIOUtils._ import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext import org.broadinstitute.dsde.workbench.util.FutureSupport +class GroupAlreadySynchronized(errorReport: ErrorReport = ErrorReport(StatusCodes.Conflict, "Group has already been synchronized")) + extends WorkbenchExceptionWithErrorReport(errorReport) + /** This class makes sure that our google groups have the right members. * * For the simple case it merely compares group membership given by directoryDAO against group membership given by googleDirectoryDAO and does the appropriate @@ -51,7 +54,15 @@ class GoogleGroupSynchronizer( IO.pure(Map.empty) } else { for { - group <- loadSamGroup(groupId, samRequestContext) + group: WorkbenchGroup <- loadSamGroup(groupId, samRequestContext) + // If group.version > group.lastSynchronizedVersion, then the group needs to be synchronized + // Else Noop + _ <- + if (group.version > group.lastSynchronizedVersion.getOrElse(0)) { + IO.unit + } else { + IO.raiseError(new GroupAlreadySynchronized) + } members <- calculateAuthDomainIntersectionIfRequired(group, samRequestContext) subGroupSyncs <- syncSubGroupsIfRequired(group, visitedGroups, samRequestContext) googleMemberEmails <- loadGoogleGroupMemberEmailsMaybeCreateGroup(group, samRequestContext) @@ -63,7 +74,7 @@ class GoogleGroupSynchronizer( addedUserSyncReports <- toAdd.toList.traverse(addMemberToGoogleGroup(group, samRequestContext)) removedUserSyncReports <- toRemove.toList.traverse(removeMemberFromGoogleGroup(group, samRequestContext)) - _ <- directoryDAO.updateSynchronizedDate(groupId, samRequestContext) + _ <- directoryDAO.updateSynchronizedDateAndVersion(group, samRequestContext) } yield Map(group.email -> Seq(addedUserSyncReports, removedUserSyncReports).flatten) ++ subGroupSyncs.flatten } diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/SamModel.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/SamModel.scala index dad99a8fe..de3449b87 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/SamModel.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/SamModel.scala @@ -215,7 +215,9 @@ consistent "has a" relationship is tracked by this ticket: https://broadworkbenc roles: Set[ResourceRoleName], actions: Set[ResourceAction], descendantPermissions: Set[AccessPolicyDescendantPermissions], - public: Boolean + public: Boolean, + version: Int = 1, + lastSynchronizedVersion: Option[Int] = None ) extends WorkbenchGroup @Lenses final case class AccessPolicyDescendantPermissions(resourceType: ResourceTypeName, actions: Set[ResourceAction], roles: Set[ResourceRoleName]) @@ -231,10 +233,18 @@ consistent "has a" relationship is tracked by this ticket: https://broadworkbenc email: WorkbenchEmail, roles: Set[ResourceRoleName], actions: Set[ResourceAction], - public: Boolean + public: Boolean, + version: Int = 1, + lastSynchronizedVersion: Option[Int] = None ) -@Lenses final case class BasicWorkbenchGroup(id: WorkbenchGroupName, members: Set[WorkbenchSubject], email: WorkbenchEmail) extends WorkbenchGroup +@Lenses final case class BasicWorkbenchGroup( + id: WorkbenchGroupName, + members: Set[WorkbenchSubject], + email: WorkbenchEmail, + version: Int = 1, + lastSynchronizedVersion: Option[Int] = None +) extends WorkbenchGroup object BasicWorkbenchGroup { def apply(workbenchGroup: WorkbenchGroup): BasicWorkbenchGroup = workbenchGroup.id match { diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ManagedGroupService.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ManagedGroupService.scala index 4b37830df..4dfad4cbd 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ManagedGroupService.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ManagedGroupService.scala @@ -83,7 +83,8 @@ class ManagedGroupService( val workbenchGroupName = WorkbenchGroupName(resource.resourceId.value) val groupMembers: Set[WorkbenchSubject] = componentPolicies.collect { // collect only member and admin policies - case AccessPolicy(id @ FullyQualifiedPolicyId(_, ManagedGroupService.memberPolicyName | ManagedGroupService.adminPolicyName), _, _, _, _, _, _) => id + case AccessPolicy(id @ FullyQualifiedPolicyId(_, ManagedGroupService.memberPolicyName | ManagedGroupService.adminPolicyName), _, _, _, _, _, _, _, _) => + id } directoryDAO.createGroup(BasicWorkbenchGroup(workbenchGroupName, groupMembers, email), accessInstructionsOpt, samRequestContext = samRequestContext) } 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 0795b0a86..dafd42dbc 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 @@ -15,18 +15,7 @@ import org.broadinstitute.dsde.workbench.sam.azure.AzureService import org.broadinstitute.dsde.workbench.sam.dataAccess.{AccessPolicyDAO, DirectoryDAO, LoadResourceAuthDomainResult} import org.broadinstitute.dsde.workbench.sam.google.GoogleExtensions import org.broadinstitute.dsde.workbench.sam.model._ -import org.broadinstitute.dsde.workbench.sam.model.api.{ - AccessPolicyMembershipRequest, - AccessPolicyMembershipResponse, - FilteredResourceFlat, - FilteredResourceFlatPolicy, - FilteredResourceHierarchical, - FilteredResourceHierarchicalPolicy, - FilteredResourceHierarchicalRole, - FilteredResourcesFlat, - FilteredResourcesHierarchical, - SamUser -} +import org.broadinstitute.dsde.workbench.sam.model.api._ import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext import java.util.UUID @@ -298,6 +287,7 @@ class ResourceService( } else IO.unit policies <- listResourcePolicies(resource, samRequestContext) _ <- accessPolicyDAO.addResourceAuthDomain(resource, authDomains, samRequestContext) + _ <- policies.traverse(p => directoryDAO.updateGroupUpdatedDateAndVersionWithSession(FullyQualifiedPolicyId(resource, p.policyName), samRequestContext)) _ <- cloudExtensions.onGroupUpdate(policies.map(p => FullyQualifiedPolicyId(resource, p.policyName)), samRequestContext) authDomains <- loadResourceAuthDomain(resource, samRequestContext) } yield authDomains @@ -871,6 +861,10 @@ class ResourceService( for { originalPolicies <- accessPolicyDAO.listAccessPolicies(policyId.resource, samRequestContext) policyChanged <- accessPolicyDAO.setPolicyIsPublic(policyId, public, samRequestContext) + _ <- directoryDAO.updateGroupUpdatedDateAndVersionWithSession( + FullyQualifiedPolicyId(policyId.resource, policyId.accessPolicyName), + samRequestContext + ) _ <- onPolicyUpdateIfChanged(policyId, originalPolicies, samRequestContext)(policyChanged) } yield () } 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 d599daaa8..a3afb1b5d 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 @@ -183,7 +183,7 @@ class MockAccessPolicyDAO(private val resourceTypes: mutable.Map[ResourceTypeNam val newPolicy = policy.copy(public = isPublic) IO { policies.put(resourceAndPolicyName, newPolicy) match { - case Some(AccessPolicy(_, _, _, _, _, _, public)) => public != isPublic + case Some(AccessPolicy(_, _, _, _, _, _, public, _, _)) => public != isPublic case _ => false } } diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockDirectoryDAO.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockDirectoryDAO.scala index 09a8f25a6..1d4e62869 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockDirectoryDAO.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockDirectoryDAO.scala @@ -257,11 +257,14 @@ class MockDirectoryDAO(val groups: mutable.Map[WorkbenchGroupIdentity, Workbench }.toSeq } - override def updateSynchronizedDate(groupId: WorkbenchGroupIdentity, samRequestContext: SamRequestContext): IO[Unit] = { - groupSynchronizedDates += groupId -> new Date() + override def updateSynchronizedDateAndVersion(group: WorkbenchGroup, samRequestContext: SamRequestContext): IO[Unit] = { + groupSynchronizedDates += group.id -> new Date() IO.unit } + override def updateGroupUpdatedDateAndVersionWithSession(groupId: WorkbenchGroupIdentity, samRequestContext: SamRequestContext): IO[Unit] = + IO.unit + override def loadSubjectEmail(subject: WorkbenchSubject, samRequestContext: SamRequestContext): IO[Option[WorkbenchEmail]] = IO { subject match { case id: WorkbenchUserId => users.get(id).map(_.email) 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 f382d67db..ee723aceb 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 @@ -713,8 +713,8 @@ class PostgresAccessPolicyDAOSpec extends AnyFreeSpec with Matchers with BeforeA dao.createResource(resource1, samRequestContext).unsafeRunSync() dao.createResource(resource2, samRequestContext).unsafeRunSync() - dirDao.updateSynchronizedDate(policy1.id, samRequestContext).unsafeRunSync() - dirDao.updateSynchronizedDate(policy3.id, samRequestContext).unsafeRunSync() + dirDao.updateSynchronizedDateAndVersion(policy1, samRequestContext).unsafeRunSync() + dirDao.updateSynchronizedDateAndVersion(policy3, samRequestContext).unsafeRunSync() dao.listSyncedAccessPolicyIdsOnResourcesConstrainedByGroup(sharedAuthDomain.id, samRequestContext).unsafeRunSync() should contain theSameElementsAs Set( policy1.id, @@ -2896,7 +2896,7 @@ class PostgresAccessPolicyDAOSpec extends AnyFreeSpec with Matchers with BeforeA dao.createPolicy(policy, samRequestContext).unsafeRunSync() dao.loadPolicy(policy.id, samRequestContext).unsafeRunSync() shouldEqual Option(policy) - val newPolicy = policy.copy(members = Set(defaultUser.id, secondUser.id), actions = Set(readAction), roles = Set.empty, public = true) + val newPolicy = policy.copy(members = Set(defaultUser.id, secondUser.id), actions = Set(readAction), roles = Set.empty, public = true, version = 2) dao.overwritePolicy(newPolicy, samRequestContext).unsafeRunSync() dao.loadPolicy(policy.id, samRequestContext).unsafeRunSync() shouldEqual Option(newPolicy) @@ -2969,7 +2969,8 @@ class PostgresAccessPolicyDAOSpec extends AnyFreeSpec with Matchers with BeforeA actions = Set(readAction), roles = Set.empty, descendantPermissions = updatedDescendantPermissions, - public = true + public = true, + version = 2 ) val testResult = for { diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAOSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAOSpec.scala index c234581cc..9d4ba0e84 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAOSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAOSpec.scala @@ -248,6 +248,8 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B val loadedGroup = dao.loadGroup(defaultGroup.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"failed to load group ${defaultGroup.id}")) loadedGroup.members should contain theSameElementsAs Set(subGroup.id) + + loadedGroup.version shouldEqual 2 } "add users to groups" in { @@ -259,6 +261,8 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B val loadedGroup = dao.loadGroup(defaultGroup.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"failed to load group ${defaultGroup.id}")) loadedGroup.members should contain theSameElementsAs Set(defaultUser.id) + + loadedGroup.version shouldEqual 2 } "add policies to groups" in { @@ -272,6 +276,8 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B val loadedGroup = dao.loadGroup(defaultGroup.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"failed to load group ${defaultGroup.id}")) loadedGroup.members should contain theSameElementsAs Set(defaultPolicy.id) + + loadedGroup.version shouldEqual 2 } "add groups to policies" in { @@ -286,6 +292,8 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B val loadedPolicy = policyDAO.loadPolicy(defaultPolicy.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"s'failed to load policy ${defaultPolicy.id}")) loadedPolicy.members should contain theSameElementsAs Set(defaultGroup.id) + + loadedPolicy.version shouldEqual 2 } "add users to policies" in { @@ -300,6 +308,8 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B val loadedPolicy = policyDAO.loadPolicy(defaultPolicy.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"s'failed to load policy ${defaultPolicy.id}")) loadedPolicy.members should contain theSameElementsAs Set(defaultUser.id) + + loadedPolicy.version shouldEqual 2 } "add policies to other policies" in { @@ -316,6 +326,8 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B val loadedPolicy = policyDAO.loadPolicy(defaultPolicy.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"s'failed to load policy ${defaultPolicy.id}")) loadedPolicy.members should contain theSameElementsAs Set(memberPolicy.id) + + loadedPolicy.version shouldEqual 2 } "trying to add a group that does not exist will fail" in { @@ -373,10 +385,13 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B dao.addGroupMember(defaultGroup.id, subGroup.id, samRequestContext).unsafeRunSync() shouldBe true val afterAdd = dao.loadGroup(defaultGroup.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"failed to load group ${defaultGroup.id}")) afterAdd.members should contain theSameElementsAs Set(subGroup.id) + afterAdd.version shouldEqual 2 + dao.removeGroupMember(defaultGroup.id, subGroup.id, samRequestContext).unsafeRunSync() shouldBe true val afterRemove = dao.loadGroup(defaultGroup.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"failed to load group ${defaultGroup.id}")) afterRemove.members shouldBe empty + afterRemove.version shouldEqual 3 } "remove users from groups" in { @@ -387,10 +402,13 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B dao.addGroupMember(defaultGroup.id, defaultUser.id, samRequestContext).unsafeRunSync() shouldBe true val afterAdd = dao.loadGroup(defaultGroup.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"failed to load group ${defaultGroup.id}")) afterAdd.members should contain theSameElementsAs Set(defaultUser.id) + afterAdd.version shouldEqual 2 + dao.removeGroupMember(defaultGroup.id, defaultUser.id, samRequestContext).unsafeRunSync() shouldBe true val afterRemove = dao.loadGroup(defaultGroup.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"failed to load group ${defaultGroup.id}")) afterRemove.members shouldBe empty + afterRemove.version shouldEqual 3 } "remove policies from groups" in { @@ -403,10 +421,13 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B dao.addGroupMember(defaultGroup.id, defaultPolicy.id, samRequestContext).unsafeRunSync() shouldBe true val afterAdd = dao.loadGroup(defaultGroup.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"failed to load group ${defaultGroup.id}")) afterAdd.members should contain theSameElementsAs Set(defaultPolicy.id) + afterAdd.version shouldEqual 2 + dao.removeGroupMember(defaultGroup.id, defaultPolicy.id, samRequestContext).unsafeRunSync() shouldBe true val afterRemove = dao.loadGroup(defaultGroup.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"failed to load group ${defaultGroup.id}")) afterRemove.members shouldBe empty + afterRemove.version shouldEqual 3 } "remove groups from policies" in { @@ -420,12 +441,16 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B dao.addGroupMember(defaultPolicy.id, defaultGroup.id, samRequestContext).unsafeRunSync() val afterAdd = policyDAO.loadPolicy(defaultPolicy.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"s'failed to load policy ${defaultPolicy.id}")) afterAdd.members should contain theSameElementsAs Set(defaultGroup.id) + afterAdd.version shouldEqual 2 + policyDAO.listFlattenedPolicyMembers(defaultPolicy.id, samRequestContext).unsafeRunSync() should contain theSameElementsAs Set(defaultUser) dao.removeGroupMember(defaultPolicy.id, defaultGroup.id, samRequestContext).unsafeRunSync() val afterRemove = policyDAO.loadPolicy(defaultPolicy.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"s'failed to load policy ${defaultPolicy.id}")) afterRemove.members shouldBe empty + afterRemove.version shouldEqual 3 + policyDAO.listFlattenedPolicyMembers(defaultPolicy.id, samRequestContext).unsafeRunSync() shouldBe empty } @@ -442,6 +467,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B val loadedPolicy = policyDAO.loadPolicy(defaultPolicy.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"s'failed to load policy ${defaultPolicy.id}")) loadedPolicy.members shouldBe empty + loadedPolicy.version shouldBe 3 } "remove policies from other policies" in { @@ -459,6 +485,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B val loadedPolicy = policyDAO.loadPolicy(defaultPolicy.id, samRequestContext).unsafeRunSync().getOrElse(fail(s"s'failed to load policy ${defaultPolicy.id}")) loadedPolicy.members shouldBe empty + loadedPolicy.version shouldBe 3 } } @@ -1320,7 +1347,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B assume(databaseEnabled, databaseEnabledClue) dao.createGroup(defaultGroup, samRequestContext = samRequestContext).unsafeRunSync() - dao.updateSynchronizedDate(defaultGroup.id, samRequestContext).unsafeRunSync() + dao.updateSynchronizedDateAndVersion(defaultGroup, samRequestContext).unsafeRunSync() val loadedDate = dao.getSynchronizedDate(defaultGroup.id, samRequestContext).unsafeRunSync().getOrElse(fail("failed to load date")) loadedDate.getTime should equal(new Date().getTime +- 2.seconds.toMillis) @@ -1332,7 +1359,7 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B policyDAO.createResource(defaultResource, samRequestContext).unsafeRunSync() policyDAO.createPolicy(defaultPolicy, samRequestContext).unsafeRunSync() - dao.updateSynchronizedDate(defaultPolicy.id, samRequestContext).unsafeRunSync() + dao.updateSynchronizedDateAndVersion(defaultPolicy, samRequestContext).unsafeRunSync() val loadedDate = dao.getSynchronizedDate(defaultPolicy.id, samRequestContext).unsafeRunSync().getOrElse(fail("failed to load date")) loadedDate.getTime should equal(new Date().getTime +- 2.seconds.toMillis) diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutesSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutesSpec.scala index 58c9cab67..8461ba421 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutesSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutesSpec.scala @@ -17,8 +17,8 @@ import org.broadinstitute.dsde.workbench.sam.TestSupport.{genSamDependencies, ge import org.broadinstitute.dsde.workbench.sam.api.SamRoutes import org.broadinstitute.dsde.workbench.sam.config.GoogleServicesConfig import org.broadinstitute.dsde.workbench.sam.mock.RealKeyMockGoogleIamDAO -import org.broadinstitute.dsde.workbench.sam.model.api.SamJsonSupport._ import org.broadinstitute.dsde.workbench.sam.model._ +import org.broadinstitute.dsde.workbench.sam.model.api.SamJsonSupport._ import org.broadinstitute.dsde.workbench.sam.model.api._ import org.broadinstitute.dsde.workbench.sam.service._ import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionSpec.scala index 5146965ca..94130b099 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionSpec.scala @@ -142,7 +142,7 @@ class GoogleExtensionSpec(_system: ActorSystem) case p: AccessPolicy => when(mockAccessPolicyDAO.loadPolicy(p.id, samRequestContext)).thenReturn(IO.pure(Option(testPolicy))) } - when(mockDirectoryDAO.updateSynchronizedDate(any[WorkbenchGroupIdentity], any[SamRequestContext])).thenReturn(IO.unit) + when(mockDirectoryDAO.updateSynchronizedDateAndVersion(any[WorkbenchGroup], any[SamRequestContext])).thenReturn(IO.unit) when(mockDirectoryDAO.getSynchronizedDate(any[WorkbenchGroupIdentity], any[SamRequestContext])) .thenReturn(IO.pure(Some(new GregorianCalendar(2017, 11, 22).getTime()))) @@ -192,7 +192,12 @@ class GoogleExtensionSpec(_system: ActorSystem) added.foreach(email => verify(mockGoogleDirectoryDAO).addMemberToGroup(target.email, WorkbenchEmail(email.value.toLowerCase))) removed.foreach(email => verify(mockGoogleDirectoryDAO).removeMemberFromGroup(target.email, WorkbenchEmail(email.value.toLowerCase))) - verify(mockDirectoryDAO).updateSynchronizedDate(target.id, samRequestContext) + + target match { + case _: BasicWorkbenchGroup => verify(mockDirectoryDAO).updateSynchronizedDateAndVersion(target, samRequestContext) + case p: AccessPolicy => + verify(mockDirectoryDAO).updateSynchronizedDateAndVersion(p.copy(members = p.members + CloudExtensions.allUsersGroupName), samRequestContext) + } } } @@ -299,7 +304,7 @@ class GoogleExtensionSpec(_system: ActorSystem) when(mockDirectoryDAO.listIntersectionGroupUsers(Set(managedGroupId, testPolicy.id), samRequestContext)) .thenReturn(IO.pure(Set(intersectionSamUserId, authorizedGoogleUserId, subIntersectionSamGroupUserId, subAuthorizedGoogleGroupUserId, addError))) - when(mockDirectoryDAO.updateSynchronizedDate(any[WorkbenchGroupIdentity], any[SamRequestContext])).thenReturn(IO.unit) + when(mockDirectoryDAO.updateSynchronizedDateAndVersion(any[WorkbenchGroup], any[SamRequestContext])).thenReturn(IO.unit) when(mockDirectoryDAO.getSynchronizedDate(any[WorkbenchGroupIdentity], any[SamRequestContext])) .thenReturn(IO.pure(Some(new GregorianCalendar(2017, 11, 22).getTime()))) @@ -331,7 +336,7 @@ class GoogleExtensionSpec(_system: ActorSystem) added.foreach(email => verify(mockGoogleDirectoryDAO).addMemberToGroup(testPolicy.email, WorkbenchEmail(email.value.toLowerCase))) removed.foreach(email => verify(mockGoogleDirectoryDAO).removeMemberFromGroup(testPolicy.email, WorkbenchEmail(email.value.toLowerCase))) - verify(mockDirectoryDAO).updateSynchronizedDate(testPolicy.id, samRequestContext) + verify(mockDirectoryDAO).updateSynchronizedDateAndVersion(testPolicy, samRequestContext) } it should "break out of cycle" in { diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/MockUserService.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/MockUserService.scala index 74a654169..4e608a733 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/MockUserService.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/MockUserService.scala @@ -93,8 +93,8 @@ class MockUserService( def getAllPetServiceAccountsForUser(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Seq[PetServiceAccount]] = directoryDAO.getAllPetServiceAccountsForUser(userId, samRequestContext) - def updateSynchronizedDate(groupId: WorkbenchGroupIdentity, samRequestContext: SamRequestContext): IO[Unit] = - directoryDAO.updateSynchronizedDate(groupId, samRequestContext) + def updateSynchronizedDateAndVersion(group: WorkbenchGroup, samRequestContext: SamRequestContext): IO[Unit] = + directoryDAO.updateSynchronizedDateAndVersion(group, samRequestContext) def loadSubjectEmail(subject: WorkbenchSubject, samRequestContext: SamRequestContext): IO[Option[WorkbenchEmail]] = directoryDAO.loadSubjectEmail(subject, samRequestContext) diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceSpec.scala index 21ec0ea85..8eadd0331 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceSpec.scala @@ -951,6 +951,48 @@ class ResourceServiceSpec } } + "addResourceAuthDomain" should "increment the group version of policy groups associated with the resource" in { + assume(databaseEnabled, databaseEnabledClue) + + constrainableResourceType.isAuthDomainConstrainable shouldEqual true + constrainableService.createResourceType(constrainableResourceType, samRequestContext).unsafeRunSync() + + constrainableService.createResourceType(managedGroupResourceType, samRequestContext).unsafeRunSync() + val managedGroupName = "fooGroup" + val secondMGroupName = "barGroup" + runAndWait(managedGroupService.createManagedGroup(ResourceId(managedGroupName), dummyUser, samRequestContext = samRequestContext)) + runAndWait(managedGroupService.createManagedGroup(ResourceId(secondMGroupName), dummyUser, samRequestContext = samRequestContext)) + + val authDomain = NonEmptyList.of(WorkbenchGroupName(managedGroupName), WorkbenchGroupName(secondMGroupName)) + val viewPolicyName = AccessPolicyName(constrainableReaderRoleName.value) + val resource = runAndWait( + constrainableService.createResource( + constrainableResourceType, + ResourceId(UUID.randomUUID().toString), + Map(viewPolicyName -> constrainablePolicyMembership), + Set.empty, + None, + dummyUser.id, + samRequestContext + ) + ) + + val resourceAndPolicyName = + FullyQualifiedPolicyId(FullyQualifiedResourceId(constrainableResourceType.name, resource.resourceId), viewPolicyName) + val policy = + policyDAO.loadPolicy(resourceAndPolicyName, samRequestContext).unsafeRunSync().getOrElse(fail(s"s'failed to load policy ${resourceAndPolicyName}")) + policy.version shouldEqual 1 + + val resourceWithAuthDomain = + runAndWait(constrainableService.addResourceAuthDomain(resource.fullyQualifiedId, authDomain.toList.toSet, dummyUser.id, samRequestContext)) + resourceWithAuthDomain shouldEqual authDomain.toList.toSet + + val updatedPolicy = + policyDAO.loadPolicy(resourceAndPolicyName, samRequestContext).unsafeRunSync().getOrElse(fail(s"s'failed to load policy ${resourceAndPolicyName}")) + updatedPolicy.version shouldEqual 2 + + } + "listUserResourceRoles" should "list the user's role when they have at least one role" in { assume(databaseEnabled, databaseEnabledClue) @@ -2174,7 +2216,7 @@ class ResourceServiceSpec policyDAO.overwritePolicy(policy.get.copy(members = Set(dummyUser.id)), samRequestContext).unsafeRunSync() val policy2 = policyDAO.loadPolicy(resourceAndPolicyName, samRequestContext).unsafeRunSync() policy2.map(_.copy(email = WorkbenchEmail(""))) should equal( - Some(AccessPolicy(resourceAndPolicyName, Set(dummyUser.id), WorkbenchEmail(""), Set(ownerRoleName), Set.empty, Set.empty, public = false)) + Some(AccessPolicy(resourceAndPolicyName, Set(dummyUser.id), WorkbenchEmail(""), Set(ownerRoleName), Set.empty, Set.empty, public = false, version = 2)) ) // call it again to ensure it does not fail @@ -2183,7 +2225,7 @@ class ResourceServiceSpec // verify the policy has not changed val policy3 = policyDAO.loadPolicy(resourceAndPolicyName, samRequestContext).unsafeRunSync() policy3.map(_.copy(email = WorkbenchEmail(""))) should equal( - Some(AccessPolicy(resourceAndPolicyName, Set(dummyUser.id), WorkbenchEmail(""), Set(ownerRoleName), Set.empty, Set.empty, public = false)) + Some(AccessPolicy(resourceAndPolicyName, Set(dummyUser.id), WorkbenchEmail(""), Set(ownerRoleName), Set.empty, Set.empty, public = false, version = 2)) ) }