diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientAuthenticator.java index 2f083163f54..72d4becc933 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientAuthenticator.java @@ -34,6 +34,7 @@ import jakarta.ws.rs.core.Response; import org.keycloak.OAuthErrorException; import org.keycloak.authentication.AuthenticationFlowError; import org.keycloak.authentication.ClientAuthenticationFlowContext; +import org.keycloak.crypto.ClientSignatureVerifierProvider; import org.keycloak.jose.jws.JWSInput; import org.keycloak.keys.loader.PublicKeyStorageManager; import org.keycloak.models.AuthenticationExecutionModel; @@ -49,6 +50,8 @@ import org.keycloak.representations.JsonWebToken; import org.keycloak.services.ServicesLogger; import org.keycloak.services.Urls; +import static org.keycloak.models.TokenManager.DEFAULT_VALIDATOR; + /** * Client authentication based on JWT signed by client private key . * See specs for more details. @@ -67,7 +70,7 @@ public class JWTClientAuthenticator extends AbstractClientAuthenticator { @Override public void authenticateClient(ClientAuthenticationFlowContext context) { - JWTClientValidator validator = new JWTClientValidator(context); + JWTClientValidator validator = new JWTClientValidator(context, getId()); if (!validator.clientAssertionParametersValidation()) return; try { @@ -90,7 +93,17 @@ public class JWTClientAuthenticator extends AbstractClientAuthenticator { boolean signatureValid; try { - JsonWebToken jwt = context.getSession().tokens().decodeClientJWT(clientAssertion, client, JsonWebToken.class); + JsonWebToken jwt = context.getSession().tokens().decodeClientJWT(clientAssertion, client, (jose, validatedClient) -> { + DEFAULT_VALIDATOR.accept(jose, validatedClient); + String signatureAlgorithm = jose.getHeader().getRawAlgorithm(); + ClientSignatureVerifierProvider signatureProvider = context.getSession().getProvider(ClientSignatureVerifierProvider.class, signatureAlgorithm); + if (signatureProvider == null) { + throw new RuntimeException("Algorithm not supported"); + } + if (!signatureProvider.isAsymmetricAlgorithm()) { + throw new RuntimeException("Algorithm is not asymmetric"); + } + }, JsonWebToken.class); signatureValid = jwt != null; } catch (RuntimeException e) { Throwable cause = e.getCause() != null ? e.getCause() : e; diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientSecretAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientSecretAuthenticator.java index 9e967e17070..07068844057 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientSecretAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientSecretAuthenticator.java @@ -18,6 +18,7 @@ package org.keycloak.authentication.authenticators.client; import org.keycloak.authentication.AuthenticationFlowError; import org.keycloak.authentication.ClientAuthenticationFlowContext; +import org.keycloak.crypto.ClientSignatureVerifierProvider; import org.keycloak.jose.jws.JWSInput; import org.keycloak.models.AuthenticationExecutionModel.Requirement; import org.keycloak.models.ClientModel; @@ -41,6 +42,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import static org.keycloak.models.TokenManager.DEFAULT_VALIDATOR; + /** * Client authentication based on JWT signed by client secret instead of private key . * See specs for more details. @@ -55,7 +58,7 @@ public class JWTClientSecretAuthenticator extends AbstractClientAuthenticator { @Override public void authenticateClient(ClientAuthenticationFlowContext context) { - JWTClientValidator validator = new JWTClientValidator(context); + JWTClientValidator validator = new JWTClientValidator(context, getId()); if (!validator.clientAssertionParametersValidation()) return; try { @@ -85,7 +88,17 @@ public class JWTClientSecretAuthenticator extends AbstractClientAuthenticator { boolean signatureValid; try { - JsonWebToken jwt = context.getSession().tokens().decodeClientJWT(clientAssertion, client, JsonWebToken.class); + JsonWebToken jwt = context.getSession().tokens().decodeClientJWT(clientAssertion, client, (jose, validatedClient) -> { + DEFAULT_VALIDATOR.accept(jose, validatedClient); + String signatureAlgorithm = jose.getHeader().getRawAlgorithm(); + ClientSignatureVerifierProvider signatureProvider = context.getSession().getProvider(ClientSignatureVerifierProvider.class, signatureAlgorithm); + if (signatureProvider == null) { + throw new RuntimeException("Algorithm not supported"); + } + if (signatureProvider.isAsymmetricAlgorithm()) { + throw new RuntimeException("Algorithm is not symmetric"); + } + }, JsonWebToken.class); signatureValid = jwt != null; //try authenticate with client rotated secret if (!signatureValid && wrapper.hasRotatedSecret() && !wrapper.isClientRotatedSecretExpired()) { diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientValidator.java b/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientValidator.java index 8d2352578eb..457295ac0f4 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientValidator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/client/JWTClientValidator.java @@ -50,6 +50,7 @@ public class JWTClientValidator { private final ClientAuthenticationFlowContext context; private final RealmModel realm; private final int currentTime; + private final String clientAuthenticatorProviderId; private MultivaluedMap params; private String clientAssertion; @@ -59,10 +60,11 @@ public class JWTClientValidator { private static final int ALLOWED_CLOCK_SKEW = 15; // sec - public JWTClientValidator(ClientAuthenticationFlowContext context) { + public JWTClientValidator(ClientAuthenticationFlowContext context, String clientAuthenticatorProviderId) { this.context = context; this.realm = context.getRealm(); this.currentTime = Time.currentTime(); + this.clientAuthenticatorProviderId = clientAuthenticatorProviderId; } public boolean clientAssertionParametersValidation() { @@ -138,6 +140,11 @@ public class JWTClientValidator { return false; } + if (!clientAuthenticatorProviderId.equals(client.getClientAuthenticatorType())) { + context.failure(AuthenticationFlowError.INVALID_CLIENT_CREDENTIALS, null); + return false; + } + return true; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSecretSignedJWTTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSecretSignedJWTTest.java index 51cd41caa5a..d8d633c239a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSecretSignedJWTTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSecretSignedJWTTest.java @@ -43,7 +43,9 @@ import org.jetbrains.annotations.NotNull; import org.junit.Rule; import org.junit.Test; import org.keycloak.OAuth2Constants; +import org.keycloak.OAuthErrorException; import org.keycloak.admin.client.resource.ClientResource; +import org.keycloak.authentication.authenticators.client.JWTClientAuthenticator; import org.keycloak.authentication.authenticators.client.JWTClientSecretAuthenticator; import org.keycloak.common.Profile; import org.keycloak.common.util.KeycloakUriBuilder; @@ -129,6 +131,41 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest { testCodeToTokenRequestSuccess(Algorithm.HS512); } + + // Issue 34547 + @Test + public void testCodeToTokenRequestSuccessWhenClientHasGeneratedKeys() throws Exception { + // Test when client has public/private keys generated despite the fact that it uses client-secret for the client authentication (and not those keys) + ApiUtil.findClientByClientId(adminClient.realm("test"), "test-app").getCertficateResource("jwt.credential").generate(); + + testCodeToTokenRequestSuccess(Algorithm.HS256); + } + + @Test + public void testCodeToTokenRequestFailureWhenClientHasPrivateKeyJWT() throws Exception { + // Setup client for "private_key_jwt" authentication + ClientResource client = ApiUtil.findClientByClientId(adminClient.realm("test"), "test-app"); + client.getCertficateResource("jwt.credential").generate(); + ClientRepresentation clientRep = client.toRepresentation(); + clientRep.setClientAuthenticatorType(JWTClientAuthenticator.PROVIDER_ID); + client.update(clientRep); + + // Client should not be able to authenticate with "client_secret_jwt" + try { + oauth.clientId("test-app"); + oauth.doLogin("test-user@localhost", "password"); + events.expectLogin().client("test-app").assertEvent(); + + String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, getClientSignedJWT(CLIENT_SECRET, 20, Algorithm.HS256)); + assertEquals(400, response.getStatusCode()); + assertEquals(OAuthErrorException.INVALID_CLIENT, response.getError()); + } finally { + clientRep.setClientAuthenticatorType(JWTClientSecretAuthenticator.PROVIDER_ID); + client.update(clientRep); + } + } + @Test public void testInvalidIssuer() throws Exception { oauth.clientId("test-app"); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java index 57afdad76be..c6cc07e9ddf 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java @@ -27,6 +27,7 @@ import org.keycloak.OAuthErrorException; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.authentication.AuthenticationFlowError; import org.keycloak.authentication.authenticators.client.JWTClientAuthenticator; +import org.keycloak.authentication.authenticators.client.JWTClientSecretAuthenticator; import org.keycloak.common.constants.ServiceAccountConstants; import org.keycloak.common.util.KeystoreUtil.KeystoreFormat; import org.keycloak.crypto.Algorithm; @@ -675,6 +676,28 @@ public class ClientAuthSignedJWTTest extends AbstractClientAuthSignedJWTTest { assertEquals(OAuthErrorException.INVALID_CLIENT, response.getError()); } + @Test + public void testAuthenticationFailsWhenClientSecretJWTAuthenticatorSet() throws Exception { + // Set client authenticator to JWT signed by client secret. + ClientResource clientResource = ApiUtil.findClientByClientId(adminClient.realm("test"), "client1"); + ClientRepresentation clientRep = clientResource.toRepresentation(); + clientRep.setClientAuthenticatorType(JWTClientSecretAuthenticator.PROVIDER_ID); + clientResource.update(clientRep); + + // It should not be possible to use private_key_jwt for the authentication + try { + String clientJwt = getClient1SignedJWT(); + + OAuthClient.AccessTokenResponse response = doClientCredentialsGrantRequest(clientJwt); + + assertEquals(400, response.getStatusCode()); + assertEquals(OAuthErrorException.UNAUTHORIZED_CLIENT, response.getError()); + } finally { + clientRep.setClientAuthenticatorType(JWTClientAuthenticator.PROVIDER_ID); + clientResource.update(clientRep); + } + } + @Test public void testMissingIdClaim() throws Exception { OAuthClient.AccessTokenResponse response = testMissingClaim("id");