From 1792af68508120b7734d22f4d0f956e4a23bb60b Mon Sep 17 00:00:00 2001 From: Takashi Norimatsu Date: Fri, 1 Mar 2024 20:13:14 +0900 Subject: [PATCH] OAuth 2.1 default profile lacks oauth-2-1-compliant setting for SecureRedirectUrisEnforcerExecutor closes #27412 Signed-off-by: Takashi Norimatsu --- .../keycloak-default-client-profiles.json | 6 +- .../OAuth2_1ConfidentialClientTest.java | 112 ++++++++++-------- .../client/OAuth2_1PublicClientTest.java | 58 ++++++--- 3 files changed, 111 insertions(+), 65 deletions(-) diff --git a/services/src/main/resources/keycloak-default-client-profiles.json b/services/src/main/resources/keycloak-default-client-profiles.json index b0fec76931..559f0a68a1 100644 --- a/services/src/main/resources/keycloak-default-client-profiles.json +++ b/services/src/main/resources/keycloak-default-client-profiles.json @@ -308,7 +308,8 @@ "configuration": { "allow-ipv4-loopback-address": "true", "allow-ipv6-loopback-address": "true", - "allow-private-use-uri-scheme": "true" + "allow-private-use-uri-scheme": "true", + "oauth-2-1-compliant": "true" } }, { @@ -346,7 +347,8 @@ "configuration": { "allow-ipv4-loopback-address": "true", "allow-ipv6-loopback-address": "true", - "allow-private-use-uri-scheme": "true" + "allow-private-use-uri-scheme": "true", + "oauth-2-1-compliant": "true" } }, { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OAuth2_1ConfidentialClientTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OAuth2_1ConfidentialClientTest.java index bf4978054f..774ceb73ee 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OAuth2_1ConfidentialClientTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OAuth2_1ConfidentialClientTest.java @@ -19,6 +19,7 @@ package org.keycloak.testsuite.client; import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.keycloak.OAuth2Constants; import org.keycloak.OAuthErrorException; @@ -36,6 +37,7 @@ import org.keycloak.representations.oidc.OIDCClientRepresentation; import org.keycloak.services.clientpolicy.ClientPolicyException; import org.keycloak.services.clientpolicy.condition.AnyClientConditionFactory; import org.keycloak.testsuite.Assert; +import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.client.resources.TestApplicationResourceUrls; import org.keycloak.testsuite.util.ClientPoliciesUtil; import org.keycloak.testsuite.util.MutualTLSUtils; @@ -57,6 +59,13 @@ public class OAuth2_1ConfidentialClientTest extends AbstractFAPITest { private static final String OAUTH2_1_CONFIDENTIAL_CLIENT_PROFILE_NAME = "oauth-2-1-for-confidential-client"; + private String validRedirectUri;; + + @Before + public void setupValidateRedirectUri() { + validRedirectUri = AssertEvents.DEFAULT_REDIRECT_URI.replace("localhost", "127.0.0.1"); + } + @After public void revertPolicies() throws ClientPolicyException { oauth.openid(true); @@ -75,7 +84,7 @@ public class OAuth2_1ConfidentialClientTest extends AbstractFAPITest { clientRep.setStandardFlowEnabled(Boolean.TRUE); clientRep.setImplicitFlowEnabled(Boolean.TRUE); clientRep.setClientAuthenticatorType(JWTClientAuthenticator.PROVIDER_ID); - + clientRep.setRedirectUris(Collections.singletonList(validRedirectUri)); }); assertEquals(JWTClientAuthenticator.PROVIDER_ID, getClientByAdmin(cId).getClientAuthenticatorType()); @@ -147,6 +156,7 @@ public class OAuth2_1ConfidentialClientTest extends AbstractFAPITest { clientConfig.setRequestUris(Collections.singletonList(TestApplicationResourceUrls.clientRequestUri())); clientConfig.setTlsClientAuthSubjectDn(MutualTLSUtils.DEFAULT_KEYSTORE_SUBJECT_DN); clientConfig.setAllowRegexPatternComparison(false); + clientRep.setRedirectUris(Collections.singletonList(validRedirectUri.replace(":8543/", "/"))); }); verifyClientSettings(getClientByAdmin(cId), X509ClientAuthenticator.PROVIDER_ID); } @@ -155,35 +165,33 @@ public class OAuth2_1ConfidentialClientTest extends AbstractFAPITest { @Test public void testOAuth2_1ProofKeyForCodeExchange() throws Exception { + String clientId = generateSuffixedName(CLIENT_NAME); + String cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> + setupValidClientExceptForRedirectUri(clientRep, OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep)) + ); + verifyClientSettings(getClientByAdmin(cId), X509ClientAuthenticator.PROVIDER_ID); + // setup profiles and policies setupPolicyOAuth2_1ConfidentialClientForAllClient(); - String clientId = generateSuffixedName(CLIENT_NAME); - String cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> { - clientRep.setStandardFlowEnabled(Boolean.TRUE); - clientRep.setClientAuthenticatorType(JWTClientAuthenticator.PROVIDER_ID); - - }); - verifyClientSettings(getClientByAdmin(cId), JWTClientAuthenticator.PROVIDER_ID); - + oauth.redirectUri(validRedirectUri); + oauth.codeChallenge(null); + oauth.codeChallengeMethod(null); + oauth.codeVerifier(null); failLoginByNotFollowingPKCE(clientId); } @Test public void testOAuth2_1RedirectUris() throws Exception { + String clientId = generateSuffixedName(CLIENT_NAME); + String cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> + setupValidClientExceptForRedirectUri(clientRep, OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep)) + ); + verifyClientSettings(getClientByAdmin(cId), X509ClientAuthenticator.PROVIDER_ID); + // setup profiles and policies setupPolicyOAuth2_1ConfidentialClientForAllClient(); - String clientId = generateSuffixedName(CLIENT_NAME); - String cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> { - clientRep.setClientAuthenticatorType(X509ClientAuthenticator.PROVIDER_ID); - OIDCAdvancedConfigWrapper clientConfig = OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep); - clientConfig.setRequestUris(Collections.singletonList(TestApplicationResourceUrls.clientRequestUri())); - clientConfig.setTlsClientAuthSubjectDn(MutualTLSUtils.DEFAULT_KEYSTORE_SUBJECT_DN); - clientConfig.setAllowRegexPatternComparison(false); - }); - verifyClientSettings(getClientByAdmin(cId), X509ClientAuthenticator.PROVIDER_ID); - faiilUpdateRedirectUrisDynamically(clientId, List.of("https://dev.example.com:8443/*")); successUpdateRedirectUrisByAdmin(cId, List.of("https://dev.example.com:8443/callback", "https://[::1]/auth/admin", @@ -193,20 +201,17 @@ public class OAuth2_1ConfidentialClientTest extends AbstractFAPITest { @Test public void testOAuth2_1OAuthMtlsSenderConstrainedToken() throws Exception { + String clientId = generateSuffixedName(CLIENT_NAME); + String cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> + setupValidClientExceptForRedirectUri(clientRep, OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep)) + ); + verifyClientSettings(getClientByAdmin(cId), X509ClientAuthenticator.PROVIDER_ID); + // setup profiles and policies setupPolicyOAuth2_1ConfidentialClientForAllClient(); - String clientId = generateSuffixedName(CLIENT_NAME); - String cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> { - clientRep.setClientAuthenticatorType(X509ClientAuthenticator.PROVIDER_ID); - OIDCAdvancedConfigWrapper clientConfig = OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep); - clientConfig.setRequestUris(Collections.singletonList(TestApplicationResourceUrls.clientRequestUri())); - clientConfig.setTlsClientAuthSubjectDn(MutualTLSUtils.DEFAULT_KEYSTORE_SUBJECT_DN); - clientConfig.setAllowRegexPatternComparison(false); - }); - verifyClientSettings(getClientByAdmin(cId), X509ClientAuthenticator.PROVIDER_ID); - oauth.clientId(clientId); + oauth.redirectUri(validRedirectUri); setValidPkce(clientId); OAuthClient.AuthorizationEndpointResponse res = oauth.doLogin(TEST_USERNAME, TEST_USERSECRET); @@ -217,6 +222,16 @@ public class OAuth2_1ConfidentialClientTest extends AbstractFAPITest { oauth.idTokenHint(tokenResponse.getIdToken()).openLogout(); } + private void testProhibitedImplicitOrHybridFlow(boolean isOpenid, String responseType, String nonce) { + oauth.openid(isOpenid); + oauth.responseType(responseType); + oauth.nonce(nonce); + oauth.redirectUri(validRedirectUri); + oauth.openLoginForm(); + assertEquals(OAuthErrorException.INVALID_REQUEST, oauth.getCurrentFragment().get(OAuth2Constants.ERROR)); + assertEquals("Implicit/Hybrid flow is prohibited.", oauth.getCurrentFragment().get(OAuth2Constants.ERROR_DESCRIPTION)); + } + private void setupPolicyOAuth2_1ConfidentialClientForAllClient() throws Exception { String json = (new ClientPoliciesUtil.ClientPoliciesBuilder()).addPolicy( (new ClientPoliciesUtil.ClientPolicyBuilder()).createPolicy("MyPolicy", "Policy for enable OAuth 2.1 confidential client profile for all clients", Boolean.TRUE) @@ -228,14 +243,17 @@ public class OAuth2_1ConfidentialClientTest extends AbstractFAPITest { updatePolicies(json); } - private void testProhibitedImplicitOrHybridFlow(boolean isOpenid, String responseType, String nonce) { - oauth.openid(isOpenid); - oauth.responseType(responseType); - oauth.nonce(nonce); - oauth.openLoginForm(); - assertEquals(OAuthErrorException.INVALID_REQUEST, oauth.getCurrentFragment().get(OAuth2Constants.ERROR)); - assertEquals("Implicit/Hybrid flow is prohibited.", oauth.getCurrentFragment().get(OAuth2Constants.ERROR_DESCRIPTION)); - } + private void setupValidClientExceptForRedirectUri(ClientRepresentation clientRep, OIDCAdvancedConfigWrapper clientConfig ) { + clientRep.setClientAuthenticatorType(X509ClientAuthenticator.PROVIDER_ID); + clientRep.setRedirectUris(Collections.singletonList(validRedirectUri)); + clientRep.setImplicitFlowEnabled(false); + clientRep.setDirectAccessGrantsEnabled(false); + clientConfig.setRequestUris(Collections.singletonList(TestApplicationResourceUrls.clientRequestUri())); + clientConfig.setTlsClientAuthSubjectDn(MutualTLSUtils.DEFAULT_KEYSTORE_SUBJECT_DN); + clientConfig.setAllowRegexPatternComparison(false); + clientConfig.setPkceCodeChallengeMethod(OAuth2Constants.PKCE_METHOD_S256); + clientConfig.setUseMtlsHoKToken(true); + }; private void setValidPkce(String clientId) throws Exception { oauth.clientId(clientId); @@ -260,16 +278,6 @@ public class OAuth2_1ConfidentialClientTest extends AbstractFAPITest { assertFalse(clientRep.isDirectAccessGrantsEnabled()); } - private void faiilUpdateRedirectUrisDynamically(String clientId, List redirectUrisList) { - try { - updateClientDynamically(clientId, (OIDCClientRepresentation clientRep) -> - clientRep.setRedirectUris(redirectUrisList)); - fail(); - } catch (ClientRegistrationException e) { - assertEquals(ERR_MSG_CLIENT_REG_FAIL, e.getMessage()); - } - } - private void successUpdateRedirectUrisByAdmin(String cId, List redirectUrisList) { try { updateClientByAdmin(cId, (ClientRepresentation clientRep) -> { @@ -283,6 +291,16 @@ public class OAuth2_1ConfidentialClientTest extends AbstractFAPITest { } } + private void faiilUpdateRedirectUrisDynamically(String clientId, List redirectUrisList) { + try { + updateClientDynamically(clientId, (OIDCClientRepresentation clientRep) -> + clientRep.setRedirectUris(redirectUrisList)); + fail(); + } catch (ClientRegistrationException e) { + assertEquals(ERR_MSG_CLIENT_REG_FAIL, e.getMessage()); + } + } + private void failAuthorizationRequest(String clientId, String redirectUri) { oauth.clientId(clientId); oauth.redirectUri(redirectUri); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OAuth2_1PublicClientTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OAuth2_1PublicClientTest.java index fe3863a38f..43c7eacfd1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OAuth2_1PublicClientTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/OAuth2_1PublicClientTest.java @@ -26,6 +26,7 @@ import org.junit.Before; import org.junit.Test; import org.keycloak.OAuth2Constants; import org.keycloak.OAuthErrorException; +import org.keycloak.authentication.authenticators.client.X509ClientAuthenticator; import org.keycloak.client.registration.ClientRegistrationException; import org.keycloak.common.Profile; import org.keycloak.common.util.SecretGenerator; @@ -43,12 +44,15 @@ import org.keycloak.services.clientpolicy.ClientPolicyException; import org.keycloak.testsuite.AssertEvents; import org.keycloak.services.clientpolicy.condition.AnyClientConditionFactory; import org.keycloak.testsuite.arquillian.annotation.EnableFeature; +import org.keycloak.testsuite.client.resources.TestApplicationResourceUrls; import org.keycloak.testsuite.util.ClientPoliciesUtil; import org.keycloak.testsuite.util.Matchers; +import org.keycloak.testsuite.util.MutualTLSUtils; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.util.JsonSerialization; import java.security.KeyPair; +import java.util.Collections; import java.util.List; import java.util.UUID; @@ -72,6 +76,13 @@ public class OAuth2_1PublicClientTest extends AbstractFAPITest { private JWK jwkEc; + private String validRedirectUri;; + + @Before + public void setupValidateRedirectUri() { + validRedirectUri = AssertEvents.DEFAULT_REDIRECT_URI.replace("localhost", "127.0.0.1"); + } + @Before public void beforeDPoPTest() throws Exception { ecKeyPair = generateEcdsaKey("secp256r1"); @@ -97,6 +108,7 @@ public class OAuth2_1PublicClientTest extends AbstractFAPITest { clientRep.setStandardFlowEnabled(Boolean.TRUE); clientRep.setImplicitFlowEnabled(Boolean.TRUE); clientRep.setPublicClient(Boolean.TRUE); + clientRep.setRedirectUris(Collections.singletonList(validRedirectUri)); }); // setup profiles and policies @@ -143,16 +155,19 @@ public class OAuth2_1PublicClientTest extends AbstractFAPITest { @Test public void testOAuth2_1ProofKeyForCodeExchange() throws Exception { + String clientId = generateSuffixedName(CLIENT_NAME); + String cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> + setupValidClientExceptForRedirectUri(clientRep, OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep)) + ); + assertEquals(OAuth2Constants.PKCE_METHOD_S256, OIDCAdvancedConfigWrapper.fromClientRepresentation(getClientByAdmin(cId)).getPkceCodeChallengeMethod()); + // setup profiles and policies setupPolicyOAuth2_1PublicClientForAllClient(); - String clientId = generateSuffixedName(CLIENT_NAME); - String cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> { - clientRep.setPublicClient(Boolean.TRUE); - clientRep.setRedirectUris(List.of(AssertEvents.DEFAULT_REDIRECT_URI)); - }); - assertEquals(OAuth2Constants.PKCE_METHOD_S256, OIDCAdvancedConfigWrapper.fromClientRepresentation(getClientByAdmin(cId)).getPkceCodeChallengeMethod()); - + oauth.redirectUri(validRedirectUri); + oauth.codeChallenge(null); + oauth.codeChallengeMethod(null); + oauth.codeVerifier(null); failLoginByNotFollowingPKCE(clientId); } @@ -173,9 +188,9 @@ public class OAuth2_1PublicClientTest extends AbstractFAPITest { String clientId = generateSuffixedName(CLIENT_NAME); String cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> { clientRep.setPublicClient(Boolean.TRUE); - clientRep.setRedirectUris(List.of(AssertEvents.DEFAULT_REDIRECT_URI)); + clientRep.setRedirectUris(List.of(validRedirectUri.replace(":8543/", "/"))); }); - assertEquals(AssertEvents.DEFAULT_REDIRECT_URI, getClientByAdmin(cId).getRedirectUris().get(0)); + assertEquals(validRedirectUri.replace(":8543/", "/"), getClientByAdmin(cId).getRedirectUris().get(0)); // update with valid redirect_uri - fail try { @@ -194,20 +209,20 @@ public class OAuth2_1PublicClientTest extends AbstractFAPITest { @Test public void testOAuth2_1DPoPSenderConstrainedToken() throws Exception { - // setup profiles and policies - setupPolicyOAuth2_1PublicClientForAllClient(); - // registration (auto-config) - success String clientId = generateSuffixedName(CLIENT_NAME); - String cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> { - clientRep.setPublicClient(Boolean.TRUE); - clientRep.setRedirectUris(List.of(AssertEvents.DEFAULT_REDIRECT_URI)); - }); + String cId = createClientByAdmin(clientId, (ClientRepresentation clientRep) -> + setupValidClientExceptForRedirectUri(clientRep, OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep)) + ); assertTrue(OIDCAdvancedConfigWrapper.fromClientRepresentation(getClientByAdmin(cId)).isUseDPoP()); + // setup profiles and policies + setupPolicyOAuth2_1PublicClientForAllClient(); + // authorization request - success setValidPkce(clientId); oauth.clientId(clientId); + oauth.redirectUri(validRedirectUri); oauth.doLogin(TEST_USER_NAME, TEST_USER_PASSWORD); // token request with DPoP Proof - success @@ -256,10 +271,21 @@ public class OAuth2_1PublicClientTest extends AbstractFAPITest { updatePolicies(json); } + private void setupValidClientExceptForRedirectUri(ClientRepresentation clientRep, OIDCAdvancedConfigWrapper clientConfig ) { + clientRep.setPublicClient(Boolean.TRUE); + clientRep.setRedirectUris(Collections.singletonList(validRedirectUri)); + clientRep.setImplicitFlowEnabled(false); + clientRep.setDirectAccessGrantsEnabled(false); + clientConfig.setRequestUris(Collections.singletonList(TestApplicationResourceUrls.clientRequestUri())); + clientConfig.setPkceCodeChallengeMethod(OAuth2Constants.PKCE_METHOD_S256); + clientConfig.setUseDPoP(true); + }; + private void testProhibitedImplicitOrHybridFlow(boolean isOpenid, String responseType, String nonce) { oauth.openid(isOpenid); oauth.responseType(responseType); oauth.nonce(nonce); + oauth.redirectUri(validRedirectUri); oauth.openLoginForm(); assertEquals(OAuthErrorException.INVALID_REQUEST, oauth.getCurrentFragment().get(OAuth2Constants.ERROR)); assertEquals("Implicit/Hybrid flow is prohibited.", oauth.getCurrentFragment().get(OAuth2Constants.ERROR_DESCRIPTION));