Repository navigation
Add OAuth 2.0 compliant redirect URI validation - #47320
Conversation
Enforce RFC 6749 Section 3.1.2 redirect URI rules (no fragments, no wildcards) via a new "OAuth 2.0 Compliant" client policy flag. This is less strict than the existing OAuth 2.1 flag, which additionally bans localhost and requires HTTPS. Also fixes the "complient" typo in the existing OAuth 2.1 field and method names. Closes keycloak#41164 Signed-off-by: Jon Koops <jonkoops@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adds an “OAuth 2.0 Compliant” mode to the Secure Redirect URIs client policy executor to enforce RFC 6749 redirect URI restrictions (notably rejecting fragments), and cleans up the prior OAuth 2.1 “complient” typo while extending test coverage.
Changes:
- Add new
oauth-2-0-compliantexecutor configuration property and expose it in the executor factory. - Extend redirect URI validation logic to reject fragments (and wildcards) when OAuth 2.0 or 2.1 compliant modes are enabled.
- Update/extend unit + integration tests, including renaming the OAuth 2.1 test method from “Complient” to “Compliant”.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/SecureRedirectUrisEnforcerExecutorTest.java | Renames OAuth 2.1 test method and adds an OAuth 2.0 compliant integration test scenario. |
| services/src/test/java/org/keycloak/services/clientpolicy/executor/SecureRedirectUrisEnforcerExecutorTest.java | Adds unit tests covering OAuth 2.0 compliant behavior for normal/loopback/private-scheme URIs. |
| services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRedirectUrisEnforcerExecutorFactory.java | Introduces the new oauth-2-0-compliant config property and documents it in the admin-config metadata. |
| services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureRedirectUrisEnforcerExecutor.java | Adds OAuth 2.0 compliant flag to configuration and applies fragment/wildcard checks for OAuth 2.0 and 2.1 compliant modes. |
| .helpText("If On, then the executor enforces OAuth 2.0 (RFC 6749) redirect URI rules. " + | ||
| "This means that URL fragments and wildcard redirect URIs are not allowed. " + | ||
| "This is less strict than 'OAuth 2.1 Compliant', which additionally disallows 'localhost' and requires HTTPS.") | ||
| .type(ProviderConfigProperty.BOOLEAN_TYPE) |
There was a problem hiding this comment.
In this new config property's help text, "If On" is inconsistent with the capitalization used in the other executor properties in this same factory (they use "If ON"). Consider standardizing the wording/capitalization for consistency in the admin UI text.
| enable(OAUTH_2_0_COMPLIANT); | ||
| Stream.of( | ||
| "https://oauth.redirect:8443/auth/callback#fragment", | ||
| "https://example.org/path#frag", |
There was a problem hiding this comment.
The new OAuth 2.0 compliance tests include a wildcard case ("https://example.org/*"), but ALLOW_WILDCARD_CONTEXT_PATH is not enabled here, so the wildcard would already be rejected even without the new OAuth 2.0 compliance logic. To actually cover the new behavior (OAuth 2.0 compliance overriding the wildcard-allowed setting), add a case that enables ALLOW_WILDCARD_CONTEXT_PATH and asserts that a wildcard redirect URI still fails when OAUTH_2_0_COMPLIANT is enabled.
| "https://example.org/path#frag", | |
| "https://example.org/path#frag" | |
| ).forEach(i->checkFail(i, false, SecureRedirectUrisEnforcerExecutor.ERR_NORMALURI)); | |
| } | |
| @Test | |
| public void failWildcardContextPath_OAuth2_0Compliant() { | |
| enable(ALLOW_WILDCARD_CONTEXT_PATH, OAUTH_2_0_COMPLIANT); | |
| Stream.of( |
| @Test | ||
| public void testSecureRedirectUrisEnforcerExecutor_OAuth2_0Compliant() throws Exception { | ||
| // register profiles - OAuth 2.0 compliant rejects fragments and wildcards | ||
| // but is less strict than OAuth 2.1 (allows localhost, HTTP, single-word schemes) | ||
| String json = (new ClientPoliciesUtil.ClientProfilesBuilder()).addProfile( | ||
| (new ClientPoliciesUtil.ClientProfileBuilder()).createProfile(PROFILE_NAME, "Le Premier Profil") | ||
| .addExecutor(SecureRedirectUrisEnforcerExecutorFactory.PROVIDER_ID, | ||
| createSecureRedirectUrisEnforcerExecutorConfig(it->{ | ||
| it.setAllowIPv4LoopbackAddress(true); | ||
| it.setAllowIPv6LoopbackAddress(true); | ||
| it.setAllowHttpScheme(true); | ||
| it.setOAuth2_0Compliant(true);})) | ||
| .toRepresentation() | ||
| ).toString(); | ||
| updateProfiles(json); | ||
|
|
||
| // register policies | ||
| json = (new ClientPoliciesUtil.ClientPoliciesBuilder()).addPolicy( | ||
| (new ClientPoliciesUtil.ClientPolicyBuilder()).createPolicy(POLICY_NAME, "La Premiere Politique", Boolean.TRUE) | ||
| .addCondition(AnyClientConditionFactory.PROVIDER_ID, | ||
| createAnyClientConditionConfig()) | ||
| .addProfile(PROFILE_NAME) | ||
| .toRepresentation() | ||
| ).toString(); | ||
| updatePolicies(json); | ||
|
|
||
| // register - fail | ||
| // fragment in redirect URI not allowed per RFC 6749 | ||
| testSecureRedirectUrisEnforcerExecutor_failRegisterByAdmin(List.of("https://example.com/callback#fragment")); | ||
|
|
||
| // register - fail | ||
| // wildcard in redirect URI not allowed | ||
| testSecureRedirectUrisEnforcerExecutor_failRegisterByAdmin(List.of("https://example.com/*")); |
There was a problem hiding this comment.
This OAuth 2.0 compliance integration test checks that a wildcard redirect URI ("https://example.com/*") fails, but the profile config here does not enable allow-wildcard-context-path, so this failure would happen even without the new OAuth 2.0 compliant flag. To validate the new behavior, add a step that enables allow-wildcard-context-path and still asserts wildcard redirect URIs are rejected when OAuth 2.0 Compliant is ON.
Enforce RFC 6749 Section 3.1.2 redirect URI rules (no fragments, no wildcards) via a new "OAuth 2.0 Compliant" client policy flag. This is less strict than the existing OAuth 2.1 flag, which additionally bans localhost and requires HTTPS.
Also fixes the "complient" typo in the existing OAuth 2.1 field and method names.
Closes #41164