[CA-675] Properly update pet service accounts in OpenDJ (#404)

broadinstitute · Feb 4, 2020 · cde1dd0 · cde1dd0
1 parent 533cbcc
commit cde1dd0
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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,15 +345,15 @@ 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)
   protected def newRegistrationDAO(): RegistrationDAO = new LdapRegistrationDAO(connectionPool, directoryConfig, TestSupport.blockingEc)
   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)
   }