Skip to content

Commit

Permalink
resolve merge conflicts and adjust AccessPolicyDAO test to avoid dead…
Browse files Browse the repository at this point in the history
…lock
  • Loading branch information
marctalbott committed Oct 4, 2018
1 parent 5284957 commit 47843f2
Show file tree
Hide file tree
Showing 13 changed files with 559 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ object Boot extends App with LazyLogging {
val googleKeyCache = new GoogleKeyCache(googleIamDAO, googleStorageDAO, googlePubSubDAO, googleServicesConfig, petServiceAccountConfig)
val notificationDAO = new PubSubNotificationDAO(googlePubSubDAO, googleServicesConfig.notificationTopic)

new GoogleExtensions(directoryDAO, accessPolicyDAO, googleDirectoryDAO, googlePubSubDAO, googleIamDAO, googleStorageDAO, googleProjectDAO, googleKeyCache, notificationDAO, googleServicesConfig, petServiceAccountConfig, resourceTypeMap(CloudExtensions.resourceTypeName))
new GoogleExtensions(directoryDAO, accessPolicyDAO, googleDirectoryDAO, googlePubSubDAO, googleIamDAO, googleStorageDAO, googleProjectDAO, googleKeyCache, notificationDAO, googleServicesConfig, petServiceAccountConfig, resourceTypeMap)
case None => NoExtensions
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ trait DirectoryDAO {
def readProxyGroup(userId: WorkbenchUserId): Future[Option[WorkbenchEmail]]

def listUsersGroups(userId: WorkbenchUserId): Future[Set[WorkbenchGroupIdentity]]
def listFlattenedGroupUsers(groupId: WorkbenchGroupIdentity): Future[Set[WorkbenchUserId]]
def listIntersectionGroupUsers(groupId: Set[WorkbenchGroupIdentity]): Future[Set[WorkbenchUserId]]
def listAncestorGroups(groupId: WorkbenchGroupIdentity): Future[Set[WorkbenchGroupIdentity]]

def enableIdentity(subject: WorkbenchSubject): Future[Unit]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,16 @@ class LdapDirectoryDAO(protected val ldapConnectionPool: LDAPConnectionPool, pro
groupsOption.getOrElse(Set.empty)
}

override def listFlattenedGroupUsers(groupId: WorkbenchGroupIdentity): Future[Set[WorkbenchUserId]] = Future {
ldapSearchStream(peopleOu, SearchScope.SUB, Filter.createEqualityFilter(Attr.memberOf, groupDn(groupId)))(unmarshalUser).map(_.id).toSet
override def listIntersectionGroupUsers(groupIds: Set[WorkbenchGroupIdentity]): Future[Set[WorkbenchUserId]] = Future {
ldapSearchStream(directoryConfig.baseDn, SearchScope.SUB, Filter.createANDFilter(groupIds.map(groupId =>
Filter.createEqualityFilter(Attr.memberOf, groupDn(groupId))).asJava))(unmarshalUserOption).collect { case Some(user) => user.id }.toSet
}

private def unmarshalUserOption(results: Entry): Option[WorkbenchUser] = {
for {
uid <- getAttribute(results, Attr.uid)
email <- getAttribute(results, Attr.email)
} yield WorkbenchUser(WorkbenchUserId(uid), getAttribute(results, Attr.googleSubjectId).map(GoogleSubjectId), WorkbenchEmail(email))
}

override def listAncestorGroups(groupId: WorkbenchGroupIdentity): Future[Set[WorkbenchGroupIdentity]] = Future {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import java.util.Date

import akka.actor.ActorSystem
import akka.http.scaladsl.model.StatusCodes
//import cats.effect.IO
//import cats.implicits._
import akka.http.scaladsl.model.headers.OAuth2BearerToken
import com.google.api.client.googleapis.json.GoogleJsonResponseException
import com.google.auth.oauth2.ServiceAccountCredentials
Expand Down Expand Up @@ -38,7 +40,7 @@ object GoogleExtensions {
val getPetPrivateKeyAction = ResourceAction("get_pet_private_key")
}

class GoogleExtensions(val directoryDAO: DirectoryDAO, val accessPolicyDAO: AccessPolicyDAO, val googleDirectoryDAO: GoogleDirectoryDAO, val googlePubSubDAO: GooglePubSubDAO, val googleIamDAO: GoogleIamDAO, val googleStorageDAO: GoogleStorageDAO, val googleProjectDAO: GoogleProjectDAO, val googleKeyCache: GoogleKeyCache, val notificationDAO: NotificationDAO, val googleServicesConfig: GoogleServicesConfig, val petServiceAccountConfig: PetServiceAccountConfig, extensionResourceType: ResourceType)(implicit val system: ActorSystem, executionContext: ExecutionContext) extends LazyLogging with FutureSupport with CloudExtensions with Retry {
class GoogleExtensions(val directoryDAO: DirectoryDAO, val accessPolicyDAO: AccessPolicyDAO, val googleDirectoryDAO: GoogleDirectoryDAO, val googlePubSubDAO: GooglePubSubDAO, val googleIamDAO: GoogleIamDAO, val googleStorageDAO: GoogleStorageDAO, val googleProjectDAO: GoogleProjectDAO, val googleKeyCache: GoogleKeyCache, val notificationDAO: NotificationDAO, val googleServicesConfig: GoogleServicesConfig, val petServiceAccountConfig: PetServiceAccountConfig, val resourceTypes: Map[ResourceTypeName, ResourceType])(implicit val system: ActorSystem, executionContext: ExecutionContext) extends LazyLogging with FutureSupport with CloudExtensions with Retry {

private val maxGroupEmailLength = 64

Expand Down Expand Up @@ -86,7 +88,7 @@ class GoogleExtensions(val directoryDAO: DirectoryDAO, val accessPolicyDAO: Acce
this
))


val extensionResourceType = resourceTypes.getOrElse(CloudExtensions.resourceTypeName, throw new Exception(s"${CloudExtensions.resourceTypeName} resource type not found"))
val ownerGoogleSubjectId = GoogleSubjectId(googleServicesConfig.serviceAccountClientId)
for {
user <- directoryDAO.loadSubjectFromGoogleSubjectId(ownerGoogleSubjectId)
Expand Down Expand Up @@ -127,6 +129,17 @@ class GoogleExtensions(val directoryDAO: DirectoryDAO, val accessPolicyDAO: Acce
case (rpn: ResourceAndPolicyName, Some(_)) => rpn.toJson.compactPrint
}
_ <- googlePubSubDAO.publishMessages(googleServicesConfig.groupSyncTopic, messagesForIdsWithSyncDates)
ancestorGroups <- Future.traverse(groupIdentities) { id => directoryDAO.listAncestorGroups(id) }
managedGroupIds = (ancestorGroups.flatten ++ groupIdentities).collect { case ResourceAndPolicyName(Resource(ResourceTypeName("managed-group"), id, _), _) => id }
_ <- Future.traverse(managedGroupIds)(onManagedGroupUpdate)
} yield ()
}

private def onManagedGroupUpdate(groupId: ResourceId): Future[Unit] = {
for {
resources <- accessPolicyDAO.listResourcesConstrainedByGroup(WorkbenchGroupName(groupId.value))
policies <- Future.traverse(resources) { resource => accessPolicyDAO.listAccessPolicies(resource).unsafeToFuture }
_ <- onGroupUpdate(policies.flatten.map(_.id).toList)
} yield ()
}

Expand Down Expand Up @@ -403,6 +416,7 @@ class GoogleExtensions(val directoryDAO: DirectoryDAO, val accessPolicyDAO: Acce
}
)
}

if(visitedGroups.contains(groupId)) {
Future.successful(Map.empty)
} else {
Expand All @@ -414,6 +428,15 @@ class GoogleExtensions(val directoryDAO: DirectoryDAO, val accessPolicyDAO: Acce

group = groupOption.getOrElse(throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, s"$groupId not found")))

members <- group match {
case accessPolicy: AccessPolicy => if (isConstrainable(accessPolicy.id.resource, accessPolicy)) {
calculateIntersectionGroup(accessPolicy.id.resource, accessPolicy)
} else {
Future.successful(accessPolicy.members)
}
case group: BasicWorkbenchGroup => Future.successful(group.members)
}

subGroupSyncs <- Future.traverse(group.members) {
case subGroup: WorkbenchGroupIdentity =>
directoryDAO.getSynchronizedDate(subGroup).flatMap{
Expand All @@ -427,7 +450,7 @@ class GoogleExtensions(val directoryDAO: DirectoryDAO, val accessPolicyDAO: Acce
case None => googleDirectoryDAO.createGroup(groupId.toString, group.email, Option(googleDirectoryDAO.lockedDownGroupSettings)) map (_ => Set.empty[String])
case Some(members) => Future.successful(members.map(_.toLowerCase).toSet)
}
samMemberEmails <- Future.traverse(group.members) {
samMemberEmails <- Future.traverse(members) {
case group: WorkbenchGroupIdentity => directoryDAO.loadSubjectEmail(group)

// use proxy group email instead of user's actual email
Expand All @@ -450,6 +473,32 @@ class GoogleExtensions(val directoryDAO: DirectoryDAO, val accessPolicyDAO: Acce
}
}

private def isConstrainable(resource: Resource, accessPolicy: AccessPolicy): Boolean = {
resourceTypes.get(resource.resourceTypeName) match {
case Some(resourceType) => resourceType.actionPatterns.exists { actionPattern =>
actionPattern.authDomainConstrainable &&
(accessPolicy.actions.exists(actionPattern.matches) ||
accessPolicy.roles.exists { accessPolicyRole =>
resourceType.roles.exists {
case resourceTypeRole@ResourceRole(`accessPolicyRole`, _) => resourceTypeRole.actions.exists(actionPattern.matches)
case _ => false
}
})
}
case None =>
throw new Exception(s"Invalid resource type specified. ${resource.resourceTypeName} is not a recognized resource type.")
}
}

private def calculateIntersectionGroup(resource: Resource, policy: AccessPolicy): Future[Set[WorkbenchUserId]] = {
for {
groups <- accessPolicyDAO.loadResourceAuthDomain(resource).unsafeToFuture
members <- directoryDAO.listIntersectionGroupUsers(groups.asInstanceOf[Set[WorkbenchGroupIdentity]] + policy.id)
} yield {
members
}
}

private[google] def toPetSAFromUser(user: WorkbenchUser): (ServiceAccountName, ServiceAccountDisplayName) = {
/*
* Service account IDs must be:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package org.broadinstitute.dsde.workbench.sam.openam

import cats.effect.IO
import org.broadinstitute.dsde.workbench.model.{WorkbenchGroupName, WorkbenchUserId}
import org.broadinstitute.dsde.workbench.model.{WorkbenchGroupIdentity, WorkbenchGroupName, WorkbenchUserId}
import org.broadinstitute.dsde.workbench.sam.model._

import scala.concurrent.Future
Expand All @@ -15,6 +15,7 @@ trait AccessPolicyDAO {
def createResource(resource: Resource): IO[Resource]
def deleteResource(resource: Resource): IO[Unit]
def loadResourceAuthDomain(resource: Resource): IO[Set[WorkbenchGroupName]]
def listResourcesConstrainedByGroup(groupId: WorkbenchGroupIdentity): Future[Set[Resource]]

def createPolicy(policy: AccessPolicy): IO[AccessPolicy]
def deletePolicy(policy: AccessPolicy): IO[Unit]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import org.broadinstitute.dsde.workbench.sam.directory.DirectorySubjectNameSuppo
import org.broadinstitute.dsde.workbench.sam.model._
import org.broadinstitute.dsde.workbench.sam.schema.JndiSchemaDAO.{Attr, ObjectClass}
import org.broadinstitute.dsde.workbench.sam.util.LdapSupport
import cats.implicits._

import scala.collection.JavaConverters._
import scala.concurrent.{ExecutionContext, Future}
Expand Down Expand Up @@ -55,6 +56,21 @@ class LdapAccessPolicyDAO(protected val ldapConnectionPool: LDAPConnectionPool,
}
}

private def unmarshalResource(results: Entry): Either[String, Resource] = {
for {
resourceTypeName <- getAttribute(results, Attr.resourceType).toRight(s"${Attr.resourceType} attribute missing")
resourceId <- getAttribute(results, Attr.resourceId).toRight(s"${Attr.resourceId} attribute missing")
} yield {
val authDomain = getAttributes(results, Attr.authDomain).map(g => WorkbenchGroupName(g)).toSet
Resource(ResourceTypeName(resourceTypeName), ResourceId(resourceId), authDomain)
}
}

override def listResourcesConstrainedByGroup(groupId: WorkbenchGroupIdentity): Future[Set[Resource]] = for {
res <- Future(ldapSearchStream(resourcesOu, SearchScope.SUB, Filter.createEqualityFilter(Attr.authDomain, groupId.toString))(unmarshalResource))
r <- res.parSequence.fold(err => Future.failed(new WorkbenchException(err)), r => Future.successful(r.toSet))
} yield r

override def createPolicy(policy: AccessPolicy): IO[AccessPolicy] = {
val attributes = List(
new Attribute("objectclass", "top", ObjectClass.policy),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,13 @@ class ResourceService(private val resourceTypes: Map[ResourceTypeName, ResourceT
private def createOrUpdatePolicy(resource: Resource, policy: ValidatableAccessPolicy): Future[AccessPolicy] = {
val resourceAndPolicyName = ResourceAndPolicyName(resource, policy.policyName)
val workbenchSubjects = policy.emailsToSubjects.values.flatten.toSet

accessPolicyDAO.loadPolicy(resourceAndPolicyName).flatMap {
case None => createPolicy(resourceAndPolicyName, workbenchSubjects, generateGroupEmail(), policy.roles, policy.actions)
case Some(accessPolicy) => accessPolicyDAO.overwritePolicy(AccessPolicy(resourceAndPolicyName, workbenchSubjects, accessPolicy.email, policy.roles, policy.actions))
} andThen {
case Success(policy) => fireGroupUpdateNotification(policy.id)
case Success(policy) =>
fireGroupUpdateNotification(policy.id)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ object TestSupport extends TestSupport{
notificationDAO,
googleServicesConfig,
petServiceAccountConfig,
configResourceTypes(CloudExtensions.resourceTypeName)))
resourceTypes))
val mockResourceService = new ResourceService(resourceTypes, policyDAO, directoryDAO, googleExt, "example.com")
val mockManagedGroupService = new ManagedGroupService(mockResourceService, resourceTypes, policyDAO, directoryDAO, googleExt, "example.com")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,44 +183,6 @@ class LdapDirectoryDAOSpec extends FlatSpec with Matchers with TestSupport with
}
}

it should "list flattened group users" in {
val userId1 = WorkbenchUserId(UUID.randomUUID().toString)
val user1 = WorkbenchUser(userId1, None, WorkbenchEmail("foo@bar.com"))
val userId2 = WorkbenchUserId(UUID.randomUUID().toString)
val user2 = WorkbenchUser(userId2, None, WorkbenchEmail("foo@bar.com"))
val userId3 = WorkbenchUserId(UUID.randomUUID().toString)
val user3 = WorkbenchUser(userId3, None, WorkbenchEmail("foo@bar.com"))

val groupName1 = WorkbenchGroupName(UUID.randomUUID().toString)
val group1 = BasicWorkbenchGroup(groupName1, Set(userId1), WorkbenchEmail("g1@example.com"))

val groupName2 = WorkbenchGroupName(UUID.randomUUID().toString)
val group2 = BasicWorkbenchGroup(groupName2, Set(userId2, groupName1), WorkbenchEmail("g2@example.com"))

val groupName3 = WorkbenchGroupName(UUID.randomUUID().toString)
val group3 = BasicWorkbenchGroup(groupName3, Set(userId3, groupName2), WorkbenchEmail("g3@example.com"))

runAndWait(dao.createUser(user1))
runAndWait(dao.createUser(user2))
runAndWait(dao.createUser(user3))
runAndWait(dao.createGroup(group1))
runAndWait(dao.createGroup(group2))
runAndWait(dao.createGroup(group3))

try {
assertResult(Set(userId1, userId2, userId3)) {
runAndWait(dao.listFlattenedGroupUsers(groupName3))
}
} finally {
runAndWait(dao.deleteUser(userId1))
runAndWait(dao.deleteUser(userId2))
runAndWait(dao.deleteUser(userId3))
runAndWait(dao.deleteGroup(groupName3))
runAndWait(dao.deleteGroup(groupName2))
runAndWait(dao.deleteGroup(groupName1))
}
}

it should "list group ancestors" in {
val groupName1 = WorkbenchGroupName(UUID.randomUUID().toString)
val group1 = BasicWorkbenchGroup(groupName1, Set(), WorkbenchEmail("g1@example.com"))
Expand Down Expand Up @@ -267,10 +229,6 @@ class LdapDirectoryDAOSpec extends FlatSpec with Matchers with TestSupport with
runAndWait(dao.addGroupMember(groupName1, groupName3))

try {
assertResult(Set(userId)) {
runAndWait(dao.listFlattenedGroupUsers(groupName3))
}

assertResult(Set(groupName1, groupName2, groupName3)) {
runAndWait(dao.listUsersGroups(userId))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ class MockDirectoryDAO(private val groups: mutable.Map[WorkbenchGroupIdentity, W
}
}

override def listFlattenedGroupUsers(groupName: WorkbenchGroupIdentity): Future[Set[WorkbenchUserId]] = Future {
listGroupUsers(groupName, Set.empty)
override def listIntersectionGroupUsers(groupIds: Set[WorkbenchGroupIdentity]): Future[Set[WorkbenchUserId]] = Future {
groupIds.flatMap(groupId => listGroupUsers(groupId, Set.empty))
}

private def listGroupUsers(groupName: WorkbenchGroupIdentity, visitedGroups: Set[WorkbenchGroupIdentity]): Set[WorkbenchUserId] = {
Expand Down
Loading

0 comments on commit 47843f2

Please sign in to comment.