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 committed Dec 11, 2024
1 parent 11c6d46 commit 85a72b3
Show file tree
Hide file tree
Showing 18 changed files with 67 additions and 75 deletions.
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()

Check warning

Code scanning / detekt

The function getDeniedPermissions is missing documentation. Warning

The function getDeniedPermissions is missing documentation.

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

fun hasPermission(call: ApplicationCall, permissionToCheck: PermissionInstanceReference): Boolean {

Check warning

Code scanning / detekt

The function hasPermission is missing documentation. Warning

The function hasPermission is missing documentation.
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(

Check warning

Code scanning / detekt

DeniedPermissionRequest is missing required documentation. Warning

DeniedPermissionRequest is missing required documentation.
val permissionId: PermissionParts,
val permissionRef: PermissionInstanceReference,

Check warning

Code scanning / detekt

The property permissionRef is missing documentation. Warning

The property permissionRef is missing documentation.
val userId: String,

Check warning

Code scanning / detekt

The property userId is missing documentation. Warning

The property userId is missing documentation.
val jwtPayload: String,

Check warning

Code scanning / detekt

The property jwtPayload is missing documentation. Warning

The property jwtPayload is missing documentation.
) {
Expand Down
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 {

Check warning

Code scanning / detekt

The function hasPermission is missing documentation. Warning

The function hasPermission is missing documentation.
return application.plugin(ModelixAuthorization).hasPermission(this, permissionToCheck)
}

fun ApplicationCall.getPermissionEvaluator(): PermissionEvaluator {
return application.plugin(ModelixAuthorization).getPermissionEvaluator(this)
}
Expand Down
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() {

Check warning

Code scanning / detekt

The function installPermissionManagementHandlers is missing documentation. Warning

The function installPermissionManagementHandlers is missing documentation.
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 {

Check warning

Code scanning / detekt

The function canGrantPermission is missing documentation. Warning

The function canGrantPermission is missing documentation.
return canGrantPermission(parsePermission(permissionId))
}

fun ApplicationCall.canGrantPermission(permissionRef: PermissionInstanceReference): Boolean {

Check warning

Code scanning / detekt

The function canGrantPermission is missing documentation. Warning

The function canGrantPermission is missing documentation.
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) {

Check warning

Code scanning / detekt

The function checkCanGranPermission is missing documentation. Warning

The function checkCanGranPermission is missing documentation.
if (!canGrantPermission(id)) {
val principal = principal<AccessTokenPrincipal>()
throw NoPermissionException(principal, null, null, "${principal?.getUserName()} has no permission '$id'")
}
}

fun ApplicationCall.parsePermission(id: String): PermissionInstanceReference {

Check warning

Code scanning / detekt

The function parsePermission is missing documentation. Warning

The function parsePermission is missing documentation.
return application.plugin(ModelixAuthorization).config.permissionSchema.let { PermissionParser(it) }.parse(id)
}
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package org.modelix.model.server

import io.ktor.server.testing.ApplicationTestBuilder
import io.ktor.server.testing.testApplication
import org.modelix.authorization.installAuthentication
import org.modelix.model.api.INode
import org.modelix.model.api.NullChildLink
import org.modelix.model.api.PBranch
Expand Down Expand Up @@ -39,7 +38,6 @@ class LazyLoadingTest {

private fun runTest(block: suspend ApplicationTestBuilder.() -> Unit) = testApplication {
application {
installAuthentication(unitTestMode = true)
installDefaultServerPlugins()
val store = InMemoryStoreClient()
val repoManager = RepositoriesManager(store)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
package org.modelix.model.server

import io.ktor.server.application.install
import io.ktor.server.resources.Resources
import io.ktor.server.routing.IgnoreTrailingSlash
import io.ktor.server.testing.ApplicationTestBuilder
import io.ktor.server.testing.testApplication
import kotlinx.coroutines.delay
import kotlinx.coroutines.withTimeout
import org.modelix.authorization.installAuthentication
import org.modelix.model.IKeyListener
import org.modelix.model.client.RestWebModelClient
import org.modelix.model.server.handlers.KeyValueLikeModelServer
Expand All @@ -24,9 +20,7 @@ class ModelClientTest {

private fun runTest(block: suspend ApplicationTestBuilder.() -> Unit) = testApplication {
application {
installAuthentication(unitTestMode = true)
install(Resources)
install(IgnoreTrailingSlash)
installDefaultServerPlugins()
KeyValueLikeModelServer(RepositoriesManager(InMemoryStoreClient())).init(this)
}
block()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import io.ktor.server.testing.ApplicationTestBuilder
import io.ktor.server.websocket.WebSockets
import kotlinx.coroutines.runBlocking
import org.modelix.authorization.ModelixAuthorization
import org.modelix.authorization.installAuthentication
import org.modelix.model.client2.ModelClientV2
import org.modelix.model.server.Main.installStatusPages
import org.modelix.model.server.handlers.Paths.registerJsonTypes
Expand Down Expand Up @@ -55,7 +54,6 @@ fun runWithNettyServer(
testBlock: suspend (server: NettyApplicationEngine) -> Unit,
) {
val nettyServer: NettyApplicationEngine = io.ktor.server.engine.embeddedServer(Netty, port = 0) {
installAuthentication(unitTestMode = true)
installDefaultServerPlugins()
setupBlock(this)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package org.modelix.model.server
import io.ktor.server.testing.ApplicationTestBuilder
import io.ktor.server.testing.testApplication
import kotlinx.coroutines.coroutineScope
import org.modelix.authorization.installAuthentication
import org.modelix.model.api.IChildLink
import org.modelix.model.api.IConceptReference
import org.modelix.model.api.INode
Expand All @@ -25,7 +24,6 @@ class PullPerformanceTest {
val storeClientWithStatistics = StoreClientWithStatistics(InMemoryStoreClient())
val repositoriesManager = RepositoriesManager(storeClientWithStatistics)
application {
installAuthentication(unitTestMode = true)
installDefaultServerPlugins()
ModelReplicationServer(repositoriesManager).init(this)
KeyValueLikeModelServer(repositoriesManager).init(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import org.junit.Assert.assertFalse
import org.modelix.authorization.installAuthentication
import org.modelix.model.api.ChildLinkFromName
import org.modelix.model.api.ConceptReference
import org.modelix.model.api.IBranch
Expand Down Expand Up @@ -117,7 +116,6 @@ class ReplicatedModelTest {

private fun runTest(block: suspend ApplicationTestBuilder.() -> Unit) = testApplication {
application {
installAuthentication(unitTestMode = true)
installDefaultServerPlugins()
val repoManager = RepositoriesManager(InMemoryStoreClient())
ModelReplicationServer(repoManager).init(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import kotlinx.coroutines.launch
import kotlinx.coroutines.withTimeout
import org.junit.jupiter.api.RepeatedTest
import org.junit.jupiter.api.RepetitionInfo
import org.modelix.authorization.installAuthentication
import org.modelix.model.ModelFacade
import org.modelix.model.VersionMerger
import org.modelix.model.api.IBranch
Expand Down Expand Up @@ -51,7 +50,6 @@ class ReplicatedRepositoryTest {

private fun runTest(block: suspend ApplicationTestBuilder.(scope: CoroutineScope) -> Unit) = testApplication {
application {
installAuthentication(unitTestMode = true)
installDefaultServerPlugins()
val storeClient = InMemoryStoreClient()
val repositoriesManager = RepositoriesManager(storeClient)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import io.ktor.server.testing.testApplication
import kotlinx.coroutines.async
import kotlinx.coroutines.sync.Mutex
import org.junit.jupiter.api.Test
import org.modelix.authorization.installAuthentication
import org.modelix.model.server.handlers.KeyValueLikeModelServer
import org.modelix.model.server.handlers.RepositoriesManager
import org.modelix.model.server.store.InMemoryStoreClient
Expand All @@ -27,7 +26,6 @@ class V1ApiTest {
val repositoriesManager = RepositoriesManager(InMemoryStoreClient())

application {
installAuthentication(unitTestMode = true)
installDefaultServerPlugins()
KeyValueLikeModelServer(repositoriesManager).init(this)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import io.mockk.every
import io.mockk.spyk
import kotlinx.serialization.json.Json
import org.junit.jupiter.api.Test
import org.modelix.authorization.installAuthentication
import org.modelix.model.server.installDefaultServerPlugins
import org.modelix.model.server.store.InMemoryStoreClient
import kotlin.test.AfterTest
Expand All @@ -24,7 +23,6 @@ class HealthApiTest {

private fun runApiTest(block: suspend ApplicationTestBuilder.() -> Unit) = testApplication {
application {
installAuthentication(unitTestMode = true)
installDefaultServerPlugins()
routing {
healthApiSpy.installRoutes(this)
Expand Down
Loading

0 comments on commit 85a72b3

Please sign in to comment.