diff --git a/common/src/main/kotlin/gg/grounds/permissions/PermissionChecker.kt b/common/src/main/kotlin/gg/grounds/permissions/PermissionChecker.kt index ec3253e..c7edbb5 100644 --- a/common/src/main/kotlin/gg/grounds/permissions/PermissionChecker.kt +++ b/common/src/main/kotlin/gg/grounds/permissions/PermissionChecker.kt @@ -64,16 +64,23 @@ class InMemoryPermissionSnapshots(initialSnapshots: Map = emptySet(), ) : Permissions { constructor( snapshots: Map, defaultScope: PermissionCheckScope = PermissionCheckScope.global(), clock: Clock = Clock.systemUTC(), - ) : this(InMemoryPermissionSnapshots(snapshots), defaultScope, clock) + negativePermissions: Set = emptySet(), + ) : this(InMemoryPermissionSnapshots(snapshots), defaultScope, clock, negativePermissions) override fun hasPermission(playerId: UUID, permission: String): Boolean { return hasPermission(playerId, permission, defaultScope) @@ -100,11 +107,13 @@ class SnapshotPermissions( return false } + val negative = permission in negativePermissions + val candidate = (snapshot.allowPatterns + snapshot.denyPatterns) .asSequence() .filterNot { it.isExpired(now) } - .filter { PermissionPattern.matches(it.pattern, permission) } + .filter { PermissionPattern.matches(it.pattern, permission, negative) } .mapNotNull { grant -> grant.toCandidate(scope) } .maxWithOrNull( compareBy { it.scopeSpecificity } @@ -147,15 +156,24 @@ class SnapshotPermissions( } private object PermissionPattern { - fun matches(pattern: String, permission: String): Boolean = - when { + fun matches(pattern: String, permission: String, negative: Boolean = false): Boolean { + if (pattern == permission) { + return true + } + // A wildcard says "everything", which is never meant to include a permission that takes + // something away. Those have to be granted by name. + if (negative) { + return false + } + return when { pattern == "*" -> true pattern.endsWith(".*") -> { val prefix = pattern.removeSuffix("*") permission.startsWith(prefix) && permission.length > prefix.length } - else -> pattern == permission + else -> false } + } fun specificity(pattern: String): Int = when { diff --git a/common/src/main/kotlin/gg/grounds/permissions/catalog/PermissionManifest.kt b/common/src/main/kotlin/gg/grounds/permissions/catalog/PermissionManifest.kt index 2fb0b68..53dd48e 100644 --- a/common/src/main/kotlin/gg/grounds/permissions/catalog/PermissionManifest.kt +++ b/common/src/main/kotlin/gg/grounds/permissions/catalog/PermissionManifest.kt @@ -60,6 +60,15 @@ data class PermissionManifestEntry( val label: String, val description: String, val supportedScopes: List, + /** + * Marks a permission whose meaning is inverted: holding it takes something away rather than + * granting it (a mute, a ban from a channel). + * + * A wildcard grant means "give this player everything", which must never be read as "mute + * them". Negative permissions are therefore only ever matched by an exact grant — see + * `SnapshotPermissions`. + */ + val negative: Boolean = false, ) enum class PermissionManifestScope { diff --git a/common/src/main/kotlin/gg/grounds/permissions/catalog/PermissionManifestCollector.kt b/common/src/main/kotlin/gg/grounds/permissions/catalog/PermissionManifestCollector.kt index c6aae69..bee0981 100644 --- a/common/src/main/kotlin/gg/grounds/permissions/catalog/PermissionManifestCollector.kt +++ b/common/src/main/kotlin/gg/grounds/permissions/catalog/PermissionManifestCollector.kt @@ -9,7 +9,21 @@ data class PermissionManifestCollectionFailure(val origin: ManifestOrigin, val r data class PermissionManifestCollection( val manifests: List, val failures: List, -) +) { + /** + * The permission keys the loaded plugins declared as negative. + * + * Only plugins running in this JVM can be asked about a permission, so their own manifests are + * the complete authority here — no catalog round-trip needed. + */ + fun negativePermissionKeys(): Set = + manifests + .asSequence() + .flatMap { it.manifest.permissions.asSequence() } + .filter { it.negative } + .map { it.key } + .toSet() +} class PermissionManifestCollector { fun collect(origins: Iterable): PermissionManifestCollection { diff --git a/common/src/test/kotlin/gg/grounds/permissions/PermissionCheckerTest.kt b/common/src/test/kotlin/gg/grounds/permissions/PermissionCheckerTest.kt index f70d256..4aff833 100644 --- a/common/src/test/kotlin/gg/grounds/permissions/PermissionCheckerTest.kt +++ b/common/src/test/kotlin/gg/grounds/permissions/PermissionCheckerTest.kt @@ -201,15 +201,69 @@ class PermissionCheckerTest { assertFalse(permissions.hasPermission(playerId, "chat.send")) } + // An operator holds ALLOW *. That says "give this player everything" — it must not hand them + // grounds.chat.muted, which takes chat away. This muted every admin on the network. + @Test + fun globalWildcardDoesNotGrantANegativePermission() { + val permissions = + permissions( + allowPatterns = listOf(allow("*")), + negativePermissions = setOf("grounds.chat.muted"), + ) + + assertFalse(permissions.hasPermission(playerId, "grounds.chat.muted")) + assertTrue(permissions.hasPermission(playerId, "grounds.chat.staff")) + } + + @Test + fun prefixWildcardDoesNotGrantANegativePermission() { + val permissions = + permissions( + allowPatterns = listOf(allow("grounds.chat.*")), + negativePermissions = setOf("grounds.chat.muted"), + ) + + assertFalse(permissions.hasPermission(playerId, "grounds.chat.muted")) + assertTrue(permissions.hasPermission(playerId, "grounds.chat.staff")) + } + + // Muting somebody still has to work — by name. + @Test + fun exactGrantStillAppliesANegativePermission() { + val permissions = + permissions( + allowPatterns = listOf(allow("grounds.chat.muted")), + negativePermissions = setOf("grounds.chat.muted"), + ) + + assertTrue(permissions.hasPermission(playerId, "grounds.chat.muted")) + } + + // A wildcard cannot grant it, so a DENY next to it has nothing to overrule — but an exact + // grant that was later revoked must still lose. + @Test + fun denyBeatsAnExactGrantOfANegativePermission() { + val permissions = + permissions( + allowPatterns = listOf(allow("grounds.chat.muted")), + denyPatterns = listOf(deny("grounds.chat.muted")), + negativePermissions = setOf("grounds.chat.muted"), + ) + + assertFalse(permissions.hasPermission(playerId, "grounds.chat.muted")) + } + private fun permissions( allowPatterns: List = emptyList(), denyPatterns: List = emptyList(), scope: PermissionCheckScope = PermissionCheckScope.global(), + negativePermissions: Set = emptySet(), ): Permissions = SnapshotPermissions( mapOf(playerId to snapshot(allowPatterns = allowPatterns, denyPatterns = denyPatterns)), defaultScope = scope, clock = clock, + negativePermissions = negativePermissions, ) private fun snapshot( diff --git a/minestom/src/main/kotlin/gg/grounds/permissions/minestom/GroundsPermissionsModule.kt b/minestom/src/main/kotlin/gg/grounds/permissions/minestom/GroundsPermissionsModule.kt index b1ed483..c9aa221 100644 --- a/minestom/src/main/kotlin/gg/grounds/permissions/minestom/GroundsPermissionsModule.kt +++ b/minestom/src/main/kotlin/gg/grounds/permissions/minestom/GroundsPermissionsModule.kt @@ -7,6 +7,7 @@ import gg.grounds.permissions.PermissionSnapshotRefreshSweep import gg.grounds.permissions.Permissions import gg.grounds.permissions.SnapshotPermissions import gg.grounds.permissions.catalog.CollectedPermissionManifest +import gg.grounds.permissions.catalog.PermissionManifestCollection import gg.grounds.runtime.GroundsModule import gg.grounds.runtime.GroundsServerContext import java.time.Clock @@ -46,11 +47,17 @@ class GroundsPermissionsModule(private val clock: Clock = Clock.systemUTC()) : G Executors.newSingleThreadExecutor { runnable -> Thread(runnable, "grounds-permissions-manifest-catalog").apply { isDaemon = true } } + // Collected before the checker is built: it has to know which permissions are negative + // before it answers its first question. Registration with the catalog service is the slow + // part and stays off-thread below. + val manifests = collectActivePermissionManifests(ctx.activeModuleProviders) + val permissions = SnapshotPermissions( snapshots = snapshots, defaultScope = config.context.toCheckScope(), clock = clock, + negativePermissions = manifests.negativePermissionKeys(), ) val loader = MinestomPermissionSnapshotLoader( @@ -105,7 +112,7 @@ class GroundsPermissionsModule(private val clock: Clock = Clock.systemUTC()) : G ctx.onShutdown { stop() } registerActivePermissionManifests( - activeProviders = ctx.activeModuleProviders, + manifests = manifests, manifestClient = manifestClient, manifestExecutor = manifestExecutor, context = config.context, @@ -133,15 +140,14 @@ class GroundsPermissionsModule(private val clock: Clock = Clock.systemUTC()) : G } private fun registerActivePermissionManifests( - activeProviders: Iterable, + manifests: PermissionManifestCollection, manifestClient: PermissionCatalogClient, manifestExecutor: ExecutorService, context: PermissionSnapshotContext, ) { try { manifestExecutor.execute { - val collection = collectActivePermissionManifests(activeProviders) - collection.failures.forEach { failure -> + manifests.failures.forEach { failure -> logger.warn( "Skipped malformed permission manifest (originId={}, originVersion={}, reason={})", failure.origin.id, @@ -151,7 +157,7 @@ class GroundsPermissionsModule(private val clock: Clock = Clock.systemUTC()) : G } val registration = MinestomPermissionManifestRegistrar(manifestClient, context) - .register(collection.manifests) + .register(manifests.manifests) registration.registered.forEach(::logManifestRegistration) registration.failures.forEach { failure -> logger.warn( diff --git a/velocity/src/main/kotlin/gg/grounds/permissions/velocity/GroundsPermissionsPlugin.kt b/velocity/src/main/kotlin/gg/grounds/permissions/velocity/GroundsPermissionsPlugin.kt index 8358d68..d26d8c4 100644 --- a/velocity/src/main/kotlin/gg/grounds/permissions/velocity/GroundsPermissionsPlugin.kt +++ b/velocity/src/main/kotlin/gg/grounds/permissions/velocity/GroundsPermissionsPlugin.kt @@ -17,6 +17,7 @@ import gg.grounds.permissions.PermissionSnapshotRefreshSweep import gg.grounds.permissions.Permissions import gg.grounds.permissions.SnapshotPermissions import gg.grounds.permissions.catalog.PermissionManifest +import gg.grounds.permissions.catalog.PermissionManifestCollection import gg.grounds.permissions.catalog.PermissionManifestCollector import io.grpc.LoadBalancerRegistry import io.grpc.NameResolverRegistry @@ -101,8 +102,17 @@ constructor( .repeat(config.refreshIntervalSeconds, TimeUnit.SECONDS) .schedule() + // Collected before the checker is built: it has to know which permissions are negative + // before it answers its first question. Registration with the catalog service is the slow + // part and stays on the scheduler below. + val manifests = collectPermissionManifests() + val permissions = - SnapshotPermissions(snapshots, defaultScope = config.context.toCheckScope()) + SnapshotPermissions( + snapshots, + defaultScope = config.context.toCheckScope(), + negativePermissions = manifests.negativePermissionKeys(), + ) this.permissions = permissions loadCommandPermissions()?.let { commandPermissions -> val router = @@ -154,7 +164,10 @@ constructor( } proxy.scheduler - .buildTask(this, Runnable { registerActivePermissionManifests(manifestClient, config) }) + .buildTask( + this, + Runnable { registerActivePermissionManifests(manifestClient, config, manifests) }, + ) .schedule() logger.info( @@ -201,10 +214,7 @@ constructor( null } - private fun registerActivePermissionManifests( - manifestClient: PermissionCatalogClient, - config: VelocityPermissionsConfig, - ) { + private fun collectPermissionManifests(): PermissionManifestCollection { val collection = PermissionManifestCollector() .collect(discoverPermissionManifestOrigins(proxy.pluginManager.plugins)) @@ -216,6 +226,14 @@ constructor( failure.reason, ) } + return collection + } + + private fun registerActivePermissionManifests( + manifestClient: PermissionCatalogClient, + config: VelocityPermissionsConfig, + collection: PermissionManifestCollection, + ) { val registration = PermissionManifestRegistrar(manifestClient, config.context) .register(collection.manifests)