Managed Group Access Instructions (#199)

* change endpoints and service to support access instructions in sam and only notify admins of access requests if instructions are not set; reading and writing of access instructions stubbed out

* add AdminGroupRoutes for setting access instructions; implement reading and writing of access instructions to ldap

* write tests for managed group access instructions

* remove unused test

* update swagger docs and add v1 routes tests

* remove unused import

* change endpoints to remove admin endpoint and have more straightforward get/set of access instructions

* replace admin endpoint with standard one; create get access instructions endpoint; change request access to return 400 if access instructions are set; add tests to improve coverage

* remove duplication in LdapDirectoryDAO; change post to put; remove groupName from accessInstructions payload

* update test name

* make changes according to pr comments from qi and dvoet

src/main/resources/reference.conf

@@ -95,11 +95,14 @@ resourceTypes = {
       "notify_admins" = {
         description = "send notifications to the admins of this group"
       }
+      "set_access_instructions" = {
+        description = "set access instructions for this group"
+      }
     }
     ownerRoleName = "admin"
     roles = {
       admin = {
-        roleActions = ["delete", "read_policies", "use", "share_policy::admin", "share_policy::member", "share_policy::admin-notifier", "read_policy::admin", "read_policy::member", "read_policy::admin-notifier"]
+        roleActions = ["delete", "read_policies", "use", "share_policy::admin", "share_policy::member", "share_policy::admin-notifier", "read_policy::admin", "read_policy::member", "read_policy::admin-notifier", "set_access_instructions"]
       }
       member = {
         roleActions = ["use"]

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

@@ -280,6 +280,50 @@ paths:
       tags:
         - Group
 
+  /api/groups/v1/{groupName}/accessInstructions:
+    put:
+      summary: sets the access instructions for a managed group
+      responses:
+        204:
+          description: Successfully set access instructions
+        500:
+          description: Internal Error
+          schema:
+            $ref: '#/definitions/ErrorReport'
+      parameters:
+      - in: path
+        description: Name of group
+        name: groupName
+        required: true
+        type: string
+      operationId: setAccessInstructions
+      tags:
+      - Group
+    get:
+      summary: gets the access instructions for a managed group
+      responses:
+        200:
+          description: Access instructions for this group
+          schema:
+            type: string
+        204:
+          description: No access instructions found for this group
+        404:
+          description: Group could not be found or you do not have the required permissions on this group
+        500:
+          description: Internal Error
+          schema:
+            $ref: '#/definitions/ErrorReport'
+      parameters:
+      - in: path
+        description: Name of group
+        name: groupName
+        required: true
+        type: string
+      operationId: getAccessInstructions
+      tags:
+      - Group
+
   /api/groups/v1/{groupName}/{policyName}:
     get:
       summary: 'Get email addresses for members of the "admin" policy'

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

@@ -33,28 +33,38 @@ trait ManagedGroupRoutes extends UserInfoDirectives with SecurityDirectives with
           get {
             handleGetGroup(managedGroup)
           } ~
-            post {
-              handleCreateGroup(managedGroup, userInfo)
-            } ~
-            delete {
-              handleDeleteGroup(managedGroup, userInfo)
-            }
+          post {
+            handleCreateGroup(managedGroup, userInfo)
+          } ~
+          delete {
+            handleDeleteGroup(managedGroup, userInfo)
+          }
         } ~
         pathPrefix("requestAccess") {
           post {
             handleRequestAccess(managedGroup, userInfo)
           }
         } ~
+        path("accessInstructions") {
+          put {
+            entity(as[ManagedGroupAccessInstructions]) { accessInstructions =>
+              handleSetAccessInstructions(managedGroup, accessInstructions, userInfo)
+            }
+          } ~
+          get {
+            handleGetAccessInstructions(managedGroup)
+          }
+        } ~
         pathPrefix(Segment) { policyName =>
           val accessPolicyName = ManagedGroupService.getPolicyName(policyName)
 
           pathEndOrSingleSlash {
             get {
               handleListEmails(managedGroup, accessPolicyName, userInfo)
             } ~
-              put {
-                handleOverwriteEmails(managedGroup, accessPolicyName, userInfo)
-              }
+            put {
+              handleOverwriteEmails(managedGroup, accessPolicyName, userInfo)
+            }
           } ~
           pathPrefix(Segment) { email =>
             pathEndOrSingleSlash {
@@ -78,7 +88,7 @@ trait ManagedGroupRoutes extends UserInfoDirectives with SecurityDirectives with
     }
   }
 
-  def handleListGroups(userInfo: UserInfo): Route = {
+  private def handleListGroups(userInfo: UserInfo): Route = {
     complete(managedGroupService.listGroups(userInfo.userId).map(StatusCodes.OK -> _))
   }
 
@@ -146,4 +156,21 @@ trait ManagedGroupRoutes extends UserInfoDirectives with SecurityDirectives with
       )
     }
   }
+
+  private def handleSetAccessInstructions(managedGroup: Resource, accessInstructions: ManagedGroupAccessInstructions, userInfo: UserInfo): Route = {
+    requireAction(managedGroup, SamResourceActions.setAccessInstructions, userInfo) {
+      complete(
+        managedGroupService.setAccessInstructions(managedGroup.resourceId, accessInstructions.value).map(_ => StatusCodes.NoContent)
+      )
+    }
+  }
+
+  private def handleGetAccessInstructions(managedGroup: Resource): Route = {
+    complete(
+      managedGroupService.getAccessInstructions(managedGroup.resourceId).map {
+        case Some(accessInstructions) => StatusCodes.OK -> Option(accessInstructions)
+        case None => StatusCodes.NoContent -> None
+      }
+    )
+  }
 }

src/main/scala/org/broadinstitute/dsde/workbench/sam/directory/DirectoryDAO.scala

@@ -4,15 +4,15 @@ import java.util.Date
 
 import org.broadinstitute.dsde.workbench.model.google._
 import org.broadinstitute.dsde.workbench.model._
-import org.broadinstitute.dsde.workbench.sam.model.BasicWorkbenchGroup
+import org.broadinstitute.dsde.workbench.sam.model.{BasicWorkbenchGroup, ResourceId}
 
 import scala.concurrent.Future
 
 /**
   * Created by dvoet on 5/26/17.
   */
 trait DirectoryDAO {
-  def createGroup(group: BasicWorkbenchGroup): Future[BasicWorkbenchGroup]
+  def createGroup(group: BasicWorkbenchGroup, accessInstructionsOpt: Option[String] = None): Future[BasicWorkbenchGroup]
   def loadGroup(groupName: WorkbenchGroupName): Future[Option[BasicWorkbenchGroup]]
   def loadGroups(groupNames: Set[WorkbenchGroupName]): Future[Seq[BasicWorkbenchGroup]]
   def loadGroupEmail(groupName: WorkbenchGroupName): Future[Option[WorkbenchEmail]]
@@ -57,4 +57,7 @@ trait DirectoryDAO {
   def deletePetServiceAccount(petServiceAccountId: PetServiceAccountId): Future[Unit]
   def getAllPetServiceAccountsForUser(userId: WorkbenchUserId): Future[Seq[PetServiceAccount]]
   def updatePetServiceAccount(petServiceAccount: PetServiceAccount): Future[PetServiceAccount]
+
+  def getManagedGroupAccessInstructions(groupName: WorkbenchGroupName): Future[Option[String]]
+  def setManagedGroupAccessInstructions(groupName: WorkbenchGroupName, accessInstructions: String): Future[Unit]
 }

src/main/scala/org/broadinstitute/dsde/workbench/sam/directory/LdapDirectoryDAO.scala

@@ -7,7 +7,7 @@ import org.broadinstitute.dsde.workbench.sam._
 import org.broadinstitute.dsde.workbench.model._
 import org.broadinstitute.dsde.workbench.model.google.{ServiceAccount, ServiceAccountDisplayName, ServiceAccountSubjectId}
 import org.broadinstitute.dsde.workbench.sam.config.DirectoryConfig
-import org.broadinstitute.dsde.workbench.sam.model.BasicWorkbenchGroup
+import org.broadinstitute.dsde.workbench.sam.model.{BasicWorkbenchGroup, ResourceId}
 import org.broadinstitute.dsde.workbench.sam.schema.JndiSchemaDAO.{Attr, ObjectClass}
 import org.broadinstitute.dsde.workbench.sam.util.LdapSupport
 
@@ -17,13 +17,19 @@ import scala.util.{Failure, Success, Try}
 
 class LdapDirectoryDAO(protected val ldapConnectionPool: LDAPConnectionPool, protected val directoryConfig: DirectoryConfig)(implicit executionContext: ExecutionContext) extends DirectoryDAO with DirectorySubjectNameSupport with LdapSupport {
 
-  override def createGroup(group: BasicWorkbenchGroup): Future[BasicWorkbenchGroup] = Future {
+  override def createGroup(group: BasicWorkbenchGroup, accessInstructionsOpt: Option[String] = None): Future[BasicWorkbenchGroup] = Future {
     val membersAttribute = if (group.members.isEmpty) None else Option(new Attribute(Attr.uniqueMember, group.members.map(subject => subjectDn(subject)).asJava))
+
+    val accessInstructionsAttr = accessInstructionsOpt.collect {
+      case accessInstructions => new Attribute(Attr.accessInstructions, accessInstructions)
+    }
+
     val attributes = Seq(
-      new Attribute("objectclass", "top", "workbenchGroup"),
-      new Attribute(Attr.email, group.email.value),
-      new Attribute(Attr.groupUpdatedTimestamp, formattedDate(new Date()))
-    ) ++ membersAttribute
+        new Attribute("objectclass", "top", "workbenchGroup"),
+        new Attribute(Attr.email, group.email.value),
+        new Attribute(Attr.groupUpdatedTimestamp, formattedDate(new Date())),
+    ) ++ membersAttribute ++
+      accessInstructionsAttr
 
     ldapConnectionPool.add(groupDn(group.id), attributes.asJava)
 
@@ -329,4 +335,15 @@ class LdapDirectoryDAO(protected val ldapConnectionPool: LDAPConnectionPool, pro
     ldapConnectionPool.modify(petDn(petServiceAccount.id), modifications.asJava)
     petServiceAccount
   }
+
+  override def getManagedGroupAccessInstructions(groupName: WorkbenchGroupName): Future[Option[String]] = {
+    Option(ldapConnectionPool.getEntry(groupDn(groupName))) match {
+      case None => Future.failed(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, s"$groupName not found")))
+      case Some(e) => Future.successful(Option(e.getAttributeValue(Attr.accessInstructions)))
+    }
+  }
+
+  override def setManagedGroupAccessInstructions(groupName: WorkbenchGroupName, accessInstructions: String): Future[Unit] = Future {
+    ldapConnectionPool.modify(groupDn(groupName), new Modification(ModificationType.REPLACE, Attr.accessInstructions, accessInstructions))
+  }
 }

src/main/scala/org/broadinstitute/dsde/workbench/sam/model/SamModel.scala

@@ -47,6 +47,8 @@ object SamJsonSupport {
 
   implicit val ManagedGroupMembershipEntryFormat = jsonFormat3(ManagedGroupMembershipEntry)
 
+  implicit val ManagedGroupAccessInstructionsFormat = ValueObjectFormat(ManagedGroupAccessInstructions)
+
   implicit val GroupSyncResponseFormat = jsonFormat2(GroupSyncResponse)
 
   implicit val CreateResourceRequestFormat = jsonFormat3(CreateResourceRequest)
@@ -57,6 +59,7 @@ object SamResourceActions {
   val alterPolicies = ResourceAction("alter_policies")
   val delete = ResourceAction("delete")
   val notifyAdmins = ResourceAction("notify_admins")
+  val setAccessInstructions = ResourceAction("set_access_instructions")
 
   def sharePolicy(policy: AccessPolicyName) = ResourceAction(s"share_policy::${policy.value}")
   def readPolicy(policy: AccessPolicyName) = ResourceAction(s"read_policy::${policy.value}")
@@ -104,5 +107,6 @@ case class AccessPolicyResponseEntry(policyName: AccessPolicyName, policy: Acces
 case class BasicWorkbenchGroup(id: WorkbenchGroupName, members: Set[WorkbenchSubject], email: WorkbenchEmail) extends WorkbenchGroup
 
 case class ManagedGroupMembershipEntry(groupName: ResourceId, role: AccessPolicyName, groupEmail: WorkbenchEmail)
+case class ManagedGroupAccessInstructions(value: String) extends ValueObject
 
 case class GroupSyncResponse(lastSyncDate: String, email: WorkbenchEmail)

src/main/scala/org/broadinstitute/dsde/workbench/sam/schema/JndiSchemaDAO.scala

@@ -31,6 +31,7 @@ object JndiSchemaDAO {
     val uniqueMember = "uniqueMember"
     val groupUpdatedTimestamp = "groupUpdatedTimestamp"
     val groupSynchronizedTimestamp = "groupSynchronizedTimestamp"
+    val accessInstructions = "accessInstructions"
     val member = "member"
     val memberOf = "isMemberOf"
     val givenName = "givenName"
@@ -271,6 +272,7 @@ class JndiSchemaDAO(protected val directoryConfig: DirectoryConfig, val schemaLo
 
     createAttributeDefinition(schema, "1.3.6.1.4.1.18060.0.4.3.2.200", Attr.groupUpdatedTimestamp, "time when group was updated", true, Option("generalizedTimeMatch"), Option("generalizedTimeOrderingMatch"), Option("1.3.6.1.4.1.1466.115.121.1.24"))
     createAttributeDefinition(schema, "1.3.6.1.4.1.18060.0.4.3.2.201", Attr.groupSynchronizedTimestamp, "time when group was synchronized", true, Option("generalizedTimeMatch"), Option("generalizedTimeOrderingMatch"), Option("1.3.6.1.4.1.1466.115.121.1.24"))
+    createAttributeDefinition(schema, "1.3.6.1.4.1.18060.0.4.3.2.202", Attr.accessInstructions, "access instructions for managed groups", true)
 
     val attrs = new BasicAttributes(true) // Ignore case
     attrs.put("NUMERICOID", "1.3.6.1.4.1.18060.0.4.3.2.100")
@@ -286,6 +288,7 @@ class JndiSchemaDAO(protected val directoryConfig: DirectoryConfig, val schemaLo
     val may = new BasicAttribute("MAY")
     may.add(Attr.groupUpdatedTimestamp)
     may.add(Attr.groupSynchronizedTimestamp)
+    may.add(Attr.accessInstructions)
     attrs.put(may)
 
     // Add the new schema object for "fooObjectClass"
@@ -308,6 +311,7 @@ class JndiSchemaDAO(protected val directoryConfig: DirectoryConfig, val schemaLo
     Try { schema.destroySubcontext("ClassDefinition/" + ObjectClass.workbenchGroup) }
     Try { schema.destroySubcontext("AttributeDefinition/" + Attr.groupSynchronizedTimestamp) }
     Try { schema.destroySubcontext("AttributeDefinition/" + Attr.groupUpdatedTimestamp) }
+    Try { schema.destroySubcontext("AttributeDefinition/" + Attr.accessInstructions) }
   }
 
   private def createPolicySchema(): Future[Unit] = withContext { ctx =>

src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ManagedGroupService.scala

@@ -19,7 +19,7 @@ class ManagedGroupService(private val resourceService: ResourceService, private
 
   def managedGroupType: ResourceType = resourceTypes.getOrElse(ManagedGroupService.managedGroupTypeName, throw new WorkbenchException(s"resource type ${ManagedGroupService.managedGroupTypeName.value} not found"))
 
-  def createManagedGroup(groupId: ResourceId, userInfo: UserInfo): Future[Resource] = {
+  def createManagedGroup(groupId: ResourceId, userInfo: UserInfo, accessInstructionsOpt: Option[String] = None): Future[Resource] = {
     def adminRole = managedGroupType.ownerRoleName
 
     val memberPolicy = ManagedGroupService.memberPolicyName -> AccessPolicyMembership(Set.empty, Set.empty, Set(ManagedGroupService.memberRoleName))
@@ -30,19 +30,19 @@ class ManagedGroupService(private val resourceService: ResourceService, private
     for {
       managedGroup <- resourceService.createResource(managedGroupType, groupId, Map(adminPolicy, memberPolicy, adminNotificationPolicy), Set.empty, userInfo)
       policies <- accessPolicyDAO.listAccessPolicies(managedGroup)
-      workbenchGroup <- createAggregateGroup(managedGroup, policies)
+      workbenchGroup <- createAggregateGroup(managedGroup, policies, accessInstructionsOpt)
       _ <- cloudExtensions.publishGroup(workbenchGroup.id)
     } yield managedGroup
   }
 
-  private def createAggregateGroup(resource: Resource, componentPolicies: Set[AccessPolicy]): Future[BasicWorkbenchGroup] = {
+  private def createAggregateGroup(resource: Resource, componentPolicies: Set[AccessPolicy], accessInstructionsOpt: Option[String]): Future[BasicWorkbenchGroup] = {
     val email = WorkbenchEmail(constructEmail(resource.resourceId.value))
     val workbenchGroupName = WorkbenchGroupName(resource.resourceId.value)
     val groupMembers: Set[WorkbenchSubject] = componentPolicies.collect {
       // collect only member and admin policies
       case AccessPolicy(id@ResourceAndPolicyName(_, ManagedGroupService.memberPolicyName | ManagedGroupService.adminPolicyName), _, _, _, _) => id
     }
-    directoryDAO.createGroup(BasicWorkbenchGroup(workbenchGroupName, groupMembers, email))
+    directoryDAO.createGroup(BasicWorkbenchGroup(workbenchGroupName, groupMembers, email), accessInstructionsOpt)
   }
 
   private def constructEmail(groupName: String) = {
@@ -131,15 +131,27 @@ class ManagedGroupService(private val resourceService: ResourceService, private
   }
 
   def requestAccess(resourceId: ResourceId, requesterUserId: WorkbenchUserId): Future[Unit] = {
-    val resourceAndPolicyName = ResourceAndPolicyName(Resource(ManagedGroupService.managedGroupTypeName, resourceId), ManagedGroupService.adminPolicyName)
-    accessPolicyDAO.listFlattenedPolicyMembers(resourceAndPolicyName).map { users =>
-      val notifications = users.map { recipientUserId =>
-        Notifications.GroupAccessRequestNotification(recipientUserId, WorkbenchGroupName(resourceId.value).value, users, requesterUserId)
-      }
-
-      cloudExtensions.fireAndForgetNotifications(notifications)
+    getAccessInstructions(resourceId).flatMap {
+      case Some(accessInstructions) =>
+        Future.failed(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.BadRequest, s"Please follow special access instructions: $accessInstructions")))
+      case None =>
+        val resourceAndPolicyName = ResourceAndPolicyName(Resource(ManagedGroupService.managedGroupTypeName, resourceId), ManagedGroupService.adminPolicyName)
+        accessPolicyDAO.listFlattenedPolicyMembers(resourceAndPolicyName).map { users =>
+          val notifications = users.map { recipientUserId =>
+            Notifications.GroupAccessRequestNotification(recipientUserId, WorkbenchGroupName(resourceId.value).value, users, requesterUserId)
+          }
+          cloudExtensions.fireAndForgetNotifications(notifications)
+        }
     }
   }
+
+  def getAccessInstructions(groupId: ResourceId): Future[Option[String]] = {
+    directoryDAO.getManagedGroupAccessInstructions(WorkbenchGroupName(groupId.value))
+  }
+
+  def setAccessInstructions(groupId: ResourceId, accessInstructions: String): Future[Unit] = {
+    directoryDAO.setManagedGroupAccessInstructions(WorkbenchGroupName(groupId.value), accessInstructions)
+  }
 }
 
 object ManagedGroupService {