libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

Client Policies: pkce-enforcer executor with client-access-type condition is not applied on client change via Admin API

Browse source 
Closes #12295
This commit is contained in:
Takashi Norimatsu 2022-06-05 08:41:28 +09:00 committed by Marek Posolda
parent 91e88f554e
commit 3889eeda30
3 changed files with 106 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientAccessTypeCondition.java
View file

@ -25,9 +25,11 @@ import org.jboss.logging.Logger;
import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.representations.idm.ClientPolicyConditionConfigurationRepresentation;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.services.clientpolicy.ClientPolicyContext;
import org.keycloak.services.clientpolicy.ClientPolicyException;
import org.keycloak.services.clientpolicy.ClientPolicyVote;
import org.keycloak.services.clientpolicy.context.ClientCRUDContext;
/**
* @author <a href="mailto:takashi.norimatsu.ws@hitachi.com">Takashi Norimatsu</a>
@ -74,10 +76,14 @@ public class ClientAccessTypeCondition extends AbstractClientPolicyConditionProv
case TOKEN_INTROSPECT:
case USERINFO_REQUEST:
case LOGOUT_REQUEST:
case UPDATE:
case UPDATED:
case REGISTERED:
if (isClientAccessTypeMatched()) return ClientPolicyVote.YES;
return ClientPolicyVote.NO;
case REGISTER:
if (isProposedClientAccessTypeMatched((ClientCRUDContext)context)) return ClientPolicyVote.YES;
return ClientPolicyVote.NO;
default:
return ClientPolicyVote.ABSTAIN;
}
@ -86,14 +92,31 @@ public class ClientAccessTypeCondition extends AbstractClientPolicyConditionProv
private String getClientAccessType() {
ClientModel client = session.getContext().getClient();
if (client == null) return null;
return getClientAccessType(client.isPublicClient(), client.isBearerOnly());
}
if (client.isPublicClient()) return ClientAccessTypeConditionFactory.TYPE_PUBLIC;
if (client.isBearerOnly()) return ClientAccessTypeConditionFactory.TYPE_BEARERONLY;
private String getProposedClientAccessType(ClientCRUDContext context) {
ClientRepresentation clientRep = context.getProposedClientRepresentation();
if (clientRep == null) return null;
return getClientAccessType(Optional.ofNullable(clientRep.isPublicClient()).orElse(Boolean.FALSE).booleanValue(),
Optional.ofNullable(clientRep.isBearerOnly()).orElse(Boolean.FALSE).booleanValue());
}
private String getClientAccessType(boolean isPublicClient, boolean isBearerOnly) {
if (isPublicClient) return ClientAccessTypeConditionFactory.TYPE_PUBLIC;
if (isBearerOnly) return ClientAccessTypeConditionFactory.TYPE_BEARERONLY;
else return ClientAccessTypeConditionFactory.TYPE_CONFIDENTIAL;
}
private boolean isClientAccessTypeMatched() {
final String accessType = getClientAccessType();
return isClientAccessTypeMatched(getClientAccessType());
}
private boolean isProposedClientAccessTypeMatched(ClientCRUDContext context) {
return isClientAccessTypeMatched(getProposedClientAccessType(context));
}
private boolean isClientAccessTypeMatched(String accessType) {
if (accessType == null) return false;
List<String> expectedAccessTypes = Optional.ofNullable(configuration.getType()).orElse(Collections.emptyList());

35
services/src/main/java/org/keycloak/services/clientpolicy/executor/ClientSecretRotationExecutor.java
View file

@ -49,23 +49,23 @@ public class ClientSecretRotationExecutor implements
@Override
public void executeOnEvent(ClientPolicyContext context) throws ClientPolicyException {
if (!session.getContext().getClient().isPublicClient() && !session.getContext().getClient()
.isBearerOnly()) {
session.setAttribute(ClientSecretConstants.CLIENT_SECRET_ROTATION_ENABLED, Boolean.TRUE);
switch (context.getEvent()) {
case REGISTERED:
case UPDATED:
switch (context.getEvent()) {
case REGISTERED:
case UPDATED:
if(isClientWithSecret(session.getContext().getClient())) {
session.setAttribute(ClientSecretConstants.CLIENT_SECRET_ROTATION_ENABLED, Boolean.TRUE);
executeOnClientCreateOrUpdate((ClientCRUDContext) context);
break;
case AUTHORIZATION_REQUEST:
case TOKEN_REQUEST:
}
break;
case AUTHORIZATION_REQUEST:
case TOKEN_REQUEST:
if(isClientWithSecret(session.getContext().getClient())) {
session.setAttribute(ClientSecretConstants.CLIENT_SECRET_ROTATION_ENABLED, Boolean.TRUE);
executeOnAuthRequest();
return;
default:
return;
}
}
break;
default:
return;
}
}
@ -80,6 +80,11 @@ public class ClientSecretRotationExecutor implements
}
private boolean isClientWithSecret(ClientModel client) {
if (client == null) return false;
return (!client.isPublicClient() && !client.isBearerOnly());
}
private void executeOnAuthRequest() {
ClientModel client = session.getContext().getClient();
OIDCClientSecretConfigWrapper wrapper = OIDCClientSecretConfigWrapper.fromClientModel(

60
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientPoliciesTest.java
View file

@ -1053,6 +1053,66 @@ public class ClientPoliciesTest extends AbstractClientPoliciesTest {
successfulLoginAndLogout(clientBetaId, null);
failLoginWithoutNonce(clientAlphaId);
// update profiles
json = (new ClientProfilesBuilder()).addProfile(
(new ClientProfileBuilder()).createProfile(PROFILE_NAME, "El Primer Perfil")
.addExecutor(PKCEEnforcerExecutorFactory.PROVIDER_ID,
createPKCEEnforceExecutorConfig(Boolean.FALSE)) // check only
.toRepresentation()
).toString();
updateProfiles(json);
// Attempt to create a confidential client without PKCE setting. Should fail
try {
createClientByAdmin(generateSuffixedName("Gamma-App"), (ClientRepresentation clientRep) -> {
clientRep.setSecret("secretGamma");
clientRep.setBearerOnly(Boolean.FALSE);
clientRep.setPublicClient(Boolean.FALSE);
});
fail();
} catch (ClientPolicyException e) {
assertEquals(OAuthErrorException.INVALID_CLIENT_METADATA, e.getMessage());
assertEquals("Invalid client metadata: code_challenge_method", e.getErrorDetail());
}
json = (new ClientProfilesBuilder()).addProfile(
(new ClientProfileBuilder()).createProfile(PROFILE_NAME, "El Primer Perfil")
.addExecutor(PKCEEnforcerExecutorFactory.PROVIDER_ID,
createPKCEEnforceExecutorConfig(Boolean.TRUE)) // enforce
.toRepresentation()
).toString();
updateProfiles(json);
authCreateClients();
String clientGammaId = createClientDynamically(generateSuffixedName("Gamma-App"), (OIDCClientRepresentation clientRep) -> {
clientRep.setClientSecret("secretGamma");
});
ClientRepresentation clientRep = getClientByAdmin(clientGammaId);
assertEquals(OAuth2Constants.PKCE_METHOD_S256, OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep).getPkceCodeChallengeMethod());
json = (new ClientProfilesBuilder()).addProfile(
(new ClientProfileBuilder()).createProfile(PROFILE_NAME, "El Primer Perfil")
.addExecutor(PKCEEnforcerExecutorFactory.PROVIDER_ID,
createPKCEEnforceExecutorConfig(Boolean.FALSE)) // check only
.toRepresentation()
).toString();
updateProfiles(json);
// Attempt to update the confidential client with not allowed PKCE setting. Should fail
try {
updateClientByAdmin(clientGammaId, (ClientRepresentation updatingClientRep) -> {
updatingClientRep.setAttributes(new HashMap<>());
updatingClientRep.getAttributes().put(OIDCConfigAttributes.PKCE_CODE_CHALLENGE_METHOD, OAuth2Constants.PKCE_METHOD_PLAIN);
});
} catch (ClientPolicyException e) {
assertEquals(OAuthErrorException.INVALID_CLIENT_METADATA, e.getMessage());
assertEquals("Invalid client metadata: code_challenge_method", e.getErrorDetail());
}
ClientRepresentation cRep = getClientByAdmin(clientGammaId);
assertEquals(OAuth2Constants.PKCE_METHOD_S256, cRep.getAttributes().get(OIDCConfigAttributes.PKCE_CODE_CHALLENGE_METHOD));
}
@Test