diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/AdminRoutes.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/AdminRoutes.scala index 3e2629276..19fde1868 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/AdminRoutes.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/AdminRoutes.scala @@ -11,7 +11,6 @@ import cats.effect.IO import org.broadinstitute.dsde.workbench.model._ import org.broadinstitute.dsde.workbench.model.google.GoogleProject import org.broadinstitute.dsde.workbench.sam.config.LiquibaseConfig -import org.broadinstitute.dsde.workbench.sam.dataAccess.DirectoryDAO import org.broadinstitute.dsde.workbench.sam.model.api.SamJsonSupport._ import org.broadinstitute.dsde.workbench.sam.model.SamResourceActions.{adminAddMember, adminReadPolicies, adminRemoveMember} import org.broadinstitute.dsde.workbench.sam.model.SamResourceTypes.resourceTypeAdminName @@ -29,7 +28,6 @@ trait AdminRoutes extends SecurityDirectives with SamRequestContextDirectives wi implicit val executionContext: ExecutionContext val resourceService: ResourceService - val directoryDAO: DirectoryDAO val managedGroupService: ManagedGroupService val liquibaseConfig: LiquibaseConfig @@ -149,12 +147,15 @@ trait AdminRoutes extends SecurityDirectives with SamRequestContextDirectives wi } } ~ pathPrefix("repairCloudAccess") { + pathEndOrSingleSlash { putWithTelemetry(samRequestContext, userIdParam(workbenchUserId)) { complete { - repairCloudAccess(workbenchUserId, samRequestContext) + userService + .repairCloudAccess(workbenchUserId, samRequestContext) .map(_ => NoContent) } } + } } } } @@ -275,17 +276,6 @@ trait AdminRoutes extends SecurityDirectives with SamRequestContextDirectives wi } } } - def repairCloudAccess(workbenchUserId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Unit] = { - for { - maybeUser <- userService - .getUser(workbenchUserId, samRequestContext = samRequestContext) - user = maybeUser.getOrElse(throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, s"User ${workbenchUserId.value} not found"))) - _ <- cloudExtensions.onUserCreate(user, samRequestContext) - groups <- directoryDAO.listUserDirectMemberships(user.id, samRequestContext) - _ <- cloudExtensions.onGroupUpdate(groups, Set(user.id), samRequestContext) - } yield IO.pure(()) - } - def requireAdminResourceAction(action: ResourceAction, resourceType: ResourceType, user: SamUser, samRequestContext: SamRequestContext): Directive0 = requireAction( diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/UserService.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/UserService.scala index e16b15bf5..f7b4fec1a 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/UserService.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/UserService.scala @@ -489,6 +489,19 @@ class UserService( def setUserAttributes(userAttributes: SamUserAttributes, samRequestContext: SamRequestContext): IO[SamUserAttributes] = directoryDAO.setUserAttributes(userAttributes, samRequestContext).map(_ => userAttributes) + + def repairCloudAccess(workbenchUserId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Unit] = { + val maybeUser = getUser(workbenchUserId, samRequestContext) + maybeUser.flatMap { + case Some(user) => + for { + _ <- cloudExtensions.onUserCreate(user, samRequestContext) + groups <- directoryDAO.listUserDirectMemberships(user.id, samRequestContext) + _ <- cloudExtensions.onGroupUpdate(groups, Set(user.id), samRequestContext) + } yield IO.pure(()) + case None => IO.raiseError(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, s"User $workbenchUserId not found"))) + } + } } object UserService { diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/api/AdminUserRoutesSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/api/AdminUserRoutesSpec.scala index feca4bbe0..b5d8b2e1d 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/api/AdminUserRoutesSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/api/AdminUserRoutesSpec.scala @@ -395,4 +395,32 @@ class AdminUserRoutesSpec extends AnyFlatSpec with Matchers with ScalatestRouteT status shouldEqual StatusCodes.NotFound } } + + "PUT /admin/v2/user/{userId}/repairCloudAccess" should "return no content when called as admin user" in { + // Arrange + val samRoutes = new MockSamRoutesBuilder(allUsersGroup) + .callAsAdminUser() // enabled "admin" user who is making the http request + .withEnabledUser(defaultUser) // "persisted/enabled" user we will check the status of + .withAllowedUser(defaultUser) + .build + + // Act and Assert + Put(s"/api/admin/v2/user/$defaultUserId/repairCloudAccess") ~> samRoutes.route ~> check { + status shouldEqual StatusCodes.NoContent + } + } + + it should "forbid the request when the requesting user is a non admin" in { + // Arrange + val samRoutes = new MockSamRoutesBuilder(allUsersGroup) + .withEnabledUser(defaultUser) // "persisted/enabled" user we will check the status of + .withAllowedUser(defaultUser) + .callAsNonAdminUser() + .build + + // Act and Assert + Put(s"/api/admin/v2/user/$defaultUserId/repairCloudAccess") ~> samRoutes.route ~> check { + status shouldEqual StatusCodes.Forbidden + } + } } 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 4e608a733..376270bbf 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 @@ -123,9 +123,6 @@ class MockUserService( def setGoogleSubjectId(userId: WorkbenchUserId, googleSubjectId: GoogleSubjectId, samRequestContext: SamRequestContext): IO[Unit] = directoryDAO.setGoogleSubjectId(userId, googleSubjectId, samRequestContext) - def listUserDirectMemberships(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[LazyList[WorkbenchGroupIdentity]] = - directoryDAO.listUserDirectMemberships(userId, samRequestContext) - def listFlattenedGroupMembers(groupName: WorkbenchGroupName, samRequestContext: SamRequestContext): IO[Set[WorkbenchUserId]] = directoryDAO.listFlattenedGroupMembers(groupName, samRequestContext) diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/UserServiceSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/UserServiceSpec.scala index 33f62462b..3e53480ac 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/UserServiceSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/UserServiceSpec.scala @@ -8,7 +8,7 @@ import cats.effect.IO import cats.effect.unsafe.implicits.{global => globalEc} import org.broadinstitute.dsde.workbench.model._ import org.broadinstitute.dsde.workbench.sam.Generator.{arbNonPetEmail => _, _} -import org.broadinstitute.dsde.workbench.sam.TestSupport.{databaseEnabled, databaseEnabledClue, truncateAll} +import org.broadinstitute.dsde.workbench.sam.TestSupport.{databaseEnabled, databaseEnabledClue, googleServicesConfig, truncateAll} import org.broadinstitute.dsde.workbench.sam.dataAccess.{DirectoryDAO, PostgresDirectoryDAO} import org.broadinstitute.dsde.workbench.sam.google.GoogleExtensions import org.broadinstitute.dsde.workbench.sam.matchers.BeSameUserMatcher.beSameUserAs @@ -715,4 +715,40 @@ class OldUserServiceSpec(_system: ActorSystem) assert(service.validateEmailAddress(WorkbenchEmail("foo@splat.bar.com"), Seq.empty, Seq("bar.com")).attempt.unsafeRunSync().isLeft) assert(service.validateEmailAddress(WorkbenchEmail("foo@bar.com"), Seq.empty, Seq("bar.com")).attempt.unsafeRunSync().isLeft) } + + "UserService repairCloudAccess" should "create a proxy group for a user and add it to any groups they are a member of" in { + assume(databaseEnabled, databaseEnabledClue) + + // Create user + val inviteeEmail = genNonPetEmail.sample.get + service.inviteUser(inviteeEmail, samRequestContext).unsafeRunSync() + val invitedUserId = dirDAO.loadSubjectFromEmail(inviteeEmail, samRequestContext).unsafeRunSync().value.asInstanceOf[WorkbenchUserId] + + val userInPostgres = dirDAO.loadUser(invitedUserId, samRequestContext).unsafeRunSync() + userInPostgres.value should { + equal(SamUser(invitedUserId, None, inviteeEmail, None, false)) + } + + val registeringUser = genWorkbenchUserGoogle.sample.get.copy(email = inviteeEmail) + runAndWait(service.createUser(registeringUser, samRequestContext)) + + verify(googleExtensions).onUserCreate(SamUser(invitedUserId, None, inviteeEmail, None, false), samRequestContext) + + verify(googleExtensions).onGroupUpdate(Seq.empty, Set(invitedUserId), samRequestContext) + + val updatedUserInPostgres = dirDAO.loadUser(invitedUserId, samRequestContext).unsafeRunSync() + updatedUserInPostgres.value shouldBe SamUser(invitedUserId, registeringUser.googleSubjectId, inviteeEmail, None, true) + + val proxyGroup = + WorkbenchGroupName(s"${googleServicesConfig.resourceNamePrefix.getOrElse("")}PROXY_${invitedUserId.value}@${googleServicesConfig.appsDomain}") + // delete proxy group + runAndWait(dirDAO.deleteGroup(proxyGroup, samRequestContext)) + + // Run test + service.repairCloudAccess(invitedUserId, samRequestContext).unsafeRunSync() + + verify(googleExtensions).onUserCreate(SamUser(invitedUserId, registeringUser.googleSubjectId, inviteeEmail, None, true), samRequestContext) + + verify(googleExtensions).onGroupUpdate(Seq(allUsersGroup.id), Set(invitedUserId), samRequestContext) + } }