[KEYCLOAK-16676] - Client attributes should not be stored if null or empty

This commit is contained in:
Pedro Igor 2021-02-02 16:00:48 -03:00 committed by Hynek Mlnařík
parent 7780badb2a
commit 0f30b3118a
7 changed files with 132 additions and 4 deletions

View file

@ -293,13 +293,26 @@ public class ClientAdapter implements ClientModel, JpaModel<ClientEntity> {
@Override
public void setAttribute(String name, String value) {
boolean valueUndefined = value == null || "".equals(value.trim());
for (ClientAttributeEntity attr : entity.getAttributes()) {
if (attr.getName().equals(name)) {
attr.setValue(value);
// clean up, so that attributes previously set with either a empty or null value are removed
// we should remove this in future versions so that new clients never store empty/null attributes
if (valueUndefined) {
removeAttribute(name);
} else {
attr.setValue(value);
}
return;
}
}
// do not create attributes if empty or null
if (valueUndefined) {
return;
}
ClientAttributeEntity attr = new ClientAttributeEntity();
attr.setName(name);
attr.setValue(value);

View file

@ -245,6 +245,13 @@ public abstract class MapClientAdapter extends AbstractClientModel<MapClientEnti
@Override
public void setAttribute(String name, String value) {
boolean valueUndefined = value == null || "".equals(value.trim());
if (valueUndefined) {
removeAttribute(name);
return;
}
entity.setAttribute(name, value);
}

View file

@ -19,11 +19,12 @@ package org.keycloak.testsuite.rest;
import org.jboss.resteasy.annotations.cache.NoCache;
import org.jboss.resteasy.spi.HttpRequest;
import org.jboss.resteasy.spi.ResteasyProviderFactory;
import org.keycloak.OAuth2Constants;
import org.keycloak.common.util.HtmlUtils;
import org.keycloak.jose.jws.JWSInput;
import org.keycloak.jose.jws.JWSInputException;
import org.keycloak.models.KeycloakSession;
import org.keycloak.representations.LogoutToken;
import org.keycloak.representations.adapters.action.LogoutAction;
import org.keycloak.representations.adapters.action.PushNotBeforeAction;
import org.keycloak.representations.adapters.action.TestAvailabilityAction;
@ -55,6 +56,7 @@ public class TestApplicationResourceProvider implements RealmResourceProvider {
private KeycloakSession session;
private final BlockingQueue<LogoutAction> adminLogoutActions;
private final BlockingQueue<LogoutToken> backChannelLogoutTokens;
private final BlockingQueue<PushNotBeforeAction> adminPushNotBeforeActions;
private final BlockingQueue<TestAvailabilityAction> adminTestAvailabilityAction;
private final TestApplicationResourceProviderFactory.OIDCClientData oidcClientData;
@ -63,10 +65,13 @@ public class TestApplicationResourceProvider implements RealmResourceProvider {
HttpRequest request;
public TestApplicationResourceProvider(KeycloakSession session, BlockingQueue<LogoutAction> adminLogoutActions,
BlockingQueue<LogoutToken> backChannelLogoutTokens,
BlockingQueue<PushNotBeforeAction> adminPushNotBeforeActions,
BlockingQueue<TestAvailabilityAction> adminTestAvailabilityAction, TestApplicationResourceProviderFactory.OIDCClientData oidcClientData) {
BlockingQueue<TestAvailabilityAction> adminTestAvailabilityAction,
TestApplicationResourceProviderFactory.OIDCClientData oidcClientData) {
this.session = session;
this.adminLogoutActions = adminLogoutActions;
this.backChannelLogoutTokens = backChannelLogoutTokens;
this.adminPushNotBeforeActions = adminPushNotBeforeActions;
this.adminTestAvailabilityAction = adminTestAvailabilityAction;
this.oidcClientData = oidcClientData;
@ -79,6 +84,13 @@ public class TestApplicationResourceProvider implements RealmResourceProvider {
adminLogoutActions.add(new JWSInput(data).readJsonContent(LogoutAction.class));
}
@POST
@Consumes(MediaType.APPLICATION_FORM_URLENCODED)
@Path("/admin/backchannelLogout")
public void backchannelLogout() throws JWSInputException {
backChannelLogoutTokens.add(new JWSInput(request.getDecodedFormParameters().getFirst(OAuth2Constants.LOGOUT_TOKEN)).readJsonContent(LogoutToken.class));
}
@POST
@Consumes(MediaType.TEXT_PLAIN_UTF_8)
@Path("/admin/k_push_not_before")
@ -100,6 +112,13 @@ public class TestApplicationResourceProvider implements RealmResourceProvider {
return adminLogoutActions.poll(10, TimeUnit.SECONDS);
}
@GET
@Produces(MediaType.APPLICATION_JSON)
@Path("/poll-backchannel-logout")
public LogoutToken getBackChannelLogoutAction() throws InterruptedException {
return backChannelLogoutTokens.poll(20, TimeUnit.SECONDS);
}
@GET
@Produces(MediaType.APPLICATION_JSON)
@Path("/poll-admin-not-before")

View file

@ -24,6 +24,7 @@ import org.keycloak.crypto.KeyType;
import org.keycloak.crypto.KeyUse;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.representations.LogoutToken;
import org.keycloak.representations.adapters.action.LogoutAction;
import org.keycloak.representations.adapters.action.PushNotBeforeAction;
import org.keycloak.representations.adapters.action.TestAvailabilityAction;
@ -41,6 +42,7 @@ import java.util.concurrent.LinkedBlockingDeque;
public class TestApplicationResourceProviderFactory implements RealmResourceProviderFactory {
private BlockingQueue<LogoutAction> adminLogoutActions = new LinkedBlockingDeque<>();
private BlockingQueue<LogoutToken> backChannelLogoutTokens = new LinkedBlockingDeque<>();
private BlockingQueue<PushNotBeforeAction> pushNotBeforeActions = new LinkedBlockingDeque<>();
private BlockingQueue<TestAvailabilityAction> testAvailabilityActions = new LinkedBlockingDeque<>();
@ -48,7 +50,8 @@ public class TestApplicationResourceProviderFactory implements RealmResourceProv
@Override
public RealmResourceProvider create(KeycloakSession session) {
TestApplicationResourceProvider provider = new TestApplicationResourceProvider(session, adminLogoutActions, pushNotBeforeActions, testAvailabilityActions, oidcClientData);
TestApplicationResourceProvider provider = new TestApplicationResourceProvider(session, adminLogoutActions,
backChannelLogoutTokens, pushNotBeforeActions, testAvailabilityActions, oidcClientData);
ResteasyProviderFactory.getInstance().injectProperties(provider);

View file

@ -17,6 +17,7 @@
package org.keycloak.testsuite.client.resources;
import org.keycloak.representations.LogoutToken;
import org.keycloak.representations.adapters.action.LogoutAction;
import org.keycloak.representations.adapters.action.PushNotBeforeAction;
import org.keycloak.representations.adapters.action.TestAvailabilityAction;
@ -39,6 +40,11 @@ public interface TestApplicationResource {
@Path("/poll-admin-logout")
LogoutAction getAdminLogoutAction();
@GET
@Produces(MediaType.APPLICATION_JSON)
@Path("/poll-backchannel-logout")
LogoutToken getBackChannelLogoutToken();
@GET
@Produces(MediaType.APPLICATION_JSON)
@Path("/poll-admin-not-before")

View file

@ -28,6 +28,7 @@ import org.keycloak.events.admin.ResourceType;
import org.keycloak.models.AccountRoles;
import org.keycloak.models.Constants;
import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
import org.keycloak.protocol.oidc.OIDCConfigAttributes;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.protocol.oidc.OIDCLoginProtocolFactory;
import org.keycloak.representations.adapters.action.GlobalRequestResult;
@ -385,6 +386,13 @@ public class ClientTest extends AbstractAdminTest {
storedClient = realm.clients().get(client.getId()).toRepresentation();
assertClient(client, storedClient);
storedClient.getAttributes().put(OIDCConfigAttributes.BACKCHANNEL_LOGOUT_URL, "");
realm.clients().get(storedClient.getId()).update(storedClient);
storedClient = realm.clients().get(client.getId()).toRepresentation();
assertFalse(storedClient.getAttributes().containsKey(OIDCConfigAttributes.BACKCHANNEL_LOGOUT_URL));
}
@Test

View file

@ -23,11 +23,15 @@ import org.junit.Test;
import org.keycloak.OAuth2Constants;
import org.keycloak.OAuthErrorException;
import org.keycloak.admin.client.resource.ClientResource;
import org.keycloak.admin.client.resource.ClientsResource;
import org.keycloak.common.util.Time;
import org.keycloak.events.Details;
import org.keycloak.jose.jws.JWSHeader;
import org.keycloak.jose.jws.JWSInput;
import org.keycloak.protocol.oidc.OIDCConfigAttributes;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.testsuite.AbstractKeycloakTest;
import org.keycloak.testsuite.Assert;
@ -328,6 +332,74 @@ public class LogoutTest extends AbstractKeycloakTest {
}
}
@Test
public void successfulKLogoutAfterEmptyBackChannelUrl() throws Exception {
ClientsResource clients = adminClient.realm(oauth.getRealm()).clients();
ClientRepresentation rep = clients.findByClientId(oauth.getClientId()).get(0);
rep.getAttributes().put(OIDCConfigAttributes.BACKCHANNEL_LOGOUT_URL, "");
clients.get(rep.getId()).update(rep);
oauth.doLogin("test-user@localhost", "password");
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
oauth.clientSessionState("client-session");
OAuthClient.AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(code, "password");
String idTokenString = tokenResponse.getIdToken();
String logoutUrl = oauth.getLogoutUrl()
.idTokenHint(idTokenString)
.postLogoutRedirectUri(oauth.APP_AUTH_ROOT)
.build();
try (CloseableHttpClient c = HttpClientBuilder.create().disableRedirectHandling().build();
CloseableHttpResponse response = c.execute(new HttpGet(logoutUrl))) {
assertThat(response, Matchers.statusCodeIsHC(Status.FOUND));
assertThat(response.getFirstHeader(HttpHeaders.LOCATION).getValue(), is(oauth.APP_AUTH_ROOT));
}
assertNotNull(testingClient.testApp().getAdminLogoutAction());
}
@Test
public void backChannelPreferenceOverKLogout() throws Exception {
ClientsResource clients = adminClient.realm(oauth.getRealm()).clients();
ClientRepresentation rep = clients.findByClientId(oauth.getClientId()).get(0);
rep.getAttributes().put(OIDCConfigAttributes.BACKCHANNEL_LOGOUT_URL, oauth.APP_ROOT + "/admin/backchannelLogout");
ClientResource clientResource = clients.get(rep.getId());
clientResource.update(rep);
try {
oauth.doLogin("test-user@localhost", "password");
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
oauth.clientSessionState("client-session");
OAuthClient.AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(code, "password");
String idTokenString = tokenResponse.getIdToken();
String logoutUrl = oauth.getLogoutUrl()
.idTokenHint(idTokenString)
.postLogoutRedirectUri(oauth.APP_AUTH_ROOT)
.build();
try (CloseableHttpClient c = HttpClientBuilder.create().disableRedirectHandling().build();
CloseableHttpResponse response = c.execute(new HttpGet(logoutUrl))) {
assertThat(response, Matchers.statusCodeIsHC(Status.FOUND));
assertThat(response.getFirstHeader(HttpHeaders.LOCATION).getValue(), is(oauth.APP_AUTH_ROOT));
}
assertNotNull(testingClient.testApp().getBackChannelLogoutToken());
} finally {
rep.getAttributes().put(OIDCConfigAttributes.BACKCHANNEL_LOGOUT_URL, "");
clientResource.update(rep);
}
}
private OAuthClient.AccessTokenResponse loginAndForceNewLoginPage() {
oauth.doLogin("test-user@localhost", "password");