8000 Add OAuth 2.0 compliant redirect URI validation by jonkoops · Pull Request #47320 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,10 @@ public static class Configuration extends ClientPolicyExecutorConfigurationRepre
protected boolean allowWildcardContextPath;
@JsonProperty(SecureRedirectUrisEnforcerExecutorFactory.ALLOW_PERMITTED_DOMAINS)
protected List<String> allowPermittedDomains = Collections.emptyList();
@JsonProperty(SecureRedirectUrisEnforcerExecutorFactory.OAUTH_2_0_COMPLIANT)
protected boolean oauth2_0compliant;
@JsonProperty(SecureRedirectUrisEnforcerExecutorFactory.OAUTH_2_1_COMPLIANT)
protected boolean oauth2_1complient;
protected boolean oauth2_1compliant;
@JsonProperty(SecureRedirectUrisEnforcerExecutorFactory.ALLOW_OPEN_REDIRECT)
protected boolean allowOpenRedirect;

Expand Down Expand Up @@ -147,12 +149,20 @@ public void setAllowPermittedDomains(List<String> permittedDomains) {
this.allowPermittedDomains = permittedDomains;
}

public boolean isOAuth2_0Compliant() {
return oauth2_0compliant;
}

public void setOAuth2_0Compliant(boolean oauth20compliant) {
this.oauth2_0compliant = oauth20compliant;
}

public boolean isOAuth2_1Compliant() {
return oauth2_1complient;
return oauth2_1compliant;
}

public void setOAuth2_1Compliant(boolean oauth21complient) {
this.oauth2_1complient = oauth21complient;
public void setOAuth2_1Compliant(boolean oauth21compliant) {
this.oauth2_1compliant = oauth21compliant;
}

public boolean isAllowOpenRedirect() {
Expand Down Expand Up @@ -382,35 +392,37 @@ boolean isValidLoopbackAddress(Predicate<URI> p) {
}
}

if (config.isOAuth2_1Compliant()) {
if (isIncludeUriFragment()) { // URL fragment is not allowed
logger.debugv("Invalid LoopbackAddress: URI fragment not allowed - OAuth 2.1 compliant - input = {0}", uri.toString());
if (config.isOAuth2_0Compliant() || config.isOAuth2_1Compliant()) {
if (isIncludeUriFragment()) {
logger.debugv("Invalid LoopbackAddress: URI fragment not allowed - input = {0}", uri.toString());
return false;
}

if (isIncludeWildcard()) { // wildcard is not allowed
logger.debugv("Invalid LoopbackAddress: Wildcard not allowed - OAuth 2.1 compliant - input = {0}", uri.toString());
if (isIncludeWildcard()) {
logger.debugv("Invalid LoopbackAddress: Wildcard not allowed - input = {0}", uri.toString());
return false;
}
}

if ("localhost".equalsIgnoreCase(uri.getHost())) { // "localhost" is not allowed.
if (config.isOAuth2_1Compliant()) {
if ("localhost".equalsIgnoreCase(uri.getHost())) {
logger.debugv("Invalid LoopbackAddress: localhost not allowed - OAuth 2.1 compliant - input = {0}", uri.toString());
return false;
}

if (isRedirectUriParameter) {
if (uri.getPort() < 0 || uri.getPort() > 65535) { // only 0 to 65535 are allowed. no port number is not allowed.
if (uri.getPort() < 0 || uri.getPort() > 65535) {
logger.debugv("Invalid LoopbackAddress: invalid port number - OAuth 2.1 compliant - redirect_uri parameter - input = {0}", uri.toString());
return false;
}
} else {
if (uri.getPort() > -1) { // any port number is not allowed
if (uri.getPort() > -1) {
logger.debugv("Invalid LoopbackAddress: port number not allowed - OAuth 2.1 compliant - input = {0}", uri.toString());
return false;
}
}

if (!p.test(uri)) return false; // additional tests for OAuth 2.1 compliant
if (!p.test(uri)) return false;
}
return true;
}
Expand All @@ -430,18 +442,20 @@ boolean isValidPrivateUseUriScheme() {
}
}

if (config.isOAuth2_1Compliant()) {
if (isIncludeUriFragment()) { // URL fragment is not allowed
logger.debugv("Invalid PrivateUseUriScheme: URI fragment not allowed - OAuth 2.1 compliant - input = {0}", uri.toString());
if (config.isOAuth2_0Compliant() || config.isOAuth2_1Compliant()) {
if (isIncludeUriFragment()) {
logger.debugv("Invalid PrivateUseUriScheme: URI fragment not allowed - input = {0}", uri.toString());
return false;
}

if (isIncludeWildcard()) { // wildcard is not allowed
logger.debugv("Invalid PrivateUseUriScheme: Wildcard not allowed - OAuth 2.1 compliant - input = {0}", uri.toString());
if (isIncludeWildcard()) {
logger.debugv("Invalid PrivateUseUriScheme: Wildcard not allowed - input = {0}", uri.toString());
return false;
}
}

if (uri.getScheme() == null || !uri.getScheme().contains(".")) { // a single word scheme name is not allowed.
if (config.isOAuth2_1Compliant()) {
if (uri.getScheme() == null || !uri.getScheme().contains(".")) {
logger.debugv("Invalid PrivateUseUriScheme: a single word scheme name is not allowed - OAuth 2.1 compliant - input = {0}", uri.toString());
return false;
}
Expand Down Expand Up @@ -477,19 +491,21 @@ boolean isValidNormalUri() {
}
}

if (config.isOAuth2_1Compliant()) {
if (!isHttps()) { // only https scheme is allowed.
logger.debugv("Invalid NormalUri: HTTP not allowed - OAuth 2.1 compliant - input = {0}", uri.toString());
if (config.isOAuth2_0Compliant() || config.isOAuth2_1Compliant()) {
if (isIncludeUriFragment()) {
logger.debugv("Invalid NormalUri: URI fragment not allowed - input = {0}", uri.toString());
return false;
}

if (isIncludeUriFragment()) { // URL fragment is not allowed.
logger.debugv("Invalid NormalUri: URI fragment not allowed - OAuth 2.1 compliant - input = {0}", uri.toString());
if (isIncludeWildcard()) {
logger.debugv("Invalid NormalUri: Wildcard not allowed - input = {0}", uri.toString());
return false;
}
}

if (isIncludeWildcard()) { // wildcard is not allowed.
logger.debugv("Invalid NormalUri: Wildcard not allowed - OAuth 2.1 compliant - input = {0}", uri.toString());
if (config.isOAuth2_1Compliant()) {
if (!isHttps()) {
logger.debugv("Invalid NormalUri: HTTP not allowed - OAuth 2.1 compliant - input = {0}", uri.toString());
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public class SecureRedirectUrisEnforcerExecutorFactory implements ClientPolicyEx
public static final String ALLOW_HTTP_SCHEME = "allow-http-scheme";
public static final String ALLOW_WILDCARD_CONTEXT_PATH = "allow-wildcard-context-path";
public static final String ALLOW_PERMITTED_DOMAINS = "allow-permitted-domains";
public static final String OAUTH_2_0_COMPLIANT = "oauth-2-0-compliant";
public static final String OAUTH_2_1_COMPLIANT = "oauth-2-1-compliant";

public static final String ALLOW_OPEN_REDIRECT = "allow-open-redirect";
Expand Down Expand Up @@ -140,6 +141,16 @@ public List<ProviderConfigProperty> getConfigProperties() {
.type(ProviderConfigProperty.MULTIVALUED_STRING_TYPE)
.add()

.property()
.name(OAUTH_2_0_COMPLIANT)
.label("OAuth 2.0 Compliant")
.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)
Comment on lines +147 to +150
Copilot AI Apr 15, 2026
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
.defaultValue(false)
.add()

.property()
.name(OAUTH_2_1_COMPLIANT)
.label("OAuth 2.1 Compliant")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import static org.keycloak.services.clientpolicy.executor.SecureRedirectUrisEnforcerExecutorFactory.ALLOW_PERMITTED_DOMAINS;
import static org.keycloak.services.clientpolicy.executor.SecureRedirectUrisEnforcerExecutorFactory.ALLOW_PRIVATE_USE_URI_SCHEME;
import static org.keycloak.services.clientpolicy.executor.SecureRedirectUrisEnforcerExecutorFactory.ALLOW_WILDCARD_CONTEXT_PATH;
import static org.keycloak.services.clientpolicy.executor.SecureRedirectUrisEnforcerExecutorFactory.OAUTH_2_0_COMPLIANT;
import static org.keycloak.services.clientpolicy.executor.SecureRedirectUrisEnforcerExecutorFactory.OAUTH_2_1_COMPLIANT;

import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -76,6 +77,7 @@ public void defaultConfiguration() {
assertFalse(configuration.isAllowPrivateUseUriScheme());
assertFalse(configuration.isAllowHttpScheme());
assertFalse(configuration.isAllowWildcardContextPath());
assertFalse(configuration.isOAuth2_0Compliant());
assertFalse(configuration.isOAuth2_1Compliant());
assertFalse(configuration.isAllowOpenRedirect());

Expand Down Expand Up @@ -339,6 +341,72 @@ public void successAllConfigurationDisabled() {
).forEach(i->checkSuccess(i, false));
}

@Test
public void failNormalUri_OAuth2_0Compliant() {
enable(OAUTH_2_0_COMPLIANT);
Stream.of(
"https://oauth.redirect:8443/auth/callback#fragment",
"https://example.org/path#frag",
Copilot AI Apr 15, 2026
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
"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(

Copilot uses AI. Check for mistakes.
"https://example.org/*"
).forEach(i->checkFail(i, false, SecureRedirectUrisEnforcerExecutor.ERR_NORMALURI));
}

@Test
public void successNormalUri_OAuth2_0Compliant() {
enable(OAUTH_2_0_COMPLIANT);
// HTTP is still allowed (unlike OAuth 2.1) when allow-http-scheme is on
enable(ALLOW_HTTP_SCHEME);
Stream.of(
"https://example.org/redirect",
"http://example.org/redirect",
"https://example.org:8443/auth/callback"
).forEach(i->checkSuccess(i, false));
}

@Test
public void failLoopbackAddress_OAuth2_0Compliant() {
enable(ALLOW_IPV4_LOOPBACK_ADDRESS, ALLOW_IPV6_LOOPBACK_ADDRESS, ALLOW_HTTP_SCHEME, OAUTH_2_0_COMPLIANT);
Stream.of(
"http://127.0.0.1/auth/admin#fragment",
"http://[::1]/auth/admin#fragment",
"http://127.0.0.1/*",
"http://[::1]/*"
).forEach(i->checkFail(i, false, SecureRedirectUrisEnforcerExecutor.ERR_LOOPBACK));
}

@Test
public void successLoopbackAddress_OAuth2_0Compliant() {
enable(ALLOW_IPV4_LOOPBACK_ADDRESS, ALLOW_IPV6_LOOPBACK_ADDRESS, ALLOW_HTTP_SCHEME, OAUTH_2_0_COMPLIANT);
// localhost is still allowed (unlike OAuth 2.1)
Stream.of(
"http://localhost/auth/admin",
"http://localhost:8080/auth/admin",
"http://127.0.0.1:8080/auth/admin",
"http://127.0.0.1/auth/admin",
"http://[::1]:8080/auth/admin",
"http://[::1]/auth/admin"
).forEach(i->checkSuccess(i, false));
}

@Test
public void failPrivateUseUriScheme_OAuth2_0Compliant() {
enable(ALLOW_PRIVATE_USE_URI_SCHEME, OAUTH_2_0_COMPLIANT);
Stream.of(
"myapp:/oauth.redirect#pinpoint",
"com.example.app:/oauth.redirect/*"
).forEach(i->checkFail(i, false, SecureRedirectUrisEnforcerExecutor.ERR_PRIVATESCHEME));
}

@Test
public void successPrivateUseUriScheme_OAuth2_0Compliant() {
enable(ALLOW_PRIVATE_USE_URI_SCHEME, OAUTH_2_0_COMPLIANT);
// single-word scheme is still allowed (unlike OAuth 2.1)
Stream.of(
"com.example.app:/oauth2redirect/example-provider",
"myapp:/oauth.redirect"
).forEach(i->checkSuccess(i, false));
}

private void permittedDomains(String... domains) {
ArrayNode arrayNode = JsonSerialization.mapper.createArrayNode();
Arrays.stream(domains).forEach(arrayNode::add);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ public void testSecureRedirectUrisEnforcerExecutor_AllowPermittedDomains() throw
}

@Test
public void testSecureRedirectUrisEnforcerExecutor_OAuth2_1Complient() throws Exception {
public void testSecureRedirectUrisEnforcerExecutor_OAuth2_1Compliant() throws Exception {
// register profiles
String json = (new ClientPoliciesUtil.ClientProfilesBuilder()).addProfile(
(new ClientPoliciesUtil.ClientProfileBuilder()).createProfile(PROFILE_NAME, "Le Premier Profil")
Expand Down Expand Up @@ -522,6 +522,62 @@ public void testSecureRedirectUrisEnforcerExecutor_OAuth2_1Complient() throws Ex
testSecureRedirectUrisEnforcerExecutor_failAuthorizationRequest(alphaClientId, "com.example.app:/oauth3redirect/example-provider");
}

@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/*"));
Comment on lines +525 to +557
Copilot AI Apr 15, 2026
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

// register - success
// localhost is still allowed (unlike OAuth 2.1)
List<String> registerResultList = testSecureRedirectUrisEnforcerExecutor_successRegisterByAdmin(
List.of("http://localhost/auth/admin"));
String alphaClientId = registerResultList.get(0);
String alphaCid = registerResultList.get(1);

// update - fail
// fragment not allowed
testSecureRedirectUrisEnforcerExecutor_failUpdateByAdmin(alphaCid,
List.of("http://localhost/auth/admin#frag"));

// update - success
// HTTP scheme and localhost are allowed under OAuth 2.0 (unlike OAuth 2.1)
testSecureRedirectUrisEnforcerExecutor_successUpdateByAdmin(alphaCid,
List.of("http://localhost/auth/redirect", "http://127.0.0.1:8080/callback"));

// authorization request - fail
// redirect_uri not match with registered redirect uris
testSecureRedirectUrisEnforcerExecutor_failAuthorizationRequest(alphaClientId, "http://localhost/auth/other");
}

@Test
public void testSecureRedirectUrisEnforcerExecutor_AllowOpenRedirect() throws Exception {
// Allow open redirect
Expand Down
Loading
0