fixes after rebase

broadinstitute · Oct 17, 2018 · 7e7ed1a · 7e7ed1a
1 parent 3e4a244
commit 7e7ed1a
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 22 deletions.
diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/Boot.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/Boot.scala
@@ -124,15 +124,15 @@ object Boot extends App with LazyLogging {
         blockingEc =>
           implicit val cs = IO.contextShift(scala.concurrent.ExecutionContext.Implicits.global)
           val accessPolicyDao = new LdapAccessPolicyDAO(ldapConnectionPool, directoryConfig, blockingEc)
-          val cloudExtention = createCloudExt(accessPolicyDao)
-          val (sRoutes, userService, resourceService, statusService) = createSamRoutes(cloudExtention, accessPolicyDao)
+          val cloudExtension = createCloudExt(accessPolicyDao)
+          val (sRoutes, userService, resourceService, statusService) = createSamRoutes(cloudExtension, accessPolicyDao)
 
           for{
             _ <- resourceService.initResourceTypes().handleErrorWith{
               case t: Throwable => IO(logger.error("FATAL - failure starting http server", t)) *> IO.raiseError(t)
             }
 
-            _ <- IO.fromFuture(IO(cloudExtention.onBoot(SamApplication(userService, resourceService, statusService))))
+            _ <- IO.fromFuture(IO(cloudExtension.onBoot(SamApplication(userService, resourceService, statusService))))
 
             binding <- IO.fromFuture(IO(Http().bindAndHandle(sRoutes.route, "0.0.0.0", 8080))).handleErrorWith{
               case t: Throwable => IO(logger.error("FATAL - failure starting http server", t)) *> IO.raiseError(t)

diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/ResourceRoutes.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/ResourceRoutes.scala
@@ -142,14 +142,14 @@ trait ResourceRoutes extends UserInfoDirectives with SecurityDirectives with Sam
                     } ~
                     pathPrefix("public") {
                       pathEndOrSingleSlash {
-                        requireOneOfAction(resource, Set(SamResourceActions.readPolicies, SamResourceActions.readPolicy(resourceAndPolicyName.accessPolicyName)), userInfo.userId) {
-                          get {
+                        get {
+                          requireOneOfAction(resource, Set(SamResourceActions.readPolicies, SamResourceActions.readPolicy(resourceAndPolicyName.accessPolicyName)), userInfo.userId) {
                             complete(resourceService.isPublic(resourceAndPolicyName))
                           }
                         } ~
-                        requireOneOfAction(resource, Set(SamResourceActions.alterPolicies, SamResourceActions.sharePolicy(resourceAndPolicyName.accessPolicyName)), userInfo.userId) {
-                          requireOneOfAction(FullyQualifiedResourceId(SamResourceTypes.resourceTypeAdminName, ResourceId(resourceType.name.value)), Set(SamResourceActions.setPublic, SamResourceActions.setPublicPolicy(resourceAndPolicyName.accessPolicyName)), userInfo.userId) {
-                            put {
+                        put {
+                          requireOneOfAction(resource, Set(SamResourceActions.alterPolicies, SamResourceActions.sharePolicy(resourceAndPolicyName.accessPolicyName)), userInfo.userId) {
+                            requireOneOfAction(FullyQualifiedResourceId(SamResourceTypes.resourceTypeAdminName, ResourceId(resourceType.name.value)), Set(SamResourceActions.setPublic, SamResourceActions.setPublicPolicy(resourceAndPolicyName.accessPolicyName)), userInfo.userId) {
                               entity(as[Boolean]) { isPublic =>
                                 complete(resourceService.setPublic(resourceAndPolicyName, isPublic).unsafeToFuture().map(_ => StatusCodes.NoContent))
                               }

diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/openam/LdapAccessPolicyDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/openam/LdapAccessPolicyDAO.scala
@@ -113,7 +113,7 @@ class LdapAccessPolicyDAO(protected val ldapConnectionPool: LDAPConnectionPool,
     val email = WorkbenchEmail(getAttribute(entry, Attr.email).get)
 
     AccessPolicy(
-      FullyQualifiedPolicyId(FullyQualifiedResourceId(resourceTypeName, resourceId), AccessPolicyName(policyName)), members, email, roles, actions)
+      FullyQualifiedPolicyId(FullyQualifiedResourceId(resourceTypeName, resourceId), AccessPolicyName(policyName)), members, email, roles, actions, isPublic)
   }
 
   override def overwritePolicyMembers(id: FullyQualifiedPolicyId, memberList: Set[WorkbenchSubject]): IO[Unit] = {

diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/PolicyEvaluatorService.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/PolicyEvaluatorService.scala
@@ -22,14 +22,14 @@ class PolicyEvaluatorService(
         role =>
           resourceType.roles.filter(_.roleName == role).flatMap(_.actions)
       }
-
       policy.actions ++ roleActions
     }
+
     for{
       rt <- IO.fromEither[ResourceType](resourceTypes.get(resource.resourceTypeName).toRight(new WorkbenchException(s"missing configuration for resourceType ${resource.resourceTypeName}")))
       isConstrained = rt.isAuthDomainConstrainable
 
-      policiesForResource <- accessPolicyDAO.listAccessPoliciesForUser(resource, userId)
+      policiesForResource <- listResourceAccessPoliciesForUser(resource, userId)
       allPolicyActions = policiesForResource.flatMap(p => allActions(p, rt))
       res <- if(isConstrained) {
         for{
@@ -86,6 +86,12 @@ class PolicyEvaluatorService(
     for {
       ripns <- accessPolicyDAO.listAccessPolicies(ManagedGroupService.managedGroupTypeName, userId)
     } yield ripns.filter(ripn => ManagedGroupService.userMembershipPolicyNames.contains(ripn.accessPolicyName))
+
+  def listResourceAccessPoliciesForUser(resource: FullyQualifiedResourceId, userId: WorkbenchUserId): IO[Set[AccessPolicy]] =
+    for {
+      policies <- accessPolicyDAO.listAccessPoliciesForUser(resource, userId)
+      publicPolicies <- accessPolicyDAO.listPublicAccessPolicies(resource)
+    } yield policies ++ publicPolicies
 }
 
 object PolicyEvaluatorService {

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
@@ -189,18 +189,11 @@ class ResourceService(private val resourceTypes: Map[ResourceTypeName, ResourceT
   }
 
   def listUserResourceRoles(resource: FullyQualifiedResourceId, userInfo: UserInfo): Future[Set[ResourceRoleName]] = {
-    listResourceAccessPoliciesForUser(resource, userInfo).map { matchingPolicies =>
+    policyEvaluatorService.listResourceAccessPoliciesForUser(resource, userInfo.userId).map { matchingPolicies =>
       matchingPolicies.flatMap(_.roles)
     }.unsafeToFuture()
   }
 
-  private def listResourceAccessPoliciesForUser(resource: FullyQualifiedResourceId, userInfo: UserInfo): IO[Set[AccessPolicy]] = {
-    for {
-      policies <- accessPolicyDAO.listAccessPoliciesForUser(resource, userInfo.userId)
-      publicPolicies <- accessPolicyDAO.listPublicAccessPolicies(resource)
-    } yield policies ++ publicPolicies
-  }
-
   /**
     * Overwrites an existing policy (keyed by resourceType/resourceId/policyName), saves a new one if it doesn't exist yet
     * @param resourceType

diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionSpec.scala
@@ -104,7 +104,7 @@ class GoogleExtensionSpec(_system: ActorSystem) extends TestKit(_system) with Fl
     val testGroup = BasicWorkbenchGroup(groupName, Set(inSamSubGroup.id, inBothSubGroup.id, inSamUserId, inBothUserId, addError), groupEmail)
     val testPolicy = AccessPolicy(
       model.FullyQualifiedPolicyId(
-        FullyQualifiedResourceId(ResourceTypeName("workspace"), ResourceId("rid")), AccessPolicyName("ap")), Set(inSamSubGroup.id, inBothSubGroup.id, inSamUserId, inBothUserId, addError), groupEmail, Set.empty, Set.empty)
+        FullyQualifiedResourceId(ResourceTypeName("workspace"), ResourceId("rid")), AccessPolicyName("ap")), Set(inSamSubGroup.id, inBothSubGroup.id, inSamUserId, inBothUserId, addError), groupEmail, Set.empty, Set.empty, isPublic = Some(true))
 
     Seq(testGroup, testPolicy).foreach { target =>
       val mockAccessPolicyDAO = mock[AccessPolicyDAO]

diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/openam/MockAccessPolicyDAO.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/openam/MockAccessPolicyDAO.scala
@@ -76,13 +76,13 @@ class MockAccessPolicyDAO(private val policies: mutable.Map[WorkbenchGroupIdenti
 
   override def listAccessPolicies(resource: FullyQualifiedResourceId): IO[Set[AccessPolicy]] = IO {
     policies.collect {
-      case (riapn @ FullyQualifiedPolicyId(`resource`, _), policy: AccessPolicy) => policy
+      case (FullyQualifiedPolicyId(`resource`, _), policy: AccessPolicy) => policy
     }.toSet
   }
 
   override def listAccessPoliciesForUser(resource: FullyQualifiedResourceId, user: WorkbenchUserId): IO[Set[AccessPolicy]] = IO {
     policies.collect {
-      case (_, policy: AccessPolicy) if policy.members.contains(user) => policy
+      case (FullyQualifiedPolicyId(`resource`, _), policy: AccessPolicy) if policy.members.contains(user) => policy
     }.toSet
 
   }