From 77c255449377d3c84218c73d03eb182087815c06 Mon Sep 17 00:00:00 2001 From: dvoet Date: Tue, 10 Dec 2024 14:21:23 -0500 Subject: [PATCH] CORE-210 add can_access action (#1604) --- README.md | 11 +- .../dsde/sam/liquibase/changelog.xml | 1 + .../20241205_prereq_action_column.xml | 15 ++ src/main/resources/reference.conf | 39 +++++- src/main/resources/swagger/api-docs.yaml | 13 +- .../dsde/workbench/sam/config/AppConfig.scala | 3 +- .../dataAccess/PostgresAccessPolicyDAO.scala | 11 +- .../workbench/sam/db/SamTypeBinders.scala | 129 +++++++++--------- .../sam/db/tables/ResourceTypeTable.scala | 14 +- .../dsde/workbench/sam/model/SamModel.scala | 3 +- .../sam/model/api/SamJsonSupport.scala | 2 +- .../sam/service/PolicyEvaluatorService.scala | 12 +- .../sam/service/ResourceService.scala | 26 +++- .../sam/service/ResourceServiceSpec.scala | 57 +++++++- .../sam/service/ResourceServiceUnitSpec.scala | 54 ++++++++ 15 files changed, 305 insertions(+), 85 deletions(-) create mode 100644 src/main/resources/org/broadinstitute/dsde/sam/liquibase/changesets/20241205_prereq_action_column.xml diff --git a/README.md b/README.md index 8e165568b7..c730b760c9 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,15 @@ The “owner” role of a resource generally will include delete action and acti Resource types define the set of available actions for all resources of that type. It also defines a set of roles and their associated actions. Roles are useful because it can be cumbersome to deal with granular actions and as a point of extensibility (when new actions are added to resource types, they can be added to roles as well, effectively adding the action to all resources with that role). Creating and maintaining resource types is achieved through [configuration](src/main/resources/reference.conf). +#### Resource Type Configuration +Configuration options for resource types are defined in the `resourceTypes` section of the configuration file. Each resource type has the following fields: +* `actionPatterns` - a set of regex patterns that are used to document and validate the actions available for resources of this type +* `roles` - a set of roles that are available for resources of this type. Roles are a collection of actions. Both roles and actions can be assigned to resource policies, but it is highly recommended to use roles because they are easier to change and affect all resources with that role as opposed to updating policies to add new actions. +* `ownerRoleName` - the name of the role that is considered the "owner" role for all resources of this type. All resources must have a policy with this role or have a parent. +* `reuseIds` - whether to allow reusing ids when creating resources of this type. This is important to prevent when using auth domains because users should not be able to delete then recreate a resource in Sam omitting the auth domain. Should be false when using UUIDs for Sam resource ids. Default is false. +* `allowLeaving` - whether to allow users to leave resources of this type, otherwise an owner must remove them. Default is false. +* `prerequisiteAction` - an optional action that must be granted before a user can perform any other actions on a resource of this type. Useful for resources that require some kind of access to a parent resource before accessing a child. + ### Public Policies There are some cases where it is desirable to grant actions or roles to all authenticated users. For example, granting read-only access to public workspaces. In this case a policy can be created that has the appropriate actions or roles and set to public. Resources with public policies show up when listing resources for a user. For this reason it is not always desirable to allow everyone to make public policies. Again, the example is public workspaces. Public workspaces show up for everyone and should be curated. @@ -62,7 +71,7 @@ Group - Create, delete, read, list, add/remove users and groups. Nested groups a ### Built In Actions * read_policies - may read all policies of a resource -* alter_policies - may change any policy of a resource +* alter_policies - may add or change any policy of a resource, use sparingly, prefer share_policy below for more control over policy structure * delete - may delete a resource * share_policy::{policy name} - may add/remove members to/from specified policy of a resource * read_policy::{policy name} - may read specified policy of a resource diff --git a/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changelog.xml b/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changelog.xml index 6239c45435..459844b6e6 100644 --- a/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changelog.xml +++ b/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changelog.xml @@ -33,4 +33,5 @@ + diff --git a/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changesets/20241205_prereq_action_column.xml b/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changesets/20241205_prereq_action_column.xml new file mode 100644 index 0000000000..1f5be04ba7 --- /dev/null +++ b/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changesets/20241205_prereq_action_column.xml @@ -0,0 +1,15 @@ + + + + + + + + + + + \ No newline at end of file diff --git a/src/main/resources/reference.conf b/src/main/resources/reference.conf index 121e1dd860..4477bb0320 100644 --- a/src/main/resources/reference.conf +++ b/src/main/resources/reference.conf @@ -227,6 +227,9 @@ resourceTypes = { roleActions = ["compute"] descendantRoles = { google-project = ["notebook-user"] + notebook-cluster = ["accessor"] + kubernetes-app = ["accessor"] + persistent-disk = ["accessor"] } } can-catalog = { @@ -798,6 +801,9 @@ resourceTypes = { } notebook-cluster = { actionPatterns = { + can_access = { + description = "necessary but not sufficient to perform any action on the notebook cluster" + } status = { description = "view notebook cluster status details and configuration" } @@ -826,17 +832,27 @@ resourceTypes = { } ownerRoleName = "creator" roles = { + # the creator role is assigned directly to the notebook-cluster resource and does not include can_access + # but can_access is required to perform any action so must be granted via the parent resource using + # either the manager or accessor role creator = { roleActions = ["status", "connect", "delete", "read_policies", "stop_start", "modify", "set_parent", "get_parent"] } manager = { - roleActions = ["status", "delete", "read_policies"] + roleActions = ["status", "delete", "read_policies", "can_access"] + } + accessor = { + roleActions = ["can_access"] } } reuseIds = false + prerequisiteAction = "can_access" } persistent-disk = { actionPatterns = { + can_access = { + description = "necessary but not sufficient to perform any action on the persistent disk" + } read = { description = "read metadata of persistent disk" } @@ -862,17 +878,27 @@ resourceTypes = { } ownerRoleName = "creator" roles = { + # the creator role is assigned directly to the persistent-disk resource and does not include can_access + # but can_access is required to perform any action so must be granted via the parent resource using + # either the manager or accessor role creator = { roleActions = ["read", "attach", "modify", "delete", "read_policies", "set_parent", "get_parent"] } manager = { - roleActions = ["delete", "read", "read_policies"] + roleActions = ["delete", "read", "read_policies", "can_access"] + } + accessor = { + roleActions = ["can_access"] } } reuseIds = false + prerequisiteAction = "can_access" } kubernetes-app = { actionPatterns = { + can_access = { + description = "necessary but not sufficient to perform any action on the kubernetes app" + } delete = { description = "delete kubernetes application" } @@ -901,14 +927,21 @@ resourceTypes = { } ownerRoleName = "creator" roles = { + # the creator role is assigned directly to the kubernetes-app resource and does not include can_access + # but can_access is required to perform any action so must be granted via the parent resource using + # either the manager or accessor role creator = { roleActions = ["delete", "connect", "update", "status", "stop", "start", "read_policies", "set_parent"] } + accessor = { + roleActions = ["can_access"] + } manager = { - roleActions = ["delete", "status", "read_policies"] + roleActions = ["delete", "status", "read_policies", "can_access"] } } reuseIds = false + prerequisiteAction = "can_access" } kubernetes-app-shared = { actionPatterns = { diff --git a/src/main/resources/swagger/api-docs.yaml b/src/main/resources/swagger/api-docs.yaml index 2b1d89feea..7d7d82afff 100755 --- a/src/main/resources/swagger/api-docs.yaml +++ b/src/main/resources/swagger/api-docs.yaml @@ -4354,7 +4354,7 @@ components: description: specification of a role for a resource ResourceType: required: - - actions + - actionPatterns - name - ownerRoleName - roles @@ -4363,7 +4363,7 @@ components: name: type: string description: The name of the resource type - actions: + actionPatterns: type: array description: List of actions that can be performed on a resource of this type @@ -4378,6 +4378,15 @@ components: type: string description: The name of the role that can perform administrative functions on a resource of this type + reuseIds: + type: boolean + description: Whether resource ids can be reused + allowLeaving: + type: boolean + description: Whether users can leave a resource of this type + prerequisiteAction: + type: string + description: An action that is required but not sufficient to perform any other action on a resource of this type description: specification of a type of resource RolesAndActions: required: diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala index 838e6c01e6..2130dae1f3 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala @@ -84,7 +84,8 @@ object AppConfig { config.as[Map[String, ResourceRole]](s"$uqPath.roles").values.toSet, ResourceRoleName(config.getString(s"$uqPath.ownerRoleName")), config.getBoolean(s"$uqPath.reuseIds"), - config.as[Option[Boolean]](s"$uqPath.allowLeaving").getOrElse(false) + config.as[Option[Boolean]](s"$uqPath.allowLeaving").getOrElse(false), + config.as[Option[String]](s"$uqPath.prerequisiteAction").map(ResourceAction) ) } } diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala index 68d7e508db..1bc3a0d9d1 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresAccessPolicyDAO.scala @@ -188,7 +188,8 @@ class PostgresAccessPolicyDAO( resourceTypeToRoles.getOrElse(resourceTypeRecord.id, Set.empty), resourceTypeRecord.ownerRoleName, resourceTypeRecord.reuseIds, - resourceTypeRecord.allowLeaving + resourceTypeRecord.allowLeaving, + resourceTypeRecord.prerequisiteAction ) } @@ -434,14 +435,16 @@ class PostgresAccessPolicyDAO( private def upsertResourceTypes(resourceTypes: Iterable[ResourceType])(implicit session: DBSession): Int = { val resourceTypeTableColumn = ResourceTypeTable.column - val resourceTypeValues = resourceTypes.map(rt => samsqls"""(${rt.name}, ${rt.ownerRoleName}, ${rt.reuseIds}, ${rt.allowLeaving})""") - samsql"""insert into ${ResourceTypeTable.table} (${resourceTypeTableColumn.name}, ${resourceTypeTableColumn.ownerRoleName}, ${resourceTypeTableColumn.reuseIds}, ${resourceTypeTableColumn.allowLeaving}) + val resourceTypeValues = + resourceTypes.map(rt => samsqls"""(${rt.name}, ${rt.ownerRoleName}, ${rt.reuseIds}, ${rt.allowLeaving}, ${rt.prerequisiteAction})""") + samsql"""insert into ${ResourceTypeTable.table} (${resourceTypeTableColumn.name}, ${resourceTypeTableColumn.ownerRoleName}, ${resourceTypeTableColumn.reuseIds}, ${resourceTypeTableColumn.allowLeaving}, ${resourceTypeTableColumn.prerequisiteAction}) values $resourceTypeValues on conflict (${ResourceTypeTable.column.name}) do update set ${resourceTypeTableColumn.ownerRoleName} = EXCLUDED.${resourceTypeTableColumn.ownerRoleName}, ${resourceTypeTableColumn.reuseIds} = EXCLUDED.${resourceTypeTableColumn.reuseIds}, - ${resourceTypeTableColumn.allowLeaving} = EXCLUDED.${resourceTypeTableColumn.allowLeaving}""".update().apply() + ${resourceTypeTableColumn.allowLeaving} = EXCLUDED.${resourceTypeTableColumn.allowLeaving}, + ${resourceTypeTableColumn.prerequisiteAction} = EXCLUDED.${resourceTypeTableColumn.prerequisiteAction}""".update().apply() } /** @param resource diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/db/SamTypeBinders.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/db/SamTypeBinders.scala index 9ed071b456..47482cc2c6 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/db/SamTypeBinders.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/db/SamTypeBinders.scala @@ -17,123 +17,123 @@ import scalikejdbc.TypeBinder object SamTypeBinders { implicit val accessInstructionsPKTypeBinder: TypeBinder[AccessInstructionsPK] = new TypeBinder[AccessInstructionsPK] { - def apply(rs: ResultSet, label: String): AccessInstructionsPK = AccessInstructionsPK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): AccessInstructionsPK = AccessInstructionsPK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): AccessInstructionsPK = nullSafe(rs.getLong(label), AccessInstructionsPK) + def apply(rs: ResultSet, index: Int): AccessInstructionsPK = nullSafe(rs.getLong(index), AccessInstructionsPK) } implicit val groupMemberPKTypeBinder: TypeBinder[GroupMemberPK] = new TypeBinder[GroupMemberPK] { - def apply(rs: ResultSet, label: String): GroupMemberPK = GroupMemberPK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): GroupMemberPK = GroupMemberPK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): GroupMemberPK = nullSafe(rs.getLong(label), GroupMemberPK) + def apply(rs: ResultSet, index: Int): GroupMemberPK = nullSafe(rs.getLong(index), GroupMemberPK) } implicit val groupPKTypeBinder: TypeBinder[GroupPK] = new TypeBinder[GroupPK] { - def apply(rs: ResultSet, label: String): GroupPK = GroupPK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): GroupPK = GroupPK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): GroupPK = nullSafe(rs.getLong(label), GroupPK) + def apply(rs: ResultSet, index: Int): GroupPK = nullSafe(rs.getLong(index), GroupPK) } implicit val policyPKTypeBinder: TypeBinder[PolicyPK] = new TypeBinder[PolicyPK] { - def apply(rs: ResultSet, label: String): PolicyPK = PolicyPK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): PolicyPK = PolicyPK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): PolicyPK = nullSafe(rs.getLong(label), PolicyPK) + def apply(rs: ResultSet, index: Int): PolicyPK = nullSafe(rs.getLong(index), PolicyPK) } implicit val effectivePolicyPKTypeBinder: TypeBinder[EffectiveResourcePolicyPK] = new TypeBinder[EffectiveResourcePolicyPK] { - def apply(rs: ResultSet, label: String): EffectiveResourcePolicyPK = EffectiveResourcePolicyPK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): EffectiveResourcePolicyPK = EffectiveResourcePolicyPK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): EffectiveResourcePolicyPK = nullSafe(rs.getLong(label), EffectiveResourcePolicyPK) + def apply(rs: ResultSet, index: Int): EffectiveResourcePolicyPK = nullSafe(rs.getLong(index), EffectiveResourcePolicyPK) } implicit val policyNameTypeBinder: TypeBinder[AccessPolicyName] = new TypeBinder[AccessPolicyName] { - def apply(rs: ResultSet, label: String): AccessPolicyName = AccessPolicyName(rs.getString(label)) - def apply(rs: ResultSet, index: Int): AccessPolicyName = AccessPolicyName(rs.getString(index)) + def apply(rs: ResultSet, label: String): AccessPolicyName = nullSafe(rs.getString(label), AccessPolicyName) + def apply(rs: ResultSet, index: Int): AccessPolicyName = nullSafe(rs.getString(index), AccessPolicyName) } implicit val resourceActionPatternPKTypeBinder: TypeBinder[ResourceActionPatternPK] = new TypeBinder[ResourceActionPatternPK] { - def apply(rs: ResultSet, label: String): ResourceActionPatternPK = ResourceActionPatternPK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): ResourceActionPatternPK = ResourceActionPatternPK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): ResourceActionPatternPK = nullSafe(rs.getLong(label), ResourceActionPatternPK) + def apply(rs: ResultSet, index: Int): ResourceActionPatternPK = nullSafe(rs.getLong(index), ResourceActionPatternPK) } implicit val resourceActionPatternNameTypeBinder: TypeBinder[ResourceActionPatternName] = new TypeBinder[ResourceActionPatternName] { - def apply(rs: ResultSet, label: String): ResourceActionPatternName = ResourceActionPatternName(rs.getString(label)) - def apply(rs: ResultSet, index: Int): ResourceActionPatternName = ResourceActionPatternName(rs.getString(index)) + def apply(rs: ResultSet, label: String): ResourceActionPatternName = nullSafe(rs.getString(label), ResourceActionPatternName) + def apply(rs: ResultSet, index: Int): ResourceActionPatternName = nullSafe(rs.getString(index), ResourceActionPatternName) } implicit val resourceActionPKTypeBinder: TypeBinder[ResourceActionPK] = new TypeBinder[ResourceActionPK] { - def apply(rs: ResultSet, label: String): ResourceActionPK = ResourceActionPK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): ResourceActionPK = ResourceActionPK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): ResourceActionPK = nullSafe(rs.getLong(label), ResourceActionPK) + def apply(rs: ResultSet, index: Int): ResourceActionPK = nullSafe(rs.getLong(index), ResourceActionPK) } implicit val resourceActionNameTypeBinder: TypeBinder[ResourceAction] = new TypeBinder[ResourceAction] { - def apply(rs: ResultSet, label: String): ResourceAction = ResourceAction(rs.getString(label)) - def apply(rs: ResultSet, index: Int): ResourceAction = ResourceAction(rs.getString(index)) + def apply(rs: ResultSet, label: String): ResourceAction = nullSafe(rs.getString(label), ResourceAction) + def apply(rs: ResultSet, index: Int): ResourceAction = nullSafe(rs.getString(index), ResourceAction) } implicit val resourceRolePKTypeBinder: TypeBinder[ResourceRolePK] = new TypeBinder[ResourceRolePK] { - def apply(rs: ResultSet, label: String): ResourceRolePK = ResourceRolePK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): ResourceRolePK = ResourceRolePK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): ResourceRolePK = nullSafe(rs.getLong(label), ResourceRolePK) + def apply(rs: ResultSet, index: Int): ResourceRolePK = nullSafe(rs.getLong(index), ResourceRolePK) } implicit val resourceRoleNameTypeBinder: TypeBinder[ResourceRoleName] = new TypeBinder[ResourceRoleName] { - def apply(rs: ResultSet, label: String): ResourceRoleName = ResourceRoleName(rs.getString(label)) - def apply(rs: ResultSet, index: Int): ResourceRoleName = ResourceRoleName(rs.getString(index)) + def apply(rs: ResultSet, label: String): ResourceRoleName = nullSafe(rs.getString(label), ResourceRoleName) + def apply(rs: ResultSet, index: Int): ResourceRoleName = nullSafe(rs.getString(index), ResourceRoleName) } implicit val resourcePKTypeBinder: TypeBinder[ResourcePK] = new TypeBinder[ResourcePK] { - def apply(rs: ResultSet, label: String): ResourcePK = ResourcePK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): ResourcePK = ResourcePK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): ResourcePK = nullSafe(rs.getLong(label), ResourcePK) + def apply(rs: ResultSet, index: Int): ResourcePK = nullSafe(rs.getLong(index), ResourcePK) } implicit val resourceIdTypeBinder: TypeBinder[ResourceId] = new TypeBinder[ResourceId] { - def apply(rs: ResultSet, label: String): ResourceId = ResourceId(rs.getString(label)) - def apply(rs: ResultSet, index: Int): ResourceId = ResourceId(rs.getString(index)) + def apply(rs: ResultSet, label: String): ResourceId = nullSafe(rs.getString(label), ResourceId) + def apply(rs: ResultSet, index: Int): ResourceId = nullSafe(rs.getString(index), ResourceId) } implicit val resourceTypePKTypeBinder: TypeBinder[ResourceTypePK] = new TypeBinder[ResourceTypePK] { - def apply(rs: ResultSet, label: String): ResourceTypePK = ResourceTypePK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): ResourceTypePK = ResourceTypePK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): ResourceTypePK = nullSafe(rs.getLong(label), ResourceTypePK) + def apply(rs: ResultSet, index: Int): ResourceTypePK = nullSafe(rs.getLong(index), ResourceTypePK) } implicit val resourceTypeNameTypeBinder: TypeBinder[ResourceTypeName] = new TypeBinder[ResourceTypeName] { - def apply(rs: ResultSet, label: String): ResourceTypeName = ResourceTypeName(rs.getString(label)) - def apply(rs: ResultSet, index: Int): ResourceTypeName = ResourceTypeName(rs.getString(index)) + def apply(rs: ResultSet, label: String): ResourceTypeName = nullSafe(rs.getString(label), ResourceTypeName) + def apply(rs: ResultSet, index: Int): ResourceTypeName = nullSafe(rs.getString(index), ResourceTypeName) } implicit val workbenchGroupNameTypeBinder: TypeBinder[WorkbenchGroupName] = new TypeBinder[WorkbenchGroupName] { - def apply(rs: ResultSet, label: String): WorkbenchGroupName = WorkbenchGroupName(rs.getString(label)) - def apply(rs: ResultSet, index: Int): WorkbenchGroupName = WorkbenchGroupName(rs.getString(index)) + def apply(rs: ResultSet, label: String): WorkbenchGroupName = nullSafe(rs.getString(label), WorkbenchGroupName) + def apply(rs: ResultSet, index: Int): WorkbenchGroupName = nullSafe(rs.getString(index), WorkbenchGroupName) } implicit val workbenchEmailTypeBinder: TypeBinder[WorkbenchEmail] = new TypeBinder[WorkbenchEmail] { - def apply(rs: ResultSet, label: String): WorkbenchEmail = WorkbenchEmail(rs.getString(label)) - def apply(rs: ResultSet, index: Int): WorkbenchEmail = WorkbenchEmail(rs.getString(index)) + def apply(rs: ResultSet, label: String): WorkbenchEmail = nullSafe(rs.getString(label), WorkbenchEmail) + def apply(rs: ResultSet, index: Int): WorkbenchEmail = nullSafe(rs.getString(index), WorkbenchEmail) } implicit val googleProjectTypeBinder: TypeBinder[GoogleProject] = new TypeBinder[GoogleProject] { - def apply(rs: ResultSet, label: String): GoogleProject = GoogleProject(rs.getString(label)) - def apply(rs: ResultSet, index: Int): GoogleProject = GoogleProject(rs.getString(index)) + def apply(rs: ResultSet, label: String): GoogleProject = nullSafe(rs.getString(label), GoogleProject) + def apply(rs: ResultSet, index: Int): GoogleProject = nullSafe(rs.getString(index), GoogleProject) } implicit val googleSubjectIdTypeBinder: TypeBinder[GoogleSubjectId] = new TypeBinder[GoogleSubjectId] { - def apply(rs: ResultSet, label: String): GoogleSubjectId = GoogleSubjectId(rs.getString(label)) - def apply(rs: ResultSet, index: Int): GoogleSubjectId = GoogleSubjectId(rs.getString(index)) + def apply(rs: ResultSet, label: String): GoogleSubjectId = nullSafe(rs.getString(label), GoogleSubjectId) + def apply(rs: ResultSet, index: Int): GoogleSubjectId = nullSafe(rs.getString(index), GoogleSubjectId) } implicit val workbenchUserIdTypeBinder: TypeBinder[WorkbenchUserId] = new TypeBinder[WorkbenchUserId] { - def apply(rs: ResultSet, label: String): WorkbenchUserId = WorkbenchUserId(rs.getString(label)) - def apply(rs: ResultSet, index: Int): WorkbenchUserId = WorkbenchUserId(rs.getString(index)) + def apply(rs: ResultSet, label: String): WorkbenchUserId = nullSafe(rs.getString(label), WorkbenchUserId) + def apply(rs: ResultSet, index: Int): WorkbenchUserId = nullSafe(rs.getString(index), WorkbenchUserId) } implicit val serviceAccountDisplayNameTypeBinder: TypeBinder[ServiceAccountDisplayName] = new TypeBinder[ServiceAccountDisplayName] { - def apply(rs: ResultSet, label: String): ServiceAccountDisplayName = ServiceAccountDisplayName(rs.getString(label)) - def apply(rs: ResultSet, index: Int): ServiceAccountDisplayName = ServiceAccountDisplayName(rs.getString(index)) + def apply(rs: ResultSet, label: String): ServiceAccountDisplayName = nullSafe(rs.getString(label), ServiceAccountDisplayName) + def apply(rs: ResultSet, index: Int): ServiceAccountDisplayName = nullSafe(rs.getString(index), ServiceAccountDisplayName) } implicit val serviceAccountSubjectIdTypeBinder: TypeBinder[ServiceAccountSubjectId] = new TypeBinder[ServiceAccountSubjectId] { - def apply(rs: ResultSet, label: String): ServiceAccountSubjectId = ServiceAccountSubjectId(rs.getString(label)) - def apply(rs: ResultSet, index: Int): ServiceAccountSubjectId = ServiceAccountSubjectId(rs.getString(index)) + def apply(rs: ResultSet, label: String): ServiceAccountSubjectId = nullSafe(rs.getString(label), ServiceAccountSubjectId) + def apply(rs: ResultSet, index: Int): ServiceAccountSubjectId = nullSafe(rs.getString(index), ServiceAccountSubjectId) } implicit val flatGroupMemberPKTypeBinder: TypeBinder[GroupMemberFlatPK] = new TypeBinder[GroupMemberFlatPK] { - def apply(rs: ResultSet, label: String): GroupMemberFlatPK = GroupMemberFlatPK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): GroupMemberFlatPK = GroupMemberFlatPK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): GroupMemberFlatPK = nullSafe(rs.getLong(label), GroupMemberFlatPK) + def apply(rs: ResultSet, index: Int): GroupMemberFlatPK = nullSafe(rs.getLong(index), GroupMemberFlatPK) } implicit val flatGroupMembershipPathPKTypeBinder: TypeBinder[GroupMembershipPath] = new TypeBinder[GroupMembershipPath] { @@ -144,43 +144,44 @@ object SamTypeBinders { } implicit val tenantIdTypeBinder: TypeBinder[TenantId] = new TypeBinder[TenantId] { - def apply(rs: ResultSet, label: String): TenantId = TenantId(rs.getString(label)) - def apply(rs: ResultSet, index: Int): TenantId = TenantId(rs.getString(index)) + def apply(rs: ResultSet, label: String): TenantId = nullSafe(rs.getString(label), TenantId) + def apply(rs: ResultSet, index: Int): TenantId = nullSafe(rs.getString(index), TenantId) } implicit val subscriptionIdTypeBinder: TypeBinder[SubscriptionId] = new TypeBinder[SubscriptionId] { - def apply(rs: ResultSet, label: String): SubscriptionId = SubscriptionId(rs.getString(label)) - def apply(rs: ResultSet, index: Int): SubscriptionId = SubscriptionId(rs.getString(index)) + def apply(rs: ResultSet, label: String): SubscriptionId = nullSafe(rs.getString(label), SubscriptionId) + def apply(rs: ResultSet, index: Int): SubscriptionId = nullSafe(rs.getString(index), SubscriptionId) } implicit val managedResourceGroupNameTypeBinder: TypeBinder[ManagedResourceGroupName] = new TypeBinder[ManagedResourceGroupName] { - def apply(rs: ResultSet, label: String): ManagedResourceGroupName = ManagedResourceGroupName(rs.getString(label)) - def apply(rs: ResultSet, index: Int): ManagedResourceGroupName = ManagedResourceGroupName(rs.getString(index)) + def apply(rs: ResultSet, label: String): ManagedResourceGroupName = nullSafe(rs.getString(label), ManagedResourceGroupName) + def apply(rs: ResultSet, index: Int): ManagedResourceGroupName = nullSafe(rs.getString(index), ManagedResourceGroupName) } implicit val managedIdentityObjectIdTypeBinder: TypeBinder[ManagedIdentityObjectId] = new TypeBinder[ManagedIdentityObjectId] { - def apply(rs: ResultSet, label: String): ManagedIdentityObjectId = ManagedIdentityObjectId(rs.getString(label)) - def apply(rs: ResultSet, index: Int): ManagedIdentityObjectId = ManagedIdentityObjectId(rs.getString(index)) + def apply(rs: ResultSet, label: String): ManagedIdentityObjectId = nullSafe(rs.getString(label), ManagedIdentityObjectId) + def apply(rs: ResultSet, index: Int): ManagedIdentityObjectId = nullSafe(rs.getString(index), ManagedIdentityObjectId) } implicit val managedIdentityDisplayNameTypeBinder: TypeBinder[ManagedIdentityDisplayName] = new TypeBinder[ManagedIdentityDisplayName] { - def apply(rs: ResultSet, label: String): ManagedIdentityDisplayName = ManagedIdentityDisplayName(rs.getString(label)) - def apply(rs: ResultSet, index: Int): ManagedIdentityDisplayName = ManagedIdentityDisplayName(rs.getString(index)) + def apply(rs: ResultSet, label: String): ManagedIdentityDisplayName = nullSafe(rs.getString(label), ManagedIdentityDisplayName) + def apply(rs: ResultSet, index: Int): ManagedIdentityDisplayName = nullSafe(rs.getString(index), ManagedIdentityDisplayName) } implicit val ManagedResourceGroupPKTypeBinder: TypeBinder[ManagedResourceGroupPK] = new TypeBinder[ManagedResourceGroupPK] { - def apply(rs: ResultSet, label: String): ManagedResourceGroupPK = ManagedResourceGroupPK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): ManagedResourceGroupPK = ManagedResourceGroupPK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): ManagedResourceGroupPK = nullSafe(rs.getLong(label), ManagedResourceGroupPK) + def apply(rs: ResultSet, index: Int): ManagedResourceGroupPK = nullSafe(rs.getLong(index), ManagedResourceGroupPK) } implicit val BillingProfileIdTypeBinder: TypeBinder[BillingProfileId] = new TypeBinder[BillingProfileId] { - def apply(rs: ResultSet, label: String): BillingProfileId = BillingProfileId(rs.getString(label)) - def apply(rs: ResultSet, index: Int): BillingProfileId = BillingProfileId(rs.getString(index)) + def apply(rs: ResultSet, label: String): BillingProfileId = nullSafe(rs.getString(label), BillingProfileId) + def apply(rs: ResultSet, index: Int): BillingProfileId = nullSafe(rs.getString(index), BillingProfileId) } implicit val lastQuotaErrorPKTypeBinder: TypeBinder[LastQuotaErrorPK] = new TypeBinder[LastQuotaErrorPK] { - def apply(rs: ResultSet, label: String): LastQuotaErrorPK = LastQuotaErrorPK(rs.getLong(label)) - def apply(rs: ResultSet, index: Int): LastQuotaErrorPK = LastQuotaErrorPK(rs.getLong(index)) + def apply(rs: ResultSet, label: String): LastQuotaErrorPK = nullSafe(rs.getLong(label), LastQuotaErrorPK) + def apply(rs: ResultSet, index: Int): LastQuotaErrorPK = nullSafe(rs.getLong(index), LastQuotaErrorPK) } + private def nullSafe[V, T >: Null](value: V, constructor: V => T): T = Option(value).map(constructor).orNull } diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/db/tables/ResourceTypeTable.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/db/tables/ResourceTypeTable.scala index 3f1acce225..d049c74a8f 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/db/tables/ResourceTypeTable.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/db/tables/ResourceTypeTable.scala @@ -1,12 +1,19 @@ package org.broadinstitute.dsde.workbench.sam.db.tables import org.broadinstitute.dsde.workbench.sam.db.{DatabaseKey, SamTypeBinders} -import org.broadinstitute.dsde.workbench.sam.model.{ResourceRoleName, ResourceTypeName} +import org.broadinstitute.dsde.workbench.sam.model.{ResourceAction, ResourceRoleName, ResourceTypeName} import org.broadinstitute.dsde.workbench.sam.db.SamParameterBinderFactory.SqlInterpolationWithSamBinders import scalikejdbc._ final case class ResourceTypePK(value: Long) extends DatabaseKey -final case class ResourceTypeRecord(id: ResourceTypePK, name: ResourceTypeName, ownerRoleName: ResourceRoleName, reuseIds: Boolean, allowLeaving: Boolean) +final case class ResourceTypeRecord( + id: ResourceTypePK, + name: ResourceTypeName, + ownerRoleName: ResourceRoleName, + reuseIds: Boolean, + allowLeaving: Boolean, + prerequisiteAction: Option[ResourceAction] +) object ResourceTypeTable extends SQLSyntaxSupportWithDefaultSamDB[ResourceTypeRecord] { override def tableName: String = "SAM_RESOURCE_TYPE" @@ -17,7 +24,8 @@ object ResourceTypeTable extends SQLSyntaxSupportWithDefaultSamDB[ResourceTypeRe rs.get(e.name), rs.get(e.ownerRoleName), rs.get(e.reuseIds), - rs.get(e.allowLeaving) + rs.get(e.allowLeaving), + rs.get(e.prerequisiteAction) ) def pkQuery(resourceTypeName: ResourceTypeName, resourceTypeTableAlias: String = "rtt"): SQLSyntax = { diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/SamModel.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/SamModel.scala index b101ee2bd6..e8e0af2bde 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/SamModel.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/SamModel.scala @@ -135,7 +135,8 @@ object UserStatusDetails { roles: Set[ResourceRole], ownerRoleName: ResourceRoleName, reuseIds: Boolean = false, - allowLeaving: Boolean = false + allowLeaving: Boolean = false, + prerequisiteAction: Option[ResourceAction] = None ) { // Ideally we'd just store this boolean in a lazy val, but this will upset the spray/akka json serializers // I can't imagine a scenario where we have enough action patterns that would make this def discernibly slow though diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/api/SamJsonSupport.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/api/SamJsonSupport.scala index a81848129a..d6ac16ea25 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/api/SamJsonSupport.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/model/api/SamJsonSupport.scala @@ -55,7 +55,7 @@ object SamJsonSupport { implicit val ResourceRoleFormat: RootJsonFormat[ResourceRole] = jsonFormat4(ResourceRole.apply) - implicit val ResourceTypeFormat: RootJsonFormat[ResourceType] = jsonFormat6(ResourceType.apply) + implicit val ResourceTypeFormat: RootJsonFormat[ResourceType] = jsonFormat7(ResourceType.apply) implicit val SamUserFormat: RootJsonFormat[SamUser] = jsonFormat8(SamUser.apply) 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 index 76fee8cf8d..e5f6657868 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/PolicyEvaluatorService.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/PolicyEvaluatorService.scala @@ -87,7 +87,7 @@ class PolicyEvaluatorService( isConstrainable = rt.isAuthDomainConstrainable allPolicyActions <- accessPolicyDAO.listUserResourceActions(resource, userId, samRequestContext) - res <- + actionsConstrainedByAuthDomain <- if (isConstrainable) { for { authDomainsResult <- accessPolicyDAO.loadResourceAuthDomain(resource, samRequestContext) @@ -104,7 +104,15 @@ class PolicyEvaluatorService( } } yield policyActions } else allPolicyActions.pure[IO] - } yield res + } yield rt.prerequisiteAction + .map { prerequisiteAction => + if (actionsConstrainedByAuthDomain.contains(prerequisiteAction)) { + actionsConstrainedByAuthDomain + } else { + Set.empty[ResourceAction] + } + } + .getOrElse(actionsConstrainedByAuthDomain) private def isMemberOfAllAuthDomainGroups( authDomains: NonEmptyList[WorkbenchGroupName], 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 index 5ba6767af8..7447406be3 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceService.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceService.scala @@ -25,7 +25,7 @@ import scala.concurrent.ExecutionContext class ResourceService( private val resourceTypes: Map[ResourceTypeName, ResourceType], private[service] val policyEvaluatorService: PolicyEvaluatorService, - private val accessPolicyDAO: AccessPolicyDAO, + private[service] val accessPolicyDAO: AccessPolicyDAO, private val directoryDAO: DirectoryDAO, private val cloudExtensions: CloudExtensions, val emailDomain: String, @@ -1116,6 +1116,16 @@ class ResourceService( resources <- listResourcesHierarchical(userId, Set(resourceTypeName), Set.empty, Set.empty, Set.empty, true, samRequestContext) } yield resources.resources.map(toUserResourcesResponse) + private def hasPrerequisiteAction(resource: FilteredResourceHierarchical): Boolean = + resourceTypes.get(resource.resourceType).flatMap(_.prerequisiteAction).forall { prereq => + resource.policies.flatMap(p => p.actions ++ p.roles.flatMap(_.actions)).contains(prereq) + } + + private def hasPrerequisiteAction(resource: FilteredResourceFlat): Boolean = + resourceTypes.get(resource.resourceType).flatMap(_.prerequisiteAction).forall { prereq => + resource.actions.contains(prereq) + } + def listResourcesFlat( samUserId: WorkbenchUserId, resourceTypeNames: Set[ResourceTypeName], @@ -1125,7 +1135,14 @@ class ResourceService( includePublic: Boolean, samRequestContext: SamRequestContext ): IO[FilteredResourcesFlat] = - accessPolicyDAO.filterResources(samUserId, resourceTypeNames, policies, roles, actions, includePublic, samRequestContext).map(groupFlat) + accessPolicyDAO + .filterResources(samUserId, resourceTypeNames, policies, roles, actions, includePublic, samRequestContext) + .map(groupFlat) + .map { result => + result.copy( + resources = result.resources.filter(hasPrerequisiteAction) + ) + } def listResourcesHierarchical( samUserId: WorkbenchUserId, @@ -1139,4 +1156,9 @@ class ResourceService( accessPolicyDAO .filterResources(samUserId, resourceTypeNames, policies, roles, actions, includePublic, samRequestContext) .map(groupHierarchical) + .map { result => + result.copy( + resources = result.resources.filter(hasPrerequisiteAction) + ) + } } diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceSpec.scala index 5e85e19e3a..6a19133e9f 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceSpec.scala @@ -133,6 +133,14 @@ class ResourceServiceSpec private val parentResourceType = ResourceType(ResourceTypeName("parent-resource-type"), defaultResourceTypeActionPatterns, parentResourceTypeRoles, ownerRoleName) val otherParentResourceType: ResourceType = parentResourceType.copy(name = ResourceTypeName("parent-resource-type-2")) + private val prerequisiteAction = ResourceAction("prerequisite_action") + private val prereqActionResourceType = ResourceType( + genResourceTypeNameExcludeManagedGroup.sample.get, + Set.empty, + Set(ResourceRole(ownerRoleName, Set(ResourceAction("delete"), ResourceAction("view")))), + ownerRoleName, + prerequisiteAction = Option(prerequisiteAction) + ) private val constrainableActionPatterns = Set(ResourceActionPattern("constrainable_view", "Can be constrained by an auth domain", true)) private val constrainableViewAction = ResourceAction("constrainable_view") @@ -158,7 +166,8 @@ class ResourceServiceSpec parentResourceType.name -> parentResourceType, childResourceType.name -> childResourceType, managedGroupResourceType.name -> managedGroupResourceType, - otherParentResourceType.name -> otherParentResourceType + otherParentResourceType.name -> otherParentResourceType, + prereqActionResourceType.name -> prereqActionResourceType ) private val policyEvaluatorService = PolicyEvaluatorService(emailDomain, resourceTypes, policyDAO, dirDAO) private val service = new ResourceService(resourceTypes, policyEvaluatorService, policyDAO, dirDAO, NoExtensions, emailDomain, Set("test.firecloud.org")) @@ -462,6 +471,52 @@ class ResourceServiceSpec ) } + it should "return empty when caller does not have prerequisite action" in { + assume(databaseEnabled, databaseEnabledClue) + + val resourceName1 = ResourceId("resource1") + val resourceName2 = ResourceId("resource2") + + service.initResourceTypes(samRequestContext).unsafeRunSync() + val resource1 = service.createResource(prereqActionResourceType, resourceName1, dummyUser, samRequestContext).unsafeRunSync() + val resource2 = service.createResource(prereqActionResourceType, resourceName2, dummyUser, samRequestContext).unsafeRunSync() + val resource3 = service.createResource(defaultResourceType, resourceName2, dummyUser, samRequestContext).unsafeRunSync() + + policyDAO + .createPolicy( + AccessPolicy( + FullyQualifiedPolicyId(resource2.fullyQualifiedId, AccessPolicyName("prereq")), + Set(dummyUser.id), + WorkbenchEmail("a@b.c"), + Set.empty, + Set(prerequisiteAction), + Set.empty, + public = false + ), + samRequestContext + ) + .unsafeRunSync() + + // missing prerequisite action so should not have any actions + assertResult(Set.empty) { + service.policyEvaluatorService + .listUserResourceActions(resource1.fullyQualifiedId, dummyUser.id, samRequestContext = samRequestContext) + .unsafeRunSync() + } + // has prerequisite action so should have actions from owner role + assertResult(prereqActionResourceType.roles.find(_.roleName.equals(ownerRoleName)).get.actions + prerequisiteAction) { + service.policyEvaluatorService + .listUserResourceActions(resource2.fullyQualifiedId, dummyUser.id, samRequestContext = samRequestContext) + .unsafeRunSync() + } + // no prerequisite action required so should have actions from owner role + assertResult(defaultResourceType.roles.find(_.roleName.equals(ownerRoleName)).get.actions) { + service.policyEvaluatorService + .listUserResourceActions(resource3.fullyQualifiedId, dummyUser.id, samRequestContext = samRequestContext) + .unsafeRunSync() + } + } + "createResource" should "detect conflict on create" in { assume(databaseEnabled, databaseEnabledClue) diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceUnitSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceUnitSpec.scala index 2487bc3d63..2276adb6b2 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceUnitSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/ResourceServiceUnitSpec.scala @@ -221,4 +221,58 @@ class ResourceServiceUnitSpec extends AnyFlatSpec with Matchers with ScalaFuture authDomainResource.authDomainGroups should be(Set(authDomainGroup1, authDomainGroup2)) authDomainResource.missingAuthDomainGroups should be(Set(authDomainGroup2)) } + + it should "handle prerequisite action in listResources" in { + val prerequisiteAction = ResourceAction("prerequisiteAction") + val localResourceService = new ResourceService( + Map( + resourceTypeName -> ResourceType( + resourceTypeName, + Set.empty, + Set.empty, + ownerRoleName, + prerequisiteAction = Some(prerequisiteAction) + ) + ), + mock[PolicyEvaluatorService], + mock[AccessPolicyDAO], + mock[DirectoryDAO], + NoExtensions, + emailDomain, + Set("test.firecloud.org") + ) + + when( + localResourceService.accessPolicyDAO.filterResources( + any[WorkbenchUserId], + any[Set[ResourceTypeName]], + any[Set[AccessPolicyName]], + any[Set[ResourceRoleName]], + any[Set[ResourceAction]], + any[Boolean], + any[SamRequestContext] + ) + ) + .thenReturn( + IO.pure( + Seq( + FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy1), None, Some(prerequisiteAction), false, None, false, false), + FilterResourcesResult(testResourceId, resourceTypeName, Some(testPolicy1), None, Some(writeAction), false, None, false, false), + FilterResourcesResult(testResourceId2, resourceTypeName, Some(testPolicy1), None, Some(writeAction), false, None, false, false) + ) + ) + ) + + val filteredResources = + localResourceService.listResourcesFlat(dummyUser.id, Set.empty, Set.empty, Set.empty, Set.empty, false, samRequestContext).unsafeRunSync() + filteredResources.resources.size should be(1) + val oneResource = filteredResources.resources.filter(_.resourceId.equals(testResourceId)).head + oneResource.actions should be(Set(prerequisiteAction, writeAction)) + + val filteredResourcesHierarchical = + localResourceService.listResourcesHierarchical(dummyUser.id, Set.empty, Set.empty, Set.empty, Set.empty, false, samRequestContext).unsafeRunSync() + filteredResourcesHierarchical.resources.size should be(1) + val oneResourceHierarchical = filteredResourcesHierarchical.resources.filter(_.resourceId.equals(testResourceId)).head + oneResourceHierarchical.policies.head.actions should be(Set(prerequisiteAction, writeAction)) + } }