Skip to content

Commit

Permalink
fix: pr feedback
Browse files Browse the repository at this point in the history
Signed-off-by: Pat Losoponkul <pat.losoponkul@iohk.io>
  • Loading branch information
Pat Losoponkul committed Jan 12, 2024
1 parent a9ea700 commit 4c459e3
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ Example JWT payload containing `ClientRole`. (Some claims are omitted for readab
"resource_access": {
"prism-agent": {
"roles": [
"agent-admin"
"admin"
]
},
"account": {
Expand Down Expand Up @@ -107,9 +107,9 @@ __Proposed agent role authorization model__
Role is a plain text that defines what level of access a user has on a system.
For the agent, it needs to support 2 roles:

1. __Admin__: `agent-admin`. Admin can never access a tenant wallet.
1. __Admin__: `admin`. Admin can never access a tenant wallet.
Agent auth layer must ignore any UMA permission to the wallet.
2. __Tenant__: `agent-tenant` or implicitly inferred if another role is not specified.
2. __Tenant__: `tenant` or implicitly inferred if another role is not specified.
Tenant must have UMA permission defined to access the wallet.

### Positive Consequences
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ agent {
autoUpgradeToRPT = ${?KEYCLOAK_UMA_AUTO_UPGRADE_RPT}

# A path of 'roles' claim in the JWT. Nested path maybe indicated by '.' separator.
# The JWT 'roles' claim is expected to be a list of the following values: [agent-admin, agent-tenant]
# The JWT 'roles' claim is expected to be a list of the following values: [admin, tenant]
rolesClaimPath = "resource_access."${agent.authentication.keycloak.clientId}".roles"
rolesClaimPath = ${?KEYKLOAK_ROLES_CLAIM_PATH}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ trait Authenticator[E <: BaseEntity] {
}

trait Authorizer[E <: BaseEntity] {
protected def authorizeWalletAccessImpl(entity: E): IO[AuthenticationError, WalletAccessContext]
protected def authorizeWalletAccessLogic(entity: E): IO[AuthenticationError, WalletAccessContext]

final def authorizeWalletAccess(entity: E): IO[AuthenticationError, WalletAccessContext] =
ZIO
Expand All @@ -59,15 +59,15 @@ trait Authorizer[E <: BaseEntity] {
.filterOrFail(_ != EntityRole.Admin)(
AuthenticationError.InvalidRole("Admin role is not allowed to access the tenant's wallet.")
)
.flatMap(_ => authorizeWalletAccessImpl(entity))
.flatMap(_ => authorizeWalletAccessLogic(entity))

def authorizeWalletAdmin(entity: E): IO[AuthenticationError, WalletAdministrationContext]
}

object EntityAuthorizer extends EntityAuthorizer

trait EntityAuthorizer extends Authorizer[Entity] {
override def authorizeWalletAccessImpl(entity: Entity): IO[AuthenticationError, WalletAccessContext] =
override def authorizeWalletAccessLogic(entity: Entity): IO[AuthenticationError, WalletAccessContext] =
ZIO.succeed(entity.walletId).map(WalletId.fromUUID).map(WalletAccessContext.apply)

override def authorizeWalletAdmin(entity: Entity): IO[AuthenticationError, WalletAdministrationContext] = {
Expand All @@ -84,8 +84,8 @@ object DefaultEntityAuthenticator extends AuthenticatorWithAuthZ[BaseEntity] {

override def isEnabled: Boolean = true
override def authenticate(credentials: Credentials): IO[AuthenticationError, BaseEntity] = ZIO.succeed(Entity.Default)
override def authorizeWalletAccessImpl(entity: BaseEntity): IO[AuthenticationError, WalletAccessContext] =
EntityAuthorizer.authorizeWalletAccessImpl(Entity.Default)
override def authorizeWalletAccessLogic(entity: BaseEntity): IO[AuthenticationError, WalletAccessContext] =
EntityAuthorizer.authorizeWalletAccessLogic(Entity.Default)
override def authorizeWalletAdmin(entity: BaseEntity): IO[AuthenticationError, WalletAdministrationContext] =
EntityAuthorizer.authorizeWalletAdmin(Entity.Default)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ case class DefaultAuthenticator(
case keycloakCredentials: JwtCredentials => keycloakAuthenticator(keycloakCredentials)
}

override def authorizeWalletAccessImpl(entity: BaseEntity): IO[AuthenticationError, WalletAccessContext] =
override def authorizeWalletAccessLogic(entity: BaseEntity): IO[AuthenticationError, WalletAccessContext] =
entity match {
case entity: Entity => EntityAuthorizer.authorizeWalletAccess(entity)
case kcEntity: KeycloakEntity => keycloakAuthenticator.authorizeWalletAccess(kcEntity)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ final class AccessToken private (token: String, claims: JwtClaim, rolesClaimPath

private def parseRole(s: String): Option[EntityRole] = {
s match {
case "agent-admin" => Some(EntityRole.Admin)
case "agent-tenant" => Some(EntityRole.Tenant)
case _ => None
case "admin" => Some(EntityRole.Admin)
case "tenant" => Some(EntityRole.Tenant)
case _ => None
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class KeycloakAuthenticatorImpl(
} else ZIO.fail(AuthenticationMethodNotEnabled("Keycloak authentication is not enabled"))
}

override def authorizeWalletAccessImpl(entity: KeycloakEntity): IO[AuthenticationError, WalletAccessContext] = {
override def authorizeWalletAccessLogic(entity: KeycloakEntity): IO[AuthenticationError, WalletAccessContext] = {
for {
walletId <- keycloakPermissionService
.listWalletPermissions(entity)
Expand Down Expand Up @@ -100,7 +100,7 @@ object KeycloakAuthenticatorImpl {
new KeycloakAuthenticator {
override def isEnabled: Boolean = false
override def authenticate(token: String): IO[AuthenticationError, KeycloakEntity] = notEnabledError
override def authorizeWalletAccessImpl(entity: KeycloakEntity): IO[AuthenticationError, WalletAccessContext] =
override def authorizeWalletAccessLogic(entity: KeycloakEntity): IO[AuthenticationError, WalletAccessContext] =
notEnabledError
override def authorizeWalletAdmin(
entity: KeycloakEntity
Expand Down
2 changes: 1 addition & 1 deletion prism-agent/service/server/src/test/resources/logback.xml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

<logger name="io.getquill" level="OFF"/>
<logger name="com.zaxxer.hikari" level="OFF"/>
<logger name="org.jboss.resteasy.client" level="OFF"/>
<logger name="org.jboss.resteasy.client" level="ERROR"/>

<!--
INFO level is too noisy because of flyway migration log when running "sbt test".
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,10 @@ object KeycloakAuthenticatorSpec
authenticator <- ZIO.service[KeycloakAuthenticator]
_ <- createUser("alice", "1234")
_ <- createUser("bob", "1234")
_ <- createClientRole("agent-admin")
_ <- createClientRole("agent-tenant")
_ <- grantClientRole("alice", "agent-admin")
_ <- grantClientRole("bob", "agent-tenant")
_ <- createClientRole("admin")
_ <- createClientRole("tenant")
_ <- grantClientRole("alice", "admin")
_ <- grantClientRole("bob", "tenant")
aliceToken <- client.getAccessToken("alice", "1234").map(_.access_token)
bobToken <- client.getAccessToken("bob", "1234").map(_.access_token)
aliceEntity <- authenticator.authenticate(aliceToken)
Expand All @@ -156,10 +156,10 @@ object KeycloakAuthenticatorSpec
client <- ZIO.service[KeycloakClient]
authenticator <- ZIO.service[KeycloakAuthenticator]
_ <- createUser("alice", "1234")
_ <- createClientRole("agent-admin")
_ <- createClientRole("agent-tenant")
_ <- grantClientRole("alice", "agent-admin")
_ <- grantClientRole("alice", "agent-tenant")
_ <- createClientRole("admin")
_ <- createClientRole("tenant")
_ <- grantClientRole("alice", "admin")
_ <- grantClientRole("alice", "tenant")
token <- client.getAccessToken("alice", "1234").map(_.access_token)
entity <- authenticator.authenticate(token)
} yield assert(entity.role)(isLeft(anything))
Expand Down Expand Up @@ -256,8 +256,8 @@ object KeycloakAuthenticatorSpec
_ <- createWalletResource(wallet.id, "wallet-1")
_ <- createUser("alice", "1234")
_ <- createResourcePermission(wallet.id, "alice")
_ <- createClientRole("agent-admin")
_ <- grantClientRole("alice", "agent-admin")
_ <- createClientRole("admin")
_ <- grantClientRole("alice", "admin")
token <- client.getAccessToken("alice", "1234").map(_.access_token)
entity <- authenticator.authenticate(token)
role <- ZIO.fromEither(entity.role)
Expand Down

0 comments on commit 4c459e3

Please sign in to comment.