policy: only require redirect_uris for the authorization_code and implicit grants

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;

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",
         }));

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
 }
 

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"],
+	}
+}