PROD-972 ID-1323 Eagerly Remove Google Group Members (#1470)

* PROD-972 Eagerly Remove Google Group Members * proxy group * revert whitespace change * remove foo * remove other debugging stuff too
broadinstitute · Jun 30, 2024 · 0b43abd · 0b43abd
1 parent ce60b3f
commit 0b43abd
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 0 deletions.
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
@@ -718,6 +718,36 @@ class GoogleExtensions(
   }
 
   override val allSubSystems: Set[Subsystems.Subsystem] = Set(Subsystems.GoogleGroups, Subsystems.GooglePubSub, Subsystems.GoogleIam)
+
+  def synchronouslyRemoveMemberFromGoogleGroup(
+      policyIdentity: FullyQualifiedPolicyId,
+      subject: WorkbenchSubject,
+      samRequestContext: SamRequestContext
+  ): IO[Unit] = {
+    val maybeEmail = subject match {
+      case userIdentity: WorkbenchUserId =>
+        getUserProxy(userIdentity)
+      case groupIdentity: WorkbenchGroupName =>
+        directoryDAO.loadGroupEmail(groupIdentity, samRequestContext)
+      case _ =>
+        IO(logger.info(s"Subject $subject is not a user or group, skipping removal from google group")).map(_ => None)
+
+    }
+    val maybePolicy = accessPolicyDAO.loadPolicy(policyIdentity, samRequestContext)
+
+    (maybeEmail, maybePolicy).tupled.flatMap {
+      case (Some(email), Some(policy)) =>
+        for {
+          _ <- IO.fromFuture(IO(googleDirectoryDAO.removeMemberFromGroup(policy.email, email)))
+        } yield logger.info(s"Synchronously removed $email for subject $subject from google group ${policy.email}")
+      case _ =>
+        IO(
+          logger.warn(
+            s"Could not remove $subject from google group for policy $policyIdentity because either the policy or subject could not be found. Policy: ${maybePolicy}, Subject: ${maybeEmail}."
+          )
+        )
+    }
+  }
 }
 
 case class GoogleExtensionsInitializer(cloudExtensions: GoogleExtensions, googleGroupSynchronizer: GoogleGroupSynchronizer) extends CloudExtensionsInitializer {

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
@@ -13,6 +13,7 @@ import org.broadinstitute.dsde.workbench.sam.audit.SamAuditModelJsonSupport._
 import org.broadinstitute.dsde.workbench.sam.audit._
 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,
@@ -472,8 +473,10 @@ class ResourceService(
               case None => IO.raiseError(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, s"policy $policyId does not exist")))
               case Some(existingPolicy) =>
                 Applicative[IO].whenA(existingPolicy.members != newMembers) {
+                  val membersToRemove = existingPolicy.members -- newMembers
                   for {
                     _ <- accessPolicyDAO.overwritePolicyMembers(policyId, newMembers, samRequestContext)
+                    _ <- membersToRemove.toList.map(member => removeSubjectFromPolicyGoogleGroupsIfChanged(policyId, member, samRequestContext)).sequence_
                     _ <- onPolicyUpdate(policyId, originalPolicies, samRequestContext)
                   } yield ()
                 }
@@ -526,7 +529,9 @@ class ResourceService(
             // short cut if access policy is unchanged
             IO.pure(newAccessPolicy)
           } else {
+            val membersToRemove = existingAccessPolicy.members -- newAccessPolicy.members
             for {
+              _ <- membersToRemove.toList.map(member => removeSubjectFromPolicyGoogleGroupsIfChanged(policyIdentity, member, samRequestContext)).sequence_
               result <- accessPolicyDAO.overwritePolicy(newAccessPolicy, samRequestContext)
               _ <- onPolicyUpdate(policyIdentity, originalPolicies, samRequestContext)
             } yield result
@@ -713,9 +718,20 @@ class ResourceService(
       originalPolicies <- accessPolicyDAO.listAccessPolicies(policyIdentity.resource, samRequestContext)
       _ <- failWhenPolicyNotExists(originalPolicies, policyIdentity)
       policyChanged <- directoryDAO.removeGroupMember(policyIdentity, subject, samRequestContext)
+      _ <- Applicative[IO].whenA(policyChanged)(removeSubjectFromPolicyGoogleGroupsIfChanged(policyIdentity, subject, samRequestContext))
       _ <- onPolicyUpdateIfChanged(policyIdentity, originalPolicies, samRequestContext)(policyChanged)
     } yield policyChanged
 
+  private def removeSubjectFromPolicyGoogleGroupsIfChanged(
+      policyId: FullyQualifiedPolicyId,
+      subject: WorkbenchSubject,
+      samRequestContext: SamRequestContext
+  ): IO[Unit] =
+    cloudExtensions match {
+      case extensions: GoogleExtensions => extensions.synchronouslyRemoveMemberFromGoogleGroup(policyId, subject, samRequestContext)
+      case _ => IO.unit
+    }
+
   def failWhenPolicyNotExists(policies: Iterable[AccessPolicy], policyId: FullyQualifiedPolicyId): IO[Unit] =
     IO.raiseUnless(policies.exists(_.id == policyId)) {
       new WorkbenchExceptionWithErrorReport(