Update secret rotation when the policy is enabled using jwt (#10853)

Closes #10666
This commit is contained in:
Marcelo Daniel Silva Sales 2022-03-23 08:25:58 +01:00 committed by GitHub
parent e493b08fa7
commit 6efa45f93e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 282 additions and 30 deletions

View file

@ -29,6 +29,7 @@ import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
import org.apache.http.client.utils.CloneUtils;
import org.jboss.logging.Logger;
import org.keycloak.OAuth2Constants;
import org.keycloak.authentication.AuthenticationFlowError;
@ -37,7 +38,10 @@ import org.keycloak.common.util.Time;
import org.keycloak.jose.jws.JWSInput;
import org.keycloak.models.AuthenticationExecutionModel.Requirement;
import org.keycloak.models.SingleUseTokenStoreProvider;
import org.keycloak.models.delegate.ClientModelLazyDelegate;
import org.keycloak.models.utils.RepresentationToModel;
import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
import org.keycloak.protocol.oidc.OIDCClientSecretConfigWrapper;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.protocol.oidc.OIDCLoginProtocolService;
import org.keycloak.models.ClientModel;
@ -50,12 +54,11 @@ import org.keycloak.services.Urls;
/**
* Client authentication based on JWT signed by client secret instead of private key .
* See <a href="http://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication">specs</a> for more details.
*
* <p>
* This is server side, which verifies JWT from client_assertion parameter, where the assertion was created on adapter side by
* org.keycloak.adapters.authentication.JWTClientSecretCredentialsProvider
*
* <p>
* TODO: Try to create abstract superclass to be shared with {@link JWTClientAuthenticator}. Most of the code can be reused
*
*/
public class JWTClientSecretAuthenticator extends AbstractClientAuthenticator {
@ -67,7 +70,7 @@ public class JWTClientSecretAuthenticator extends AbstractClientAuthenticator {
public void authenticateClient(ClientAuthenticationFlowContext context) {
//KEYCLOAK-19461: Needed for quarkus resteasy implementation throws exception when called with mediaType authentication/json in OpenShiftTokenReviewEndpoint
if(!isFormDataRequest(context.getHttpRequest())) {
if (!isFormDataRequest(context.getHttpRequest())) {
Response challengeResponse = ClientAuthUtil.errorResponse(Response.Status.BAD_REQUEST.getStatusCode(), "invalid_client", "Parameter client_assertion_type is missing");
context.challenge(challengeResponse);
return;
@ -145,10 +148,23 @@ public class JWTClientSecretAuthenticator extends AbstractClientAuthenticator {
return;
}
//
OIDCClientSecretConfigWrapper wrapper = OIDCClientSecretConfigWrapper.fromClientModel(client);
if (wrapper.isClientSecretExpired()) {
context.failure(AuthenticationFlowError.INVALID_CLIENT_CREDENTIALS, null);
return;
}
//
boolean signatureValid;
try {
JsonWebToken jwt = context.getSession().tokens().decodeClientJWT(clientAssertion, client, JsonWebToken.class);
signatureValid = jwt != null;
//try authenticate with client rotated secret
if (!signatureValid && wrapper.hasRotatedSecret() && !wrapper.isClientRotatedSecretExpired()) {
jwt = context.getSession().tokens().decodeClientJWT(clientAssertion, wrapper.toRotatedClientModel(), JsonWebToken.class);
signatureValid = jwt != null;
}
} catch (RuntimeException e) {
Throwable cause = e.getCause() != null ? e.getCause() : e;
throw new RuntimeException("Signature on JWT token by client secret failed validation", cause);

View file

@ -35,6 +35,12 @@ public abstract class AbstractClientConfigWrapper {
return value;
}
protected Object getAttributes() {
if (clientModel != null) return clientModel.getAttributes();
else
return clientRep.getAttributes();
}
protected void setAttribute(String attrKey, String attrValue) {
if (clientModel != null) {
if (attrValue != null) {

View file

@ -1,15 +1,18 @@
package org.keycloak.protocol.oidc;
import java.io.InvalidObjectException;
import java.security.MessageDigest;
import java.text.SimpleDateFormat;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.keycloak.common.util.Time;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientSecretConstants;
import org.keycloak.models.delegate.ClientModelLazyDelegate;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.utils.StringUtil;
@ -33,8 +36,7 @@ public class OIDCClientSecretConfigWrapper extends AbstractClientConfigWrapper {
return new OIDCClientSecretConfigWrapper(client, null);
}
public static OIDCClientSecretConfigWrapper fromClientRepresentation(
ClientRepresentation clientRep) {
public static OIDCClientSecretConfigWrapper fromClientRepresentation(ClientRepresentation clientRep) {
return new OIDCClientSecretConfigWrapper(null, clientRep);
}
@ -86,8 +88,7 @@ public class OIDCClientSecretConfigWrapper extends AbstractClientConfigWrapper {
}
public boolean hasRotatedSecret() {
return StringUtil.isNotBlank(getAttribute(CLIENT_ROTATED_SECRET)) && StringUtil.isNotBlank(
getAttribute(CLIENT_ROTATED_SECRET_CREATION_TIME));
return StringUtil.isNotBlank(getAttribute(CLIENT_ROTATED_SECRET)) && StringUtil.isNotBlank(getAttribute(CLIENT_ROTATED_SECRET_CREATION_TIME));
}
public String getClientRotatedSecret() {
@ -121,13 +122,10 @@ public class OIDCClientSecretConfigWrapper extends AbstractClientConfigWrapper {
public void updateClientRepresentationAttributes(ClientRepresentation rep) {
rep.getAttributes().put(CLIENT_ROTATED_SECRET, getAttribute(CLIENT_ROTATED_SECRET));
rep.getAttributes()
.put(CLIENT_SECRET_CREATION_TIME, getAttribute(CLIENT_SECRET_CREATION_TIME));
rep.getAttributes().put(CLIENT_SECRET_CREATION_TIME, getAttribute(CLIENT_SECRET_CREATION_TIME));
rep.getAttributes().put(CLIENT_SECRET_EXPIRATION, getAttribute(CLIENT_SECRET_EXPIRATION));
rep.getAttributes().put(CLIENT_ROTATED_SECRET_CREATION_TIME,
getAttribute(CLIENT_ROTATED_SECRET_CREATION_TIME));
rep.getAttributes().put(CLIENT_ROTATED_SECRET_EXPIRATION_TIME,
getAttribute(CLIENT_ROTATED_SECRET_EXPIRATION_TIME));
rep.getAttributes().put(CLIENT_ROTATED_SECRET_CREATION_TIME, getAttribute(CLIENT_ROTATED_SECRET_CREATION_TIME));
rep.getAttributes().put(CLIENT_ROTATED_SECRET_EXPIRATION_TIME, getAttribute(CLIENT_ROTATED_SECRET_EXPIRATION_TIME));
}
public boolean hasClientSecretExpirationTime() {
@ -145,29 +143,24 @@ public class OIDCClientSecretConfigWrapper extends AbstractClientConfigWrapper {
public boolean isClientSecretExpired() {
if (hasClientSecretExpirationTime()) {
if (getClientSecretExpirationTime() < Time.currentTime()) {
return true;
}
return getClientSecretExpirationTime() < Time.currentTime();
}
return false;
}
public int getClientRotatedSecretExpirationTime() {
if (hasClientRotatedSecretExpirationTime()) {
return Integer.valueOf(
getAttribute(ClientSecretConstants.CLIENT_ROTATED_SECRET_EXPIRATION_TIME));
return Integer.valueOf(getAttribute(ClientSecretConstants.CLIENT_ROTATED_SECRET_EXPIRATION_TIME));
}
return 0;
}
public void setClientRotatedSecretExpirationTime(Integer expiration) {
setAttribute(ClientSecretConstants.CLIENT_ROTATED_SECRET_EXPIRATION_TIME,
expiration != null ? String.valueOf(expiration) : null);
setAttribute(ClientSecretConstants.CLIENT_ROTATED_SECRET_EXPIRATION_TIME, expiration != null ? String.valueOf(expiration) : null);
}
public boolean hasClientRotatedSecretExpirationTime() {
return StringUtil.isNotBlank(
getAttribute(ClientSecretConstants.CLIENT_ROTATED_SECRET_EXPIRATION_TIME));
return StringUtil.isNotBlank(getAttribute(ClientSecretConstants.CLIENT_ROTATED_SECRET_EXPIRATION_TIME));
}
public boolean isClientRotatedSecretExpired() {
@ -214,4 +207,26 @@ public class OIDCClientSecretConfigWrapper extends AbstractClientConfigWrapper {
return "";
}
}
public ReadOnlyRotatedSecretClientModel toRotatedClientModel() throws InvalidObjectException {
if (Objects.isNull(this.clientModel))
throw new InvalidObjectException(getClass().getCanonicalName() + " does not have an attribute of type " + ClientModel.class.getCanonicalName());
return new ReadOnlyRotatedSecretClientModel();
}
/**
* Representation of a client model that passes information from a rotated secret. The goal is to act as a decorator/DTO just providing information and not updating objects persistently.
*/
public class ReadOnlyRotatedSecretClientModel extends ClientModelLazyDelegate {
private ReadOnlyRotatedSecretClientModel() {
super(() -> OIDCClientSecretConfigWrapper.this.clientModel);
}
@Override
public String getSecret() {
return OIDCClientSecretConfigWrapper.this.getClientRotatedSecret();
}
}
}

View file

@ -16,11 +16,20 @@
*/
package org.keycloak.testsuite.oauth;
import static org.junit.Assert.assertEquals;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.core.Response;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.http.HttpStatus;
import org.apache.http.NameValuePair;
import org.apache.http.client.entity.UrlEncodedFormEntity;
import org.apache.http.client.methods.CloseableHttpResponse;
@ -29,35 +38,62 @@ import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.DefaultHttpClient;
import org.apache.http.message.BasicNameValuePair;
import org.jboss.logging.Logger;
import org.jetbrains.annotations.NotNull;
import org.junit.Rule;
import org.junit.Test;
import org.keycloak.OAuth2Constants;
import org.keycloak.adapters.authentication.JWTClientSecretCredentialsProvider;
import org.keycloak.admin.client.resource.ClientResource;
import org.keycloak.authentication.authenticators.client.JWTClientSecretAuthenticator;
import org.keycloak.common.Profile;
import org.keycloak.common.util.KeycloakUriBuilder;
import org.keycloak.common.util.Time;
import org.keycloak.common.util.UriUtils;
import org.keycloak.constants.ServiceUrlConstants;
import org.keycloak.crypto.Algorithm;
import org.keycloak.events.Details;
import org.keycloak.models.ClientSecretConstants;
import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
import org.keycloak.representations.JsonWebToken;
import org.keycloak.representations.idm.ClientPoliciesRepresentation;
import org.keycloak.representations.idm.ClientProfilesRepresentation;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.EventRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.services.clientpolicy.ClientPolicyException;
import org.keycloak.services.clientpolicy.condition.ClientAccessTypeCondition;
import org.keycloak.services.clientpolicy.condition.ClientAccessTypeConditionFactory;
import org.keycloak.services.clientpolicy.executor.ClientSecretRotationExecutor;
import org.keycloak.services.clientpolicy.executor.ClientSecretRotationExecutorFactory;
import org.keycloak.testsuite.AbstractKeycloakTest;
import org.keycloak.testsuite.Assert;
import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.admin.AbstractAdminTest;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer;
import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
import org.keycloak.testsuite.util.ClientPoliciesUtil;
import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.ServerURLs;
import org.keycloak.util.JsonSerialization;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
@AuthServerContainerExclude(AuthServer.REMOTE)
@EnableFeature(value = Profile.Feature.CLIENT_SECRET_ROTATION)
public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest {
private static final Logger logger = Logger.getLogger(ClientAuthSecretSignedJWTTest.class);
private static final String REALM_NAME = "test";
private static final String PROFILE_NAME = "ClientSecretRotationProfile";
private static final String POLICY_NAME = "ClientSecretRotationPolicy";
private static final String OIDC = "openid-connect";
private static final ObjectMapper objectMapper = new ObjectMapper();
@Rule
public AssertEvents events = new AssertEvents(this);
@ -130,7 +166,7 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest {
testCodeToTokenRequestSuccess(Algorithm.HS384);
} catch (Exception e) {
Assert.fail();
fail();
} finally {
clientResource = ApiUtil.findClientByClientId(adminClient.realm(realmName), clientId);
clientRep = clientResource.toRepresentation();
@ -161,7 +197,7 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest {
assertEquals(400, response.getStatusCode());
assertEquals("invalid_client", response.getError());
} catch (Exception e) {
Assert.fail();
fail();
} finally {
clientResource = ApiUtil.findClientByClientId(adminClient.realm(realmName), clientId);
clientRep = clientResource.toRepresentation();
@ -169,7 +205,7 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest {
clientResource.update(clientRep);
}
}
private void testCodeToTokenRequestSuccess(String algorithm) throws Exception {
oauth.clientId("test-app");
oauth.doLogin("test-user@localhost", "password");
@ -189,6 +225,32 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest {
.assertEvent();
}
/**
* When there is a client secret rotation, the client must be able to authenticate itself by the rotated secret and the new secret. (As long as both secrets remain valid)
*
* @throws Exception
*/
@Test
public void authenticateWithValidClientSecretWhenRotationPolicyIsEnable() throws Exception {
String cidConfidential= createClientByAdmin("jwt-client","jwt-client","password");
ClientResource clientResource = adminClient.realm(REALM_NAME).clients().get(cidConfidential);
configureDefaultProfileAndPolicy();
String firstSecret = clientResource.getSecret().getValue();
//generate new secret, rotate the secret
String newSecret = clientResource.generateNewSecret().getValue();
assertThat(firstSecret, not(equalTo(newSecret)));
oauth.clientId("jwt-client");
oauth.doLogin("test-user@localhost", "password");
EventRepresentation loginEvent = events.expectLogin().client("jwt-client").assertEvent();
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, getClientSignedJWT(firstSecret, 20, Algorithm.HS256));
assertThat(response.getStatusCode(), is(HttpStatus.SC_OK));
}
// TEST ERRORS
@Test
@ -201,7 +263,7 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest {
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, getClientSignedJWT("ppassswordd", 20));
// https://tools.ietf.org/html/rfc6749#section-5.2
assertEquals(400, response.getStatusCode());
assertEquals("unauthorized_client", response.getError());
@ -246,6 +308,34 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest {
assertEquals("unauthorized_client", response.getError());
}
/**
* After a secret rotation the client should not be able to authenticate after the rotated secret expires
*
* @throws Exception
*/
@Test
public void authenticateWithInvalidRotatedClientSecretPolicyIsEnable() throws Exception {
String cidConfidential= createClientByAdmin("jwt-client","jwt-client","password");
ClientResource clientResource = adminClient.realm(REALM_NAME).clients().get(cidConfidential);
configureDefaultProfileAndPolicy();
String firstSecret = clientResource.getSecret().getValue();
//generate new secret, rotate the secret
String newSecret = clientResource.generateNewSecret().getValue();
assertThat(firstSecret, not(equalTo(newSecret)));
//force rotated secret expiration
setTimeOffset(31);
oauth.clientId("jwt-client");
oauth.doLogin("test-user@localhost", "password");
EventRepresentation loginEvent = events.expectLogin().client("jwt-client").assertEvent();
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, getClientSignedJWT(firstSecret, 20, Algorithm.HS256));
assertThat(response.getStatusCode(), is(HttpStatus.SC_BAD_REQUEST));
}
private String getClientSignedJWT(String secret, int timeout) {
return getClientSignedJWT(secret, timeout, Algorithm.HS256);
}
@ -285,4 +375,129 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest {
}
}
//support methods
private void configureDefaultProfileAndPolicy() throws Exception {
// register profiles
ClientPoliciesUtil.ClientProfileBuilder profileBuilder = new ClientPoliciesUtil.ClientProfileBuilder();
ClientSecretRotationExecutor.Configuration profileConfig = getClientProfileConfiguration(60, 30, 20);
doConfigProfileAndPolicy(profileBuilder, profileConfig);
}
private void doConfigProfileAndPolicy(ClientPoliciesUtil.ClientProfileBuilder profileBuilder,
ClientSecretRotationExecutor.Configuration profileConfig) throws Exception {
String json = (new ClientPoliciesUtil.ClientProfilesBuilder()).addProfile(
profileBuilder.createProfile(PROFILE_NAME, "Enable Client Secret Rotation")
.addExecutor(ClientSecretRotationExecutorFactory.PROVIDER_ID, profileConfig)
.toRepresentation()).toString();
updateProfiles(json);
// register policies
ClientAccessTypeCondition.Configuration config = new ClientAccessTypeCondition.Configuration();
config.setType(Arrays.asList(ClientAccessTypeConditionFactory.TYPE_CONFIDENTIAL));
json = (new ClientPoliciesUtil.ClientPoliciesBuilder()).addPolicy(
(new ClientPoliciesUtil.ClientPolicyBuilder()).createPolicy(POLICY_NAME,
"Policy for Client Secret Rotation",
Boolean.TRUE).addCondition(ClientAccessTypeConditionFactory.PROVIDER_ID, config)
.addProfile(PROFILE_NAME).toRepresentation()).toString();
updatePolicies(json);
}
protected void updateProfiles(String json) throws ClientPolicyException {
try {
ClientProfilesRepresentation clientProfiles = JsonSerialization.readValue(json,
ClientProfilesRepresentation.class);
adminClient.realm(REALM_NAME).clientPoliciesProfilesResource()
.updateProfiles(clientProfiles);
} catch (BadRequestException e) {
throw new ClientPolicyException("update profiles failed",
e.getResponse().getStatusInfo().toString());
} catch (Exception e) {
throw new ClientPolicyException("update profiles failed", e.getMessage());
}
}
protected void updateProfiles(ClientProfilesRepresentation reps) throws ClientPolicyException {
updateProfiles(convertToProfilesJson(reps));
}
protected void updatePolicies(String json) throws ClientPolicyException {
try {
ClientPoliciesRepresentation clientPolicies = json == null ? null
: JsonSerialization.readValue(json, ClientPoliciesRepresentation.class);
adminClient.realm(REALM_NAME).clientPoliciesPoliciesResource()
.updatePolicies(clientPolicies);
} catch (BadRequestException e) {
throw new ClientPolicyException("update policies failed",
e.getResponse().getStatusInfo().toString());
} catch (IOException e) {
throw new ClientPolicyException("update policies failed", e.getMessage());
}
}
protected String convertToProfilesJson(ClientProfilesRepresentation reps) {
String json = null;
try {
json = objectMapper.writeValueAsString(reps);
} catch (JsonProcessingException e) {
fail();
}
return json;
}
@NotNull
private ClientSecretRotationExecutor.Configuration getClientProfileConfiguration(
int expirationPeriod, int rotatedExpirationPeriod, int remainExpirationPeriod) {
ClientSecretRotationExecutor.Configuration profileConfig = new ClientSecretRotationExecutor.Configuration();
profileConfig.setExpirationPeriod(expirationPeriod);
profileConfig.setRotatedExpirationPeriod(rotatedExpirationPeriod);
profileConfig.setRemainExpirationPeriod(remainExpirationPeriod);
return profileConfig;
}
protected String createClientByAdmin(String clientId, String clientName, String clientSecret) throws ClientPolicyException {
ClientRepresentation clientRep = getClientRepresentation(clientId, clientName, clientSecret);
Response resp = adminClient.realm(REALM_NAME).clients().create(clientRep);
if (resp.getStatus() == Response.Status.BAD_REQUEST.getStatusCode()) {
String respBody = resp.readEntity(String.class);
Map<String, String> responseJson = null;
try {
responseJson = JsonSerialization.readValue(respBody, Map.class);
} catch (IOException e) {
fail();
}
throw new ClientPolicyException(responseJson.get(OAuth2Constants.ERROR),
responseJson.get(OAuth2Constants.ERROR_DESCRIPTION));
}
resp.close();
assertEquals(Response.Status.CREATED.getStatusCode(), resp.getStatus());
// registered components will be removed automatically when a test method finishes regardless of its success or failure.
String cId = ApiUtil.getCreatedId(resp);
testContext.getOrCreateCleanup(REALM_NAME).addClientUuid(cId);
return cId;
}
@NotNull
private ClientRepresentation getClientRepresentation(String clientId, String clientName, String clientSecret) {
ClientRepresentation clientRep = new ClientRepresentation();
clientRep.setClientId(clientId);
clientRep.setName(clientName);
clientRep.setSecret(clientSecret);
clientRep.setAttributes(new HashMap<>());
clientRep.getAttributes()
.put(ClientSecretConstants.CLIENT_SECRET_CREATION_TIME,
String.valueOf(Time.currentTime()));
clientRep.setProtocol(OIDC);
clientRep.setBearerOnly(Boolean.FALSE);
clientRep.setPublicClient(Boolean.FALSE);
clientRep.setServiceAccountsEnabled(Boolean.TRUE);
clientRep.setStandardFlowEnabled(Boolean.TRUE);
clientRep.setImplicitFlowEnabled(Boolean.TRUE);
clientRep.setClientAuthenticatorType(JWTClientSecretAuthenticator.PROVIDER_ID);
clientRep.setRedirectUris(Collections.singletonList(
ServerURLs.getAuthServerContextRoot() + "/auth/realms/master/app/auth"));
return clientRep;
}
}