diff --git a/server-spi/src/main/java/org/keycloak/services/clientpolicy/ClientPolicyEvent.java b/server-spi/src/main/java/org/keycloak/services/clientpolicy/ClientPolicyEvent.java index 659c020a97..d362f31911 100644 --- a/server-spi/src/main/java/org/keycloak/services/clientpolicy/ClientPolicyEvent.java +++ b/server-spi/src/main/java/org/keycloak/services/clientpolicy/ClientPolicyEvent.java @@ -32,6 +32,7 @@ public enum ClientPolicyEvent { UNREGISTER, AUTHORIZATION_REQUEST, TOKEN_REQUEST, + SERVICE_ACCOUNT_TOKEN_REQUEST, TOKEN_REFRESH, TOKEN_REVOKE, TOKEN_INTROSPECT, diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java index 20b9b4ea50..2e5250d724 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java @@ -74,6 +74,7 @@ import org.keycloak.services.CorsErrorResponseException; import org.keycloak.services.ServicesLogger; import org.keycloak.services.Urls; import org.keycloak.services.clientpolicy.ClientPolicyException; +import org.keycloak.services.clientpolicy.context.ServiceAccountTokenRequestContext; import org.keycloak.services.clientpolicy.context.TokenRefreshContext; import org.keycloak.services.clientpolicy.context.TokenRequestContext; import org.keycloak.services.managers.AppAuthManager; @@ -443,21 +444,7 @@ public class TokenEndpoint { responseBuilder.generateRefreshToken(); } - // KEYCLOAK-6771 Certificate Bound Token - // https://tools.ietf.org/html/draft-ietf-oauth-mtls-08#section-3 - if (OIDCAdvancedConfigWrapper.fromClientModel(client).isUseMtlsHokToken()) { - AccessToken.CertConf certConf = MtlsHoKTokenUtil.bindTokenWithClientCertificate(request, session); - if (certConf != null) { - responseBuilder.getAccessToken().setCertConf(certConf); - if (OIDCAdvancedConfigWrapper.fromClientModel(client).isUseRefreshToken()) { - responseBuilder.getRefreshToken().setCertConf(certConf); - } - } else { - event.error(Errors.INVALID_REQUEST); - throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_REQUEST, - "Client Certification missing for MTLS HoK Token Binding", Response.Status.BAD_REQUEST); - } - } + checkMtlsHoKToken(responseBuilder, OIDCAdvancedConfigWrapper.fromClientModel(client).isUseRefreshToken()); if (TokenUtil.isOIDCRequest(scopeParam)) { responseBuilder.generateIDToken().generateAccessTokenHash(); @@ -483,6 +470,24 @@ public class TokenEndpoint { return cors.builder(Response.ok(res).type(MediaType.APPLICATION_JSON_TYPE)).build(); } + private void checkMtlsHoKToken(TokenManager.AccessTokenResponseBuilder responseBuilder, boolean useRefreshToken) { + // KEYCLOAK-6771 Certificate Bound Token + // https://tools.ietf.org/html/draft-ietf-oauth-mtls-08#section-3 + if (OIDCAdvancedConfigWrapper.fromClientModel(client).isUseMtlsHokToken()) { + AccessToken.CertConf certConf = MtlsHoKTokenUtil.bindTokenWithClientCertificate(request, session); + if (certConf != null) { + responseBuilder.getAccessToken().setCertConf(certConf); + if (useRefreshToken) { + responseBuilder.getRefreshToken().setCertConf(certConf); + } + } else { + event.error(Errors.INVALID_REQUEST); + throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_REQUEST, + "Client Certification missing for MTLS HoK Token Binding", Response.Status.BAD_REQUEST); + } + } + } + private void checkParamsForPkceEnforcedClient(String codeVerifier, String codeChallenge, String codeChallengeMethod, String authUserId, String authUsername) { // check whether code verifier is specified if (codeVerifier == null) { @@ -753,6 +758,13 @@ public class TokenEndpoint { userSession.setNote(ServiceAccountConstants.CLIENT_HOST, clientConnection.getRemoteHost()); userSession.setNote(ServiceAccountConstants.CLIENT_ADDRESS, clientConnection.getRemoteAddr()); + try { + session.clientPolicy().triggerOnEvent(new ServiceAccountTokenRequestContext(formParams, clientSessionCtx.getClientSession())); + } catch (ClientPolicyException cpe) { + event.error(cpe.getError()); + throw new CorsErrorResponseException(cors, cpe.getError(), cpe.getErrorDetail(), Response.Status.BAD_REQUEST); + } + updateUserSessionFromClientAuth(userSession); TokenManager.AccessTokenResponseBuilder responseBuilder = tokenManager.responseBuilder(realm, client, event, session, userSession, clientSessionCtx) @@ -765,6 +777,8 @@ public class TokenEndpoint { responseBuilder.getAccessToken().setSessionState(null); } + checkMtlsHoKToken(responseBuilder, useRefreshToken); + String scopeParam = clientSessionCtx.getClientSession().getNote(OAuth2Constants.SCOPE); if (TokenUtil.isOIDCRequest(scopeParam)) { responseBuilder.generateIDToken().generateAccessTokenHash(); diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientAccessTypeCondition.java b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientAccessTypeCondition.java index a58abb2b64..6db2725c18 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientAccessTypeCondition.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientAccessTypeCondition.java @@ -68,6 +68,7 @@ public class ClientAccessTypeCondition extends AbstractClientPolicyConditionProv switch (context.getEvent()) { case AUTHORIZATION_REQUEST: case TOKEN_REQUEST: + case SERVICE_ACCOUNT_TOKEN_REQUEST: case TOKEN_REFRESH: case TOKEN_REVOKE: case TOKEN_INTROSPECT: diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientRolesCondition.java b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientRolesCondition.java index 53c964dfe4..e5a483c157 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientRolesCondition.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientRolesCondition.java @@ -70,6 +70,7 @@ public class ClientRolesCondition extends AbstractClientPolicyConditionProvider< switch (context.getEvent()) { case AUTHORIZATION_REQUEST: case TOKEN_REQUEST: + case SERVICE_ACCOUNT_TOKEN_REQUEST: case TOKEN_REFRESH: case TOKEN_REVOKE: case TOKEN_INTROSPECT: diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientScopesCondition.java b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientScopesCondition.java index af67184e1a..0f9fd8665f 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientScopesCondition.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientScopesCondition.java @@ -37,6 +37,7 @@ import org.keycloak.services.clientpolicy.ClientPolicyContext; import org.keycloak.services.clientpolicy.ClientPolicyException; import org.keycloak.services.clientpolicy.ClientPolicyVote; import org.keycloak.services.clientpolicy.context.AuthorizationRequestContext; +import org.keycloak.services.clientpolicy.context.ServiceAccountTokenRequestContext; import org.keycloak.services.clientpolicy.context.TokenRequestContext; /** @@ -91,6 +92,9 @@ public class ClientScopesCondition extends AbstractClientPolicyConditionProvider case TOKEN_REQUEST: if (isScopeMatched(((TokenRequestContext)context).getParseResult().getClientSession())) return ClientPolicyVote.YES; return ClientPolicyVote.NO; + case SERVICE_ACCOUNT_TOKEN_REQUEST: + if (isScopeMatched(((ServiceAccountTokenRequestContext)context).getClientSession())) return ClientPolicyVote.YES; + return ClientPolicyVote.NO; case BACKCHANNEL_AUTHENTICATION_REQUEST: if (isScopeMatched(((BackchannelAuthenticationRequestContext)context).getParsedRequest())) return ClientPolicyVote.YES; return ClientPolicyVote.NO; diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/context/ServiceAccountTokenRequestContext.java b/services/src/main/java/org/keycloak/services/clientpolicy/context/ServiceAccountTokenRequestContext.java new file mode 100644 index 0000000000..71c5948bed --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/context/ServiceAccountTokenRequestContext.java @@ -0,0 +1,53 @@ +/* + * 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.services.clientpolicy.context; + +import javax.ws.rs.core.MultivaluedMap; + +import org.keycloak.models.AuthenticatedClientSessionModel; +import org.keycloak.services.clientpolicy.ClientPolicyContext; +import org.keycloak.services.clientpolicy.ClientPolicyEvent; + +/** + * @author Marek Posolda + */ +public class ServiceAccountTokenRequestContext implements ClientPolicyContext { + + private final MultivaluedMap params; + private final AuthenticatedClientSessionModel clientSession; + + public ServiceAccountTokenRequestContext(MultivaluedMap params, + AuthenticatedClientSessionModel clientSession) { + this.params = params; + this.clientSession = clientSession; + } + + @Override + public ClientPolicyEvent getEvent() { + return ClientPolicyEvent.SERVICE_ACCOUNT_TOKEN_REQUEST; + } + + public MultivaluedMap getParams() { + return params; + } + + public AuthenticatedClientSessionModel getClientSession() { + return clientSession; + } +} diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/ConfidentialClientAcceptExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/ConfidentialClientAcceptExecutor.java index fd3238eb12..ef23ed6255 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/ConfidentialClientAcceptExecutor.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/ConfidentialClientAcceptExecutor.java @@ -45,6 +45,7 @@ public class ConfidentialClientAcceptExecutor implements ClientPolicyExecutorPro switch (context.getEvent()) { case AUTHORIZATION_REQUEST: case TOKEN_REQUEST: + case SERVICE_ACCOUNT_TOKEN_REQUEST: case BACKCHANNEL_AUTHENTICATION_REQUEST: case BACKCHANNEL_TOKEN_REQUEST: checkIsConfidentialClient(); diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/HolderOfKeyEnforcerExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/HolderOfKeyEnforcerExecutor.java index 6bf4f0e5bb..09bf186d91 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/HolderOfKeyEnforcerExecutor.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/HolderOfKeyEnforcerExecutor.java @@ -90,6 +90,7 @@ public class HolderOfKeyEnforcerExecutor implements ClientPolicyExecutorProvider validate(clientUpdateContext.getProposedClientRepresentation()); break; case TOKEN_REQUEST: + case SERVICE_ACCOUNT_TOKEN_REQUEST: case BACKCHANNEL_TOKEN_REQUEST: AccessToken.CertConf certConf = MtlsHoKTokenUtil.bindTokenWithClientCertificate(request, session); if (certConf == null) { diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureClientAuthenticatorExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureClientAuthenticatorExecutor.java index bc297dadd7..8433e208e3 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureClientAuthenticatorExecutor.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureClientAuthenticatorExecutor.java @@ -93,6 +93,7 @@ public class SecureClientAuthenticatorExecutor implements ClientPolicyExecutorPr validateDuringClientCRUD(clientUpdateContext.getProposedClientRepresentation()); break; case TOKEN_REQUEST: + case SERVICE_ACCOUNT_TOKEN_REQUEST: case TOKEN_REFRESH: case TOKEN_REVOKE: case TOKEN_INTROSPECT: diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmForSignedJwtExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmForSignedJwtExecutor.java index 9be02c0ee9..92cabbecc4 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmForSignedJwtExecutor.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureSigningAlgorithmForSignedJwtExecutor.java @@ -77,6 +77,7 @@ public class SecureSigningAlgorithmForSignedJwtExecutor implements ClientPolicyE public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyException { switch (context.getEvent()) { case TOKEN_REQUEST: + case SERVICE_ACCOUNT_TOKEN_REQUEST: case TOKEN_REFRESH: case TOKEN_REVOKE: case TOKEN_INTROSPECT: diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/services/clientpolicy/executor/TestRaiseExeptionExecutor.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/services/clientpolicy/executor/TestRaiseExeptionExecutor.java index 35da3419ae..6178097111 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/services/clientpolicy/executor/TestRaiseExeptionExecutor.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/services/clientpolicy/executor/TestRaiseExeptionExecutor.java @@ -51,6 +51,7 @@ public class TestRaiseExeptionExecutor implements ClientPolicyExecutorProvider { + clientRep.setSecret(clientSecret); + clientRep.setStandardFlowEnabled(Boolean.FALSE); + clientRep.setImplicitFlowEnabled(Boolean.FALSE); + clientRep.setServiceAccountsEnabled(Boolean.TRUE); + clientRep.setPublicClient(Boolean.FALSE); + clientRep.setBearerOnly(Boolean.FALSE); + }); + + // register profiles + String json = (new ClientProfilesBuilder()).addProfile( + (new ClientProfileBuilder()).createProfile(PROFILE_NAME, "Den Forste Profilen") + .addExecutor(TestRaiseExeptionExecutorFactory.PROVIDER_ID, null) + .toRepresentation() + ).toString(); + updateProfiles(json); + + // register policies + json = (new ClientPoliciesBuilder()).addPolicy( + (new ClientPolicyBuilder()).createPolicy(POLICY_NAME, "La Premiere Politique", Boolean.TRUE) + .addCondition(AnyClientConditionFactory.PROVIDER_ID, createAnyClientConditionConfig()) + .addProfile(PROFILE_NAME) + .toRepresentation() + ).toString(); + updatePolicies(json); + + String origClientId = oauth.getClientId(); + oauth.clientId("service-account-app"); + try { + OAuthClient.AccessTokenResponse response = oauth.doClientCredentialsGrantAccessTokenRequest("app-secret"); + assertEquals(400, response.getStatusCode()); + assertEquals(ClientPolicyEvent.SERVICE_ACCOUNT_TOKEN_REQUEST.toString(), response.getError()); + assertEquals("Exception thrown intentionally", response.getErrorDescription()); + } finally { + oauth.clientId(origClientId); + } + } + private List getAttributeMultivalued(ClientRepresentation clientRep, String attrKey) { String attrValue = Optional.ofNullable(clientRep.getAttributes()).orElse(Collections.emptyMap()).get(attrKey); if (attrValue == null) return Collections.emptyList(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/hok/HoKTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/hok/HoKTest.java index 71d7e0034d..3ce8116dda 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/hok/HoKTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/hok/HoKTest.java @@ -18,6 +18,7 @@ import java.security.MessageDigest; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.function.Supplier; import javax.ws.rs.client.Client; import javax.ws.rs.client.ClientBuilder; @@ -65,6 +66,7 @@ import org.keycloak.testsuite.util.KeycloakModelUtils; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.UserInfoClientUtil; import org.keycloak.testsuite.util.OAuthClient.AccessTokenResponse; +import org.keycloak.testsuite.util.WaitUtils; import org.keycloak.util.JsonSerialization; import org.openqa.selenium.WebDriver; @@ -77,7 +79,7 @@ public class HoKTest extends AbstractTestRealmKeycloakTest { @Different protected WebDriver driver2; - private static final List CLIENT_LIST = Arrays.asList("test-app", "named-test-app"); + private static final List CLIENT_LIST = Arrays.asList("test-app", "named-test-app", "service-account-client"); public static class HoKAssertEvents extends AssertEvents { @@ -133,6 +135,10 @@ public class HoKTest extends AbstractTestRealmKeycloakTest { confApp.setSecret("secret1"); confApp.setServiceAccountsEnabled(Boolean.TRUE); + ClientRepresentation serviceAccountApp = KeycloakModelUtils.createClient(testRealm, "service-account-client"); + serviceAccountApp.setSecret("secret1"); + serviceAccountApp.setServiceAccountsEnabled(Boolean.TRUE); + ClientRepresentation pubApp = KeycloakModelUtils.createClient(testRealm, "public-cli"); pubApp.setPublicClient(Boolean.TRUE); @@ -634,16 +640,46 @@ public class HoKTest extends AbstractTestRealmKeycloakTest { } + @Test + public void serviceAccountWithClientCertificate() throws Exception { + oauth.clientId("service-account-client"); + + AccessTokenResponse response; + + Supplier previous = oauth.getHttpClient(); + + try { + // Request without HoK should fail + oauth.httpClient(MutualTLSUtils::newCloseableHttpClientWithoutKeyStoreAndTrustStore); + response = oauth.doClientCredentialsGrantAccessTokenRequest("secret1"); + assertEquals(400, response.getStatusCode()); + assertEquals(OAuthErrorException.INVALID_REQUEST, response.getError()); + assertEquals("Client Certification missing for MTLS HoK Token Binding", response.getErrorDescription()); + + // Request with HoK - success + oauth.httpClient(MutualTLSUtils::newCloseableHttpClientWithDefaultKeyStoreAndTrustStore); + response = oauth.doClientCredentialsGrantAccessTokenRequest("secret1"); + assertEquals(200, response.getStatusCode()); + + // Success Pattern + verifyHoKTokenCertThumbPrint(response, MutualTLSUtils.getThumbprintFromDefaultClientCert(), false); + } catch (IOException ioe) { + throw new RuntimeException(ioe); + } finally { + oauth.httpClient(previous); + } + } + private void verifyHoKTokenDefaultCertThumbPrint(AccessTokenResponse response) throws Exception { - verifyHoKTokenCertThumbPrint(response, MutualTLSUtils.getThumbprintFromDefaultClientCert()); + verifyHoKTokenCertThumbPrint(response, MutualTLSUtils.getThumbprintFromDefaultClientCert(), true); } private void verifyHoKTokenOtherCertThumbPrint(AccessTokenResponse response) throws Exception { - verifyHoKTokenCertThumbPrint(response, MutualTLSUtils.getThumbprintFromOtherClientCert()); + verifyHoKTokenCertThumbPrint(response, MutualTLSUtils.getThumbprintFromOtherClientCert(), true); } - private void verifyHoKTokenCertThumbPrint(AccessTokenResponse response, String certThumbPrint) { + private void verifyHoKTokenCertThumbPrint(AccessTokenResponse response, String certThumbPrint, boolean checkRefreshToken) { JWSInput jws = null; AccessToken at = null; try { @@ -654,13 +690,15 @@ public class HoKTest extends AbstractTestRealmKeycloakTest { } assertTrue(MessageDigest.isEqual(certThumbPrint.getBytes(), at.getCertConf().getCertThumbprint().getBytes())); - RefreshToken rt = null; - try { - jws = new JWSInput(response.getRefreshToken()); - rt = jws.readJsonContent(RefreshToken.class); - } catch (JWSInputException e) { - Assert.fail(e.toString()); + if (checkRefreshToken) { + RefreshToken rt = null; + try { + jws = new JWSInput(response.getRefreshToken()); + rt = jws.readJsonContent(RefreshToken.class); + } catch (JWSInputException e) { + Assert.fail(e.toString()); + } + assertTrue(MessageDigest.isEqual(certThumbPrint.getBytes(), rt.getCertConf().getCertThumbprint().getBytes())); } - assertTrue(MessageDigest.isEqual(certThumbPrint.getBytes(), rt.getCertConf().getCertThumbprint().getBytes())); } }