Skip to content

Commit

Permalink
Fix app roles; introduce resource-specific role enums
Browse files Browse the repository at this point in the history
  • Loading branch information
rtitle committed Sep 12, 2024
1 parent 362ceac commit f999c28
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,13 @@ object ProjectAction {
final case object CreateApp extends ProjectAction {
val asString = "create_kubernetes_app"
}

// TODO the below actions will be removed after we migrate to Sam hierarchical resources
// for notebook-cluster and persistent-disk. See: https://broadworkbench.atlassian.net/browse/IA-5059

// Other exists because there are other project actions not used by Leo
final case class Other(asString: String) extends ProjectAction

// TODO we'd like to remove the below actions at the project level, and control these
// actions with policies at the resource level instead.
// See https://broadworkbench.atlassian.net/browse/IA-2093
final case object GetRuntimeStatus extends ProjectAction {
val asString = "list_notebook_cluster"
}
Expand Down Expand Up @@ -262,8 +263,68 @@ object PrivateAzureStorageAccountAction {
sealerate.collect[PrivateAzureStorageAccountAction].map(a => (a.asString, a)).toMap
}

// TODO [IA-4608] merge with SamPolicyName
/** Represents a role in Sam, permitting a set of actions on a resource of a certain type. */
sealed trait RuntimeRole extends Product with Serializable {
def asString: String
override def toString = asString

Check warning on line 268 in core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala

View check run for this annotation

Codecov / codecov/patch

core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala#L268

Added line #L268 was not covered by tests
}
object RuntimeRole {
final case object Creator extends RuntimeRole {
val asString = "creator"

Check warning on line 272 in core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala

View check run for this annotation

Codecov / codecov/patch

core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala#L272

Added line #L272 was not covered by tests
}

final case object Manager extends RuntimeRole {
val asString = "manager"

Check warning on line 276 in core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala

View check run for this annotation

Codecov / codecov/patch

core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala#L276

Added line #L276 was not covered by tests
}
}

sealed trait PersistentDiskRole extends Product with Serializable {
def asString: String
override def toString = asString

Check warning on line 282 in core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala

View check run for this annotation

Codecov / codecov/patch

core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala#L282

Added line #L282 was not covered by tests
}
object PersistentDiskRole {
final case object Creator extends PersistentDiskRole {
val asString = "creator"

Check warning on line 286 in core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala

View check run for this annotation

Codecov / codecov/patch

core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala#L286

Added line #L286 was not covered by tests
}

final case object Manager extends PersistentDiskRole {
val asString = "manager"

Check warning on line 290 in core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala

View check run for this annotation

Codecov / codecov/patch

core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala#L290

Added line #L290 was not covered by tests
}
}

sealed trait AppRole extends Product with Serializable {
def asString: String
override def toString = asString

Check warning on line 296 in core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala

View check run for this annotation

Codecov / codecov/patch

core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala#L296

Added line #L296 was not covered by tests
}

object AppRole {
final case object Creator extends AppRole {
val asString = "creator"

Check warning on line 301 in core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala

View check run for this annotation

Codecov / codecov/patch

core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala#L301

Added line #L301 was not covered by tests
}

final case object Manager extends AppRole {
val asString = "manager"

Check warning on line 305 in core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala

View check run for this annotation

Codecov / codecov/patch

core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala#L305

Added line #L305 was not covered by tests
}
}

sealed trait SharedAppRole extends Product with Serializable {
def asString: String
override def toString = asString

Check warning on line 311 in core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala

View check run for this annotation

Codecov / codecov/patch

core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala#L311

Added line #L311 was not covered by tests
}

object SharedAppRole {
final case object Owner extends SharedAppRole {
val asString = "owner"

Check warning on line 316 in core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala

View check run for this annotation

Codecov / codecov/patch

core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala#L316

Added line #L316 was not covered by tests
}

final case object User extends SharedAppRole {
val asString = "user"

Check warning on line 320 in core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala

View check run for this annotation

Codecov / codecov/patch

core/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/samModels.scala#L320

Added line #L320 was not covered by tests
}
}

/**
* Deprecated: use resource-type specific role enums (RuntimeRole, PersistentDiskRole, etc).
*/
@Deprecated
sealed trait SamRole extends Product with Serializable {
def asString: String
override def toString = asString
Expand All @@ -289,8 +350,10 @@ object SamRole {
val stringToRole = sealerate.collect[SamRole].map(p => (p.asString, p)).toMap
}

// TODO [IA-4608] merge with SamRole
/** Represents a role in Sam, permitting a set of actions on a resource of a certain type. */
/**
* Deprecated: don't use an enum to represent policy names.
*/
@Deprecated
sealed trait SamPolicyName extends Serializable with Product
object SamPolicyName {
final case object Creator extends SamPolicyName {
Expand All @@ -316,7 +379,7 @@ object SamPolicyName {
}

final case class SamPolicyEmail(email: WorkbenchEmail) extends AnyVal
final case class SamPolicyData(memberEmails: List[WorkbenchEmail], roles: List[SamRole])
final case class SamPolicyData(memberEmails: List[WorkbenchEmail], roles: List[String])

sealed abstract class AppAccessScope
object AppAccessScope {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ object HttpSamDAO {
implicit val samPolicyDataDecoder: Decoder[SamPolicyData] = Decoder.instance { x =>
for {
memberEmails <- x.downField("memberEmails").as[List[WorkbenchEmail]]
roles <- x.downField("roles").as[List[SamRole]]
roles <- x.downField("roles").as[List[String]]

Check warning on line 486 in http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpSamDAO.scala

View check run for this annotation

Codecov / codecov/patch

http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/dao/HttpSamDAO.scala#L486

Added line #L486 was not covered by tests
} yield SamPolicyData(memberEmails, roles)
}
implicit val syncStatusDecoder: Decoder[SyncStatusResponse] = Decoder.instance { x =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ class SamServiceInterp[F[_]](apiClientProvider: SamApiClientProvider[F],
.mapValues(p =>
new AccessPolicyMembershipRequest()
.memberEmails(p.memberEmails.map(_.value).asJava)
.roles(p.roles.map(_.asString).asJava)
.roles(p.roles.asJava)
)
.toMap

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class DiskServiceInterp[F[_]: Parallel](config: PersistentDiskConfig,
samResource,
Some(googleProject),
None,
Map("creator" -> SamPolicyData(List(userEmail), List(SamRole.Creator)))
getDiskSamPolicyMap(userEmail)
)
// TODO: do we need to introduce pre status here?
savedDisk <- persistentDiskQuery.save(disk).transaction
Expand Down Expand Up @@ -530,6 +530,9 @@ object DiskServiceInterp {
workspaceId
)
}

private[service] def getDiskSamPolicyMap(userEmail: WorkbenchEmail): Map[String, SamPolicyData] =
Map("creator" -> SamPolicyData(List(userEmail), List(PersistentDiskRole.Creator.asString)))
}

case class PersistentDiskAlreadyExistsException(googleProject: GoogleProject,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import org.broadinstitute.dsde.workbench.leonardo.db.KubernetesServiceDbQueries.
import org.broadinstitute.dsde.workbench.leonardo.db._
import org.broadinstitute.dsde.workbench.leonardo.http.service.LeoAppServiceInterp.{
checkIfCanBeDeleted,
getAppSamPolicyMap,
isPatchVersionDifference
}
import org.broadinstitute.dsde.workbench.leonardo.model.SamResourceAction._
Expand Down Expand Up @@ -167,7 +168,7 @@ final class LeoAppServiceInterp[F[_]: Parallel](config: AppServiceConfig,
samResourceId,
Some(googleProject),
None,
Map("creator" -> SamPolicyData(List(userEmail), List(SamRole.Creator)))
getAppSamPolicyMap(userEmail, req.accessScope)
)
saveCluster <- F.fromEither(
getSavableCluster(userEmail, cloudContext, req.autopilot.isDefined, ctx.now)
Expand Down Expand Up @@ -786,7 +787,7 @@ final class LeoAppServiceInterp[F[_]: Parallel](config: AppServiceConfig,
samResourceId,
None,
Some(workspaceId),
Map("creator" -> SamPolicyData(List(userEmail), List(SamRole.Creator)))
getAppSamPolicyMap(userEmail, req.accessScope)
)

// Save or retrieve a KubernetesCluster record for the app
Expand Down Expand Up @@ -1685,6 +1686,20 @@ object LeoAppServiceInterp {
if (deletable) Right(())
else Left(s"${appType} can not be deleted in ${appStatus} status.")
}

/**
* Shared apps are represented as kubernetes-app-shared resources in Sam and have an "owner" role.
* Private apps are represented as kubernetes-app resources in Sam and have a "creator" role.
*/
private[http] def getAppSamPolicyMap(userEmail: WorkbenchEmail,
accessScope: Option[AppAccessScope]
): Map[String, SamPolicyData] =
accessScope match {
case Some(AppAccessScope.WorkspaceShared) =>
Map("owner" -> SamPolicyData(List(userEmail), List(SharedAppRole.Owner.asString)))

Check warning on line 1699 in http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/http/service/LeoAppServiceInterp.scala

View check run for this annotation

Codecov / codecov/patch

http/src/main/scala/org/broadinstitute/dsde/workbench/leonardo/http/service/LeoAppServiceInterp.scala#L1699

Added line #L1699 was not covered by tests
case _ =>
Map("creator" -> SamPolicyData(List(userEmail), List(AppRole.Creator.asString)))
}
}

case class AppNotFoundException(cloudContext: CloudContext, appName: AppName, traceId: TraceId, extraMsg: String)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,10 @@ import org.broadinstitute.dsde.workbench.leonardo.config._
import org.broadinstitute.dsde.workbench.leonardo.dao.DockerDAO
import org.broadinstitute.dsde.workbench.leonardo.dao.sam.SamService
import org.broadinstitute.dsde.workbench.leonardo.db._
import org.broadinstitute.dsde.workbench.leonardo.http.service.DiskServiceInterp.getDiskSamPolicyMap
import org.broadinstitute.dsde.workbench.leonardo.model.SamResourceAction.{
// do not remove `projectSamResourceAction`; it is implicit
projectSamResourceAction,
// do not remove `runtimeSamResourceAction`; it is implicit
runtimeSamResourceAction,
// do not remove `workspaceSamResourceAction`; it is implicit
workspaceSamResourceAction
}
import org.broadinstitute.dsde.workbench.leonardo.http.service.RuntimeServiceInterp._
Expand Down Expand Up @@ -209,11 +207,12 @@ class RuntimeServiceInterp[F[_]: Parallel](
.parTraverse(s => validateBucketObjectUri(userEmail, petToken, s, context.traceId))
_ <- context.span.traverse(s => F.delay(s.addAnnotation("Done validating buckets")))
// Create a notebook-cluster Sam resource with a cretor policy and the google project as the parent
_ <- samService.createResource(userInfo.accessToken.token,
samResource,
Some(googleProject),
None,
Map("creator" -> SamPolicyData(List(userEmail), List(SamRole.Creator)))
_ <- samService.createResource(
userInfo.accessToken.token,
samResource,
Some(googleProject),
None,
getRuntimeSamPolicyMap(userEmail)
)
_ <- context.span.traverse(s => F.delay(s.addAnnotation("Done Sam createResource")))
runtimeConfigToSave = LeoLenses.runtimeConfigPrism.reverseGet(runtimeConfig)
Expand Down Expand Up @@ -1245,11 +1244,12 @@ object RuntimeServiceInterp {
)
)
// Create a persistent-disk Sam resource with a creator policy and the google project as the parent
_ <- samService.createResource(userInfo.accessToken.token,
samResource,
Some(googleProject),
None,
Map("creator" -> SamPolicyData(List(userEmail), List(SamRole.Creator)))
_ <- samService.createResource(
userInfo.accessToken.token,
samResource,
Some(googleProject),
None,
getDiskSamPolicyMap(userEmail)
)
pd <- persistentDiskQuery.save(diskBeforeSave).transaction
} yield PersistentDiskRequestResult(pd, true)
Expand Down Expand Up @@ -1352,7 +1352,7 @@ object RuntimeServiceInterp {
samResource,
None,
Some(workspaceId),
Map("creator" -> SamPolicyData(List(userEmail), List(SamRole.Creator)))
getDiskSamPolicyMap(userEmail)
)
pd <- persistentDiskQuery.save(diskBeforeSave).transaction
} yield PersistentDiskRequestResult(pd, true)
Expand All @@ -1374,6 +1374,8 @@ object RuntimeServiceInterp {
}
}

private[service] def getRuntimeSamPolicyMap(userEmail: WorkbenchEmail): Map[String, SamPolicyData] =
Map("creator" -> SamPolicyData(List(userEmail), List(RuntimeRole.Creator.asString)))
}

final case class PersistentDiskRequestResult(disk: PersistentDisk, creationNeeded: Boolean)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import org.broadinstitute.dsde.workbench.leonardo.config.PersistentDiskConfig
import org.broadinstitute.dsde.workbench.leonardo.dao._
import org.broadinstitute.dsde.workbench.leonardo.dao.sam.SamService
import org.broadinstitute.dsde.workbench.leonardo.db._
import org.broadinstitute.dsde.workbench.leonardo.http.service.DiskServiceInterp.getDiskSamPolicyMap
import org.broadinstitute.dsde.workbench.leonardo.http.service.RuntimeServiceInterp.getRuntimeSamPolicyMap
// do not remove: `projectSamResourceAction`, `runtimeSamResourceAction`, `workspaceSamResourceAction`, `wsmResourceSamResourceAction`; `AppSamResourceAction` they are implicit
import org.broadinstitute.dsde.workbench.leonardo.model.SamResourceAction.{
projectSamResourceAction,
Expand Down Expand Up @@ -195,7 +197,7 @@ class RuntimeV2ServiceInterp[F[_]: Parallel](
samResource,
None,
Some(workspaceId),
Map("creator" -> SamPolicyData(List(userEmail), List(SamRole.Creator)))
getDiskSamPolicyMap(userEmail)
)
disk <- persistentDiskQuery.save(pd).transaction
} yield disk.id
Expand Down Expand Up @@ -226,7 +228,7 @@ class RuntimeV2ServiceInterp[F[_]: Parallel](
samResource,
None,
Some(workspaceId),
Map("creator" -> SamPolicyData(List(userEmail), List(SamRole.Creator)))
getRuntimeSamPolicyMap(userEmail)
)

savedRuntime <- clusterQuery.save(runtimeToSave).transaction
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import org.broadinstitute.dsde.workbench.leonardo.model.LeoInternalServerError
import org.broadinstitute.dsde.workbench.leonardo.{
LeonardoTestSuite,
RuntimeAction,
RuntimeRole,
SamPolicyData,
SamResourceType,
SamRole
SamResourceType
}
import org.http4s.headers.Authorization
import org.http4s.{AuthScheme, Credentials}
Expand Down Expand Up @@ -404,7 +404,7 @@ class SamServiceInterpSpec extends AnyFunSpecLike with LeonardoTestSuite with Be
runtimeSamResource,
None,
Some(workspaceId),
Map("aPolicy" -> SamPolicyData(List(userEmail), List(SamRole.Creator)))
Map("aPolicy" -> SamPolicyData(List(userEmail), List(RuntimeRole.Creator.asString)))
)
.unsafeRunSync()

Expand Down

0 comments on commit f999c28

Please sign in to comment.