From 4f122a3100d6a32a5a5dd08ed037a24360125b7b Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Thu, 19 Dec 2024 10:47:40 +0100 Subject: [PATCH] Setup Regal to lint policies and clean them up --- .github/workflows/ci.yaml | 7 +- .github/workflows/coverage.yaml | 2 +- policies/.regal/config.yaml | 6 + policies/Makefile | 23 +- .../authorization_grant.rego | 40 ++-- .../authorization_grant_test.rego | 95 ++++----- .../client_registration.rego | 183 ++++++++-------- .../client_registration_test.rego | 200 ++++++++++-------- policies/{ => email}/email.rego | 16 +- policies/email/email_test.rego | 29 +++ policies/email_test.rego | 26 --- policies/{ => register}/register.rego | 24 ++- policies/register/register_test.rego | 58 +++++ policies/register_test.rego | 52 ----- policies/util/coveralls.rego | 16 +- 15 files changed, 401 insertions(+), 376 deletions(-) create mode 100644 policies/.regal/config.yaml rename policies/{ => authorization_grant}/authorization_grant.rego (64%) rename policies/{ => authorization_grant}/authorization_grant_test.rego (58%) rename policies/{ => client_registration}/client_registration.rego (61%) rename policies/{ => client_registration}/client_registration_test.rego (56%) rename policies/{ => email}/email.rego (71%) create mode 100644 policies/email/email_test.rego delete mode 100644 policies/email_test.rego rename policies/{ => register}/register.rego (52%) create mode 100644 policies/register/register_test.rego delete mode 100644 policies/register_test.rego diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index cc2b2c215..167b91808 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -33,7 +33,12 @@ jobs: - name: Setup OPA uses: open-policy-agent/setup-opa@v2.2.0 with: - version: 0.64.1 + version: 0.70.0 + + - name: Setup Regal + uses: StyraInc/setup-regal@v1 + with: + version: 0.29.2 - name: Lint policies working-directory: ./policies diff --git a/.github/workflows/coverage.yaml b/.github/workflows/coverage.yaml index 074c6a044..98d3a47a4 100644 --- a/.github/workflows/coverage.yaml +++ b/.github/workflows/coverage.yaml @@ -29,7 +29,7 @@ jobs: - name: Setup OPA uses: open-policy-agent/setup-opa@v2.2.0 with: - version: 0.64.1 + version: 0.70.0 - name: Run OPA tests with coverage working-directory: ./policies diff --git a/policies/.regal/config.yaml b/policies/.regal/config.yaml new file mode 100644 index 000000000..2088bdbf0 --- /dev/null +++ b/policies/.regal/config.yaml @@ -0,0 +1,6 @@ +rules: + style: + external-reference: + level: ignore + line-length: + level: ignore diff --git a/policies/Makefile b/policies/Makefile index 291304a6d..60f037e48 100644 --- a/policies/Makefile +++ b/policies/Makefile @@ -1,24 +1,28 @@ # Set to 1 to run OPA through Docker DOCKER := 0 PODMAN := 0 -OPA_DOCKER_IMAGE := docker.io/openpolicyagent/opa:0.64.1-debug +OPA_DOCKER_IMAGE := docker.io/openpolicyagent/opa:0.70.0-debug +REGAL_DOCKER_IMAGE := ghcr.io/styrainc/regal:0.29.2 INPUTS := \ - client_registration.rego \ - register.rego \ - authorization_grant.rego \ - email.rego + client_registration/client_registration.rego \ + register/register.rego \ + authorization_grant/authorization_grant.rego \ + email/email.rego ifeq ($(DOCKER), 1) OPA := docker run -i -v $(shell pwd):/policies:ro -w /policies --rm $(OPA_DOCKER_IMAGE) OPA_RW := docker run -i -v $(shell pwd):/policies -w /policies --rm $(OPA_DOCKER_IMAGE) + 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_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) else OPA := opa OPA_RW := opa + REGAL := regal endif policy.wasm: $(INPUTS) @@ -38,16 +42,17 @@ fmt: .PHONY: test test: - $(OPA) test --schema ./schema/ -v ./*.rego + $(OPA) test --schema ./schema/ --ignore schema -v ./ .PHONY: coverage coverage: - $(OPA) test --coverage ./*.rego | $(OPA) eval --format pretty \ + $(OPA) test --coverage --schema ./schema/ --ignore schema ./ | $(OPA) eval --format pretty \ --stdin-input \ --data util/coveralls.rego \ data.coveralls.from_opa > coverage.json .PHONY: lint lint: - $(OPA) fmt -d --fail ./*.rego util/*.rego - $(OPA) check --strict --schema schema/ ./*.rego util/*.rego + $(OPA) fmt -d --fail . + $(OPA) check --strict --schema schema/ --ignore schema . + $(REGAL) lint . diff --git a/policies/authorization_grant.rego b/policies/authorization_grant/authorization_grant.rego similarity index 64% rename from policies/authorization_grant.rego rename to policies/authorization_grant/authorization_grant.rego index 0ce1d63db..362ba080e 100644 --- a/policies/authorization_grant.rego +++ b/policies/authorization_grant/authorization_grant.rego @@ -3,39 +3,39 @@ # - input: schema["authorization_grant_input"] package authorization_grant -import future.keywords.in +import rego.v1 default allow := false -allow { +allow if { count(violation) == 0 } # Users can request admin scopes if either: # 1. They are in the admin_users list -can_request_admin(user) { +can_request_admin(user) if { some admin_user in data.admin_users user.username == admin_user } # 2. They have the can_request_admin flag set to true -can_request_admin(user) { +can_request_admin(user) if { user.can_request_admin } -interactive_grant_type("authorization_code") = true +interactive_grant_type("authorization_code") := true -interactive_grant_type("urn:ietf:params:oauth:grant-type:device_code") = true +interactive_grant_type("urn:ietf:params:oauth:grant-type:device_code") := true # Special case to make empty scope work -allowed_scope("") = true +allowed_scope("") := true -allowed_scope("openid") = true +allowed_scope("openid") := true -allowed_scope("email") = true +allowed_scope("email") := true # This grants access to Synapse's admin API endpoints -allowed_scope("urn:synapse:admin:*") { +allowed_scope("urn:synapse:admin:*") if { # Synapse doesn't support user-less tokens yet, so access to the admin API # can only be used with an authorization_code grant or a device code grant # as the user is present @@ -44,39 +44,41 @@ allowed_scope("urn:synapse:admin:*") { } # This grants access to the /graphql API endpoint -allowed_scope("urn:mas:graphql:*") = true +allowed_scope("urn:mas:graphql:*") := true # This makes it possible to query and do anything in the GraphQL API as an admin -allowed_scope("urn:mas:admin") { +allowed_scope("urn:mas:admin") if { interactive_grant_type(input.grant_type) can_request_admin(input.user) } # This makes it possible to get the admin scope for clients that are allowed -allowed_scope("urn:mas:admin") { +allowed_scope("urn:mas:admin") if { input.grant_type == "client_credentials" some client in data.admin_clients input.client.id == client } -allowed_scope(scope) { +allowed_scope(scope) if { # Grant access to the C-S API only if there is a user interactive_grant_type(input.grant_type) - regex.match("^urn:matrix:org.matrix.msc2967.client:device:[A-Za-z0-9._~!$&'()*+,;=:@/-]{10,}$", scope) + regex.match(`^urn:matrix:org.matrix.msc2967.client:device:[A-Za-z0-9._~!$&'()*+,;=:@/-]{10,}$`, scope) } -allowed_scope("urn:matrix:org.matrix.msc2967.client:api:*") { +allowed_scope("urn:matrix:org.matrix.msc2967.client:api:*") if { # Grant access to the C-S API only if there is a user interactive_grant_type(input.grant_type) } -violation[{"msg": msg}] { +# METADATA +# entrypoint: true +violation contains {"msg": msg} if { some scope in split(input.scope, " ") not allowed_scope(scope) msg := sprintf("scope '%s' not allowed", [scope]) } -violation[{"msg": "only one device scope is allowed at a time"}] { +violation contains {"msg": "only one device scope is allowed at a time"} if { scope_list := split(input.scope, " ") - count({key | scope_list[key]; startswith(scope_list[key], "urn:matrix:org.matrix.msc2967.client:device:")}) > 1 + count({scope | some scope in scope_list; startswith(scope, "urn:matrix:org.matrix.msc2967.client:device:")}) > 1 } diff --git a/policies/authorization_grant_test.rego b/policies/authorization_grant/authorization_grant_test.rego similarity index 58% rename from policies/authorization_grant_test.rego rename to policies/authorization_grant/authorization_grant_test.rego index fc30f353f..3594bfac2 100644 --- a/policies/authorization_grant_test.rego +++ b/policies/authorization_grant/authorization_grant_test.rego @@ -1,155 +1,134 @@ -package authorization_grant +package authorization_grant_test + +import data.authorization_grant +import rego.v1 user := {"username": "john"} client := {"client_id": "client"} -test_standard_scopes { - allow with input.user as user +test_standard_scopes if { + authorization_grant.allow with input.user as user with input.client as client with input.scope as "" - allow with input.user as user + authorization_grant.allow with input.user as user with input.client as client with input.scope as "openid" - allow with input.user as user + authorization_grant.allow with input.user as user with input.client as client with input.scope as "email" - allow with input.user as user + authorization_grant.allow with input.user as user with input.client as client with input.scope as "openid email" # Not supported yet - not allow with input.user as user + not authorization_grant.allow with input.user as user with input.client as client with input.scope as "phone" # Not supported yet - not allow with input.user as user + not authorization_grant.allow with input.user as user with input.client as client with input.scope as "profile" } -test_matrix_scopes { - allow with input.user as user +test_matrix_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:api:*" - allow with input.user as user + 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:api:*" - not allow with input.user as user + not authorization_grant.allow with input.user as user with input.client as client with input.grant_type as "client_credentials" with input.scope as "urn:matrix:org.matrix.msc2967.client:api:*" } -test_device_scopes { - allow with input.user as user +test_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" - allow with input.user as user + 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" # Too short - not allow with input.user as user + 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" # Multiple device scope - not allow with input.user as user + 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" # Allowed with the device code grant - allow with input.user as user + 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" - # Not allowed for the client credentials grant - not allow with input.client as client + # 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" } -test_synapse_admin_scopes { - allow with input.user as user - with input.client as client - with data.admin_users as ["john"] - with input.grant_type as "authorization_code" - with input.scope as "urn:synapse:admin:*" +test_synapse_admin_scopes if { + some grant_type in ["authorization_code", "urn:ietf:params:oauth:grant-type:device_code"] - allow with input.user as user + authorization_grant.allow with input.user as user with input.client as client with data.admin_users as ["john"] - with input.grant_type as "urn:ietf:params:oauth:grant-type:device_code" - with input.scope as "urn:synapse:admin:*" - - not allow with input.user as user - with input.client as client - with data.admin_users as [] - with input.grant_type as "authorization_code" - with input.scope as "urn:synapse:admin:*" - - not allow with input.user as user - with input.client as client - with data.admin_users as [] - with input.grant_type as "urn:ietf:params:oauth:grant-type:device_code" + with input.grant_type as grant_type with input.scope as "urn:synapse:admin:*" - allow with input.user as user - with input.user.can_request_admin as true + not authorization_grant.allow with input.user as user with input.client as client with data.admin_users as [] - with input.grant_type as "authorization_code" + with input.grant_type as grant_type with input.scope as "urn:synapse:admin:*" - allow with input.user as user + authorization_grant.allow with input.user as user with input.user.can_request_admin as true with input.client as client with data.admin_users as [] - with input.grant_type as "urn:ietf:params:oauth:grant-type:device_code" + with input.grant_type as grant_type with input.scope as "urn:synapse:admin:*" - not allow with input.user as user + not authorization_grant.allow with input.user as user with input.user.can_request_admin as false with input.client as client with data.admin_users as [] - with input.grant_type as "authorization_code" - with input.scope as "urn:synapse:admin:*" - - not allow with input.user as user - with input.user.can_request_admin as false - with input.client as client - with data.admin_users as [] - with input.grant_type as "urn:ietf:params:oauth:grant-type:device_code" + with input.grant_type as grant_type with input.scope as "urn:synapse:admin:*" } -test_mas_scopes { - allow with input.user as user +test_mas_scopes if { + authorization_grant.allow with input.user as user with input.client as client with input.scope as "urn:mas:graphql:*" - allow with input.user as user + authorization_grant.allow with input.user as user with input.client as client with data.admin_users as ["john"] with input.grant_type as "authorization_code" with input.scope as "urn:mas:admin" - not allow with input.user as user + not authorization_grant.allow with input.user as user with input.client as client with data.admin_users as [] with input.grant_type as "authorization_code" diff --git a/policies/client_registration.rego b/policies/client_registration/client_registration.rego similarity index 61% rename from policies/client_registration.rego rename to policies/client_registration/client_registration.rego index cd95b0dd1..4f3e28516 100644 --- a/policies/client_registration.rego +++ b/policies/client_registration/client_registration.rego @@ -3,26 +3,27 @@ # - input: schema["client_registration_input"] package client_registration -import future.keywords.in +import rego.v1 default allow := false -allow { +allow if { count(violation) == 0 } -parse_uri(url) = obj { +parse_uri(url) := obj if { is_string(url) - [matches] := regex.find_all_string_submatch_n("^(?P[a-z][a-z0-9+.-]*):(?://(?P((?:(?:[a-z0-9]|[a-z0-9][a-z0-9-]*[a-z0-9])\\.)*(?:[a-z0-9]|[a-z0-9][a-z0-9-]*[a-z0-9])|127.0.0.1|0.0.0.0|\\[::1\\])(?::(?P[0-9]+))?))?(?P/[A-Za-z0-9/.-]*)$", url, 1) + url_regex := `^(?P[a-z][a-z0-9+.-]*):(?://(?P((?:(?:[a-z0-9]|[a-z0-9][a-z0-9-]*[a-z0-9])\.)*(?:[a-z0-9]|[a-z0-9][a-z0-9-]*[a-z0-9])|127.0.0.1|0.0.0.0|\[::1\])(?::(?P[0-9]+))?))?(?P/[A-Za-z0-9/.-]*)$` + [matches] := regex.find_all_string_submatch_n(url_regex, url, 1) obj := {"scheme": matches[1], "authority": matches[2], "host": matches[3], "port": matches[4], "path": matches[5]} } -secure_url(x) { +secure_url(x) if { x data.client_registration.allow_insecure_uris } -secure_url(x) { +secure_url(x) if { url := parse_uri(x) url.scheme == "https" @@ -36,14 +37,14 @@ secure_url(x) { url.port == "" } -host_matches_client_uri(x) { +host_matches_client_uri(x) if { x # Do not check we allow host mismatch data.client_registration.allow_host_mismatch } -host_matches_client_uri(x) { +host_matches_client_uri(x) if { x # Do not check if the client_uri is missing and we allow that @@ -51,99 +52,41 @@ host_matches_client_uri(x) { not data.client_metadata.client_uri } -host_matches_client_uri(x) { +host_matches_client_uri(x) if { client_uri := parse_uri(input.client_metadata.client_uri) uri := parse_uri(x) is_subdomain(client_uri.host, uri.host) } -violation[{"msg": "missing client_uri"}] { - not data.client_registration.allow_missing_client_uri - not input.client_metadata.client_uri -} - -violation[{"msg": "invalid client_uri"}] { - not secure_url(input.client_metadata.client_uri) -} - -violation[{"msg": "invalid tos_uri"}] { - input.client_metadata.tos_uri - not secure_url(input.client_metadata.tos_uri) -} - -violation[{"msg": "tos_uri not on the same host as the client_uri"}] { - input.client_metadata.tos_uri - not host_matches_client_uri(input.client_metadata.tos_uri) -} - -violation[{"msg": "invalid policy_uri"}] { - input.client_metadata.policy_uri - not secure_url(input.client_metadata.policy_uri) -} - -violation[{"msg": "policy_uri not on the same host as the client_uri"}] { - input.client_metadata.policy_uri - not host_matches_client_uri(input.client_metadata.policy_uri) -} - -violation[{"msg": "invalid logo_uri"}] { - input.client_metadata.logo_uri - not secure_url(input.client_metadata.logo_uri) -} - -violation[{"msg": "logo_uri not on the same host as the client_uri"}] { - input.client_metadata.logo_uri - not host_matches_client_uri(input.client_metadata.logo_uri) -} - # If the grant_types is missing, we assume it is authorization_code -uses_grant_type("authorization_code") { - not input.client_metadata.grant_types +uses_grant_type("authorization_code", client_metadata) if { + not client_metadata.grant_types } # Else, we check that the grant_types contains the given grant_type -uses_grant_type(grant_type) { - some gt in input.client_metadata.grant_types - gt == grant_type +uses_grant_type(grant_type, client_metadata) if { + some grant in client_metadata.grant_types + grant == grant_type } # Consider a client public if the authentication method is none -is_public_client { +is_public_client if { input.client_metadata.token_endpoint_auth_method == "none" } -requires_redirect_uris { - uses_grant_type("authorization_code") -} - -requires_redirect_uris { - uses_grant_type("implicit") -} - -violation[{"msg": "client_credentials grant_type requires some form of client authentication"}] { - uses_grant_type("client_credentials") - is_public_client -} - -violation[{"msg": "missing redirect_uris"}] { - requires_redirect_uris - not input.client_metadata.redirect_uris -} - -violation[{"msg": "invalid redirect_uris: it must be an array"}] { - not is_array(input.client_metadata.redirect_uris) +requires_redirect_uris if { + uses_grant_type("authorization_code", input.client_metadata) } -violation[{"msg": "invalid redirect_uris: it must have at least one redirect_uri"}] { - requires_redirect_uris - count(input.client_metadata.redirect_uris) == 0 +requires_redirect_uris if { + uses_grant_type("implicit", input.client_metadata) } # Used to verify that a reverse-dns formatted scheme is a strict subdomain of # another host. # This is used so a redirect_uri like 'com.example.app:/' works for # a 'client_uri' of 'https://example.com/' -reverse_dns_match(host, reverse_dns) { +reverse_dns_match(host, reverse_dns) if { is_string(host) is_string(reverse_dns) @@ -158,7 +101,7 @@ reverse_dns_match(host, reverse_dns) { } # Used to verify that all the various URIs are subdomains of the client_uri -is_subdomain(host, subdomain) { +is_subdomain(host, subdomain) if { is_string(host) is_string(subdomain) @@ -172,27 +115,21 @@ is_subdomain(host, subdomain) { array.slice(subdomain_parts, 0, count(host_parts)) == host_parts } -valid_native_redirector(x) { - url := parse_uri(x) - is_localhost(url.host) - url.scheme == "http" -} +is_localhost("localhost") -is_localhost(host) { - host == "localhost" -} +is_localhost("127.0.0.1") -is_localhost(host) { - host == "127.0.0.1" -} +is_localhost("[::1]") -is_localhost(host) { - host == "[::1]" +valid_native_redirector(x) if { + url := parse_uri(x) + is_localhost(url.host) + url.scheme == "http" } # Custom schemes should match the client_uri, reverse-dns style # e.g. io.element.app:/ matches https://app.element.io/ -valid_native_redirector(x) { +valid_native_redirector(x) if { url := parse_uri(x) url.scheme != "http" url.scheme != "https" @@ -203,17 +140,71 @@ valid_native_redirector(x) { reverse_dns_match(client_uri.host, url.scheme) } -valid_redirect_uri(uri) { +valid_redirect_uri(uri) if { input.client_metadata.application_type == "native" valid_native_redirector(uri) } -valid_redirect_uri(uri) { +valid_redirect_uri(uri) if { secure_url(uri) host_matches_client_uri(uri) } -violation[{"msg": "invalid redirect_uri", "redirect_uri": redirect_uri}] { +# METADATA +# entrypoint: true +violation contains {"msg": "missing client_uri"} if { + not data.client_registration.allow_missing_client_uri + not input.client_metadata.client_uri +} + +violation contains {"msg": "invalid client_uri"} if { + not secure_url(input.client_metadata.client_uri) +} + +violation contains {"msg": "invalid tos_uri"} if { + not secure_url(input.client_metadata.tos_uri) +} + +violation contains {"msg": "tos_uri not on the same host as the client_uri"} if { + not host_matches_client_uri(input.client_metadata.tos_uri) +} + +violation contains {"msg": "invalid policy_uri"} if { + not secure_url(input.client_metadata.policy_uri) +} + +violation contains {"msg": "policy_uri not on the same host as the client_uri"} if { + not host_matches_client_uri(input.client_metadata.policy_uri) +} + +violation contains {"msg": "invalid logo_uri"} if { + not secure_url(input.client_metadata.logo_uri) +} + +violation contains {"msg": "logo_uri not on the same host as the client_uri"} if { + not host_matches_client_uri(input.client_metadata.logo_uri) +} + +violation contains {"msg": "client_credentials grant_type requires some form of client authentication"} if { + uses_grant_type("client_credentials", input.client_metadata) + is_public_client +} + +violation contains {"msg": "missing redirect_uris"} if { + requires_redirect_uris + not input.client_metadata.redirect_uris +} + +violation contains {"msg": "invalid redirect_uris: it must be an array"} if { + not is_array(input.client_metadata.redirect_uris) +} + +violation contains {"msg": "invalid redirect_uris: it must have at least one redirect_uri"} if { + requires_redirect_uris + count(input.client_metadata.redirect_uris) == 0 +} + +violation contains {"msg": "invalid redirect_uri", "redirect_uri": redirect_uri} if { some redirect_uri in input.client_metadata.redirect_uris not valid_redirect_uri(redirect_uri) } diff --git a/policies/client_registration_test.rego b/policies/client_registration/client_registration_test.rego similarity index 56% rename from policies/client_registration_test.rego rename to policies/client_registration/client_registration_test.rego index fe4f077e2..5bf563d27 100644 --- a/policies/client_registration_test.rego +++ b/policies/client_registration/client_registration_test.rego @@ -1,273 +1,297 @@ -package client_registration +package client_registration_test -test_valid { - allow with input.client_metadata as { +import rego.v1 + +import data.client_registration + +test_valid if { + client_registration.allow with input.client_metadata as { "grant_types": ["authorization_code"], "client_uri": "https://example.com/", "redirect_uris": ["https://example.com/callback"], } } -test_missing_client_uri { - not allow with input.client_metadata as {"grant_types": []} +test_missing_client_uri if { + not client_registration.allow with input.client_metadata as {"grant_types": []} - allow with input.client_metadata as {"grant_types": []} - with data.client_registration.allow_missing_client_uri as true + client_registration.allow with input.client_metadata as {"grant_types": []} + with client_registration.allow_missing_client_uri as true } -test_insecure_client_uri { - not allow with input.client_metadata as { +test_insecure_client_uri if { + not client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "http://example.com/", } } -test_tos_uri { - allow with input.client_metadata as { +test_tos_uri if { + client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "tos_uri": "https://example.com/tos", } +} +test_tos_uri_insecure if { # Insecure - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "tos_uri": "http://example.com/tos", } # Insecure, but allowed by the config - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "tos_uri": "http://example.com/tos", } - with data.client_registration.allow_insecure_uris as true + with client_registration.allow_insecure_uris as true +} +test_tos_uri_host_mismatch if { # Host mistmatch - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "tos_uri": "https://example.org/tos", } # TOS on a subdomain of the client_uri host is allowed - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "tos_uri": "https://tos.example.com/", } # Host mistmatch, but allowed by the config - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "tos_uri": "https://example.org/tos", } - with data.client_registration.allow_host_mismatch as true + with client_registration.allow_host_mismatch as true } -test_logo_uri { - allow with input.client_metadata as { +test_logo_uri if { + client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "logo_uri": "https://example.com/logo.png", } +} +test_logo_uri_insecure if { # Insecure - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "logo_uri": "http://example.com/logo.png", } # Insecure, but allowed by the config - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "logo_uri": "http://example.com/logo.png", } - with data.client_registration.allow_insecure_uris as true + with client_registration.allow_insecure_uris as true +} +test_logo_uri_host_mismatch if { # Host mistmatch - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "logo_uri": "https://example.org/logo.png", } # Logo on a subdomain of the client_uri host is allowed - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "logo_uri": "https://static.example.com/logo.png", } # Host mistmatch, but allowed by the config - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "logo_uri": "https://example.org/logo.png", } - with data.client_registration.allow_host_mismatch as true + with client_registration.allow_host_mismatch as true } -test_policy_uri { - allow with input.client_metadata as { +test_policy_uri if { + client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "policy_uri": "https://example.com/policy", } +} +test_policy_uri_insecure if { # Insecure - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "policy_uri": "http://example.com/policy", } # Insecure, but allowed by the config - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "policy_uri": "http://example.com/policy", } - with data.client_registration.allow_insecure_uris as true + with client_registration.allow_insecure_uris as true +} +test_policy_uri_host_mismatch if { # Host mistmatch - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "policy_uri": "https://example.org/policy", } # Policy on a subdomain of the client_uri host is allowed - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "policy_uri": "https://policy.example.com/", } # Host mistmatch, but allowed by the config - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "grant_types": [], "client_uri": "https://example.com/", "policy_uri": "https://example.org/policy", } - with data.client_registration.allow_host_mismatch as true + with client_registration.allow_host_mismatch as true } -test_redirect_uris { +test_redirect_uris if { # Missing redirect_uris - not allow with input.client_metadata as {"client_uri": "https://example.com/"} + not client_registration.allow with input.client_metadata as {"client_uri": "https://example.com/"} # redirect_uris is not an array - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "client_uri": "https://example.com/", "redirect_uris": "https://example.com/callback", } # Empty redirect_uris - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "client_uri": "https://example.com/", "redirect_uris": [], } # Not required for the client_credentials grant - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "grant_types": ["client_credentials"], "client_uri": "https://example.com/", } # Required for the authorization_code grant - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "grant_types": ["client_credentials", "refresh_token", "authorization_code"], "client_uri": "https://example.com/", } # Required for the implicit grant - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "grant_types": ["client_credentials", "implicit"], "client_uri": "https://example.com/", } } -test_web_redirect_uri { - allow with input.client_metadata as { +test_web_redirect_uri if { + client_registration.allow with input.client_metadata as { "application_type": "web", "client_uri": "https://example.com/", "redirect_uris": ["https://example.com/second/callback", "https://example.com/callback"], } +} +test_web_redirect_uri_insecure if { # Insecure URL - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "application_type": "web", "client_uri": "https://example.com/", "redirect_uris": ["http://example.com/callback", "https://example.com/callback"], } # Insecure URL, but allowed by the config - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "application_type": "web", "client_uri": "https://example.com/", "redirect_uris": ["http://example.com/callback", "https://example.com/callback"], } - with data.client_registration.allow_insecure_uris as true + with client_registration.allow_insecure_uris as true +} +test_web_redirect_uri_host_mismatch if { # Host mismatch - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "application_type": "web", "client_uri": "https://example.com/", "redirect_uris": ["https://example.com/second/callback", "https://example.org/callback"], } # Host mismatch, but allowed by the config - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "application_type": "web", "client_uri": "https://example.com/", "redirect_uris": ["https://example.com/second/callback", "https://example.org/callback"], } - with data.client_registration.allow_host_mismatch as true + with client_registration.allow_host_mismatch as true # Redirect URI on a subdomain of the client_uri host is allowed - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "application_type": "web", "client_uri": "https://example.com/", "redirect_uris": ["https://app.example.com/callback"], } +} +test_web_redirect_uri_no_custom_scheme if { # No custom scheme allowed - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "application_type": "web", "client_uri": "https://example.com/", "redirect_uris": ["com.example.app:/callback"], } +} +test_web_redirect_uri_localhost_not_allowed if { # localhost not allowed - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "application_type": "web", "client_uri": "https://example.com/", "redirect_uris": ["http://locahost:1234/callback"], } # localhost not allowed - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "application_type": "web", "client_uri": "https://example.com/", "redirect_uris": ["http://127.0.0.1:1234/callback"], } # localhost not allowed - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "application_type": "web", "client_uri": "https://example.com/", "redirect_uris": ["http://[::1]:1234/callback"], } } -test_native_redirect_uri { +test_native_redirect_uri_allowed if { # This has all the redirect URIs types we're supporting for native apps - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "application_type": "native", "client_uri": "https://example.com/", "redirect_uris": [ @@ -278,89 +302,89 @@ test_native_redirect_uri { "http://127.0.0.1:1234/callback", "http://[::1]/callback", "http://[::1]:1234/callback", + "https://example.com/callback", ], } +} - # We still allow matching URLs for native apps - allow with input.client_metadata as { - "application_type": "native", - "client_uri": "https://example.com/", - "redirect_uris": ["https://example.com/"], - } - +test_native_redirect_uri_denied_domain if { # But not insecure - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "application_type": "native", "client_uri": "https://example.com/", "redirect_uris": ["http://example.com/"], } # And not a mismatch - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "application_type": "native", "client_uri": "https://example.com/", "redirect_uris": ["http://bad.com/"], } +} +test_native_redirect_uri_denied_on_localhost if { # We don't allow HTTPS on localhost - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "application_type": "native", "client_uri": "https://example.com/", "redirect_uris": ["https://localhost:1234/"], } # Ensure we're not allowing localhost as a prefix - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "application_type": "native", "client_uri": "https://example.com/", "redirect_uris": ["http://localhost.com/"], } +} +test_native_redirect_uri_custom_scheme if { # For custom schemes, it should match the client_uri hostname - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "application_type": "native", "client_uri": "https://example.com/", "redirect_uris": ["org.example.app:/callback"], } } -test_reverse_dns_match { - client_uri := parse_uri("https://element.io/") - redirect_uri := parse_uri("io.element.app:/callback") - reverse_dns_match(client_uri.host, redirect_uri.scheme) +test_reverse_dns_match_parse if { + client_uri := client_registration.parse_uri("https://element.io/") + redirect_uri := client_registration.parse_uri("io.element.app:/callback") + client_registration.reverse_dns_match(client_uri.host, redirect_uri.scheme) } -test_client_credentials_grant { +test_client_credentials_grant if { # Allowed for confidential clients - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "grant_types": ["client_credentials"], "token_endpoint_auth_method": "client_secret_basic", "client_uri": "https://example.com/", } - allow with input.client_metadata as { + client_registration.allow with input.client_metadata as { "grant_types": ["client_credentials"], # If omitted, defaults to "client_secret_basic" "client_uri": "https://example.com/", } # Disallowed for public clients - not allow with input.client_metadata as { + not client_registration.allow with input.client_metadata as { "grant_types": ["client_credentials"], "token_endpoint_auth_method": "none", "client_uri": "https://example.com/", } } -test_is_subdomain { - is_subdomain("example.com", "example.com") - is_subdomain("example.com", "app.example.com") - not is_subdomain("example.com", "example.org") - not is_subdomain("test.com", "example.com") +test_is_subdomain if { + client_registration.is_subdomain("example.com", "example.com") + client_registration.is_subdomain("example.com", "app.example.com") + not client_registration.is_subdomain("example.com", "example.org") + not client_registration.is_subdomain("test.com", "example.com") } -test_reverse_dns_match { - reverse_dns_match("example.com", "com.example") - reverse_dns_match("example.com", "com.example.app") - not reverse_dns_match("example.com", "org.example") - not reverse_dns_match("test.com", "com.example") +test_reverse_dns_match if { + client_registration.reverse_dns_match("example.com", "com.example") + client_registration.reverse_dns_match("example.com", "com.example.app") + not client_registration.reverse_dns_match("example.com", "org.example") + not client_registration.reverse_dns_match("test.com", "com.example") } diff --git a/policies/email.rego b/policies/email/email.rego similarity index 71% rename from policies/email.rego rename to policies/email/email.rego index fecad108f..00103da26 100644 --- a/policies/email.rego +++ b/policies/email/email.rego @@ -3,32 +3,34 @@ # - input: schema["email_input"] package email -import future.keywords.in +import rego.v1 default allow := false -allow { +allow if { count(violation) == 0 } # Allow any domains if the data.allowed_domains array is not set -email_domain_allowed { +domain_allowed if { not data.allowed_domains } # Allow an email only if its domain is in the list of allowed domains -email_domain_allowed { +domain_allowed if { [_, domain] := split(input.email, "@") some allowed_domain in data.allowed_domains glob.match(allowed_domain, ["."], domain) } -violation[{"msg": "email domain is not allowed"}] { - not email_domain_allowed +# METADATA +# entrypoint: true +violation contains {"msg": "email domain is not allowed"} if { + not domain_allowed } # Deny emails with their domain in the domains banlist -violation[{"msg": "email domain is banned"}] { +violation contains {"msg": "email domain is banned"} if { [_, domain] := split(input.email, "@") some banned_domain in data.banned_domains glob.match(banned_domain, ["."], domain) diff --git a/policies/email/email_test.rego b/policies/email/email_test.rego new file mode 100644 index 000000000..0adcdfad2 --- /dev/null +++ b/policies/email/email_test.rego @@ -0,0 +1,29 @@ +package email_test + +import data.email +import rego.v1 + +test_allow_all_domains if { + email.allow with input.email as "hello@staging.element.io" +} + +test_allowed_domain if { + email.allow with input.email as "hello@staging.element.io" + with data.allowed_domains as ["*.element.io"] +} + +test_not_allowed_domain if { + not email.allow with input.email as "hello@staging.element.io" + with data.allowed_domains as ["example.com"] +} + +test_banned_domain if { + not email.allow with input.email as "hello@staging.element.io" + with data.banned_domains as ["*.element.io"] +} + +test_banned_subdomain if { + not email.allow with input.email as "hello@staging.element.io" + with data.allowed_domains as ["*.element.io"] + with data.banned_domains as ["staging.element.io"] +} diff --git a/policies/email_test.rego b/policies/email_test.rego deleted file mode 100644 index efda51bd5..000000000 --- a/policies/email_test.rego +++ /dev/null @@ -1,26 +0,0 @@ -package email - -test_allow_all_domains { - allow with input.email as "hello@staging.element.io" -} - -test_allowed_domain { - allow with input.email as "hello@staging.element.io" - with data.allowed_domains as ["*.element.io"] -} - -test_not_allowed_domain { - not allow with input.email as "hello@staging.element.io" - with data.allowed_domains as ["example.com"] -} - -test_banned_domain { - not allow with input.email as "hello@staging.element.io" - with data.banned_domains as ["*.element.io"] -} - -test_banned_subdomain { - not allow with input.email as "hello@staging.element.io" - with data.allowed_domains as ["*.element.io"] - with data.banned_domains as ["staging.element.io"] -} diff --git a/policies/register.rego b/policies/register/register.rego similarity index 52% rename from policies/register.rego rename to policies/register/register.rego index 051935499..4507a10ca 100644 --- a/policies/register.rego +++ b/policies/register/register.rego @@ -3,38 +3,40 @@ # - input: schema["register_input"] package register -import data.email as email_policy +import rego.v1 -import future.keywords.in +import data.email as email_policy default allow := false -allow { +allow if { count(violation) == 0 } -violation[{"field": "username", "msg": "username too short"}] { +# METADATA +# entrypoint: true +violation contains {"field": "username", "msg": "username too short"} if { count(input.username) <= 2 } -violation[{"field": "username", "msg": "username too long"}] { +violation contains {"field": "username", "msg": "username too long"} if { count(input.username) > 64 } -violation[{"field": "username", "msg": "username contains invalid characters"}] { - not regex.match("^[a-z0-9.=_/-]+$", input.username) +violation contains {"field": "username", "msg": "username contains invalid characters"} if { + not regex.match(`^[a-z0-9.=_/-]+$`, input.username) } -violation[{"msg": "unspecified registration method"}] { +violation contains {"msg": "unspecified registration method"} if { not input.registration_method } -violation[{"msg": "unknown registration method"}] { +violation contains {"msg": "unknown registration method"} if { not input.registration_method in ["password", "upstream-oauth2"] } # Check that we supplied an email for password registration -violation[{"field": "email", "msg": "email required for password-based registration"}] { +violation contains {"field": "email", "msg": "email required for password-based registration"} if { input.registration_method == "password" not input.email @@ -42,7 +44,7 @@ violation[{"field": "email", "msg": "email required for password-based registrat # Check if the email is valid using the email policy # and add the email field to the violation object -violation[object.union({"field": "email"}, v)] { +violation contains object.union({"field": "email"}, v) if { # Check if we have an email set in the input input.email diff --git a/policies/register/register_test.rego b/policies/register/register_test.rego new file mode 100644 index 000000000..0d270ec26 --- /dev/null +++ b/policies/register/register_test.rego @@ -0,0 +1,58 @@ +package register_test + +import data.register +import rego.v1 + +mock_registration := { + "registration_method": "password", + "username": "hello", + "email": "hello@staging.element.io", +} + +test_allow_all_domains if { + register.allow with input as mock_registration +} + +test_allowed_domain if { + register.allow with input as mock_registration + with data.allowed_domains as ["*.element.io"] +} + +test_not_allowed_domain if { + not register.allow with input as mock_registration + with data.allowed_domains as ["example.com"] +} + +test_banned_domain if { + not register.allow with input as mock_registration + with data.banned_domains as ["*.element.io"] +} + +test_banned_subdomain if { + not register.allow with input as mock_registration + with data.allowed_domains as ["*.element.io"] + with data.banned_domains as ["staging.element.io"] +} + +test_email_required if { + not register.allow with input as {"username": "hello", "registration_method": "password"} +} + +test_no_email if { + register.allow with input as {"username": "hello", "registration_method": "upstream-oauth2"} +} + +test_short_username if { + not register.allow with input as {"username": "a", "registration_method": "upstream-oauth2"} +} + +test_long_username if { + not register.allow with input as { + "username": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "registration_method": "upstream-oauth2", + } +} + +test_invalid_username if { + not register.allow with input as {"username": "hello world", "registration_method": "upstream-oauth2"} +} diff --git a/policies/register_test.rego b/policies/register_test.rego deleted file mode 100644 index 4d8d5476a..000000000 --- a/policies/register_test.rego +++ /dev/null @@ -1,52 +0,0 @@ -package register - -mock_registration := { - "registration_method": "password", - "username": "hello", - "email": "hello@staging.element.io", -} - -test_allow_all_domains { - allow with input as mock_registration -} - -test_allowed_domain { - allow with input as mock_registration - with data.allowed_domains as ["*.element.io"] -} - -test_not_allowed_domain { - not allow with input as mock_registration - with data.allowed_domains as ["example.com"] -} - -test_banned_domain { - not allow with input as mock_registration - with data.banned_domains as ["*.element.io"] -} - -test_banned_subdomain { - not allow with input as mock_registration - with data.allowed_domains as ["*.element.io"] - with data.banned_domains as ["staging.element.io"] -} - -test_email_required { - not allow with input as {"username": "hello", "registration_method": "password"} -} - -test_no_email { - allow with input as {"username": "hello", "registration_method": "upstream-oauth2"} -} - -test_short_username { - not allow with input as {"username": "a", "registration_method": "upstream-oauth2"} -} - -test_long_username { - not allow with input as {"username": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "registration_method": "upstream-oauth2"} -} - -test_invalid_username { - not allow with input as {"username": "hello world", "registration_method": "upstream-oauth2"} -} diff --git a/policies/util/coveralls.rego b/policies/util/coveralls.rego index 881c7f6fc..c25208552 100644 --- a/policies/util/coveralls.rego +++ b/policies/util/coveralls.rego @@ -1,6 +1,6 @@ -package coveralls +package util -import future.keywords +import rego.v1 from_opa := {"source_files": coverage} @@ -9,7 +9,7 @@ coverage contains obj if { obj := {"name": file, "coverage": to_lines(report)} } -covered_map(report) = cm if { +covered_map(report) := cm if { covered := object.get(report, "covered", []) cm := {line: 1 | some item in covered @@ -17,7 +17,7 @@ covered_map(report) = cm if { } } -not_covered_map(report) = ncm if { +not_covered_map(report) := ncm if { not_covered := object.get(report, "not_covered", []) ncm := {line: 0 | some item in not_covered @@ -25,7 +25,7 @@ not_covered_map(report) = ncm if { } } -to_lines(report) = lines if { +to_lines(report) := lines if { cm := covered_map(report) ncm := not_covered_map(report) keys := sort([line | some line, _ in object.union(cm, ncm)]) @@ -37,15 +37,15 @@ to_lines(report) = lines if { ] } -to_value(cm, _, line) = 1 if { +to_value(cm, _, line) := 1 if { cm[line] } -to_value(_, ncm, line) = 0 if { +to_value(_, ncm, line) := 0 if { ncm[line] } -to_value(cm, ncm, line) = null if { +to_value(cm, ncm, line) := null if { not cm[line] not ncm[line] }