Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ID-1306 CBAS Submission, Filtering by Resource Parent, and Getting Resource Creator #1457

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,5 @@
<include file="changesets/20231019_sam_user_attributes_table.xml" relativeToChangelogFile="true"/>
<include file="changesets/20240417_action_managed_identities.xml" relativeToChangelogFile="true"/>
<include file="changesets/20240416_add_sam_rac_tables.xml" relativeToChangelogFile="true"/>
<include file="changesets/20240613_resource_created_by.xml" relativeToChangelogFile="true"/>
</databaseChangeLog>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<databaseChangeLog logicalFilePath="dummy"
xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:ext="http://www.liquibase.org/xml/ns/dbchangelog-ext"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog-ext http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-ext.xsd http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.4.xsd">

<changeSet logicalFilePath="dummy" author="tlangs" id="resource_created_by">
<addColumn tableName="sam_resource">
<column name="created_by" type="VARCHAR">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the column created_by isn't just useful for submissions. Its really odd that Sam doesn't know what user created a resource!

<constraints nullable="true"/>
</column>
</addColumn>
<addForeignKeyConstraint baseTableName="sam_resource" baseColumnNames="created_by" constraintName="FK_SR_CREATOR" referencedTableName="SAM_USER" referencedColumnNames="id" onDelete="SET NULL" />
</changeSet>

</databaseChangeLog>
29 changes: 29 additions & 0 deletions src/main/resources/reference.conf
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ resourceTypes = {
wds-instance = ["owner"]
kubernetes-app = ["manager"]
kubernetes-app-shared = ["owner", "user"]
cbas-submission = ["owner"]
}
}
application = {
Expand All @@ -188,6 +189,7 @@ resourceTypes = {
google-project = ["pet-creator"]
wds-instance = ["writer"]
kubernetes-app-shared = ["user"]
cbas-submission = ["reader"]
}
}
reader = {
Expand All @@ -198,6 +200,7 @@ resourceTypes = {
google-project = ["pet-creator"]
wds-instance = ["reader"]
kubernetes-app-shared = ["user"]
cbas-submission = ["reader"]
}
}
discoverer = {
Expand Down Expand Up @@ -1674,6 +1677,32 @@ resourceTypes = {
allowLeaving = false
reuseIds = true
}

cbas-submission = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will allow CBAS to no longer need to store Sam User IDs!

actionPatterns = {
abort = {
description = "Abort this cbas-submission"
}
read = {
description = "read information about the submission"
}
create_with_parent = {
description = "Enables creating the request object with a parent"
}
}
ownerRoleName = "owner"
roles = {
owner = {
roleActions = ["abort", "create_with_parent"]
includedRoles = ["reader"]
}
Comment on lines +1698 to +1701

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How hard is it to change these after creation if we wanted to (say) have a "canceller" role separate from the owner/creator?

reader = {
roleActions = ["read", "read_creator"]
}
}
allowLeaving = false
reuseIds = false
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will allow CBAS to no longer need to store Sam User IDs!

}


Expand Down
11 changes: 10 additions & 1 deletion src/main/resources/swagger/api-docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1739,7 +1739,7 @@ paths:
This endpoint is ONLY for determining the state of the user's permissions.
It does not take into account enabled status, or Terms of Service Compliance, nor does it take into account Auth Domains.
To check if a user is allowed to execute an action on a resource, use /api/resources/v2/{resourceTypeName}/{resourceId}/action/{action}
By default, public resources are not included. Including public resources incurs a 3x cost of DB performance.
By default, public resources are not included. Including public resources might incur a 3x cost of DB performance.
operationId: listResourcesV2
parameters:
- name: format
Expand Down Expand Up @@ -1789,6 +1789,15 @@ paths:
type: array
items:
type: string
- name: parentResources
in: query
description: Fully Qualified Resource IDs of parent resources to filter on. Formatted as `resourceTypeName:resourceId`.
required: false
explode: false
schema:
type: array
items:
type: string
- name: includePublic
in: query
description: Include public resources in the response
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,16 @@ trait ResourceRoutes extends SamUserDirectives with SecurityDirectives with SamM
}
}
}
} ~
pathPrefix("creator") {
requireAction(resource, SamResourceActions.readCreator, samUser.id, samRequestContext) {
pathEndOrSingleSlash {
complete(resourceService.getResourceCreator(resource, samRequestContext).map {
case Some(response) => StatusCodes.OK -> response.email
case None => throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, "resource creator not found"))
})
}
}
}
}
}
Expand Down Expand Up @@ -605,38 +615,65 @@ trait ResourceRoutes extends SamUserDirectives with SecurityDirectives with SamM
"policies".as[String].?,
"roles".as[String].?,
"actions".as[String].?,
"parentResources".as[String].?,
"includePublic" ? false,
"format".as[String] ? "hierarchical"
) { (resourceTypes: Option[String], policies: Option[String], roles: Option[String], actions: Option[String], includePublic: Boolean, format: String) =>
format match {
case "flat" =>
complete {
resourceService
.listResourcesFlat(
samUser.id,
resourceTypes.map(_.split(",").map(ResourceTypeName(_)).toSet).getOrElse(Set.empty),
policies.map(_.split(",").map(AccessPolicyName(_)).toSet).getOrElse(Set.empty),
roles.map(_.split(",").map(ResourceRoleName(_)).toSet).getOrElse(Set.empty),
actions.map(_.split(",").map(ResourceAction(_)).toSet).getOrElse(Set.empty),
includePublic,
samRequestContext
)
.map(StatusCodes.OK -> _)
}
case "hierarchical" =>
complete {
resourceService
.listResourcesHierarchical(
samUser.id,
resourceTypes.map(_.split(",").map(ResourceTypeName(_)).toSet).getOrElse(Set.empty),
policies.map(_.split(",").map(AccessPolicyName(_)).toSet).getOrElse(Set.empty),
roles.map(_.split(",").map(ResourceRoleName(_)).toSet).getOrElse(Set.empty),
actions.map(_.split(",").map(ResourceAction(_)).toSet).getOrElse(Set.empty),
includePublic,
samRequestContext
)
.map(StatusCodes.OK -> _)
}
}
) {
(
resourceTypes: Option[String],
policies: Option[String],
roles: Option[String],
actions: Option[String],
parentResources: Option[String],
includePublic: Boolean,
format: String
) =>
val parsedResourceTypes = resourceTypes.map(_.split(",").map(ResourceTypeName(_)).toSet).getOrElse(Set.empty)
val parsedPolicies = policies.map(_.split(",").map(AccessPolicyName(_)).toSet).getOrElse(Set.empty)
val parsedRoles = roles.map(_.split(",").map(ResourceRoleName(_)).toSet).getOrElse(Set.empty)
val parsedActions = actions.map(_.split(",").map(ResourceAction(_)).toSet).getOrElse(Set.empty)
val parsedParentResourceIds = parentResources
Comment on lines +631 to +635
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a little cleanup here to keep our code DRY

.map(
_.split(",")
.map { r =>
val splitted = r.split(":")
FullyQualifiedResourceId(ResourceTypeName(splitted(0)), ResourceId(splitted(1)))
}
.toSet
)
.getOrElse(Set.empty)
format match {
case "flat" =>
complete {
resourceService
.listResourcesFlat(
samUser.id,
parsedResourceTypes,
parsedPolicies,
parsedRoles,
parsedActions,
parsedParentResourceIds,
includePublic,
samRequestContext
)
.map(StatusCodes.OK -> _)
}
case "hierarchical" =>
complete {
resourceService
.listResourcesHierarchical(
samUser.id,
parsedResourceTypes,
parsedPolicies,
parsedRoles,
parsedActions,
parsedParentResourceIds,
includePublic,
samRequestContext
)
.map(StatusCodes.OK -> _)
}
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ trait UserRoutesV2 extends SamUserDirectives with SamRequestContextDirectives {
Set.empty,
Set(ResourceRoleName("user")),
Set.empty,
Set.empty,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Listing all user resources doesn't need to filter on parent, so an empty set is provided here.

includePublic = false,
samRequestContext
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,13 @@ trait AccessPolicyDAO {
policies: Set[AccessPolicyName],
roles: Set[ResourceRoleName],
actions: Set[ResourceAction],
parentResourceIds: Set[FullyQualifiedResourceId],
includePublic: Boolean,
samRequestContext: SamRequestContext
): IO[Seq[FilterResourcesResult]]

def getResourceCreator(resource: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[Option[WorkbenchUserId]]

}

sealed abstract class LoadResourceAuthDomainResult
Expand Down
Loading
Loading