Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CORE-242 bulk membership update #1613

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions src/main/resources/swagger/api-docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))))
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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*
Expand All @@ -1038,7 +1039,6 @@ from ${GroupMemberTable as groupMemberTable}
}
removeAllGroupMembers(groupId)
insertGroupMembers(groupId, memberList)
updateGroupUpdatedDateAndVersion(id)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an internal function, calling updateGroupUpdatedDateAndVersion is redundant

}

override def overwritePolicy(newPolicy: AccessPolicy, samRequestContext: SamRequestContext): IO[AccessPolicy] =
Expand All @@ -1056,7 +1056,7 @@ from ${GroupMemberTable as groupMemberTable}
newPolicy
)
setPolicyIsPublicInternal(policyPK, newPolicy.public)

updateGroupUpdatedDateAndVersion(newPolicy.id)
newPolicy
}

Expand Down Expand Up @@ -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]] = {
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
Loading
Loading