Don't allow impersonate disabled users or service accounts

Closes https://github.com/keycloak/keycloak/issues/21106
This commit is contained in:
rmartinc 2023-06-26 17:10:50 +02:00 committed by Marek Posolda
parent b8149d66ca
commit a5a2753d11
2 changed files with 41 additions and 7 deletions

View file

@ -331,6 +331,14 @@ public class UserResource {
ProfileHelper.requireFeature(Profile.Feature.IMPERSONATION); ProfileHelper.requireFeature(Profile.Feature.IMPERSONATION);
auth.users().requireImpersonate(user); auth.users().requireImpersonate(user);
if (!user.isEnabled()) {
throw ErrorResponse.error("User is disabled", Status.BAD_REQUEST);
}
if (user.getServiceAccountClientLink() != null) {
throw ErrorResponse.error("Service accounts cannot be impersonated", Status.BAD_REQUEST);
}
RealmModel authenticatedRealm = auth.adminAuth().getRealm(); RealmModel authenticatedRealm = auth.adminAuth().getRealm();
// if same realm logout before impersonation // if same realm logout before impersonation
boolean sameRealm = false; boolean sameRealm = false;

View file

@ -24,6 +24,7 @@ import org.apache.http.impl.client.BasicCookieStore;
import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.util.EntityUtils; import org.apache.http.util.EntityUtils;
import org.hamcrest.MatcherAssert;
import org.jboss.arquillian.graphene.page.Page; import org.jboss.arquillian.graphene.page.Page;
import org.jboss.resteasy.client.jaxrs.ResteasyClient; import org.jboss.resteasy.client.jaxrs.ResteasyClient;
import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder; import org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder;
@ -56,7 +57,6 @@ import org.keycloak.testsuite.AbstractKeycloakTest;
import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.auth.page.AuthRealm; import org.keycloak.testsuite.auth.page.AuthRealm;
import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.AppPage;
import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.util.*; import org.keycloak.testsuite.util.*;
import org.openqa.selenium.Cookie; import org.openqa.selenium.Cookie;
@ -104,9 +104,6 @@ public class ImpersonationTest extends AbstractKeycloakTest {
@Page @Page
protected AppPage appPage; protected AppPage appPage;
@Page
protected LoginPage loginPage;
private String impersonatedUserId; private String impersonatedUserId;
@Override @Override
@ -181,6 +178,20 @@ public class ImpersonationTest extends AbstractKeycloakTest {
testForbiddenImpersonation("bad-impersonator", "test"); testForbiddenImpersonation("bad-impersonator", "test");
} }
@Test
public void testImpersonationFailsForDisabledUser() {
UserResource impersonatedUserResource = adminClient.realms().realm("test").users().get(impersonatedUserId);
UserRepresentation impersonatedUserRepresentation = impersonatedUserResource.toRepresentation();
impersonatedUserRepresentation.setEnabled(false);
impersonatedUserResource.update(impersonatedUserRepresentation);
try {
testBadRequestImpersonation("impersonator", "test", impersonatedUserId, "test", "User is disabled");
} finally {
impersonatedUserRepresentation.setEnabled(true);
impersonatedUserResource.update(impersonatedUserRepresentation);
}
}
@Test @Test
public void testImpersonateByMastertBadImpersonator() { public void testImpersonateByMastertBadImpersonator() {
String userId; String userId;
@ -246,6 +257,9 @@ public class ImpersonationTest extends AbstractKeycloakTest {
// Impersonation // Impersonation
testSuccessfulServiceAccountImpersonation(user, "test"); testSuccessfulServiceAccountImpersonation(user, "test");
// test impersonation over the service account fails
testBadRequestImpersonation("impersonator", "test", user.getId(), "test", "Service accounts cannot be impersonated");
// Remove test client // Remove test client
ApiUtil.findClientByClientId(realm, "service-account-cl").remove(); ApiUtil.findClientByClientId(realm, "service-account-cl").remove();
} }
@ -331,7 +345,7 @@ public class ImpersonationTest extends AbstractKeycloakTest {
.collect(Collectors.toSet()); .collect(Collectors.toSet());
Assert.assertNotNull(cookies); Assert.assertNotNull(cookies);
Assert.assertThat(cookies, is(not(empty()))); MatcherAssert.assertThat(cookies, is(not(empty())));
return cookies; return cookies;
} catch (IOException e) { } catch (IOException e) {
@ -344,7 +358,19 @@ public class ImpersonationTest extends AbstractKeycloakTest {
client.realms().realm("test").users().get(impersonatedUserId).impersonate(); client.realms().realm("test").users().get(impersonatedUserId).impersonate();
Assert.fail("Expected ClientErrorException wasn't thrown."); Assert.fail("Expected ClientErrorException wasn't thrown.");
} catch (ClientErrorException e) { } catch (ClientErrorException e) {
Assert.assertThat(e.getMessage(), containsString("403 Forbidden")); MatcherAssert.assertThat(e.getMessage(), containsString("403 Forbidden"));
}
}
protected void testBadRequestImpersonation(String admin, String adminRealm, String impersonatedId,
String impersonatedRealm, String errorExpected) {
try (Keycloak client = createAdminClient(adminRealm, establishClientId(adminRealm), admin)) {
client.realms().realm(impersonatedRealm).users().get(impersonatedId).impersonate();
Assert.fail("Expected ClientErrorException wasn't thrown.");
} catch (ClientErrorException e) {
Assert.assertEquals(Response.Status.BAD_REQUEST, e.getResponse().getStatusInfo());
ErrorRepresentation error = e.getResponse().readEntity(ErrorRepresentation.class);
Assert.assertEquals(errorExpected, error.getErrorMessage());
} }
} }
@ -438,7 +464,7 @@ public class ImpersonationTest extends AbstractKeycloakTest {
.collect(Collectors.toSet()); .collect(Collectors.toSet());
Assert.assertNotNull(cookies); Assert.assertNotNull(cookies);
Assert.assertThat(cookies, is(not(empty()))); MatcherAssert.assertThat(cookies, is(not(empty())));
return cookies; return cookies;
} catch (IOException e) { } catch (IOException e) {