From cde1dd0556f02b77f36fee4f98534b0430ffeb0e Mon Sep 17 00:00:00 2001 From: Marc Talbott Date: Tue, 4 Feb 2020 10:51:44 -0500 Subject: [PATCH] [CA-675] Properly update pet service accounts in OpenDJ (#404) --- .../sam/directory/LdapRegistrationDAO.scala | 7 +++++++ .../workbench/sam/directory/RegistrationDAO.scala | 1 + .../dsde/workbench/sam/google/GoogleExtensions.scala | 1 + .../workbench/sam/google/GoogleExtensionSpec.scala | 12 +++++++----- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/directory/LdapRegistrationDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/directory/LdapRegistrationDAO.scala index 08d91fd55..cea5f82d4 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/directory/LdapRegistrationDAO.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/directory/LdapRegistrationDAO.scala @@ -148,6 +148,13 @@ class LdapRegistrationDAO( override def deletePetServiceAccount(petServiceAccountId: PetServiceAccountId): IO[Unit] = executeLdap(IO(ldapConnectionPool.delete(petDn(petServiceAccountId)))) + override def updatePetServiceAccount(petServiceAccount: PetServiceAccount): IO[PetServiceAccount] = { + val modifications = createPetServiceAccountAttributes(petServiceAccount).map { attribute => + new Modification(ModificationType.REPLACE, attribute.getName, attribute.getRawValues) + } + executeLdap(IO(ldapConnectionPool.modify(petDn(petServiceAccount.id), modifications.asJava))) *> IO.pure(petServiceAccount) + } + override def setGoogleSubjectId(userId: WorkbenchUserId, googleSubjectId: GoogleSubjectId): IO[Unit] = executeLdap(IO(ldapConnectionPool.modify(userDn(userId), new Modification(ModificationType.ADD, Attr.googleSubjectId, googleSubjectId.value)))) } diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/directory/RegistrationDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/directory/RegistrationDAO.scala index 74b81decf..586cdc0c1 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/directory/RegistrationDAO.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/directory/RegistrationDAO.scala @@ -18,5 +18,6 @@ trait RegistrationDAO { def createPetServiceAccount(petServiceAccount: PetServiceAccount): IO[PetServiceAccount] def loadPetServiceAccount(petServiceAccountId: PetServiceAccountId): IO[Option[PetServiceAccount]] def deletePetServiceAccount(petServiceAccountId: PetServiceAccountId): IO[Unit] + def updatePetServiceAccount(petServiceAccount: PetServiceAccount): IO[PetServiceAccount] def setGoogleSubjectId(userId: WorkbenchUserId, googleSubjectId: GoogleSubjectId): IO[Unit] } diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensions.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensions.scala index 4d14c39f3..97567f10d 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensions.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensions.scala @@ -342,6 +342,7 @@ class GoogleExtensions( // pet already exists in ldap, but a new SA was created so update ldap with new SA info case (Some(p), None) => directoryDAO.updatePetServiceAccount(p.copy(serviceAccount = serviceAccount)) + registrationDAO.updatePetServiceAccount(p.copy(serviceAccount = serviceAccount)) // everything already existed case (Some(p), Some(_)) => IO.pure(p) 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 caa2bbbe4..f732fadd2 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 @@ -278,7 +278,7 @@ class GoogleExtensionSpec(_system: ActorSystem) extends TestKit(_system) with Fl } "GoogleExtension" should "get a pet service account for a user" in { - val (dirDAO: DirectoryDAO, mockGoogleIamDAO: MockGoogleIamDAO, mockGoogleDirectoryDAO: MockGoogleDirectoryDAO, googleExtensions: GoogleExtensions, service: UserService, defaultUserId: WorkbenchUserId, defaultUserEmail: WorkbenchEmail, defaultUserProxyEmail: WorkbenchEmail, createDefaultUser: CreateWorkbenchUser) = initPetTest + val (dirDAO: DirectoryDAO, _: RegistrationDAO, mockGoogleIamDAO: MockGoogleIamDAO, mockGoogleDirectoryDAO: MockGoogleDirectoryDAO, googleExtensions: GoogleExtensions, service: UserService, defaultUserId: WorkbenchUserId, defaultUserEmail: WorkbenchEmail, defaultUserProxyEmail: WorkbenchEmail, createDefaultUser: CreateWorkbenchUser) = initPetTest // create a user val newUser = service.createUser(createDefaultUser).futureValue @@ -327,8 +327,9 @@ class GoogleExtensionSpec(_system: ActorSystem) extends TestKit(_system) with Fl } - private def initPetTest: (DirectoryDAO, MockGoogleIamDAO, MockGoogleDirectoryDAO, GoogleExtensions, UserService, WorkbenchUserId, WorkbenchEmail, WorkbenchEmail, CreateWorkbenchUser) = { + private def initPetTest: (DirectoryDAO, RegistrationDAO, MockGoogleIamDAO, MockGoogleDirectoryDAO, GoogleExtensions, UserService, WorkbenchUserId, WorkbenchEmail, WorkbenchEmail, CreateWorkbenchUser) = { val dirDAO = newDirectoryDAO() + val regDAO = newRegistrationDAO() clearDatabase() @@ -344,7 +345,7 @@ class GoogleExtensionSpec(_system: ActorSystem) extends TestKit(_system) with Fl val defaultUserProxyEmail = WorkbenchEmail(s"PROXY_newuser123@${googleServicesConfig.appsDomain}") val defaultUser = CreateWorkbenchUser(defaultUserId, GoogleSubjectId(defaultUserId.value), defaultUserEmail) - (dirDAO, mockGoogleIamDAO, mockGoogleDirectoryDAO, googleExtensions, service, defaultUserId, defaultUserEmail, defaultUserProxyEmail, defaultUser) + (dirDAO, regDAO, mockGoogleIamDAO, mockGoogleDirectoryDAO, googleExtensions, service, defaultUserId, defaultUserEmail, defaultUserProxyEmail, defaultUser) } protected def newDirectoryDAO(): DirectoryDAO = new PostgresDirectoryDAO(TestSupport.dbRef, TestSupport.blockingEc) @@ -352,7 +353,7 @@ class GoogleExtensionSpec(_system: ActorSystem) extends TestKit(_system) with Fl protected def newAccessPolicyDAO(): AccessPolicyDAO = new PostgresAccessPolicyDAO(TestSupport.dbRef, TestSupport.blockingEc) it should "attach existing service account to pet" in { - val (dirDAO: DirectoryDAO, mockGoogleIamDAO: MockGoogleIamDAO, mockGoogleDirectoryDAO: MockGoogleDirectoryDAO, googleExtensions: GoogleExtensions, service: UserService, defaultUserId: WorkbenchUserId, defaultUserEmail: WorkbenchEmail, defaultUserProxyEmail: WorkbenchEmail, createDefaultUser: CreateWorkbenchUser) = initPetTest + val (dirDAO: DirectoryDAO, _: RegistrationDAO, mockGoogleIamDAO: MockGoogleIamDAO, mockGoogleDirectoryDAO: MockGoogleDirectoryDAO, googleExtensions: GoogleExtensions, service: UserService, defaultUserId: WorkbenchUserId, defaultUserEmail: WorkbenchEmail, defaultUserProxyEmail: WorkbenchEmail, createDefaultUser: CreateWorkbenchUser) = initPetTest val googleProject = GoogleProject("testproject") val defaultUser = WorkbenchUser(createDefaultUser.id, None, createDefaultUser.email) @@ -370,7 +371,7 @@ class GoogleExtensionSpec(_system: ActorSystem) extends TestKit(_system) with Fl } it should "recreate service account when missing for pet" in { - val (dirDAO: DirectoryDAO, mockGoogleIamDAO: MockGoogleIamDAO, mockGoogleDirectoryDAO: MockGoogleDirectoryDAO, googleExtensions: GoogleExtensions, service: UserService, defaultUserId: WorkbenchUserId, defaultUserEmail: WorkbenchEmail, defaultUserProxyEmail: WorkbenchEmail, createDefaultUser: CreateWorkbenchUser) = initPetTest + val (dirDAO: DirectoryDAO, regDAO: RegistrationDAO, mockGoogleIamDAO: MockGoogleIamDAO, mockGoogleDirectoryDAO: MockGoogleDirectoryDAO, googleExtensions: GoogleExtensions, service: UserService, defaultUserId: WorkbenchUserId, defaultUserEmail: WorkbenchEmail, defaultUserProxyEmail: WorkbenchEmail, createDefaultUser: CreateWorkbenchUser) = initPetTest // create a user val newUser = service.createUser(createDefaultUser).futureValue @@ -387,6 +388,7 @@ class GoogleExtensionSpec(_system: ActorSystem) extends TestKit(_system) with Fl val petServiceAccount2 = googleExtensions.createUserPetServiceAccount(defaultUser, googleProject).unsafeRunSync() petServiceAccount.serviceAccount shouldNot equal(petServiceAccount2.serviceAccount) + regDAO.loadPetServiceAccount(petServiceAccount.id).unsafeRunSync() shouldBe Some(petServiceAccount2) mockGoogleIamDAO.findServiceAccount(googleProject, petServiceAccount.serviceAccount.email).futureValue shouldBe Some(petServiceAccount2.serviceAccount) }