From d4374f37ae658c1c4189b0ac47b81a3c30a2b563 Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 24 May 2021 09:24:22 +0200 Subject: [PATCH] KEYCLOAK-18258 Not possible to login with public client, which was confidential with custom client authenticator set --- .../ClientAuthenticationFlow.java | 5 +- .../testsuite/oidc/OIDCPublicClientTest.java | 115 ++++++++++++++++++ .../OpenShiftTokenReviewEndpointTest.java | 13 +- 3 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCPublicClientTest.java diff --git a/services/src/main/java/org/keycloak/authentication/ClientAuthenticationFlow.java b/services/src/main/java/org/keycloak/authentication/ClientAuthenticationFlow.java index 8e6eda6789..3f58179408 100755 --- a/services/src/main/java/org/keycloak/authentication/ClientAuthenticationFlow.java +++ b/services/src/main/java/org/keycloak/authentication/ClientAuthenticationFlow.java @@ -75,8 +75,9 @@ public class ClientAuthenticationFlow implements AuthenticationFlow { if (client != null) { String expectedClientAuthType = client.getClientAuthenticatorType(); - // Fallback to secret just in case (for backwards compatibility) - if (expectedClientAuthType == null) { + // Fallback to secret just in case (for backwards compatibility). Also for public clients, ignore the "clientAuthenticatorType", which is set to them and stick to the + // default, which set the client just based on "client_id" parameter + if (expectedClientAuthType == null || client.isPublicClient()) { expectedClientAuthType = KeycloakModelUtils.getDefaultClientAuthenticatorType(); ServicesLogger.LOGGER.authMethodFallback(client.getClientId(), expectedClientAuthType); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCPublicClientTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCPublicClientTest.java new file mode 100644 index 0000000000..6c48783b49 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCPublicClientTest.java @@ -0,0 +1,115 @@ +/* + * Copyright 2021 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.keycloak.testsuite.oidc; + +import java.security.Security; +import java.util.List; + +import org.bouncycastle.jce.provider.BouncyCastleProvider; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.keycloak.OAuth2Constants; +import org.keycloak.admin.client.resource.ClientResource; +import org.keycloak.authentication.authenticators.client.JWTClientAuthenticator; +import org.keycloak.events.Details; +import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.EventRepresentation; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.testsuite.AbstractKeycloakTest; +import org.keycloak.testsuite.AssertEvents; +import org.keycloak.testsuite.admin.ApiUtil; +import org.keycloak.testsuite.util.ClientManager; +import org.keycloak.testsuite.util.OAuthClient; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.keycloak.testsuite.admin.AbstractAdminTest.loadJson; + +/** + * @author Marek Posolda + */ +public class OIDCPublicClientTest extends AbstractKeycloakTest { + + @Rule + public AssertEvents events = new AssertEvents(this); + + + @Override + public void beforeAbstractKeycloakTest() throws Exception { + super.beforeAbstractKeycloakTest(); + } + + @BeforeClass + public static void addBouncyCastleProvider() { + if (Security.getProvider("BC") == null) Security.addProvider(new BouncyCastleProvider()); + } + + @Before + public void clientConfiguration() { + ClientManager.realm(adminClient.realm("test")).clientId("test-app").directAccessGrant(true); + /* + * Configure the default client ID. Seems like OAuthClient is keeping the state of clientID + * For example: If some test case configure oauth.clientId("sample-public-client"), other tests + * will faile and the clientID will always be "sample-public-client + * @see AccessTokenTest#testAuthorizationNegotiateHeaderIgnored() + */ + oauth.clientId("test-app"); + } + + @Override + public void addTestRealms(List testRealms) { + RealmRepresentation realm = loadJson(getClass().getResourceAsStream("/testrealm.json"), RealmRepresentation.class); + testRealms.add(realm); + } + + + // KEYCLOAK-18258 + @Test + public void accessTokenRequest() throws Exception { + // Update client to use custom client authenticator + ClientResource clientResource = ApiUtil.findClientByClientId(adminClient.realms().realm("test"), "test-app"); + ClientRepresentation clientRep = clientResource.toRepresentation(); + clientRep.setClientAuthenticatorType(JWTClientAuthenticator.PROVIDER_ID); + clientResource.update(clientRep); + + // Switch client to public client now + clientRep = clientResource.toRepresentation(); + Assert.assertEquals(JWTClientAuthenticator.PROVIDER_ID, clientRep.getClientAuthenticatorType()); + clientRep.setPublicClient(true); + clientResource.update(clientRep); + + // It should be possible to authenticate + oauth.doLogin("test-user@localhost", "password"); + + EventRepresentation loginEvent = events.expectLogin().assertEvent(); + + String sessionId = loginEvent.getSessionId(); + String codeId = loginEvent.getDetails().get(Details.CODE_ID); + + String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, null); + + assertEquals(200, response.getStatusCode()); + assertNotNull(response.getAccessToken()); + EventRepresentation event = events.expectCodeToToken(codeId, sessionId).assertEvent(); + } + +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/openshift/OpenShiftTokenReviewEndpointTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/openshift/OpenShiftTokenReviewEndpointTest.java index 226d30b137..2bba0ec091 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/openshift/OpenShiftTokenReviewEndpointTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/openshift/OpenShiftTokenReviewEndpointTest.java @@ -12,6 +12,7 @@ import org.junit.Test; import org.keycloak.OAuth2Constants; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.RealmResource; +import org.keycloak.authentication.authenticators.client.ClientIdAndSecretAuthenticator; import org.keycloak.common.util.Base64Url; import org.keycloak.crypto.Algorithm; import org.keycloak.events.Details; @@ -314,7 +315,9 @@ public class OpenShiftTokenReviewEndpointTest extends AbstractTestRealmKeycloakT clientRep.setPublicClient(true); testRealm().clients().get(clientRep.getId()).update(clientRep); try { - new Review().invoke().assertError(401, "Public client is not permitted to invoke token review endpoint"); + new Review() + .clientAuthMethod(ClientIdAndSecretAuthenticator.PROVIDER_ID) + .invoke().assertError(401, "Public client is not permitted to invoke token review endpoint"); } finally { clientRep.setPublicClient(false); clientRep.setSecret("password"); @@ -332,6 +335,7 @@ public class OpenShiftTokenReviewEndpointTest extends AbstractTestRealmKeycloakT private InvokeRunnable runAfterTokenRequest; private String token; + private String clientAuthMethod = "testsuite-client-dummy"; private int responseStatus; private OpenShiftTokenReviewResponseRepresentation response; @@ -345,6 +349,11 @@ public class OpenShiftTokenReviewEndpointTest extends AbstractTestRealmKeycloakT return this; } + public Review clientAuthMethod(String clientAuthMethod) { + this.clientAuthMethod = clientAuthMethod; + return this; + } + public Review runAfterTokenRequest(InvokeRunnable runnable) { this.runAfterTokenRequest = runnable; return this; @@ -360,7 +369,7 @@ public class OpenShiftTokenReviewEndpointTest extends AbstractTestRealmKeycloakT String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); OAuthClient.AccessTokenResponse accessTokenResponse = oauth.doAccessTokenRequest(code, "password"); - events.expectCodeToToken(loginEvent.getDetails().get(Details.CODE_ID), loginEvent.getSessionId()).detail("client_auth_method", "testsuite-client-dummy").user(userId).assertEvent(); + events.expectCodeToToken(loginEvent.getDetails().get(Details.CODE_ID), loginEvent.getSessionId()).detail("client_auth_method", this.clientAuthMethod).user(userId).assertEvent(); token = accessTokenResponse.getAccessToken(); }