diff --git a/docs/documentation/server_admin/topics/clients/client-policies.adoc b/docs/documentation/server_admin/topics/clients/client-policies.adoc index 4c4837130fc..09992d352c1 100644 --- a/docs/documentation/server_admin/topics/clients/client-policies.adoc +++ b/docs/documentation/server_admin/topics/clients/client-policies.adoc @@ -126,6 +126,7 @@ One of several purposes for this executor is to realize the security requirement * Enforce Client Registration Access Token * Enforce checking if a client is the one to which an intent was issued in a use case where an intent is issued before starting an authorization code flow to get an access token like UK OpenBanking * Enforce prohibiting implicit and hybrid flow +* Enforce checking if a PAR request includes necessary parameters included by an authorization request [[_client_policy_profile]] === Profile diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutor.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutor.java new file mode 100644 index 00000000000..4abed9bfe0f --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutor.java @@ -0,0 +1,92 @@ +/* + * Copyright 2023 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.executor; + +import java.util.Map; +import java.util.Set; + +import org.jboss.logging.Logger; +import org.keycloak.OAuthErrorException; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.SingleUseObjectProvider; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; +import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequestParserProcessor; +import org.keycloak.protocol.oidc.endpoints.request.RequestUriType; +import org.keycloak.protocol.oidc.par.endpoints.ParEndpoint; +import org.keycloak.representations.idm.ClientPolicyExecutorConfigurationRepresentation; +import org.keycloak.services.clientpolicy.ClientPolicyContext; +import org.keycloak.services.clientpolicy.ClientPolicyException; +import org.keycloak.services.clientpolicy.context.PreAuthorizationRequestContext; + +import jakarta.ws.rs.core.MultivaluedMap; + +/** + * @author Takashi Norimatsu + */ +public class SecureParContentsExecutor implements ClientPolicyExecutorProvider { + + protected final KeycloakSession session; + private static final Logger logger = Logger.getLogger(SecureParContentsExecutor.class); + + public SecureParContentsExecutor(KeycloakSession session) { + this.session = session; + } + + @Override + public String getProviderId() { + return SecureParContentsExecutorFactory.PROVIDER_ID; + } + + @Override + public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyException { + switch (context.getEvent()) { + case PRE_AUTHORIZATION_REQUEST: + PreAuthorizationRequestContext preAuthorizationRequestContext = (PreAuthorizationRequestContext)context; + checkValidParContents(preAuthorizationRequestContext); + break; + default: + return; + } + } + + private void checkValidParContents(PreAuthorizationRequestContext preAuthorizationRequestContext) throws ClientPolicyException { + MultivaluedMap requestParameters = preAuthorizationRequestContext.getRequestParameters(); + String requestUri = requestParameters.getFirst(OIDCLoginProtocol.REQUEST_URI_PARAM); + if (requestUri == null) { + throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "request_uri not included."); + } + if (requestUri != null && AuthorizationEndpointRequestParserProcessor.getRequestUriType(requestUri) != RequestUriType.PAR) { + throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "PAR request_uri not included."); + } + + String key = requestUri.substring(ParEndpoint.REQUEST_URI_PREFIX_LENGTH); + SingleUseObjectProvider singleUseStore = session.singleUseObjects(); + Map retrievedRequest = singleUseStore.get(key); + if (retrievedRequest == null) { + throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "PAR not found. not issued or used multiple times."); + } + + Set queryParameterNames = requestParameters.keySet(); + for (String queryParamName : queryParameterNames) { + if (!retrievedRequest.keySet().contains(queryParamName) && !OIDCLoginProtocol.REQUEST_URI_PARAM.equals(queryParamName)) { + singleUseStore.remove(key); + throw new ClientPolicyException(OAuthErrorException.INVALID_REQUEST, "PAR request did not include necessary parameters"); + } + } + } +} diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutorFactory.java b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutorFactory.java new file mode 100644 index 00000000000..b11efa7fcfa --- /dev/null +++ b/services/src/main/java/org/keycloak/services/clientpolicy/executor/SecureParContentsExecutorFactory.java @@ -0,0 +1,67 @@ +/* + * Copyright 2023 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.executor; + +import java.util.Collections; +import java.util.List; + +import org.keycloak.Config.Scope; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.KeycloakSessionFactory; +import org.keycloak.provider.ProviderConfigProperty; + +/** + * @author Takashi Norimatsu + */ +public class SecureParContentsExecutorFactory implements ClientPolicyExecutorProviderFactory { + + public static final String PROVIDER_ID = "secure-par-content"; + + @Override + public ClientPolicyExecutorProvider create(KeycloakSession session) { + return new SecureParContentsExecutor(session); + } + + @Override + public void init(Scope config) { + } + + @Override + public void postInit(KeycloakSessionFactory factory) { + } + + @Override + public void close() { + } + + @Override + public String getId() { + return PROVIDER_ID; + } + + @Override + public String getHelpText() { + return "It checks if a PAR request includes necessary parameters included by an authorization request."; + } + + @Override + public List getConfigProperties() { + return Collections.emptyList(); + } + +} diff --git a/services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProviderFactory b/services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProviderFactory index 531f8ee6cf3..280afeee582 100644 --- a/services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProviderFactory +++ b/services/src/main/resources/META-INF/services/org.keycloak.services.clientpolicy.executor.ClientPolicyExecutorProviderFactory @@ -20,4 +20,5 @@ org.keycloak.services.clientpolicy.executor.RejectRequestExecutorFactory org.keycloak.services.clientpolicy.executor.IntentClientBindCheckExecutorFactory org.keycloak.services.clientpolicy.executor.SuppressRefreshTokenRotationExecutorFactory org.keycloak.services.clientpolicy.executor.RegistrationAccessTokenRotationDisabledExecutorFactory -org.keycloak.services.clientpolicy.executor.RejectImplicitGrantExecutorFactory \ No newline at end of file +org.keycloak.services.clientpolicy.executor.RejectImplicitGrantExecutorFactory +org.keycloak.services.clientpolicy.executor.SecureParContentsExecutorFactory \ No newline at end of file diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesTest.java index eb5ac6d388c..b2c2ae3151f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/policies/ClientPoliciesTest.java @@ -108,6 +108,7 @@ import org.keycloak.services.clientpolicy.executor.RejectImplicitGrantExecutorFa import org.keycloak.services.clientpolicy.executor.RejectRequestExecutorFactory; import org.keycloak.services.clientpolicy.executor.RejectResourceOwnerPasswordCredentialsGrantExecutorFactory; import org.keycloak.services.clientpolicy.executor.SecureClientAuthenticatorExecutorFactory; +import org.keycloak.services.clientpolicy.executor.SecureParContentsExecutorFactory; import org.keycloak.services.clientpolicy.executor.SecureSessionEnforceExecutorFactory; import org.keycloak.services.clientpolicy.executor.SecureSigningAlgorithmForSignedJwtExecutorFactory; import org.keycloak.testsuite.arquillian.annotation.EnableFeature; @@ -119,6 +120,7 @@ import org.keycloak.testsuite.util.ClientPoliciesUtil.ClientPoliciesBuilder; import org.keycloak.testsuite.util.ClientPoliciesUtil.ClientPolicyBuilder; import org.keycloak.testsuite.util.ClientPoliciesUtil.ClientProfileBuilder; import org.keycloak.testsuite.util.ClientPoliciesUtil.ClientProfilesBuilder; +import org.keycloak.testsuite.util.OAuthClient.ParResponse; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.RoleBuilder; import org.keycloak.testsuite.util.ServerURLs; @@ -1223,4 +1225,50 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest { assertEquals(expectedError, oauth.getCurrentFragment().get(OAuth2Constants.ERROR)); assertEquals(expectedErrorDescription, oauth.getCurrentFragment().get(OAuth2Constants.ERROR_DESCRIPTION)); } + + @Test + public void testSecureParContentsExecutor() throws Exception { + // register profiles + String json = (new ClientProfilesBuilder()).addProfile( + (new ClientProfileBuilder()).createProfile(PROFILE_NAME, "Le Premier Profil") + .addExecutor(SecureParContentsExecutorFactory.PROVIDER_ID, null) + .toRepresentation() + ).toString(); + updateProfiles(json); + + String clientBetaId = generateSuffixedName("Beta-App"); + createClientByAdmin(clientBetaId, (ClientRepresentation clientRep) -> { + clientRep.setSecret("secretBeta"); + }); + + // 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); + + // Pushed Authorization Request + ParResponse pResp = oauth.doPushedAuthorizationRequest(clientBetaId, "secretBeta"); + assertEquals(201, pResp.getStatusCode()); + String requestUri = pResp.getRequestUri(); + + oauth.requestUri(requestUri); + oauth.clientId(clientBetaId); + oauth.openLoginForm(); + assertTrue(errorPage.isCurrent()); + assertEquals("PAR request did not include necessary parameters", errorPage.getError()); + + oauth.requestUri(null); + pResp = oauth.doPushedAuthorizationRequest(clientBetaId, "secretBeta"); + assertEquals(201, pResp.getStatusCode()); + requestUri = pResp.getRequestUri(); + oauth.requestUri(requestUri); + + oauth.stateParamHardcoded(null); + successfulLoginAndLogout(clientBetaId, "secretBeta"); + } }