From 1262a2b2a87be090435df40dd9b9ab3aaf08b527 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Fri, 31 Oct 2025 15:07:19 +0000 Subject: [PATCH 1/5] Drive-by contributing doc update for policies makefile --- docs/development/contributing.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/development/contributing.md b/docs/development/contributing.md index 1bb04e730..4f2472314 100644 --- a/docs/development/contributing.md +++ b/docs/development/contributing.md @@ -72,6 +72,7 @@ Make sure your code adheres to our Rust and TypeScript code style by running: - `cargo +nightly fmt` (with the nightly toolchain installed) - `npm run format` in the `frontend` directory + - `make fmt` in the `policies` directory (if changed) When updating SQL queries in the `crates/storage-pg/` crate, you may need to update the `sqlx` introspection data. To do this, make sure to install `cargo-sqlx` (`cargo install sqlx-cli`) and: @@ -86,6 +87,7 @@ While you're developing and before submitting a patch, you'll want to test your - Run `cargo clippy --workspace` to lint the Rust code. - Run `npm run lint` in the `frontend` directory to lint the frontend code. +- Run `make fmt` and `make lint` in the `policies` directory to format and lint the included policy. ### Run the tests @@ -93,6 +95,10 @@ If you haven't already, install [Cargo-Nextest](https://nexte.st/docs/installati - Run the tests to the backend by running `cargo nextest run --workspace`. This requires a connection to a PostgreSQL database, set via the `DATABASE_URL` environment variable. - Run the tests to the frontend by running `npm run test` in the `frontend` directory. +- To run the tests for the included policy, change to the `policies` directory and run one of: + - `make test` (needs OpenPolicyAgent installed) + - `make PODMAN=1 test` (runs inside a container; needs Podman installed) + - `make DOCKER=1 test` (runs inside a container; needs Docker installed) ## 8. Submit a pull request From c8184fd5aa223d02645a462c041075c9ece3806a Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Fri, 31 Oct 2025 15:07:29 +0000 Subject: [PATCH 2/5] Drive-by podman Makefile fix --- policies/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/policies/Makefile b/policies/Makefile index 421990d07..0d515b904 100644 --- a/policies/Makefile +++ b/policies/Makefile @@ -24,9 +24,9 @@ ifeq ($(DOCKER), 1) REGAL := docker run -i -v $(shell pwd):/policies:ro -w /policies --rm $(REGAL_DOCKER_IMAGE) else ifeq ($(PODMAN), 1) # When running rootless, the volume directory may need to be given global write permissions on the host - OPA := podman run -i -v $(shell pwd):/policies:ro:Z -w /policies --rm $(OPA_DOCKER_IMAGE) + OPA := podman run -i -v $(shell pwd):/policies:ro,Z -w /policies --rm $(OPA_DOCKER_IMAGE) OPA_RW := podman run -i -v $(shell pwd):/policies:Z -w /policies --rm $(OPA_DOCKER_IMAGE) - REGAL := podman run -i -v $(shell pwd):/policies:ro:Z -w /policies --rm $(REGAL_DOCKER_IMAGE) + REGAL := podman run -i -v $(shell pwd):/policies:ro,Z -w /policies --rm $(REGAL_DOCKER_IMAGE) else OPA := opa OPA_RW := opa From f45d9c1291a57af5b0a0654a98e832dd716e6e6f Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Fri, 31 Oct 2025 15:12:45 +0000 Subject: [PATCH 3/5] Update tests to prepare for needing C-S API scope --- .../authorization_grant_test.rego | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/policies/authorization_grant/authorization_grant_test.rego b/policies/authorization_grant/authorization_grant_test.rego index fdde534d0..bbcac5748 100644 --- a/policies/authorization_grant/authorization_grant_test.rego +++ b/policies/authorization_grant/authorization_grant_test.rego @@ -78,65 +78,65 @@ test_unstable_device_scopes if { authorization_grant.allow with input.user as user with input.client as client with input.grant_type as "authorization_code" - with input.scope as "urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd01" + with input.scope as "urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd01" authorization_grant.allow with input.user as user with input.client as client with input.grant_type as "authorization_code" - with input.scope as "urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd01-asdasdsa1-2313" + with input.scope as "urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd01-asdasdsa1-2313" # Too short not authorization_grant.allow with input.user as user with input.client as client with input.grant_type as "authorization_code" - with input.scope as "urn:matrix:org.matrix.msc2967.client:device:abcd" + with input.scope as "urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:abcd" # Multiple device scope not authorization_grant.allow with input.user as user with input.client as client with input.grant_type as "authorization_code" - with input.scope as "urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd01 urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd02" + with input.scope as "urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd01 urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd02" # Allowed with the device code grant authorization_grant.allow with input.user as user with input.client as client with input.grant_type as "urn:ietf:params:oauth:grant-type:device_code" - with input.scope as "urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd01" + with input.scope as "urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd01" # Not authorization_grant.allowed for the client credentials grant not authorization_grant.allow with input.client as client with input.grant_type as "client_credentials" - with input.scope as "urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd01" + with input.scope as "urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd01" } test_stable_device_scopes if { authorization_grant.allow with input.user as user with input.client as client with input.grant_type as "authorization_code" - with input.scope as "urn:matrix:client:device:AAbbCCdd01" + with input.scope as "urn:matrix:client:api:* urn:matrix:client:device:AAbbCCdd01" authorization_grant.allow with input.user as user with input.client as client with input.grant_type as "authorization_code" - with input.scope as "urn:matrix:client:device:AAbbCCdd01-asdasdsa1-2313" + with input.scope as "urn:matrix:client:api:* urn:matrix:client:device:AAbbCCdd01-asdasdsa1-2313" # Too short not authorization_grant.allow with input.user as user with input.client as client with input.grant_type as "authorization_code" - with input.scope as "urn:matrix:client:device:abcd" + with input.scope as "urn:matrix:client:api:* urn:matrix:client:device:abcd" # Multiple device scope not authorization_grant.allow with input.user as user with input.client as client with input.grant_type as "authorization_code" - with input.scope as "urn:matrix:client:device:AAbbCCdd01 urn:matrix:client:device:AAbbCCdd02" + with input.scope as "urn:matrix:client:api:* urn:matrix:client:device:AAbbCCdd01 urn:matrix:client:device:AAbbCCdd02" # Allowed with the device code grant authorization_grant.allow with input.user as user with input.client as client with input.grant_type as "urn:ietf:params:oauth:grant-type:device_code" - with input.scope as "urn:matrix:client:device:AAbbCCdd01" + with input.scope as "urn:matrix:client:api:* urn:matrix:client:device:AAbbCCdd01" # Not authorization_grant.allowed for the client credentials grant not authorization_grant.allow with input.client as client From aeabc9cbf29e27a0f6fb9be08c26f3729335ce1b Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Fri, 31 Oct 2025 15:13:07 +0000 Subject: [PATCH 4/5] Only allow C-S device scopes when the C-S API scope has been requested It'd be weird for a client to request a device on the client-server API but yet not request any client-server API scopes to use it with. By adding this restriction, we can then create a partial index on the oauth2_sessions table to quickly identify sessions that have C-S API scopes and use this as a proxy metric for how many sessions may have device scopes. This in turn makes it feasible to efficiently limit the number of 'devices' a user has, or more precisely: the number of sessions with client-server API access. We can't do the same for device scopes themselves because, other than nastiness like parsing the JSON stringification of the scope list, it's not feasible to identify device scopes within a Postgres index predicate. Part of: #4339 --- .../authorization_grant.rego | 26 +++++++++++++++++++ .../authorization_grant_test.rego | 16 +++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/policies/authorization_grant/authorization_grant.rego b/policies/authorization_grant/authorization_grant.rego index b409cc889..79f737af1 100644 --- a/policies/authorization_grant/authorization_grant.rego +++ b/policies/authorization_grant/authorization_grant.rego @@ -98,6 +98,26 @@ uses_stable_scopes if { count({scope | some scope in scope_list; startswith(scope, "urn:matrix:client:")}) > 0 } +has_device_scope if { + scope_list := split(input.scope, " ") + count({scope | some scope in scope_list; startswith(scope, "urn:matrix:client:device:")}) > 0 +} + +has_device_scope if { + scope_list := split(input.scope, " ") + count({scope | some scope in scope_list; startswith(scope, "urn:matrix:org.matrix.msc2967.client:device:")}) > 0 +} + +has_cs_api_scope if { + scope_list := split(input.scope, " ") + count({scope | some scope in scope_list; startswith(scope, "urn:matrix:client:api:")}) > 0 +} + +has_cs_api_scope if { + scope_list := split(input.scope, " ") + count({scope | some scope in scope_list; startswith(scope, "urn:matrix:org.matrix.msc2967.client:api:")}) > 0 +} + # METADATA # entrypoint: true violation contains {"msg": msg} if { @@ -116,6 +136,12 @@ violation contains {"msg": "only one device scope is allowed at a time"} if { count({scope | some scope in scope_list; startswith(scope, "urn:matrix:client:device:")}) > 1 } +# Prevent the creation of C-S API devices for sessions that don't have C-S API access. +violation contains {"msg": "device scopes are only allowed when the client-server API scope is requested"} if { + has_device_scope + not has_cs_api_scope +} + violation contains {"msg": "request cannot mix unstable and stable scopes"} if { uses_stable_scopes uses_unstable_scopes diff --git a/policies/authorization_grant/authorization_grant_test.rego b/policies/authorization_grant/authorization_grant_test.rego index bbcac5748..6634eacb9 100644 --- a/policies/authorization_grant/authorization_grant_test.rego +++ b/policies/authorization_grant/authorization_grant_test.rego @@ -141,7 +141,21 @@ test_stable_device_scopes if { # Not authorization_grant.allowed for the client credentials grant not authorization_grant.allow with input.client as client with input.grant_type as "client_credentials" - with input.scope as "urn:matrix:client:device:AAbbCCdd01" + with input.scope as "urn:matrix:client:api:* urn:matrix:client:device:AAbbCCdd01" +} + +test_device_scope_only_with_cs_api_scope if { + not authorization_grant.allow with input.user as user + with input.client as client + with input.grant_type as "authorization_code" + # Requested a device scope but no C-S API scope: +with input.scope as "urn:matrix:client:device:AAbbCCdd01" + + not authorization_grant.allow with input.user as user + with input.client as client + with input.grant_type as "authorization_code" + # Requested a device scope but no C-S API scope: +with input.scope as "urn:matrix:org.matrix.msc2967.client:device:AAbbCCdd01" } test_mix_stable_and_unstable_scopes if { From 5bbc26cb16e0b651d921f96409e003e57c80ac65 Mon Sep 17 00:00:00 2001 From: reivilibre Date: Wed, 5 Nov 2025 15:40:12 +0000 Subject: [PATCH 5/5] Update docs/development/contributing.md Co-authored-by: Quentin Gliech --- docs/development/contributing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development/contributing.md b/docs/development/contributing.md index 4f2472314..27367b946 100644 --- a/docs/development/contributing.md +++ b/docs/development/contributing.md @@ -96,7 +96,7 @@ If you haven't already, install [Cargo-Nextest](https://nexte.st/docs/installati - Run the tests to the backend by running `cargo nextest run --workspace`. This requires a connection to a PostgreSQL database, set via the `DATABASE_URL` environment variable. - Run the tests to the frontend by running `npm run test` in the `frontend` directory. - To run the tests for the included policy, change to the `policies` directory and run one of: - - `make test` (needs OpenPolicyAgent installed) + - `make test` (needs Open Policy Agent installed) - `make PODMAN=1 test` (runs inside a container; needs Podman installed) - `make DOCKER=1 test` (runs inside a container; needs Docker installed)