Skip to content

Commit

Permalink
[CORE-58] Cascade policy deletion (#1564)
Browse files Browse the repository at this point in the history
* fix affected policy groups and add test
  • Loading branch information
calypsomatic authored Oct 31, 2024
1 parent baae571 commit 58be30b
Show file tree
Hide file tree
Showing 6 changed files with 207 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ trait AccessPolicyDAO {
samRequestContext: SamRequestContext
): IO[Seq[FilterResourcesResult]]

def checkPolicyGroupsInUse(resourceId: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[List[Map[String, String]]]
def findPolicyGroupsInUse(
resourceId: FullyQualifiedResourceId,
samRequestContext: SamRequestContext
): IO[List[(FullyQualifiedPolicyId, FullyQualifiedPolicyId)]]

}

sealed abstract class LoadResourceAuthDomainResult
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,50 @@ class PostgresAccessPolicyDAO(
}
}

// Return value: [(policyToUpdate, policyToRemove)]
override def findPolicyGroupsInUse(
resourceId: FullyQualifiedResourceId,
samRequestContext: SamRequestContext
): IO[List[(FullyQualifiedPolicyId, FullyQualifiedPolicyId)]] =
readOnlyTransaction("findAffectedPolicyGroups", samRequestContext) { implicit session =>
val groupMemberTable = GroupMemberTable.syntax("group_member_table")
val policyTable = PolicyTable.syntax("policy_table")
val parentPolicyTable = PolicyTable.syntax("parent_policy_table")
val resourceType = ResourceTypeTable.syntax("resource_type")
val resourceTable = ResourceTable.syntax("resource_table")

val query = samsql"""
WITH resourcePolicies as (
SELECT ${policyTable.groupId} as childGroupId, ${policyTable.name} as policyName
FROM ${PolicyTable as policyTable}
WHERE ${policyTable.resourceId} = (${loadResourcePKSubQuery(resourceId)})
)
SELECT resourcePolicies.policyName as childPolicyName, ${parentPolicyTable.name} as parentPolicyName, ${resourceTable.name} as parentPolicyResourceName, ${resourceType.name} as parentPolicyResourceType
from ${GroupMemberTable as groupMemberTable}
JOIN resourcePolicies ON ${groupMemberTable.memberGroupId} = resourcePolicies.childGroupId
JOIN ${PolicyTable as parentPolicyTable} ON ${parentPolicyTable.groupId} = ${groupMemberTable.groupId}
JOIN ${ResourceTable as resourceTable} ON ${resourceTable.id} = ${parentPolicyTable.resourceId}
JOIN ${ResourceTypeTable as resourceType} ON ${resourceType.id} = ${resourceTable.resourceTypeId}
"""
query
.map { rs =>
val parentPolicyResourceType = rs.get[ResourceTypeName]("parentPolicyResourceType")
val parentPolicyResourceId = rs.get[ResourceId]("parentPolicyResourceName")
val parentPolciyAccessName = rs.get[AccessPolicyName]("parentPolicyName")
val memberPolicyAccessName = rs.get[AccessPolicyName]("childPolicyName")

val parentPolicyFullResourceId =
FullyQualifiedResourceId(parentPolicyResourceType, parentPolicyResourceId)
val parentPolicyFullId = FullyQualifiedPolicyId(parentPolicyFullResourceId, parentPolciyAccessName)
val memberPolicyFullId = FullyQualifiedPolicyId(resourceId, memberPolicyAccessName)

(parentPolicyFullId, memberPolicyFullId)
}
.list()
.apply()

}

private def deleteAllResourcePolicies(resourceId: FullyQualifiedResourceId, samRequestContext: SamRequestContext)(implicit
session: DBSession
): Unit = {
Expand All @@ -975,37 +1019,6 @@ class PostgresAccessPolicyDAO(
}
}

override def checkPolicyGroupsInUse(resourceId: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[List[Map[String, String]]] = {
val g = GroupTable.syntax("g")
val pg = GroupTable.syntax("pg") // problematic group
val gm = GroupMemberTable.syntax("gm")
val p = PolicyTable.syntax("p")

readOnlyTransaction("checkPolicyGroupsInUse", samRequestContext) { implicit session =>
val problematicGroupsQuery =
samsql"""select ${g.result.id}, ${g.result.name}, array_agg(${pg.name}) as ${pg.resultName.name}
from ${GroupTable as g}
join ${GroupMemberTable as gm} on ${g.id} = ${gm.memberGroupId}
join ${GroupTable as pg} on ${gm.groupId} = ${pg.id}
where ${g.id} in
(select distinct ${gm.result.memberGroupId}
from ${GroupMemberTable as gm}
join ${PolicyTable as p} on ${gm.memberGroupId} = ${p.groupId}
where ${p.resourceId} = (${loadResourcePKSubQuery(resourceId)}))
group by ${g.id}, ${g.name}"""
problematicGroupsQuery
.map(rs =>
Map(
"groupId" -> rs.get[GroupPK](g.resultName.id).value.toString,
"groupName" -> rs.get[String](g.resultName.name),
"still used in group(s):" -> rs.get[String](pg.resultName.name)
)
)
.list()
.apply()
}
}

override def loadPolicy(resourceAndPolicyName: FullyQualifiedPolicyId, samRequestContext: SamRequestContext): IO[Option[AccessPolicy]] =
listPolicies(resourceAndPolicyName.resource, limitOnePolicy = Option(resourceAndPolicyName.accessPolicyName), samRequestContext).map(_.headOption)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -432,30 +432,20 @@ class ResourceService(
def deleteResource(resource: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[Unit] =
for {
_ <- checkNoChildren(resource, samRequestContext)
_ <- checkNoPoliciesInUse(resource, samRequestContext)

// remove from cloud first so a failure there does not leave sam in a bad state
_ <- cloudDeletePolicies(resource, samRequestContext)
_ <- deleteActionManagedIdentitiesForResource(resource, samRequestContext)

// leave a tomb stone if the resource type does not allow reuse
leaveTombStone = !resourceTypes(resource.resourceTypeName).reuseIds
affectedPolicies <- accessPolicyDAO.findPolicyGroupsInUse(resource, samRequestContext) // New method
_ <- affectedPolicies.traverse { case (policyToUpdate, policyToRemove) => removeSubjectFromPolicy(policyToUpdate, policyToRemove, samRequestContext) }
_ <- accessPolicyDAO.deleteResource(resource, leaveTombStone, samRequestContext)

_ <- AuditLogger.logAuditEventIO(samRequestContext, ResourceEvent(ResourceDeleted, resource))
} yield ()

private def checkNoPoliciesInUse(resource: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[Unit] =
accessPolicyDAO.checkPolicyGroupsInUse(resource, samRequestContext).flatMap { problematicGroups =>
if (problematicGroups.nonEmpty)
IO.raiseError(
new WorkbenchExceptionWithErrorReport( // throws a 500 since that's the current behavior
ErrorReport(StatusCodes.InternalServerError, s"Foreign Key Violation(s) while deleting group(s): ${problematicGroups}")
)
)
else IO.unit
}

private def deleteActionManagedIdentitiesForResource(resource: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[Unit] =
azureService
.map { service =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,10 @@ class MockAccessPolicyDAO(private val resourceTypes: mutable.Map[ResourceTypeNam
policies -= policy
}

override def checkPolicyGroupsInUse(resourceId: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[List[Map[String, String]]] =
IO.pure(List.empty)
override def findPolicyGroupsInUse(
resourceId: FullyQualifiedResourceId,
samRequestContext: SamRequestContext
): IO[List[(FullyQualifiedPolicyId, FullyQualifiedPolicyId)]] = IO.pure(List.empty)

override def listAccessPolicies(
resourceTypeName: ResourceTypeName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3553,6 +3553,99 @@ class PostgresAccessPolicyDAOSpec extends AnyFreeSpec with Matchers with BeforeA
dao.listResourcesUsingAuthDomain(authDomainGroupName, samRequestContext).unsafeRunSync() shouldEqual Set.empty
}
}

"findAffectedPolicyGroups" - {
"returns parent and member policy groups" in {
assume(databaseEnabled, databaseEnabledClue)

// Create a resource with a policy
dao.createResourceType(resourceType, samRequestContext).unsafeRunSync()

val user = Generator.genWorkbenchUserBoth.sample.get
dirDao.createUser(user, samRequestContext).unsafeRunSync()

val parentResourceFullyQualifiedId = FullyQualifiedResourceId(resourceType.name, ResourceId("parent_resource"))
val childResourceFullyQualifiedId = FullyQualifiedResourceId(resourceType.name, ResourceId("child_resource"))

val parentPolicy = AccessPolicy(
FullyQualifiedPolicyId(parentResourceFullyQualifiedId, AccessPolicyName("parentPolicyName")),
Set(user.id),
WorkbenchEmail("[email protected]"),
resourceType.roles.map(_.roleName),
Set(readAction, writeAction),
Set.empty,
false
)
val childPolicy = AccessPolicy(
FullyQualifiedPolicyId(childResourceFullyQualifiedId, AccessPolicyName("childPolicyName")),
Set(user.id),
WorkbenchEmail("[email protected]"),
resourceType.roles.map(_.roleName),
Set(readAction, writeAction),
Set.empty,
false
)

val otherParentResourceFullyQualifiedId = FullyQualifiedResourceId(resourceType.name, ResourceId("other_parent_resource"))
val otherParentPolicy = AccessPolicy(
FullyQualifiedPolicyId(otherParentResourceFullyQualifiedId, AccessPolicyName("otherParentPolicyName")),
Set(user.id),
WorkbenchEmail("[email protected]"),
resourceType.roles.map(_.roleName),
Set(readAction, writeAction),
Set.empty,
false
)

val otherChildPolicy = AccessPolicy(
FullyQualifiedPolicyId(childResourceFullyQualifiedId, AccessPolicyName("otherChildPolicyName")),
Set(user.id),
WorkbenchEmail("[email protected]"),
resourceType.roles.map(_.roleName),
Set(readAction, writeAction),
Set.empty,
false
)

val thirdPolicy = AccessPolicy(
FullyQualifiedPolicyId(childResourceFullyQualifiedId, AccessPolicyName("thirdPolicyName")),
Set(user.id),
WorkbenchEmail("[email protected]"),
resourceType.roles.map(_.roleName),
Set(readAction, writeAction),
Set.empty,
false
)

val parentResource =
Resource(parentResourceFullyQualifiedId.resourceTypeName, parentResourceFullyQualifiedId.resourceId, Set.empty, Set(parentPolicy))
val childResource =
Resource(
childResourceFullyQualifiedId.resourceTypeName,
childResourceFullyQualifiedId.resourceId,
Set.empty,
Set(childPolicy, otherChildPolicy, thirdPolicy)
)
val otherResource = Resource(
otherParentResourceFullyQualifiedId.resourceTypeName,
otherParentResourceFullyQualifiedId.resourceId,
Set.empty,
Set(otherParentPolicy)
)
dao.createResource(parentResource, samRequestContext).unsafeRunSync()
dao.createResource(childResource, samRequestContext).unsafeRunSync()
dao.createResource(otherResource, samRequestContext).unsafeRunSync()

dirDao.addGroupMember(otherParentPolicy.id, otherChildPolicy.id, samRequestContext).unsafeRunSync()

// Add child policy to parent policy
dirDao.addGroupMember(parentPolicy.id, childPolicy.id, samRequestContext).unsafeRunSync()

val policyGroups = dao.findPolicyGroupsInUse(childResourceFullyQualifiedId, samRequestContext).unsafeRunSync()

policyGroups should contain theSameElementsAs List((parentPolicy.id, childPolicy.id), (otherParentPolicy.id, otherChildPolicy.id))
}
}
}

private def uuid: String =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2278,7 +2278,7 @@ class ResourceServiceSpec
testDeleteResource(managedGroupResourceType)
}

it should "delete any action managed identites for the resource while it deletes the resource" in {
it should "delete any action managed identities for the resource while it deletes the resource" in {
assume(databaseEnabled, databaseEnabledClue)

val resource = FullyQualifiedResourceId(defaultResourceType.name, ResourceId("my-resource"))
Expand Down Expand Up @@ -2378,6 +2378,64 @@ class ResourceServiceSpec
assert(policyDAO.listAccessPolicies(parentResource, samRequestContext).unsafeRunSync().nonEmpty)
}

it should "delete any policy members from groups before deleting resource" in {
assume(databaseEnabled, databaseEnabledClue)

// Create a resource with a policy
val ownerRoleName = ResourceRoleName("owner")
val resourceType = ResourceType(
defaultResourceType.name,
Set(SamResourceActionPatterns.delete, ResourceActionPattern("view", "", false)),
Set(ResourceRole(ownerRoleName, Set(ResourceAction("delete"), ResourceAction("view")))),
ownerRoleName
)
val resourceName = ResourceId("resource")

runAndWait(service.createResourceType(resourceType, samRequestContext))

// creating policy and to refer to by policy identifiers
val policyMembership = AccessPolicyMembershipRequest(Set(dummyUser.email), Set(ResourceAction("view")), Set(ownerRoleName), Option(Set.empty))
val policyName = AccessPolicyName("foo")
val resource =
runAndWait(service.createResource(resourceType, resourceName, Map(policyName -> policyMembership), Set.empty, None, dummyUser.id, samRequestContext))
val policies: Seq[AccessPolicyResponseEntry] =
service.listResourcePolicies(FullyQualifiedResourceId(resourceType.name, resourceName), samRequestContext).unsafeRunSync()

// creating policy to refer to by policy email
val resourceName2 = ResourceId("resource2")
val policyMembership2 = AccessPolicyMembershipRequest(Set(dummyUser.email), Set(ResourceAction("view")), Set(ownerRoleName), None, None)
val policyName2 = AccessPolicyName("foo2")
val resource2 =
runAndWait(
service.createResource(resourceType, resourceName2, Map(policyName2 -> policyMembership2), Set.empty, None, dummyUser.id, samRequestContext)
)
val policies2: Seq[AccessPolicyResponseEntry] =
service.listResourcePolicies(FullyQualifiedResourceId(resourceType.name, resourceName2), samRequestContext).unsafeRunSync()

// Add second resource to first policy
policies2.foreach { policy =>
runAndWait(
service.addSubjectToPolicy(
FullyQualifiedPolicyId(FullyQualifiedResourceId(resourceType.name, resourceName), policyName),
FullyQualifiedPolicyId(FullyQualifiedResourceId(resourceType.name, resourceName2), policyName2),
samRequestContext
)
)
}
val policy2Emails = policies2.map(p => p.email).toSet
// Verify that policies are in group
val updatedPolicies: Seq[AccessPolicyResponseEntry] =
service.listResourcePolicies(FullyQualifiedResourceId(resourceType.name, resourceName), samRequestContext).unsafeRunSync()
policy2Emails.subsetOf(updatedPolicies.head.policy.memberEmails) shouldBe true
// Delete resource; should not throw an error
runAndWait(service.deleteResource(FullyQualifiedResourceId(resourceType.name, resourceName2), samRequestContext))

// Verify that policies are no longer in group
val updatedPolicies2: Seq[AccessPolicyResponseEntry] =
service.listResourcePolicies(FullyQualifiedResourceId(resourceType.name, resourceName), samRequestContext).unsafeRunSync()
policy2Emails.subsetOf(updatedPolicies2.head.policy.memberEmails) shouldBe false
}

"validatePolicy" should "succeed with a correct policy" in {
val emailToMaybeSubject = Map(dummyUser.email -> Option(dummyUser.id.asInstanceOf[WorkbenchSubject]))
val policy = service.ValidatableAccessPolicy(AccessPolicyName("a"), emailToMaybeSubject, Set(ownerRoleName), Set(ResourceAction("alter_policies")), Set())
Expand Down

0 comments on commit 58be30b

Please sign in to comment.