diff --git a/env/local.env b/env/local.env index d35ac44bb..ade39d250 100644 --- a/env/local.env +++ b/env/local.env @@ -5,7 +5,7 @@ export ADMIN_SERVICE_ACCOUNT_4="src/main/resources/rendered/admin-service-accoun export ADMIN_SERVICE_ACCOUNT_5="src/main/resources/rendered/admin-service-account-5.json" export SERVICE_ACCOUNT_ADMINS="service-admin@dev.test.firecloud.org, service-admin2@dev.test.firecloud.org" export AZURE_ENABLED="false" -export AZURE_SERVICE_CATALOG_APPS_ENABLED="false" +export AZURE_SERVICE_CATALOG_ENABLED="false" export AZURE_MANAGED_APP_WORKLOAD_CLIENT_ID="661e243c-5ef9-4a9c-9be3-b7f5585828b3" export EMAIL_DOMAIN="dev.test.firecloud.org" export ENVIRONMENT="dev" diff --git a/env/test.env b/env/test.env index d3125fd67..176fdd6df 100644 --- a/env/test.env +++ b/env/test.env @@ -6,7 +6,7 @@ export AZURE_MANAGED_APP_CLIENT_SECRET="foo" export AZURE_MANAGED_APP_TENANT_ID="foo" export AZURE_MANAGED_APP_WORKLOAD_CLIENT_ID="foo" export AZURE_ALLOW_MANAGED_IDENTITY_USER_CREATION="false" -export AZURE_SERVICE_CATALOG_APPS_ENABLED="false" +export AZURE_SERVICE_CATALOG_ENABLED="false" export EMAIL_DOMAIN="dev.test.firecloud.org" export ENVIRONMENT="local" export GOOGLE_APPS_DOMAIN="test.firecloud.org" diff --git a/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/provider/SamProviderSpec.scala b/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/provider/SamProviderSpec.scala index e972c241c..e924f93ab 100644 --- a/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/provider/SamProviderSpec.scala +++ b/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/provider/SamProviderSpec.scala @@ -137,7 +137,7 @@ class SamProviderSpec when { googleExt.getArbitraryPetServiceAccountToken(any[SamUser], any[Set[String]], any[SamRequestContext]) } thenReturn { - Future.successful("aToken") + IO.pure("aToken") } ) } yield () 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 17575edf0..6239c4543 100644 --- a/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changelog.xml +++ b/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changelog.xml @@ -32,4 +32,5 @@ + diff --git a/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changesets/20240820_descendant_auth_domains.xml b/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changesets/20240820_descendant_auth_domains.xml new file mode 100644 index 000000000..97a57d865 --- /dev/null +++ b/src/main/resources/org/broadinstitute/dsde/sam/liquibase/changesets/20240820_descendant_auth_domains.xml @@ -0,0 +1,27 @@ + + + + + + with recursive + ancestor_resource(resource_id, group_id) as ( + select ad.resource_id, ad.group_id + from sam_resource_auth_domain ad + union + select childResource.id, ancestorResource.group_id + from SAM_RESOURCE childResource + join ancestor_resource ancestorResource on ancestorResource.resource_id = childResource.resource_parent_id + ) + + insert into sam_resource_auth_domain(resource_id, group_id) + select ar.resource_id, ar.group_id + from ancestor_resource ar + on conflict do nothing + + + + diff --git a/src/main/resources/swagger/api-docs.yaml b/src/main/resources/swagger/api-docs.yaml index eb96ece53..432f2ccc8 100755 --- a/src/main/resources/swagger/api-docs.yaml +++ b/src/main/resources/swagger/api-docs.yaml @@ -1109,6 +1109,61 @@ paths: application/json: schema: $ref: '#/components/schemas/ErrorReport' + /api/google/v1/petServiceAccount/{project}/{email}/token: + post: + tags: + - Google + summary: gets a token for the user's pet service account, get_pet_private_key + action on cloud-extension/google required + operationId: getUserPetServiceAccountToken + parameters: + - name: project + in: path + description: Google project of the pet + required: true + schema: + type: string + - name: email + in: path + description: User's email address + required: true + schema: + type: string + requestBody: + description: Scopes for the token + content: + 'application/json': + schema: + $ref: '#/components/schemas/ArrayOfScopes' + required: true + responses: + 200: + description: an access token for the users pet service account + content: + application/json: + schema: + type: string + 403: + description: caller has some access to cloud-extension/google but not to + the get_pet_private_key action + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorReport' + 404: + description: user does not exist or caller does not have any access to cloud-extension/google + resource + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorReport' + 500: + description: Internal Server Error + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorReport' + x-codegen-request-body-name: scopes /api/google/v1/petServiceAccount/{email}/key: get: tags: @@ -1149,6 +1204,55 @@ paths: application/json: schema: $ref: '#/components/schemas/ErrorReport' + /api/google/v1/petServiceAccount/{email}/token: + post: + tags: + - Google + summary: gets a token for the user's arbitrary pet service account, get_pet_private_key + action on cloud-extension/google required + operationId: getUserArbitraryPetServiceAccountToken + parameters: + - name: email + in: path + description: User's email address + required: true + schema: + type: string + requestBody: + description: Scopes for the token + content: + 'application/json': + schema: + $ref: '#/components/schemas/ArrayOfScopes' + required: true + responses: + 200: + description: an access token for the users pet service account + content: + application/json: + schema: + type: string + 403: + description: caller has some access to cloud-extension/google but not to + the get_pet_private_key action + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorReport' + 404: + description: user does not exist or caller does not have any access to cloud-extension/google + resource + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorReport' + 500: + description: Internal Server Error + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorReport' + x-codegen-request-body-name: scopes /api/google/v1/user/signedUrlForBlob: post: tags: @@ -1802,8 +1906,8 @@ paths: tags: - Resources summary: List resources the calling user has been granted policies/roles/actions on. - This endpoint MUST NOT be used to determine if a user has an action on a resource. - This endpoint is ONLY for determining the state of the user's permissions. + This endpoint MUST NOT be used to determine if a user has an action on a resource. + 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. @@ -4518,7 +4622,7 @@ components: type: boolean description: true if there is a rolling acceptance window active. This means that users who have not accepted the latest terms of service - version (and who have accepted the previous version) will be allowed to use the system + version (and who have accepted the previous version) will be allowed to use the system for a period of time after the terms of service have been updated. UserTermsOfServiceDetails: required: 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 index 82cf8e141..5f17cf02e 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/ResourceRoutes.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/ResourceRoutes.scala @@ -408,7 +408,7 @@ trait ResourceRoutes extends SamUserDirectives with SecurityDirectives with SamM patchWithTelemetry(samRequestContext, resourceParams(resource): _*) { requireAction(resource, SamResourceActions.updateAuthDomain, samUser.id, samRequestContext) { entity(as[Set[WorkbenchGroupName]]) { authDomains => - complete(resourceService.addResourceAuthDomain(resource, authDomains, samUser.id, samRequestContext).map { response => + complete(resourceService.addResourceAuthDomain(resource, authDomains, Option(samUser.id), samRequestContext).map { response => StatusCodes.OK -> response }) } diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutes.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutes.scala index 641e73f6d..b1d08eced 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutes.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutes.scala @@ -42,27 +42,13 @@ trait GoogleExtensionRoutes extends ExtensionRoutes with SamUserDirectives with samUser.id, samRequestContext ) { - path(Segment / "key") { userEmail => + pathPrefix(Segment) { userEmail => val email = WorkbenchEmail(userEmail) - getWithTelemetry(samRequestContext, "userEmail" -> email) { - complete { - import spray.json._ - googleExtensions.getArbitraryPetServiceAccountKey(email, samRequestContext) map { - // parse json to ensure it is json and tells akka http the right content-type - case Some(key) => StatusCodes.OK -> key.parseJson - case None => - throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, "pet service account not found")) - } - } - } - } ~ - path(Segment / Segment) { (project, userEmail) => - val email = WorkbenchEmail(userEmail) - val googleProject = GoogleProject(project) - getWithTelemetry(samRequestContext, "userEmail" -> email, "googleProject" -> googleProject) { + path("key") { + getWithTelemetry(samRequestContext, "userEmail" -> email) { complete { import spray.json._ - googleExtensions.getPetServiceAccountKey(email, googleProject, samRequestContext) map { + googleExtensions.getArbitraryPetServiceAccountKey(email, samRequestContext) map { // parse json to ensure it is json and tells akka http the right content-type case Some(key) => StatusCodes.OK -> key.parseJson case None => @@ -70,6 +56,50 @@ trait GoogleExtensionRoutes extends ExtensionRoutes with SamUserDirectives with } } } + } ~ + path("token") { + postWithTelemetry(samRequestContext, "userEmail" -> email) { + entity(as[Set[String]]) { scopes => + complete { + googleExtensions.getArbitraryPetServiceAccountToken(email, scopes, samRequestContext).map { + case Some(token) => StatusCodes.OK -> JsString(token) + case None => + throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, "pet service account not found")) + } + } + } + } + } + } ~ + pathPrefix(Segment / Segment) { (project, userEmail) => + val email = WorkbenchEmail(userEmail) + val googleProject = GoogleProject(project) + pathEndOrSingleSlash { + getWithTelemetry(samRequestContext, "userEmail" -> email, "googleProject" -> googleProject) { + complete { + import spray.json._ + googleExtensions.getPetServiceAccountKey(email, googleProject, samRequestContext) map { + // parse json to ensure it is json and tells akka http the right content-type + case Some(key) => StatusCodes.OK -> key.parseJson + case None => + throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, "pet service account not found")) + } + } + } + } ~ + path("token") { + postWithTelemetry(samRequestContext, "userEmail" -> email) { + entity(as[Set[String]]) { scopes => + complete { + googleExtensions.getPetServiceAccountToken(email, googleProject, scopes, samRequestContext).map { + case Some(token) => StatusCodes.OK -> JsString(token) + case None => + throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, "pet service account not found")) + } + } + } + } + } } } } ~ diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensions.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensions.scala index a90e54e4d..518736e39 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensions.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensions.scala @@ -455,30 +455,55 @@ class GoogleExtensions( key <- googleKeyCache.getKey(pet) } yield key - def getPetServiceAccountToken(user: SamUser, project: GoogleProject, scopes: Set[String], samRequestContext: SamRequestContext): Future[String] = - getPetServiceAccountKey(user, project, samRequestContext).unsafeToFuture().flatMap { key => - getAccessTokenUsingJson(key, scopes) + def getPetServiceAccountToken(user: SamUser, project: GoogleProject, scopes: Set[String], samRequestContext: SamRequestContext): IO[String] = + getPetServiceAccountKey(user, project, samRequestContext).flatMap { key => + IO.fromFuture(IO(getAccessTokenUsingJson(key, scopes))) } + def getPetServiceAccountToken( + userEmail: WorkbenchEmail, + project: GoogleProject, + scopes: Set[String], + samRequestContext: SamRequestContext + ): IO[Option[String]] = + for { + subject <- directoryDAO.loadSubjectFromEmail(userEmail, samRequestContext) + token <- subject match { + case Some(userId: WorkbenchUserId) => + getPetServiceAccountToken(SamUser(userId, None, userEmail, None, false), project, scopes, samRequestContext).map(Option(_)) + case _ => IO.pure(None) + } + } yield token + def getArbitraryPetServiceAccountKey(userEmail: WorkbenchEmail, samRequestContext: SamRequestContext): IO[Option[String]] = for { subject <- directoryDAO.loadSubjectFromEmail(userEmail, samRequestContext) key <- subject match { case Some(userId: WorkbenchUserId) => - IO.fromFuture(IO(getArbitraryPetServiceAccountKey(SamUser(userId, None, userEmail, None, false), samRequestContext))).map(Option(_)) + getArbitraryPetServiceAccountKey(SamUser(userId, None, userEmail, None, false), samRequestContext).map(Option(_)) case _ => IO.none } } yield key - def getArbitraryPetServiceAccountKey(user: SamUser, samRequestContext: SamRequestContext): Future[String] = - getDefaultServiceAccountForShellProject(user, samRequestContext) + def getArbitraryPetServiceAccountKey(user: SamUser, samRequestContext: SamRequestContext): IO[String] = + IO.fromFuture(IO(getDefaultServiceAccountForShellProject(user, samRequestContext))) + + def getArbitraryPetServiceAccountToken(userEmail: WorkbenchEmail, scopes: Set[String], samRequestContext: SamRequestContext): IO[Option[String]] = + for { + subject <- directoryDAO.loadSubjectFromEmail(userEmail, samRequestContext) + token <- subject match { + case Some(userId: WorkbenchUserId) => + getArbitraryPetServiceAccountToken(SamUser(userId, None, userEmail, None, false), scopes, samRequestContext).map(Option(_)) + case _ => IO.none + } + } yield token - def getArbitraryPetServiceAccountToken(user: SamUser, scopes: Set[String], samRequestContext: SamRequestContext): Future[String] = + def getArbitraryPetServiceAccountToken(user: SamUser, scopes: Set[String], samRequestContext: SamRequestContext): IO[String] = getArbitraryPetServiceAccountKey(user, samRequestContext).flatMap { key => - getAccessTokenUsingJson(key, scopes) + IO.fromFuture(IO(getAccessTokenUsingJson(key, scopes))) } - private def getDefaultServiceAccountForShellProject(user: SamUser, samRequestContext: SamRequestContext): Future[String] = { + private[google] def getDefaultServiceAccountForShellProject(user: SamUser, samRequestContext: SamRequestContext): Future[String] = { val projectName = s"fc-${googleServicesConfig.environment.substring(0, Math.min(googleServicesConfig.environment.length(), 5))}-${user.id.value}" // max 30 characters. subject ID is 21 for { @@ -690,7 +715,7 @@ class GoogleExtensions( val bucket = GcsBucketName(blobId.getBucket) val objectName = GcsBlobName(blobId.getName) for { - petKey <- IO.fromFuture(IO(getArbitraryPetServiceAccountKey(samUser, samRequestContext))) + petKey <- getArbitraryPetServiceAccountKey(samUser, samRequestContext) serviceAccountCredentials = ServiceAccountCredentials.fromStream(new ByteArrayInputStream(petKey.getBytes())) url <- getSignedUrl(samUser, bucket, objectName, duration, urlParamsMap, serviceAccountCredentials) } yield url 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 39ff9b3a5..c7adfe304 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 @@ -184,7 +184,8 @@ class ResourceService( } /** This method only persists the resource and then overwrites/creates the policies for that resource. Be very careful if calling this method directly because - * it will not validate the resource or its policies. If you want to create a Resource, use createResource() which will also perform critical validations + * it will not validate the resource or its policies. If you want to create a Resource, use createResource() which will also perform critical validations. If + * the parent has an auth domain, it will be added the auth domain of the child. * * @param resourceType * @param resourceId @@ -202,7 +203,13 @@ class ResourceService( samRequestContext: SamRequestContext ) = { val accessPolicies = policies.map(constructAccessPolicy(resourceType, resourceId, _, public = false)) // can't set public at create time - accessPolicyDAO.createResource(Resource(resourceType.name, resourceId, authDomain, accessPolicies = accessPolicies, parent = parentOpt), samRequestContext) + for { + inheritedAuthDomains <- parentOpt.map(loadResourceAuthDomain(_, samRequestContext)).getOrElse(IO.pure(Set.empty)) + resource <- accessPolicyDAO.createResource( + Resource(resourceType.name, resourceId, inheritedAuthDomains ++ authDomain, accessPolicies = accessPolicies, parent = parentOpt), + samRequestContext + ) + } yield resource } private def constructAccessPolicy(resourceType: ResourceType, resourceId: ResourceId, validatableAccessPolicy: ValidatableAccessPolicy, public: Boolean) = @@ -230,7 +237,8 @@ class ResourceService( ownerPolicyErrors <- IO.pure(validateOwnerPolicyExists(resourceType, policies, parentOpt)) policyErrors <- policies.toList.traverse(policy => validatePolicy(resourceType, resourceId, policy)).map(_.flatten) authDomainErrors <- validateAuthDomain(resourceType, authDomain, userId, samRequestContext) - } yield (resourceIdErrors ++ ownerPolicyErrors ++ policyErrors ++ authDomainErrors).toSeq + childAuthDomainErrors <- validateChildAuthDomain(authDomain, parentOpt, samRequestContext) + } yield (resourceIdErrors ++ ownerPolicyErrors ++ policyErrors ++ authDomainErrors ++ childAuthDomainErrors).toSeq private val validUrlSafePattern = "[-a-zA-Z0-9._~%]+".r @@ -278,6 +286,23 @@ class ResourceService( } else None } + /** If an auth domain is specified for a child resource, it must contain all of the groups of the parent auth domain. Containing additional groups is allowed. + */ + private def validateChildAuthDomain( + childAuthDomain: Set[WorkbenchGroupName], + parentOpt: Option[FullyQualifiedResourceId], + samRequestContext: SamRequestContext + ): IO[Option[ErrorReport]] = + parentOpt + .traverse { parent => + loadResourceAuthDomain(parent, samRequestContext).map { parentAuthDomain => + if (childAuthDomain.nonEmpty && !parentAuthDomain.forall(childAuthDomain.contains)) { + Option(ErrorReport("Child resource auth domain must contain all of the groups of the parent auth domain")) + } else None + } + } + .map(_.flatten) + private def validateAuthDomainPermissions( authDomain: Set[WorkbenchGroupName], userId: WorkbenchUserId, @@ -320,27 +345,62 @@ class ResourceService( } } map { listOfUsePermissions => listOfUsePermissions.isEmpty || listOfUsePermissions.forall(identity) } + /** Adds groups to a resource's auth domain. If the resource is a parent, the auth domain of all children will be updated as well. + * @param resource + * the resource to add the auth domain groups to + * @param authDomains + * groups to add to the auth domain + * @param userId + * optional, if provided, the user must have access to the new auth domain groups + * @param samRequestContext + * @return + * the complete new auth domain of the resource + */ def addResourceAuthDomain( resource: FullyQualifiedResourceId, authDomains: Set[WorkbenchGroupName], - userId: WorkbenchUserId, + userId: Option[WorkbenchUserId], samRequestContext: SamRequestContext ): IO[Set[WorkbenchGroupName]] = for { resourceType <- getResourceType(resource.resourceTypeName) - _ <- validateAuthDomain(resourceType.get, authDomains, userId, samRequestContext) - accessPolicies <- accessPolicyDAO.listAccessPolicies(resource, samRequestContext) - _ <- - if (accessPolicies.exists(_.public)) { - IO.raiseError(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.BadRequest, "Cannot add an auth domain group to a public resource"))) - } else IO.unit - policies <- listResourcePolicies(resource, samRequestContext) - _ <- accessPolicyDAO.addResourceAuthDomain(resource, authDomains, samRequestContext) - _ <- policies.traverse(p => directoryDAO.updateGroupUpdatedDateAndVersionWithSession(FullyQualifiedPolicyId(resource, p.policyName), samRequestContext)) - _ <- cloudExtensions.onGroupUpdate(policies.map(p => FullyQualifiedPolicyId(resource, p.policyName)), Set.empty, samRequestContext) + error <- userId.map(validateAuthDomain(resourceType.get, authDomains, _, samRequestContext)).getOrElse(IO.none) + _ <- IO.raiseWhen(error.isDefined)(new WorkbenchExceptionWithErrorReport(error.get.copy(statusCode = Option(StatusCodes.BadRequest)))) + resourceAndDescendants <- listResourceAndDescendants(resource, samRequestContext) + _ <- resourceAndDescendants.traverse(validateNoPublicPolicies(_, samRequestContext)) + _ <- resourceAndDescendants.traverse(accessPolicyDAO.addResourceAuthDomain(_, authDomains, samRequestContext)) + + // sync groups because new auth domain can change group membership + policies <- resourceAndDescendants.traverse(accessPolicyDAO.listAccessPolicies(_, samRequestContext)).map(_.flatten) + _ <- policies.traverse(p => directoryDAO.updateGroupUpdatedDateAndVersionWithSession(p.id, samRequestContext)) + _ <- cloudExtensions.onGroupUpdate(policies.map(_.id), Set.empty, samRequestContext) authDomains <- loadResourceAuthDomain(resource, samRequestContext) } yield authDomains + private def validateNoPublicPolicies(resource: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[Unit] = + accessPolicyDAO.listAccessPolicies(resource, samRequestContext).map { policies => + if (policies.exists(_.public)) { + throw new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.BadRequest, s"Cannot add an auth domain group to a public resource $resource")) + } + } + + /** List the resource and all of its descendants. Recursively calls accessPolicyDAO.listResourceChildren. An alternate implementation could be to use a + * recursive query in the database but this is a little more concise and easier to understand at the expense of more db queries. But there are not likely to + * be huge depths of hierarchical resources and this is a seldom called function. + * + * @param resource + * @return + * the resource and all of its descendants + */ + private def listResourceAndDescendants(resource: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[List[FullyQualifiedResourceId]] = + accessPolicyDAO.listResourceChildren(resource, samRequestContext).flatMap { children => + children.toList + .traverse { child => + listResourceAndDescendants(child, samRequestContext) + } + .map(_.flatten.appended(resource)) + } + @VisibleForTesting def createPolicy( policyIdentity: FullyQualifiedPolicyId, @@ -916,32 +976,12 @@ class ResourceService( def getResourceParent(resourceId: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[Option[FullyQualifiedResourceId]] = accessPolicyDAO.getResourceParent(resourceId, samRequestContext) - /** In this iteration of hierarchical resources, we do not allow child resources to be in an auth domain because it would introduce additional complications - * when keeping Sam policies with their Google Groups. For more details, see - * https://docs.google.com/document/d/10qGxsV9BeM6-N_Zk27_JIayE509B8LUQBGiGrqB0taY/edit#heading=h.dxz6xjtnz9la - */ def setResourceParent(childResource: FullyQualifiedResourceId, parentResource: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[Unit] = for { - authDomain <- accessPolicyDAO.loadResourceAuthDomain(childResource, samRequestContext) - _ <- authDomain match { - case LoadResourceAuthDomainResult.NotConstrained => - for { - _ <- accessPolicyDAO.setResourceParent(childResource, parentResource, samRequestContext) - _ <- AuditLogger.logAuditEventIO(samRequestContext, ResourceEvent(ResourceParentUpdated, childResource, Set(ResourceChange(parentResource)))) - } yield () - case LoadResourceAuthDomainResult.Constrained(_) => - IO.raiseError( - new WorkbenchExceptionWithErrorReport( - ErrorReport(StatusCodes.BadRequest, "Cannot set the parent for a constrained resource") - ) - ) - case LoadResourceAuthDomainResult.ResourceNotFound => - IO.raiseError( - new WorkbenchExceptionWithErrorReport( - ErrorReport(StatusCodes.NotFound, "Resource not found") - ) - ) - } + parentAuthDomain <- loadResourceAuthDomain(parentResource, samRequestContext) + _ <- IO.whenA(parentAuthDomain.nonEmpty)(addResourceAuthDomain(childResource, parentAuthDomain, None, samRequestContext).void) + _ <- accessPolicyDAO.setResourceParent(childResource, parentResource, samRequestContext) + _ <- AuditLogger.logAuditEventIO(samRequestContext, ResourceEvent(ResourceParentUpdated, childResource, Set(ResourceChange(parentResource)))) } yield () def deleteResourceParent(resourceId: FullyQualifiedResourceId, samRequestContext: SamRequestContext): IO[Boolean] = diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/NewGoogleExtensionsSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/NewGoogleExtensionsSpec.scala index c25faefbe..918f47f99 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/NewGoogleExtensionsSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/NewGoogleExtensionsSpec.scala @@ -4,34 +4,38 @@ import akka.actor.ActorSystem import akka.testkit.TestKit import cats.effect.IO import com.google.auth.oauth2.ServiceAccountCredentials +import fs2.Stream import org.broadinstitute.dsde.workbench.RetryConfig import org.broadinstitute.dsde.workbench.dataaccess.NotificationDAO import org.broadinstitute.dsde.workbench.google.{GoogleDirectoryDAO, GoogleIamDAO, GoogleKmsService, GoogleProjectDAO, GooglePubSubDAO, GoogleStorageDAO} import org.broadinstitute.dsde.workbench.google2.{GcsBlobName, GoogleStorageService} import org.broadinstitute.dsde.workbench.model.TraceId +import org.broadinstitute.dsde.workbench.model.google.GcsBucketName import org.broadinstitute.dsde.workbench.sam.Generator.{ genFirecloudEmail, genGcsBlobName, genGcsBucketName, genGoogleProject, + genNonPetEmail, + genOAuth2BearerToken, genPetServiceAccount, - genWorkbenchUserGoogle + genWorkbenchUserGoogle, + genWorkbenchUserId } import org.broadinstitute.dsde.workbench.sam.TestSupport import org.broadinstitute.dsde.workbench.sam.dataAccess.{AccessPolicyDAO, DirectoryDAO, PostgresDistributedLockDAO} import org.broadinstitute.dsde.workbench.sam.mock.RealKeyMockGoogleIamDAO +import org.broadinstitute.dsde.workbench.sam.model.api.SamUser import org.broadinstitute.dsde.workbench.sam.model.{ResourceType, ResourceTypeName} import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext -import org.mockito.IdiomaticMockito -import org.mockito.Mockito.{RETURNS_SMART_NULLS, doReturn} -import fs2.Stream -import org.broadinstitute.dsde.workbench.model.google.GcsBucketName -import org.scalatest.{Inside, OptionValues} -import org.scalatest.concurrent.ScalaFutures -import org.scalatest.matchers.should.Matchers import org.mockito.ArgumentMatchersSugar._ +import org.mockito.IdiomaticMockito +import org.mockito.Mockito.{RETURNS_SMART_NULLS, clearInvocations, doReturn, verifyNoInteractions} import org.mockito.MockitoSugar.verify +import org.scalatest.concurrent.ScalaFutures import org.scalatest.freespec.AnyFreeSpecLike +import org.scalatest.matchers.should.Matchers +import org.scalatest.{Inside, OptionValues} import java.net.URL import java.util.concurrent.TimeUnit @@ -166,7 +170,7 @@ class NewGoogleExtensionsSpec(_system: ActorSystem) val arbitraryPetServiceAccount = genPetServiceAccount.sample.get val arbitraryPetServiceAccountKey = RealKeyMockGoogleIamDAO.generateNewRealKey(arbitraryPetServiceAccount.serviceAccount.email)._2 - doReturn(Future.successful(arbitraryPetServiceAccountKey)) + doReturn(IO.pure(arbitraryPetServiceAccountKey)) .when(googleExtensions) .getArbitraryPetServiceAccountKey(eqTo(newGoogleUser), any[SamRequestContext]) @@ -206,4 +210,168 @@ class NewGoogleExtensionsSpec(_system: ActorSystem) } } } + "GoogleExtensions: Pet Service Accounts" - { + val subject = genWorkbenchUserId.sample.get + val email = genNonPetEmail.sample.get + val user = SamUser(subject, None, email, None, false) + val googleProject = genGoogleProject.sample.get + val scopes = Set("scope1", "scope2") + val petServiceAccount = genPetServiceAccount.sample.get + val expectedKey = RealKeyMockGoogleIamDAO.generateNewRealKey(petServiceAccount.serviceAccount.email)._2 + val expectedToken = genOAuth2BearerToken.sample.get.token + + val mockDirectoryDAO = mock[DirectoryDAO](RETURNS_SMART_NULLS) + val mockGoogleKeyCache = mock[GoogleKeyCache](RETURNS_SMART_NULLS) + + val googleExtensions: GoogleExtensions = spy( + new GoogleExtensions( + mock[PostgresDistributedLockDAO[IO]](RETURNS_SMART_NULLS), + mockDirectoryDAO, + mock[AccessPolicyDAO](RETURNS_SMART_NULLS), + mock[GoogleDirectoryDAO](RETURNS_SMART_NULLS), + mock[GooglePubSubDAO](RETURNS_SMART_NULLS), + mock[GooglePubSubDAO](RETURNS_SMART_NULLS), + mock[GooglePubSubDAO](RETURNS_SMART_NULLS), + mock[GoogleIamDAO](RETURNS_SMART_NULLS), + mock[GoogleStorageDAO](RETURNS_SMART_NULLS), + mock[GoogleProjectDAO](RETURNS_SMART_NULLS), + mockGoogleKeyCache, + mock[NotificationDAO](RETURNS_SMART_NULLS), + mock[GoogleKmsService[IO]](RETURNS_SMART_NULLS), + mock[GoogleStorageService[IO]](RETURNS_SMART_NULLS), + TestSupport.googleServicesConfig, + TestSupport.petServiceAccountConfig, + Map.empty[ResourceTypeName, ResourceType], + genFirecloudEmail.sample.get + ) + ) + + doReturn(IO.some(subject)) + .when(mockDirectoryDAO) + .loadSubjectFromEmail(eqTo(email), any[SamRequestContext]) + + doReturn(IO.pure(petServiceAccount)) + .when(googleExtensions) + .createUserPetServiceAccount(eqTo(user), eqTo(googleProject), any[SamRequestContext]) + + doReturn(Future.successful(expectedToken)) + .when(googleExtensions) + .getAccessTokenUsingJson(eqTo(expectedKey), eqTo(scopes)) + + doReturn(Future.successful(expectedKey)) + .when(googleExtensions) + .getDefaultServiceAccountForShellProject(eqTo(user), any[SamRequestContext]) + + doReturn(IO.pure(expectedKey)) + .when(mockGoogleKeyCache) + .getKey(eqTo(petServiceAccount)) + + "getPetServiceAccountKey" - { + "gets a key for an email" in { + clearInvocations(mockDirectoryDAO, googleExtensions, mockGoogleKeyCache) + + val key = runAndWait(googleExtensions.getPetServiceAccountKey(email, googleProject, samRequestContext)) + + key should be(Some(expectedKey)) + + verify(mockDirectoryDAO).loadSubjectFromEmail(eqTo(email), any[SamRequestContext]) + verify(googleExtensions).createUserPetServiceAccount(eqTo(user), eqTo(googleProject), any[SamRequestContext]) + verify(mockGoogleKeyCache).getKey(eqTo(petServiceAccount)) + } + + "gets a key for a SamUser" in { + clearInvocations(mockDirectoryDAO, googleExtensions, mockGoogleKeyCache) + + val key = runAndWait(googleExtensions.getPetServiceAccountKey(user, googleProject, samRequestContext)) + + key should be(expectedKey) + + verifyNoInteractions(mockDirectoryDAO) + verify(googleExtensions).createUserPetServiceAccount(eqTo(user), eqTo(googleProject), any[SamRequestContext]) + verify(mockGoogleKeyCache).getKey(eqTo(petServiceAccount)) + } + + "getPetServiceAccountToken" - { + "gets a token for an email" in { + clearInvocations(mockDirectoryDAO, googleExtensions, mockGoogleKeyCache) + + val token = runAndWait(googleExtensions.getPetServiceAccountToken(email, googleProject, scopes, samRequestContext)) + + token should be(Some(expectedToken)) + + verify(mockDirectoryDAO).loadSubjectFromEmail(eqTo(email), any[SamRequestContext]) + verify(googleExtensions).createUserPetServiceAccount(eqTo(user), eqTo(googleProject), any[SamRequestContext]) + verify(mockGoogleKeyCache).getKey(eqTo(petServiceAccount)) + verify(googleExtensions).getAccessTokenUsingJson(eqTo(expectedKey), eqTo(scopes)) + + } + "gets a token for a SamUser" in { + clearInvocations(mockDirectoryDAO, googleExtensions, mockGoogleKeyCache) + + val token = runAndWait(googleExtensions.getPetServiceAccountToken(user, googleProject, scopes, samRequestContext)) + + token should be(expectedToken) + + verifyNoInteractions(mockDirectoryDAO) + verify(googleExtensions).createUserPetServiceAccount(eqTo(user), eqTo(googleProject), any[SamRequestContext]) + verify(mockGoogleKeyCache).getKey(eqTo(petServiceAccount)) + verify(googleExtensions).getAccessTokenUsingJson(eqTo(expectedKey), eqTo(scopes)) + } + } + + "getArbitraryPetServiceAccountKey" - { + "gets a key for an email" in { + clearInvocations(mockDirectoryDAO, googleExtensions, mockGoogleKeyCache) + + val key = runAndWait(googleExtensions.getArbitraryPetServiceAccountKey(email, samRequestContext)) + + key should be(Some(expectedKey)) + + verify(mockDirectoryDAO).loadSubjectFromEmail(eqTo(email), any[SamRequestContext]) + verifyNoInteractions(mockGoogleKeyCache) + verify(googleExtensions).getDefaultServiceAccountForShellProject(eqTo(user), any[SamRequestContext]) + } + + "gets a key for a SamUser" in { + clearInvocations(mockDirectoryDAO, googleExtensions, mockGoogleKeyCache) + + val key = runAndWait(googleExtensions.getArbitraryPetServiceAccountKey(user, samRequestContext)) + + key should be(expectedKey) + + verifyNoInteractions(mockDirectoryDAO) + verifyNoInteractions(mockGoogleKeyCache) + verify(googleExtensions).getDefaultServiceAccountForShellProject(eqTo(user), any[SamRequestContext]) + } + } + + "getArbitraryPetServiceAccountToken" - { + "gets a token for an email" in { + clearInvocations(mockDirectoryDAO, googleExtensions, mockGoogleKeyCache) + + val token = runAndWait(googleExtensions.getArbitraryPetServiceAccountToken(email, scopes, samRequestContext)) + + token should be(Some(expectedToken)) + + verify(mockDirectoryDAO).loadSubjectFromEmail(eqTo(email), any[SamRequestContext]) + verifyNoInteractions(mockGoogleKeyCache) + verify(googleExtensions).getDefaultServiceAccountForShellProject(eqTo(user), any[SamRequestContext]) + verify(googleExtensions).getAccessTokenUsingJson(eqTo(expectedKey), eqTo(scopes)) + } + + "gets a token for a SamUser" in { + clearInvocations(mockDirectoryDAO, googleExtensions, mockGoogleKeyCache) + + val token = runAndWait(googleExtensions.getArbitraryPetServiceAccountToken(user, scopes, samRequestContext)) + + token should be(expectedToken) + + verifyNoInteractions(mockDirectoryDAO) + verifyNoInteractions(mockGoogleKeyCache) + verify(googleExtensions).getDefaultServiceAccountForShellProject(eqTo(user), any[SamRequestContext]) + verify(googleExtensions).getAccessTokenUsingJson(eqTo(expectedKey), eqTo(scopes)) + } + } + } + } } 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 a40b51f3d..081d1c80e 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 @@ -4,6 +4,7 @@ import akka.http.scaladsl.model.StatusCodes import cats.data.NonEmptyList import cats.effect.IO import cats.effect.unsafe.implicits.{global => globalEc} +import cats.implicits.toTraverseOps import ch.qos.logback.classic.spi.ILoggingEvent import ch.qos.logback.classic.{Level, Logger} import ch.qos.logback.core.read.ListAppender @@ -52,7 +53,6 @@ import org.slf4j.LoggerFactory import java.util.UUID import scala.concurrent.ExecutionContext.Implicits.global -import scala.concurrent.duration._ import scala.jdk.CollectionConverters._ import scala.util.Random @@ -956,6 +956,129 @@ class ResourceServiceSpec fryAccess shouldEqual false } + it should "inherit parent auth domains" in { + assume(databaseEnabled, databaseEnabledClue) + + constrainableResourceType.isAuthDomainConstrainable shouldEqual true + constrainableService.createResourceType(constrainableResourceType, samRequestContext).unsafeRunSync() + constrainableService.createResourceType(managedGroupResourceType, samRequestContext).unsafeRunSync() + + val parentAuthDomain = Set(WorkbenchGroupName("parentGroup")) + managedGroupService.createManagedGroup(ResourceId("parentGroup"), dummyUser, samRequestContext = samRequestContext).unsafeRunSync() + + val parentResource = service + .createResource( + constrainableResourceType, + ResourceId(UUID.randomUUID().toString), + Map.newBuilder + .addOne(AccessPolicyName("policy") -> AccessPolicyMembershipRequest(Set(dummyUser.email), Set.empty, Set(constrainableReaderRoleName))) + .result(), + parentAuthDomain, + None, + dummyUser.id, + samRequestContext + ) + .unsafeRunSync() + val childResource = + service + .createResource( + constrainableResourceType, + ResourceId(UUID.randomUUID().toString), + Map.empty, + Set.empty, + Option(parentResource.fullyQualifiedId), + dummyUser.id, + samRequestContext + ) + .unsafeRunSync() + + constrainableService + .loadResourceAuthDomain(childResource.fullyQualifiedId, samRequestContext) + .unsafeRunSync() should contain theSameElementsAs parentAuthDomain + } + + it should "pass if auth domains includes parent's" in { + assume(databaseEnabled, databaseEnabledClue) + + constrainableResourceType.isAuthDomainConstrainable shouldEqual true + constrainableService.createResourceType(constrainableResourceType, samRequestContext).unsafeRunSync() + constrainableService.createResourceType(managedGroupResourceType, samRequestContext).unsafeRunSync() + + val parentAuthDomain = Set(WorkbenchGroupName("parentGroup")) + managedGroupService.createManagedGroup(ResourceId("parentGroup"), dummyUser, samRequestContext = samRequestContext).unsafeRunSync() + val childAuthDomain = parentAuthDomain + WorkbenchGroupName("childGroup") + managedGroupService.createManagedGroup(ResourceId("childGroup"), dummyUser, samRequestContext = samRequestContext).unsafeRunSync() + + val parentResource = service + .createResource( + constrainableResourceType, + ResourceId(UUID.randomUUID().toString), + Map(AccessPolicyName("policy") -> constrainablePolicyMembership), + parentAuthDomain, + None, + dummyUser.id, + samRequestContext + ) + .unsafeRunSync() + val childResource = + service + .createResource( + constrainableResourceType, + ResourceId(UUID.randomUUID().toString), + Map.empty, + childAuthDomain, + Option(parentResource.fullyQualifiedId), + dummyUser.id, + samRequestContext + ) + .unsafeRunSync() + + constrainableService + .loadResourceAuthDomain(childResource.fullyQualifiedId, samRequestContext) + .unsafeRunSync() should contain theSameElementsAs childAuthDomain + } + + it should "fail if auth domains conflict with parent" in { + assume(databaseEnabled, databaseEnabledClue) + + constrainableResourceType.isAuthDomainConstrainable shouldEqual true + constrainableService.createResourceType(constrainableResourceType, samRequestContext).unsafeRunSync() + constrainableService.createResourceType(managedGroupResourceType, samRequestContext).unsafeRunSync() + + val parentAuthDomain = Set(WorkbenchGroupName("parentGroup")) + managedGroupService.createManagedGroup(ResourceId("parentGroup"), dummyUser, samRequestContext = samRequestContext).unsafeRunSync() + val childAuthDomain = Set(WorkbenchGroupName("childGroup")) + managedGroupService.createManagedGroup(ResourceId("childGroup"), dummyUser, samRequestContext = samRequestContext).unsafeRunSync() + + val parentResource = service + .createResource( + constrainableResourceType, + ResourceId(UUID.randomUUID().toString), + Map.newBuilder + .addOne(AccessPolicyName("policy") -> AccessPolicyMembershipRequest(Set(dummyUser.email), Set.empty, Set(constrainableReaderRoleName))) + .result(), + parentAuthDomain, + None, + dummyUser.id, + samRequestContext + ) + .unsafeRunSync() + val error = intercept[WorkbenchExceptionWithErrorReport] { + service + .createResource( + constrainableResourceType, + ResourceId(UUID.randomUUID().toString), + Map.empty, + childAuthDomain, + Option(parentResource.fullyQualifiedId), + dummyUser.id, + samRequestContext + ) + .unsafeRunSync() + } + error.errorReport.statusCode shouldEqual Option(StatusCodes.BadRequest) + } + "Loading an auth domain" should "fail when the resource does not exist" in { assume(databaseEnabled, databaseEnabledClue) @@ -1105,7 +1228,7 @@ class ResourceServiceSpec policy.version shouldEqual 1 val resourceWithAuthDomain = - runAndWait(constrainableService.addResourceAuthDomain(resource.fullyQualifiedId, authDomain.toList.toSet, dummyUser.id, samRequestContext)) + runAndWait(constrainableService.addResourceAuthDomain(resource.fullyQualifiedId, authDomain.toList.toSet, None, samRequestContext)) resourceWithAuthDomain shouldEqual authDomain.toList.toSet val updatedPolicy = @@ -1114,6 +1237,145 @@ class ResourceServiceSpec } + it should "add auth domains to resource and descendants" in { + assume(databaseEnabled, databaseEnabledClue) + + val accessPolicies = Map( + AccessPolicyName("constrainable") -> constrainablePolicyMembership + ) + + val authDomain = Set(WorkbenchGroupName("authDomain")) + val authDomain2 = Set(WorkbenchGroupName("authDomain2")) + val testResult = for { + _ <- service.createResourceType(constrainableResourceType, samRequestContext) + _ <- service.createResourceType(managedGroupResourceType, samRequestContext) + + _ <- managedGroupService.createManagedGroup(ResourceId("authDomain"), dummyUser, samRequestContext = samRequestContext) + _ <- managedGroupService.createManagedGroup(ResourceId("authDomain2"), dummyUser, samRequestContext = samRequestContext) + parentResource <- service.createResource( + constrainableResourceType, + ResourceId("parent"), + accessPolicies, + authDomain, + None, + dummyUser.id, + samRequestContext + ) + childResource <- service.createResource( + constrainableResourceType, + ResourceId("child"), + accessPolicies, + Set.empty, + Option(parentResource.fullyQualifiedId), + dummyUser.id, + samRequestContext + ) + grandchild <- service.createResource( + constrainableResourceType, + ResourceId("grandchild"), + accessPolicies, + Set.empty, + Option(childResource.fullyQualifiedId), + dummyUser.id, + samRequestContext + ) + updated <- constrainableService.addResourceAuthDomain(parentResource.fullyQualifiedId, authDomain2, Option(dummyUser.id), samRequestContext) + allResourceIds = List(parentResource.fullyQualifiedId, childResource.fullyQualifiedId, grandchild.fullyQualifiedId) + allADs <- allResourceIds.traverse(constrainableService.loadResourceAuthDomain(_, samRequestContext)) + allPolicies <- allResourceIds.traverse(policyDAO.listAccessPolicies(_, samRequestContext)) + } yield { + updated shouldBe authDomain2 ++ authDomain + allADs.foreach(_ shouldBe updated) + allPolicies.flatten.foreach(_.version shouldBe 2) + } + + testResult.unsafeRunSync() + } + + it should "throw if any has public policies" in { + assume(databaseEnabled, databaseEnabledClue) + + val accessPolicies = Map( + AccessPolicyName("constrainable") -> constrainablePolicyMembership + ) + + val authDomain = Set(WorkbenchGroupName("authDomain")) + val testResult = for { + _ <- service.createResourceType(constrainableResourceType, samRequestContext) + _ <- service.createResourceType(managedGroupResourceType, samRequestContext) + + _ <- managedGroupService.createManagedGroup(ResourceId("authDomain"), dummyUser, samRequestContext = samRequestContext) + parentResource <- service.createResource( + constrainableResourceType, + ResourceId("parent"), + accessPolicies, + Set.empty, + None, + dummyUser.id, + samRequestContext + ) + childResource <- service.createResource( + constrainableResourceType, + ResourceId("child"), + accessPolicies, + Set.empty, + Option(parentResource.fullyQualifiedId), + dummyUser.id, + samRequestContext + ) + grandchild <- service.createResource( + constrainableResourceType, + ResourceId("grandchild"), + accessPolicies, + Set.empty, + Option(childResource.fullyQualifiedId), + dummyUser.id, + samRequestContext + ) + // pick a random resource to set public, this test should work for any of the 3 + probeResourceId = Random.shuffle(List(parentResource.fullyQualifiedId, childResource.fullyQualifiedId, grandchild.fullyQualifiedId)).head + _ <- constrainableService.setPublic(FullyQualifiedPolicyId(probeResourceId, accessPolicies.head._1), true, samRequestContext) + _ <- constrainableService.addResourceAuthDomain(parentResource.fullyQualifiedId, authDomain, Option(dummyUser.id), samRequestContext) + } yield {} + + val error = intercept[WorkbenchExceptionWithErrorReport] { + testResult.unsafeRunSync() + } + + error.errorReport.statusCode shouldEqual Option(StatusCodes.BadRequest) + } + + it should "throw if auth domain does not exist" in { + assume(databaseEnabled, databaseEnabledClue) + + val accessPolicies = Map( + AccessPolicyName("constrainable") -> constrainablePolicyMembership + ) + + val authDomain = Set(WorkbenchGroupName("authDomain")) + val testResult = for { + _ <- service.createResourceType(constrainableResourceType, samRequestContext) + _ <- service.createResourceType(managedGroupResourceType, samRequestContext) + + resource <- service.createResource( + constrainableResourceType, + ResourceId("parent"), + accessPolicies, + Set.empty, + None, + dummyUser.id, + samRequestContext + ) + _ <- constrainableService.addResourceAuthDomain(resource.fullyQualifiedId, authDomain, Option(dummyUser.id), samRequestContext) + } yield {} + + val error = intercept[WorkbenchExceptionWithErrorReport] { + testResult.unsafeRunSync() + } + + error.errorReport.statusCode shouldEqual Option(StatusCodes.BadRequest) + } + "listUserResourceRoles" should "list the user's role when they have at least one role" in { assume(databaseEnabled, databaseEnabledClue) @@ -2122,7 +2384,7 @@ class ResourceServiceSpec runAndWait(service.validatePolicy(defaultResourceType, ResourceId(""), policy)) shouldBe empty } - "validatePolicy" should "fail with an incorrect policy" in { + it should "fail with an incorrect policy" in { val emailToMaybeSubject = Map(dummyUser.email -> Option(dummyUser.id.asInstanceOf[WorkbenchSubject])) val policy = service.ValidatableAccessPolicy(AccessPolicyName("a"), emailToMaybeSubject, Set(ResourceRoleName("bad_name")), Set(ResourceAction("bad_action")), Set()) @@ -2136,7 +2398,7 @@ class ResourceServiceSpec maybeErrorReport.value.message should include("invalid role") } - "validateRoles" should "succeed with role included in listed roles" in { + it should "succeed with role included in listed roles" in { service.validateRoles(defaultResourceType, Set(ownerRoleName)) shouldBe empty } @@ -2147,6 +2409,10 @@ class ResourceServiceSpec maybeErrorReport.value.message should include("invalid action") } + it should "succeed with action included in listed actions" in { + service.validateActions(defaultResourceType, Set(ResourceAction("alter_policies"))) shouldBe empty + } + "validateResourceTypeAdminDescendantPermissions" should "succeed if resource type admin matches resource" in { service.validateResourceTypeAdminDescendantPermissions( resourceTypeAdmin, @@ -2181,10 +2447,6 @@ class ResourceServiceSpec ) } - "validateActions" should "succeed with action included in listed actions" in { - service.validateActions(defaultResourceType, Set(ResourceAction("alter_policies"))) shouldBe empty - } - "add/remove SubjectToPolicy" should "add/remove subject and tolerate prior (non)existence" in { assume(databaseEnabled, databaseEnabledClue) @@ -2754,40 +3016,106 @@ class ResourceServiceSpec testPolicy.email ) - implicit val patienceConfig = PatienceConfig(5.seconds) testResult.unsafeRunSync() } - "setResourceParent" should "throw if the child resource has an auth domain" in { + "setResourceParent" should "inherit parent's auth domain for all descendants" in { assume(databaseEnabled, databaseEnabledClue) - val childAccessPolicies = Map( + val accessPolicies = Map( AccessPolicyName("constrainable") -> constrainablePolicyMembership ) + val authDomain = Set(WorkbenchGroupName("authDomain")) val testResult = for { _ <- service.createResourceType(constrainableResourceType, samRequestContext) _ <- service.createResourceType(managedGroupResourceType, samRequestContext) _ <- managedGroupService.createManagedGroup(ResourceId("authDomain"), dummyUser, samRequestContext = samRequestContext) + parentResource <- service.createResource( + constrainableResourceType, + ResourceId("parent"), + accessPolicies, + authDomain, + None, + dummyUser.id, + samRequestContext + ) childResource <- service.createResource( constrainableResourceType, ResourceId("child"), - childAccessPolicies, - Set(WorkbenchGroupName("authDomain")), + accessPolicies, + Set.empty, None, dummyUser.id, samRequestContext ) - parentResource <- service.createResource(constrainableResourceType, ResourceId("parent"), dummyUser, samRequestContext) + grandchild <- service.createResource( + constrainableResourceType, + ResourceId("grandchild"), + accessPolicies, + Set.empty, + Option(childResource.fullyQualifiedId), + dummyUser.id, + samRequestContext + ) _ <- service.setResourceParent(childResource.fullyQualifiedId, parentResource.fullyQualifiedId, samRequestContext) - } yield () - - val exception = intercept[WorkbenchExceptionWithErrorReport] { - testResult.unsafeRunSync() + childAD <- constrainableService.loadResourceAuthDomain(childResource.fullyQualifiedId, samRequestContext) + grandchildAD <- constrainableService.loadResourceAuthDomain(grandchild.fullyQualifiedId, samRequestContext) + } yield { + childAD shouldBe authDomain + grandchildAD shouldBe authDomain } - exception.errorReport.statusCode shouldBe Option(StatusCodes.BadRequest) + testResult.unsafeRunSync() + } + + it should "fail when parent has auth domain and descendant has public policy" in { + assume(databaseEnabled, databaseEnabledClue) + + val accessPolicies = Map( + AccessPolicyName("constrainable") -> constrainablePolicyMembership + ) + + val authDomain = Set(WorkbenchGroupName("authDomain")) + val testResult = for { + _ <- service.createResourceType(constrainableResourceType, samRequestContext) + _ <- service.createResourceType(managedGroupResourceType, samRequestContext) + + _ <- managedGroupService.createManagedGroup(ResourceId("authDomain"), dummyUser, samRequestContext = samRequestContext) + parentResource <- service.createResource( + constrainableResourceType, + ResourceId("parent"), + accessPolicies, + authDomain, + None, + dummyUser.id, + samRequestContext + ) + childResource <- service.createResource( + constrainableResourceType, + ResourceId("child"), + accessPolicies, + Set.empty, + None, + dummyUser.id, + samRequestContext + ) + grandchild <- service.createResource( + constrainableResourceType, + ResourceId("grandchild"), + accessPolicies, + Set.empty, + Option(childResource.fullyQualifiedId), + dummyUser.id, + samRequestContext + ) + _ <- service.setPublic(FullyQualifiedPolicyId(grandchild.fullyQualifiedId, accessPolicies.head._1), public = true, samRequestContext) + _ <- service.setResourceParent(childResource.fullyQualifiedId, parentResource.fullyQualifiedId, samRequestContext) + } yield () + + val error = intercept[WorkbenchExceptionWithErrorReport](testResult.unsafeRunSync()) + error.errorReport.statusCode shouldBe Some(StatusCodes.BadRequest) } "deletePolicy" should "delete the policy" in {