From b94241d946cf96ac390f71f5bf7037cd6ad26c65 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 560ebc2202e8fff923de7e90f4c098e86cf2c052 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 532e4a4794c52a2a8eb4cd7f6c7c4714a12f2abb 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 e2a08d23654b0aed6add9d179c3698e50e29dae8 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 58ffe37678ffb32713a371806a689b5c7bacfa37 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)