bulk membership update implemented but not tested

src/main/resources/swagger/api-docs.yaml

@@ -1975,6 +1975,49 @@ paths:
                 oneOf:
                   - $ref: '#/components/schemas/FilteredResourcesFlatResponse'
                   - $ref: '#/components/schemas/FilteredResourcesHierarchicalResponse'
+  /api/resources/v2/bulkMembershipUpdate:
+    post:
+      tags:
+        - Resources
+      summary: Bulk update the membership for resources
+      description: |
+        This endpoint is used to add and remove members of multiple policies across multiple resource in one api call.
+        Each policy is updated in order in its own transaction. If a policy does not exist on a resource, 
+        processing stops with an error but all updates up to that point are committed. Missing member emails or policies fail fast. Additions are processed
+        before removals so if a subject is in both the add and remove lists for a policy, the ultimate result is removal.
+        This call is idempotent, if members being added already exist or members being removed do not exist, nothing will happen.
+      operationId: bulkMembershipUpdateV2
+      requestBody:
+        description: The details of the membership updates
+        content:
+          'application/json':
+            schema:
+              type: array
+              items:
+                $ref: '#/components/schemas/BulkMembershipUpdateRequestV2'
+        required: true
+      responses:
+        200:
+          description: Successfully updated membership
+          content: { }
+        400:
+          description: members or policies are not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorReport'
+        403:
+          description: Caller does not have permission to update the membership of one or more of the policies or one or more resources do not exist
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorReport'
+        500:
+          description: Internal Server Error
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/ErrorReport'
   /api/resources/v2/{resourceTypeName}:
     get:
       tags:
@@ -5062,6 +5105,51 @@ components:
           description: Actions on the role
           items:
             type: string
+    BulkMembershipUpdateRequestV2:
+      type: object
+      required:
+        - resourceTypeName
+        - resourceId
+        - policyUpdates
+      properties:
+        resourceTypeName:
+          type: string
+          description: The type of the resource
+        resourceId:
+          type: string
+          description: The id of the resource
+        policyUpdates:
+          type: array
+          items:
+            $ref: '#/components/schemas/PolicyMembershipUpdate'
+    PolicyMembershipUpdate:
+      type: object
+      required:
+          - policyName
+      properties:
+        policyName:
+          type: string
+          description: The name of the policy
+        addEmails:
+          type: array
+          description: Emails to add to the policy
+          items:
+            type: string
+        addPolicies:
+          type: array
+          description: Other policies to add to the specified policy
+          items:
+            $ref: '#/components/schemas/PolicyIdentifiers'
+        removeEmails:
+          type: array
+          description: Emails to remove from the policy
+          items:
+            type: string
+        removePolicies:
+          type: array
+          description: Other policies to remove from the specified policy
+          items:
+              $ref: '#/components/schemas/PolicyIdentifiers'
   securitySchemes:
     oidc:
       type: oauth2

src/main/scala/org/broadinstitute/dsde/workbench/sam/api/ResourceRoutes.scala

@@ -14,7 +14,7 @@ import org.broadinstitute.dsde.workbench.sam.config.LiquibaseConfig
 import org.broadinstitute.dsde.workbench.sam.model.RootPrimitiveJsonSupport._
 import org.broadinstitute.dsde.workbench.sam.model.api.SamJsonSupport._
 import org.broadinstitute.dsde.workbench.sam.model._
-import org.broadinstitute.dsde.workbench.sam.model.api.{AccessPolicyMembershipRequest, SamUser}
+import org.broadinstitute.dsde.workbench.sam.model.api.{AccessPolicyMembershipRequest, BulkMembershipUpdate, SamUser}
 import org.broadinstitute.dsde.workbench.sam.model.api.FilteredResourcesHierarchical._
 import org.broadinstitute.dsde.workbench.sam.model.api.FilteredResourcesFlat._
 import org.broadinstitute.dsde.workbench.sam.service.ResourceService
@@ -131,6 +131,20 @@ trait ResourceRoutes extends SamUserDirectives with SecurityDirectives with SamM
             listUserResources(samUser, samRequestContext)
           }
         } ~
+        path("bulkMembershipUpdate") {
+          postWithTelemetry(samRequestContext) {
+            import BulkMembershipUpdate.bulkMembershipUpdateFormat
+            entity(as[Seq[BulkMembershipUpdate]]) { membershipUpdates =>
+              verifyBulkMembershipUpdateAccess(membershipUpdates, samUser.id, samRequestContext) {
+                complete(
+                  resourceService
+                    .bulkMembershipUpdate(membershipUpdates, samRequestContext)
+                    .map(_ => StatusCodes.OK)
+                )
+              }
+            }
+          }
+        } ~
         pathPrefix(Segment) { resourceTypeName =>
           withNonAdminResourceType(ResourceTypeName(resourceTypeName)) { resourceType =>
             pathEndOrSingleSlash {

src/main/scala/org/broadinstitute/dsde/workbench/sam/api/SecurityDirectives.scala

@@ -2,12 +2,14 @@ package org.broadinstitute.dsde.workbench.sam.api
 
 import akka.http.scaladsl.model.StatusCodes
 import akka.http.scaladsl.server
-import akka.http.scaladsl.server.Directives.onSuccess
-import akka.http.scaladsl.server.{Directive0, Directives}
+import akka.http.scaladsl.server.Directives.{complete, handleExceptions, onSuccess}
+import akka.http.scaladsl.server.{Directive0, Directives, ExceptionHandler}
 import cats.effect.IO
 import org.broadinstitute.dsde.workbench.model.{ErrorReport, WorkbenchExceptionWithErrorReport, WorkbenchUserId}
 import org.broadinstitute.dsde.workbench.sam.ImplicitConversions.ioOnSuccessMagnet
 import org.broadinstitute.dsde.workbench.sam._
+import org.broadinstitute.dsde.workbench.sam.model.SamResourceTypes.resourceTypeAdminName
+import org.broadinstitute.dsde.workbench.sam.model.api.BulkMembershipUpdate
 import org.broadinstitute.dsde.workbench.sam.model.{FullyQualifiedResourceId, ResourceAction, ResourceType, SamResourceActions, SamResourceTypes}
 import org.broadinstitute.dsde.workbench.sam.service.{PolicyEvaluatorService, ResourceService}
 import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext
@@ -211,4 +213,46 @@ trait SecurityDirectives {
         IO.pure(true)
     }
   }
+
+  def verifyBulkMembershipUpdateAccess(
+      membershipUpdates: Seq[BulkMembershipUpdate],
+      userId: WorkbenchUserId,
+      samRequestContext: SamRequestContext
+  ): Directive0 = {
+    val requireDirectives = membershipUpdates.flatMap { membershipUpdate =>
+      if (membershipUpdate.resourceTypeName == resourceTypeAdminName) {
+        Seq(
+          Directives.mapInnerRoute { innerRoute =>
+            Directives.failWith(
+              new WorkbenchExceptionWithErrorReport(
+                ErrorReport(StatusCodes.BadRequest, "Please use the admin resourceTypes routes to view and make changes to admin resource policies.")
+              )
+            )
+          }
+        )
+      } else {
+        val resource = FullyQualifiedResourceId(membershipUpdate.resourceTypeName, membershipUpdate.resourceId)
+        membershipUpdate.policyUpdates.map { policyUpdate =>
+          requireOneOfAction(
+            resource,
+            Set(SamResourceActions.sharePolicy(policyUpdate.policyName), SamResourceActions.alterPolicies),
+            userId,
+            samRequestContext
+          )
+        }
+      }
+    }
+    // requireOneOfAction may return Not Found but we want to return Forbidden instead in this case
+    changeNotFoundToForbidden & requireDirectives.foldLeft(Directives.pass)(_ & _)
+  }
+
+  private val changeNotFoundToForbidden: Directive0 = {
+    import org.broadinstitute.dsde.workbench.model.ErrorReportJsonSupport._
+    import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._
+
+    handleExceptions(ExceptionHandler {
+      case withErrorReport: WorkbenchExceptionWithErrorReport if withErrorReport.errorReport.statusCode.contains(StatusCodes.NotFound) =>
+        complete((StatusCodes.Forbidden, withErrorReport.errorReport.copy(statusCode = Option(StatusCodes.Forbidden))))
+    })
+  }
 }

src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/AccessPolicyDAO.scala

@@ -135,6 +135,13 @@ trait AccessPolicyDAO {
       samRequestContext: SamRequestContext
   ): IO[List[(FullyQualifiedPolicyId, FullyQualifiedPolicyId)]]
 
+  def addAndRemovePolicyMembers(
+      policyId: FullyQualifiedPolicyId,
+      addSubjects: Set[WorkbenchSubject],
+      removeSubjects: Set[WorkbenchSubject],
+      samRequestContext: SamRequestContext
+  ): IO[Int]
+
 }
 
 sealed abstract class LoadResourceAuthDomainResult

src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala

@@ -1028,6 +1028,7 @@ from ${GroupMemberTable as groupMemberTable}
   override def overwritePolicyMembers(id: FullyQualifiedPolicyId, memberList: Set[WorkbenchSubject], samRequestContext: SamRequestContext): IO[Unit] =
     serializableWriteTransaction("overwritePolicyMembers", samRequestContext) { implicit session =>
       overwritePolicyMembersInternal(id, memberList)
+      updateGroupUpdatedDateAndVersion(id)
     }
 
   // Steps: Delete every member from the underlying group and then add all of the new members. Do this in a *single*
@@ -1038,7 +1039,6 @@ from ${GroupMemberTable as groupMemberTable}
     }
     removeAllGroupMembers(groupId)
     insertGroupMembers(groupId, memberList)
-    updateGroupUpdatedDateAndVersion(id)
   }
 
   override def overwritePolicy(newPolicy: AccessPolicy, samRequestContext: SamRequestContext): IO[AccessPolicy] =
@@ -1056,7 +1056,7 @@ from ${GroupMemberTable as groupMemberTable}
         newPolicy
       )
       setPolicyIsPublicInternal(policyPK, newPolicy.public)
-
+      updateGroupUpdatedDateAndVersion(newPolicy.id)
       newPolicy
     }
 
@@ -1661,7 +1661,11 @@ from ${GroupMemberTable as groupMemberTable}
   override def setPolicyIsPublic(policyId: FullyQualifiedPolicyId, isPublic: Boolean, samRequestContext: SamRequestContext): IO[Boolean] =
     serializableWriteTransaction("setPolicyIsPublic", samRequestContext) { implicit session =>
       val policyPK = loadPolicyPK(policyId)
-      setPolicyIsPublicInternal(policyPK, isPublic) > 0
+      val changed = setPolicyIsPublicInternal(policyPK, isPublic) > 0
+      if (changed) {
+        updateGroupUpdatedDateAndVersion(policyId)
+      }
+      changed
     }
 
   override def getResourceParent(resource: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[Option[FullyQualifiedResourceId]] = {
@@ -1975,6 +1979,28 @@ from ${GroupMemberTable as groupMemberTable}
       .filter(r => roles.isEmpty || r.role.exists(role => roles.contains(role)))
       .filter(r => actions.isEmpty || r.action.exists(action => actions.contains(action))) ++ privateResources
 
+  override def addAndRemovePolicyMembers(
+      policyId: FullyQualifiedPolicyId,
+      addSubjects: Set[WorkbenchSubject],
+      removeSubjects: Set[WorkbenchSubject],
+      samRequestContext: SamRequestContext
+  ): IO[Int] =
+    serializableWriteTransaction("addAndRemovePolicyMembers", samRequestContext) { implicit session =>
+      val groupId = samsql"${workbenchGroupIdentityToGroupPK(policyId)}".map(rs => rs.get[GroupPK](1)).single().apply().getOrElse {
+        throw new WorkbenchException(s"Group for policy [$policyId] not found")
+      }
+      val insertedCount = insertGroupMembers(groupId, addSubjects)
+      // there is no bulk removeGroupMembers because figuring out which records to remove from the flat structure
+      // can't be done with an in clause because it needs to use array functions
+      val removedCount = removeSubjects
+        .map { subject =>
+          removeGroupMember(policyId, subject)
+        }
+        .count(identity) // counts all that were true
+      updateGroupUpdatedDateAndVersion(policyId)
+      insertedCount + removedCount
+    }
+
   private def recreateEffectivePolicyRolesTableEntry(resourceTypeNames: Set[ResourceTypeName])(implicit session: DBSession): Int = {
     val resource = ResourceTable.syntax("resource")
     val policyResource = ResourceTable.syntax("policyResource")

src/main/scala/org/broadinstitute/dsde/workbench/sam/model/api/BulkMembershipUpdate.scala

@@ -0,0 +1,27 @@
+package org.broadinstitute.dsde.workbench.sam.model.api
+
+import org.broadinstitute.dsde.workbench.model.WorkbenchEmail
+import org.broadinstitute.dsde.workbench.sam.model.{AccessPolicyName, PolicyIdentifiers, ResourceId, ResourceTypeName}
+import spray.json.RootJsonFormat
+
+case class PolicyMembershipUpdate(
+    policyName: AccessPolicyName,
+    addEmails: Set[WorkbenchEmail],
+    addPolicies: Set[PolicyIdentifiers],
+    removeEmails: Set[WorkbenchEmail],
+    removePolicies: Set[PolicyIdentifiers]
+)
+object PolicyMembershipUpdate {
+  import spray.json.DefaultJsonProtocol._
+  import SamJsonSupport._
+  import org.broadinstitute.dsde.workbench.model.WorkbenchIdentityJsonSupport._
+  implicit val policyMembershipUpdateFormat: RootJsonFormat[PolicyMembershipUpdate] = jsonFormat5(PolicyMembershipUpdate.apply)
+}
+
+case class BulkMembershipUpdate(resourceTypeName: ResourceTypeName, resourceId: ResourceId, policyUpdates: Seq[PolicyMembershipUpdate])
+object BulkMembershipUpdate {
+  import spray.json.DefaultJsonProtocol._
+  import SamJsonSupport._
+  import PolicyMembershipUpdate.policyMembershipUpdateFormat
+  implicit val bulkMembershipUpdateFormat: RootJsonFormat[BulkMembershipUpdate] = jsonFormat3(BulkMembershipUpdate.apply)
+}