diff --git a/crates/handlers/src/graphql/tests.rs b/crates/handlers/src/graphql/tests.rs index 4500dd35..1af08990 100644 --- a/crates/handlers/src/graphql/tests.rs +++ b/crates/handlers/src/graphql/tests.rs @@ -366,12 +366,9 @@ async fn test_oauth2_client_credentials(pool: PgPool) { let request = Request::post(mas_router::OAuth2RegistrationEndpoint::PATH).json(serde_json::json!({ "client_uri": "https://example.com/", - // XXX: we shouldn't have to specify the redirect URI here, but the policy denies it for now - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], "token_endpoint_auth_method": "client_secret_post", "grant_types": ["client_credentials"], - "response_types": [], })); let response = state.request(request).await; diff --git a/crates/handlers/src/oauth2/introspection.rs b/crates/handlers/src/oauth2/introspection.rs index 7aed08df..83cf9d38 100644 --- a/crates/handlers/src/oauth2/introspection.rs +++ b/crates/handlers/src/oauth2/introspection.rs @@ -392,10 +392,6 @@ mod tests { let request = Request::post(OAuth2RegistrationEndpoint::PATH).json(json!({ "contacts": ["hello@introspecting.com"], "client_uri": "https://introspecting.com/", - // XXX: even though we don't use the authorization_code flow, we need to specify at - // least one redirect_uri - "redirect_uris": ["https://introspecting.com/"], - "response_types": [], "grant_types": [], "token_endpoint_auth_method": "client_secret_basic", })); @@ -558,10 +554,6 @@ mod tests { let request = Request::post(OAuth2RegistrationEndpoint::PATH).json(json!({ "contacts": ["hello@introspecting.com"], "client_uri": "https://introspecting.com/", - // XXX: even though we don't use the authorization_code flow, we need to specify at - // least one redirect_uri - "redirect_uris": ["https://introspecting.com/"], - "response_types": [], "grant_types": [], "token_endpoint_auth_method": "client_secret_basic", })); diff --git a/policies/client_registration.rego b/policies/client_registration.rego index 54fd9abe..e63628af 100644 --- a/policies/client_registration.rego +++ b/policies/client_registration.rego @@ -109,7 +109,37 @@ violation[{"msg": "empty contacts"}] { count(input.client_metadata.contacts) == 0 } +# If the grant_types is missing, we assume it is authorization_code +uses_grant_type("authorization_code") { + not input.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 +} + +# Consider a client public if the authentication method is none +is_public_client { + 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 } @@ -118,6 +148,7 @@ violation[{"msg": "invalid redirect_uris"}] { } violation[{"msg": "empty redirect_uris"}] { + requires_redirect_uris count(input.client_metadata.redirect_uris) == 0 } diff --git a/policies/client_registration_test.rego b/policies/client_registration_test.rego index 63625367..a80d21a9 100644 --- a/policies/client_registration_test.rego +++ b/policies/client_registration_test.rego @@ -2,6 +2,7 @@ package client_registration test_valid { allow with input.client_metadata as { + "grant_types": ["authorization_code"], "client_uri": "https://example.com/", "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], @@ -10,12 +11,12 @@ test_valid { test_missing_client_uri { not allow with input.client_metadata as { - "redirect_uris": ["https://example.com/callback"], + "grant_types": [], "contacts": ["contact@example.com"], } allow with input.client_metadata as { - "redirect_uris": ["https://example.com/callback"], + "grant_types": [], "contacts": ["contact@example.com"], } with data.client_registration.allow_missing_client_uri as true @@ -23,50 +24,50 @@ test_missing_client_uri { test_insecure_client_uri { not allow with input.client_metadata as { + "grant_types": [], "client_uri": "http://example.com/", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } } test_tos_uri { allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "tos_uri": "https://example.com/tos", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } # Insecure not allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "tos_uri": "http://example.com/tos", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } # Insecure, but allowed by the config allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "tos_uri": "http://example.com/tos", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } with data.client_registration.allow_insecure_uris as true # Host mistmatch not allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "tos_uri": "https://example.org/tos", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } # Host mistmatch, but allowed by the config allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "tos_uri": "https://example.org/tos", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } with data.client_registration.allow_host_mismatch as true @@ -74,42 +75,42 @@ test_tos_uri { test_logo_uri { allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "logo_uri": "https://example.com/logo.png", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } # Insecure not allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "logo_uri": "http://example.com/logo.png", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } # Insecure, but allowed by the config allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "logo_uri": "http://example.com/logo.png", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } with data.client_registration.allow_insecure_uris as true # Host mistmatch not allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "logo_uri": "https://example.org/logo.png", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } # Host mistmatch, but allowed by the config allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "logo_uri": "https://example.org/logo.png", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } with data.client_registration.allow_host_mismatch as true @@ -117,42 +118,42 @@ test_logo_uri { test_policy_uri { allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "policy_uri": "https://example.com/policy", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } # Insecure not allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "policy_uri": "http://example.com/policy", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } # Insecure, but allowed by the config allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "policy_uri": "http://example.com/policy", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } with data.client_registration.allow_insecure_uris as true # Host mistmatch not allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "policy_uri": "https://example.org/policy", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } # Host mistmatch, but allowed by the config allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", "policy_uri": "https://example.org/policy", - "redirect_uris": ["https://example.com/callback"], "contacts": ["contact@example.com"], } with data.client_registration.allow_host_mismatch as true @@ -178,6 +179,27 @@ test_redirect_uris { "redirect_uris": [], "contacts": ["contact@example.com"], } + + # Not required for the client_credentials grant + allow with input.client_metadata as { + "grant_types": ["client_credentials"], + "client_uri": "https://example.com/", + "contacts": ["contact@example.com"], + } + + # Required for the authorization_code grant + not allow with input.client_metadata as { + "grant_types": ["client_credentials", "refresh_token", "authorization_code"], + "client_uri": "https://example.com/", + "contacts": ["contact@example.com"], + } + + # Required for the implicit grant + not allow with input.client_metadata as { + "grant_types": ["client_credentials", "implicit"], + "client_uri": "https://example.com/", + "contacts": ["contact@example.com"], + } } test_web_redirect_uri { @@ -330,28 +352,52 @@ test_reverse_dns_match { test_contacts { # Missing contacts not allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", - "redirect_uris": ["https://example.com/callback"], } # Missing contacts, but allowed by config allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", - "redirect_uris": ["https://example.com/callback"], } with data.client_registration.allow_missing_contacts as true # contacts is not an array not allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", - "redirect_uris": ["https://example.com/callback"], "contacts": "contact@example.com", } # Empty contacts not allow with input.client_metadata as { + "grant_types": [], "client_uri": "https://example.com/", - "redirect_uris": ["https://example.com/callback"], "contacts": [], } } + +test_client_credentials_grant { + # Allowed for confidential clients + allow with input.client_metadata as { + "grant_types": ["client_credentials"], + "token_endpoint_auth_method": "client_secret_basic", + "client_uri": "https://example.com/", + "contacts": ["contact@example.com"], + } + allow with input.client_metadata as { + "grant_types": ["client_credentials"], + # If omitted, defaults to "client_secret_basic" + "client_uri": "https://example.com/", + "contacts": ["contact@example.com"], + } + + # Disallowed for public clients + not allow with input.client_metadata as { + "grant_types": ["client_credentials"], + "token_endpoint_auth_method": "none", + "client_uri": "https://example.com/", + "contacts": ["contact@example.com"], + } +}