diff --git a/policies/client_registration.rego b/policies/client_registration.rego index a41375cf3..54fd9abe7 100644 --- a/policies/client_registration.rego +++ b/policies/client_registration.rego @@ -17,6 +17,11 @@ parse_uri(url) = obj { obj := {"scheme": matches[1], "authority": matches[2], "host": matches[3], "port": matches[4], "path": matches[5]} } +secure_url(x) { + x + data.client_registration.allow_insecure_uris +} + secure_url(x) { url := parse_uri(x) url.scheme == "https" @@ -31,6 +36,21 @@ secure_url(x) { url.port == "" } +host_matches_client_uri(x) { + x + + # Do not check we allow host mismatch + data.client_registration.allow_host_mismatch +} + +host_matches_client_uri(x) { + x + + # Do not check if the client_uri is missing and we allow that + data.client_registration.allow_missing_client_uri + not data.client_metadata.client_uri +} + host_matches_client_uri(x) { client_uri := parse_uri(input.client_metadata.client_uri) uri := parse_uri(x) @@ -43,43 +63,36 @@ violation[{"msg": "missing client_uri"}] { } violation[{"msg": "invalid client_uri"}] { - not data.client_registration.allow_insecure_uris not secure_url(input.client_metadata.client_uri) } violation[{"msg": "invalid tos_uri"}] { input.client_metadata.tos_uri - not data.client_registration.allow_insecure_uris 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 data.client_registration.allow_host_mismatch not host_matches_client_uri(input.client_metadata.tos_uri) } violation[{"msg": "invalid policy_uri"}] { input.client_metadata.policy_uri - not data.client_registration.allow_insecure_uris 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 data.client_registration.allow_host_mismatch not host_matches_client_uri(input.client_metadata.policy_uri) } violation[{"msg": "invalid logo_uri"}] { input.client_metadata.logo_uri - not data.client_registration.allow_insecure_uris 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 data.client_registration.allow_host_mismatch not host_matches_client_uri(input.client_metadata.logo_uri) } @@ -108,22 +121,6 @@ violation[{"msg": "empty redirect_uris"}] { count(input.client_metadata.redirect_uris) == 0 } -violation[{"msg": "invalid redirect_uri", "redirect_uri": redirect_uri}] { - # For 'web' apps, we should verify that redirect_uris are secure - input.client_metadata.application_type != "native" - some redirect_uri in input.client_metadata.redirect_uris - not data.client_registration.allow_host_mismatch - not host_matches_client_uri(redirect_uri) -} - -violation[{"msg": "invalid redirect_uri"}] { - # For 'web' apps, we should verify that redirect_uris are secure - input.client_metadata.application_type != "native" - some redirect_uri in input.client_metadata.redirect_uris - not data.client_registration.allow_insecure_uris - not secure_url(redirect_uri) -} - # 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 @@ -173,11 +170,17 @@ valid_native_redirector(x) { reverse_dns_match(client_uri.host, url.scheme) } -violation[{"msg": "invalid redirect_uri"}] { - # For 'native' apps, we need to check that the redirect_uri is either - # a custom scheme, or localhost - # TODO: this might not be right, because of app-associated domains on mobile? +valid_redirect_uri(uri) { input.client_metadata.application_type == "native" - some redirect_uri in input.client_metadata.redirect_uris - not valid_native_redirector(redirect_uri) + valid_native_redirector(uri) +} + +valid_redirect_uri(uri) { + secure_url(uri) + host_matches_client_uri(uri) +} + +violation[{"msg": "invalid redirect_uri", "redirect_uri": redirect_uri}] { + 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_test.rego index 02bd78530..63625367f 100644 --- a/policies/client_registration_test.rego +++ b/policies/client_registration_test.rego @@ -272,14 +272,15 @@ test_native_redirect_uri { "contacts": ["contact@example.com"], } - # We don't allow HTTP URLs other than localhost - not allow with input.client_metadata as { + # 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/"], "contacts": ["contact@example.com"], } + # But not insecure not allow with input.client_metadata as { "application_type": "native", "client_uri": "https://example.com/", @@ -287,6 +288,14 @@ test_native_redirect_uri { "contacts": ["contact@example.com"], } + # And not a mismatch + not allow with input.client_metadata as { + "application_type": "native", + "client_uri": "https://example.com/", + "redirect_uris": ["http://bad.com/"], + "contacts": ["contact@example.com"], + } + # We don't allow HTTPS on localhost not allow with input.client_metadata as { "application_type": "native",