Skip to content

Commit

Permalink
fix(authorization): allow only resource owners/admins to manage permi…
Browse files Browse the repository at this point in the history 
…ssions

The previous behavior of allowing to grant your own permission to others might be too risky.
It's hardcoded to
  • Loading branch information
@slisson
slisson committed Dec 11, 2024
1 parent 11c6d46 commit f2d2afb
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 44 deletions.
12 changes: 9 additions & 3 deletions authorization/src/main/kotlin/org/modelix/authorization/AuthorizationPlugin.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import io.ktor.server.routing.get
import io.ktor.server.routing.routing
import io.ktor.util.AttributeKey
import org.modelix.authorization.permissions.PermissionEvaluator
import org.modelix.authorization.permissions.PermissionInstanceReference
import org.modelix.authorization.permissions.PermissionParser
import org.modelix.authorization.permissions.PermissionParts
import org.modelix.authorization.permissions.SchemaInstance
import java.nio.charset.StandardCharsets
Expand Down Expand Up @@ -169,11 +171,15 @@ class ModelixAuthorizationPluginInstance(val config: ModelixAuthorizationConfig)
private val deniedPermissionRequests: MutableSet<DeniedPermissionRequest> = Collections.synchronizedSet(LinkedHashSet())
private val permissionCache = CacheBuilder.newBuilder()
.expireAfterWrite(5, TimeUnit.SECONDS)
.build<Pair<AccessTokenPrincipal, PermissionParts>, Boolean>()
.build<Pair<AccessTokenPrincipal, PermissionInstanceReference>, Boolean>()

fun getDeniedPermissions(): Set<DeniedPermissionRequest> = deniedPermissionRequests.toSet()

fun hasPermission(call: ApplicationCall, permissionToCheck: PermissionParts): Boolean {
return hasPermission(call, PermissionParser(config.permissionSchema).parse(permissionToCheck))
}

fun hasPermission(call: ApplicationCall, permissionToCheck: PermissionInstanceReference): Boolean {
if (!config.permissionCheckingEnabled()) return true

val principal = call.principal<AccessTokenPrincipal>() ?: throw NotLoggedInException()
Expand All @@ -184,7 +190,7 @@ class ModelixAuthorizationPluginInstance(val config: ModelixAuthorizationConfig)
if (userId != null) {
synchronized(deniedPermissionRequests) {
deniedPermissionRequests += DeniedPermissionRequest(
permissionId = permissionToCheck,
permissionRef = permissionToCheck,
userId = userId,
jwtPayload = principal.jwt.payload,
)
Expand Down Expand Up @@ -222,7 +228,7 @@ class ModelixAuthorizationPluginInstance(val config: ModelixAuthorizationConfig)
}

data class DeniedPermissionRequest(
val permissionId: PermissionParts,
val permissionRef: PermissionInstanceReference,
val userId: String,
val jwtPayload: String,
) {
Expand Down
5 changes: 5 additions & 0 deletions authorization/src/main/kotlin/org/modelix/authorization/KtorAuthUtils.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import io.ktor.server.request.header
import io.ktor.server.routing.Route
import io.ktor.util.pipeline.PipelineContext
import org.modelix.authorization.permissions.PermissionEvaluator
import org.modelix.authorization.permissions.PermissionInstanceReference
import org.modelix.authorization.permissions.PermissionParts

internal const val MODELIX_JWT_AUTH = "modelixJwtAuth"
Expand Down Expand Up @@ -49,6 +50,10 @@ fun ApplicationCall.hasPermission(permissionToCheck: PermissionParts): Boolean {
return application.plugin(ModelixAuthorization).hasPermission(this, permissionToCheck)
}

fun ApplicationCall.hasPermission(permissionToCheck: PermissionInstanceReference): Boolean {
return application.plugin(ModelixAuthorization).hasPermission(this, permissionToCheck)
}

fun ApplicationCall.getPermissionEvaluator(): PermissionEvaluator {
return application.plugin(ModelixAuthorization).getPermissionEvaluator(this)
}
Expand Down
72 changes: 46 additions & 26 deletions authorization/src/main/kotlin/org/modelix/authorization/PermissionManagementPage.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import io.ktor.server.application.ApplicationCall
import io.ktor.server.application.application
import io.ktor.server.application.call
import io.ktor.server.application.plugin
import io.ktor.server.auth.principal
import io.ktor.server.html.respondHtml
import io.ktor.server.request.receiveParameters
import io.ktor.server.response.respond
Expand All @@ -26,8 +27,8 @@ import kotlinx.html.td
import kotlinx.html.textInput
import kotlinx.html.th
import kotlinx.html.tr
import org.modelix.authorization.permissions.PermissionParts
import org.modelix.authorization.permissions.PermissionSchemaBase
import org.modelix.authorization.permissions.PermissionInstanceReference
import org.modelix.authorization.permissions.PermissionParser

fun Route.installPermissionManagementHandlers() {
route("permissions") {
Expand All @@ -42,9 +43,7 @@ fun Route.installPermissionManagementHandlers() {
val roleId = formParameters["roleId"]
require(userId != null || roleId != null) { "userId or roleId required" }
val permissionId = requireNotNull(formParameters["permissionId"]) { "permissionId not specified" }

// a user can grant his own permission to other users
checkPermission(PermissionParts.fromString(permissionId))
call.checkCanGranPermission(permissionId)

if (userId != null) {
application.plugin(ModelixAuthorization).config.accessControlPersistence.update {
Expand All @@ -59,12 +58,12 @@ fun Route.installPermissionManagementHandlers() {
call.respond("Granted $permissionId to ${userId ?: roleId}")
}
post("remove-grant") {
call.checkPermission(PermissionSchemaBase.permissionData.write)
val formParameters = call.receiveParameters()
val userId = formParameters["userId"]
val roleId = formParameters["roleId"]
require(userId != null || roleId != null) { "userId or roleId required" }
val permissionId = requireNotNull(formParameters["permissionId"]) { "permissionId not specified" }
call.checkCanGranPermission(permissionId)
if (userId != null) {
application.plugin(ModelixAuthorization).config.accessControlPersistence.update {
it.withoutGrantToUser(userId, permissionId)
Expand Down Expand Up @@ -138,7 +137,7 @@ fun HTML.buildPermissionManagementPage(call: ApplicationCall, pluginInstance: Mo
th { +"Permission" }
}
for ((userId, permission) in pluginInstance.config.accessControlPersistence.read().grantsToUsers.flatMap { entry -> entry.value.map { entry.key to it } }) {
if (!call.hasPermission(PermissionParts.fromString(permission))) continue
if (!call.canGrantPermission(permission)) continue

tr {
td {
Expand Down Expand Up @@ -174,7 +173,7 @@ fun HTML.buildPermissionManagementPage(call: ApplicationCall, pluginInstance: Mo
th { +"Permission" }
}
for ((roleId, permission) in pluginInstance.config.accessControlPersistence.read().grantsToRoles.flatMap { entry -> entry.value.map { entry.key to it } }) {
if (!call.hasPermission(PermissionParts.fromString(permission))) continue
if (!call.canGrantPermission(permission)) continue

tr {
td {
Expand Down Expand Up @@ -213,32 +212,30 @@ fun HTML.buildPermissionManagementPage(call: ApplicationCall, pluginInstance: Mo
th { +"Grant" }
}
for (deniedPermission in pluginInstance.getDeniedPermissions()) {
if (!call.hasPermission(deniedPermission.permissionId)) continue
if (!call.canGrantPermission(deniedPermission.permissionRef)) continue

val userId = deniedPermission.userId
tr {
td {
+userId.orEmpty()
+userId
}
td {
+deniedPermission.permissionId.fullId
+deniedPermission.permissionRef.toPermissionParts().fullId
}
td {
if (userId != null) {
val evaluator = pluginInstance.createPermissionEvaluator()
val permissionInstance = evaluator.instantiatePermission(deniedPermission.permissionId)
val candidates = (setOf(permissionInstance) + permissionInstance.transitiveIncludedIn())
postForm(action = "grant") {
hiddenInput {
name = "userId"
value = userId
}
for (candidate in candidates) {
div {
submitInput {
name = "permissionId"
value = candidate.ref.toString()
}
val evaluator = pluginInstance.createPermissionEvaluator()
val permissionInstance = evaluator.instantiatePermission(deniedPermission.permissionRef)
val candidates = (setOf(permissionInstance) + permissionInstance.transitiveIncludedIn())
postForm(action = "grant") {
hiddenInput {
name = "userId"
value = userId
}
for (candidate in candidates) {
div {
submitInput {
name = "permissionId"
value = candidate.ref.toString()
}
}
}
Expand All @@ -249,3 +246,26 @@ fun HTML.buildPermissionManagementPage(call: ApplicationCall, pluginInstance: Mo
}
}
}

fun ApplicationCall.canGrantPermission(permissionId: String): Boolean {
return canGrantPermission(parsePermission(permissionId))
}

fun ApplicationCall.canGrantPermission(permissionRef: PermissionInstanceReference): Boolean {
val resources = generateSequence(permissionRef.resource) { it.parent }
return resources.any {
// hardcoded admin/owner to keep it simple and not having to introduce a permission schema for permissions
hasPermission(PermissionInstanceReference("admin", it)) || hasPermission(PermissionInstanceReference("owner", it))
}
}

fun ApplicationCall.checkCanGranPermission(id: String) {
if (!canGrantPermission(id)) {
val principal = principal<AccessTokenPrincipal>()
throw NoPermissionException(principal, null, null, "${principal?.getUserName()} has no permission '$id'")
}
}

fun ApplicationCall.parsePermission(id: String): PermissionInstanceReference {
return application.plugin(ModelixAuthorization).config.permissionSchema.let { PermissionParser(it) }.parse(id)
}
5 changes: 4 additions & 1 deletion authorization/src/main/kotlin/org/modelix/authorization/permissions/PermissionEvaluator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ class PermissionEvaluator(val schemaInstance: SchemaInstance) {
}

fun instantiatePermission(permissionId: PermissionParts): SchemaInstance.ResourceInstance.PermissionInstance {
val permissionRef = parser.parse(permissionId)
return instantiatePermission(parser.parse(permissionId))
}

fun instantiatePermission(permissionRef: PermissionInstanceReference): SchemaInstance.ResourceInstance.PermissionInstance {
val instance = schemaInstance.instantiatePermission(permissionRef)
hasPermission(permissionRef) // permissions are instantiated during the check
return instance
Expand Down
16 changes: 2 additions & 14 deletions model-server/src/main/kotlin/org/modelix/model/server/ModelServerPermissionSchema.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import org.modelix.model.lazy.RepositoryId
object ModelServerPermissionSchema {
private const val MODEL_SERVER = "model-server"
private const val ADMIN = "admin"
private const val PERMISSION_SCHEMA = "permission-schema"
private const val WRITE = "write"
private const val READ = "read"
private const val LEGACY_USER_DEFINED_ENTRIES = "legacy-user-defined-entries"
Expand All @@ -35,28 +34,17 @@ object ModelServerPermissionSchema {
}
}

resource(PERMISSION_SCHEMA) {
permission(WRITE) {
includedIn(MODEL_SERVER, ADMIN)
permission(READ)
}
}

resource(LEGACY_USER_DEFINED_ENTRIES) {
permission(READ) {
includedIn(MODEL_SERVER, ADMIN)
}
permission(WRITE) {
includedIn(MODEL_SERVER, ADMIN)
permission(READ)
}
}

resource(LEGACY_GLOBAL_OBJECTS) {
permission(READ) {
includedIn(MODEL_SERVER, ADMIN)
}
permission(ADD) {
includedIn(MODEL_SERVER, ADMIN)
permission(READ)
}
}

Expand Down

0 comments on commit f2d2afb

Please sign in to comment.