mirror of
https://github.com/keycloak/keycloak.git
synced 2026-01-10 15:32:05 -03:30
Make user read-only and a proper error message when the user federation provider is not available
Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
This commit is contained in:
parent
d65c17ebc7
commit
54d2451b35
@ -225,6 +225,16 @@ This implicit fallback mechanism ensures a smooth upgrade process for deployment
|
||||
|
||||
For more information about fine-grained admin permissions, see link:{adminguide_finegrained_link}[{adminguide_finegrained_name}].
|
||||
|
||||
=== Errors when searching users from LDAP will not fail the request anymore and local users will be returned
|
||||
|
||||
Until now, failures when searching for users from an LDAP user federation provider caused the whole request to fail and no users were returned.
|
||||
In this release, if an error occurs during the search, local users will still be returned and the error will be logged at the `ERROR` level,
|
||||
so that administrators can investigate the root cause of the problem and fix any issue with their LDAP configuration or connectivity
|
||||
with the LDAP server.
|
||||
|
||||
This change improves the resilience of the system when there are temporary issues with the LDAP server, ensuring that local users can still be accessed even if the LDAP search fails.
|
||||
If a local user is linked to a failing LDAP provider, the user will be marked as disabled and read-only until the LDAP server is available again.
|
||||
|
||||
// ------------------------ Deprecated features ------------------------ //
|
||||
== Deprecated features
|
||||
|
||||
|
||||
@ -173,9 +173,8 @@ public class UserStorageManager extends AbstractStorageManager<UserStorageProvid
|
||||
return validated;
|
||||
} catch (Exception e) {
|
||||
logger.warnf(e, "User storage provider %s failed during federated user validation", model.getName());
|
||||
return new ReadOnlyUserModelDelegate(user, false, ignore -> new ReadOnlyException("The user is read-only. The user storage provider '" + model.getName() + "' is currently unavailable. Check the server logs for more details."));
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
private ReadOnlyUserModelDelegate deleteFederatedUser(RealmModel realm, UserModel user) {
|
||||
|
||||
@ -47,6 +47,11 @@ public class ReadOnlyUserModelDelegate extends UserModelDelegate {
|
||||
this.exceptionCreator = exceptionCreator;
|
||||
}
|
||||
|
||||
public ReadOnlyUserModelDelegate(UserModel delegate, boolean enabled, Function<String, RuntimeException> exceptionCreator) {
|
||||
this(delegate, exceptionCreator);
|
||||
this.enabled = enabled;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void setUsername(String username) {
|
||||
throw readOnlyException("username");
|
||||
|
||||
@ -37,7 +37,7 @@ import org.keycloak.federation.kerberos.KerberosFederationProvider;
|
||||
import org.keycloak.models.LDAPConstants;
|
||||
import org.keycloak.models.RealmModel;
|
||||
import org.keycloak.representations.idm.ComponentRepresentation;
|
||||
import org.keycloak.representations.idm.OAuth2ErrorRepresentation;
|
||||
import org.keycloak.representations.idm.ErrorRepresentation;
|
||||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
import org.keycloak.representations.idm.UserRepresentation;
|
||||
import org.keycloak.representations.userprofile.config.UPAttribute;
|
||||
@ -50,11 +50,13 @@ import org.keycloak.testsuite.util.LDAPRule;
|
||||
import org.keycloak.testsuite.util.LDAPTestUtils;
|
||||
import org.keycloak.testsuite.util.UserBuilder;
|
||||
|
||||
import static org.hamcrest.Matchers.is;
|
||||
import static org.hamcrest.Matchers.not;
|
||||
import static org.hamcrest.Matchers.contains;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.hamcrest.MatcherAssert.assertThat;
|
||||
import static org.junit.Assert.assertNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.junit.Assert.fail;
|
||||
import static org.keycloak.testsuite.util.userprofile.UserProfileUtil.setUserProfileConfiguration;
|
||||
import static org.keycloak.util.JsonSerialization.writeValueAsString;
|
||||
@ -235,29 +237,47 @@ public class LDAPAdminRestApiTest extends AbstractLDAPTest {
|
||||
String newUserId1 = createUserExpectSuccess(user1);
|
||||
getCleanup().addUserId(newUserId1);
|
||||
|
||||
List<ComponentRepresentation> storageProviders = testRealm().components().query(testRealm().toRepresentation().getId(), UserStorageProvider.class.getName());
|
||||
String realmId = testRealm().toRepresentation().getId();
|
||||
List<ComponentRepresentation> storageProviders = testRealm().components().query(realmId, UserStorageProvider.class.getName());
|
||||
ComponentRepresentation ldapProvider = storageProviders.get(0);
|
||||
List<String> originalUrl = ldapProvider.getConfig().get(LDAPConstants.CONNECTION_URL);
|
||||
|
||||
getCleanup().addCleanup(new AutoCloseable() {
|
||||
@Override
|
||||
public void close() {
|
||||
ldapProvider.getConfig().put(LDAPConstants.CONNECTION_URL, originalUrl);
|
||||
testRealm().components().component(ldapProvider.getId()).update(ldapProvider);
|
||||
}
|
||||
getCleanup().addCleanup(() -> {
|
||||
ldapProvider.getConfig().put(LDAPConstants.CONNECTION_URL, originalUrl);
|
||||
testRealm().components().component(ldapProvider.getId()).update(ldapProvider);
|
||||
});
|
||||
|
||||
ldapProvider.getConfig().put(LDAPConstants.CONNECTION_URL, List.of("ldap://invalid"));
|
||||
testRealm().components().component(ldapProvider.getId()).update(ldapProvider);
|
||||
|
||||
List<UserRepresentation> search = testRealm().users().search("*", -1, -1, true);
|
||||
assertThat(search.isEmpty(), is(false));
|
||||
user1 = search.stream().filter(u -> u.getUsername().equals("admintestuser1")).findFirst().orElseThrow();
|
||||
assertThat(user1.getAttributes().containsKey(LDAPConstants.LDAP_ID), is(true));
|
||||
assertThat(user1.isEnabled(), is(false));
|
||||
|
||||
UserResource userResource = testRealm().users().get(newUserId1);
|
||||
|
||||
try {
|
||||
List<UserRepresentation> search = testRealm().users().search("*", -1, -1, true);
|
||||
Assert.fail("Should fail because LDAP is in failing state");
|
||||
userResource.update(user1);
|
||||
Assert.fail("Not expected to successfully update user");
|
||||
} catch (WebApplicationException expected) {
|
||||
Response response = expected.getResponse();
|
||||
OAuth2ErrorRepresentation error = response.readEntity(OAuth2ErrorRepresentation.class);
|
||||
assertEquals("unknown_error", error.getError());
|
||||
ErrorRepresentation error = response.readEntity(ErrorRepresentation.class);
|
||||
assertTrue(error.getErrorMessage().contains("The user is read-only. The user storage provider 'test-ldap' is currently unavailable. Check the server logs for more details."));
|
||||
}
|
||||
|
||||
// fix the LDAP connection configuration and try again to update the user
|
||||
storageProviders = testRealm().components().query(realmId, UserStorageProvider.class.getName());
|
||||
ComponentRepresentation ldapProviderValid = storageProviders.get(0);
|
||||
ldapProviderValid.getConfig().put(LDAPConstants.CONNECTION_URL, originalUrl);
|
||||
testRealm().components().component(ldapProviderValid.getId()).update(ldapProviderValid);
|
||||
user1 = userResource.toRepresentation();
|
||||
user1.setLastName("changed");
|
||||
userResource.update(user1);
|
||||
user1 = userResource.toRepresentation();
|
||||
assertTrue(user1.isEnabled());
|
||||
assertEquals("changed", user1.getLastName());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@ -35,8 +35,6 @@ import org.keycloak.models.RoleModel;
|
||||
import org.keycloak.models.UserModel;
|
||||
import org.keycloak.models.cache.CachedUserModel;
|
||||
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
|
||||
import org.keycloak.representations.AccessToken;
|
||||
import org.keycloak.representations.RefreshToken;
|
||||
import org.keycloak.representations.idm.ComponentRepresentation;
|
||||
import org.keycloak.representations.idm.EventRepresentation;
|
||||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
@ -161,33 +159,15 @@ public class UserStorageFailureTest extends AbstractTestRealmKeycloakTest {
|
||||
.detail(Details.REDIRECT_URI, OAuthClient.AUTH_SERVER_ROOT + "/offline-client")
|
||||
.assertEvent();
|
||||
|
||||
final String sessionId = loginEvent.getSessionId();
|
||||
String codeId = loginEvent.getDetails().get(Details.CODE_ID);
|
||||
|
||||
String code = oauth.parseLoginResponse().getCode();
|
||||
|
||||
AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(code);
|
||||
AccessToken token = oauth.verifyToken(tokenResponse.getAccessToken());
|
||||
String offlineTokenString = tokenResponse.getRefreshToken();
|
||||
RefreshToken offlineToken = oauth.parseRefreshToken(offlineTokenString);
|
||||
events.clear();
|
||||
|
||||
evictUser(FailableHardcodedStorageProvider.username);
|
||||
|
||||
toggleForceFail(true);
|
||||
|
||||
// make sure failure is turned on
|
||||
testingClient.server().run(session -> {
|
||||
RealmModel realm = session.realms().getRealmByName(AuthRealm.TEST);
|
||||
try {
|
||||
UserModel user = session.users().getUserByUsername(realm, FailableHardcodedStorageProvider.username);
|
||||
Assert.fail();
|
||||
} catch (Exception e) {
|
||||
Assert.assertEquals("FORCED FAILURE", e.getMessage());
|
||||
|
||||
}
|
||||
});
|
||||
|
||||
controller.stop(suiteContext.getAuthServerInfo().getQualifier());
|
||||
controller.start(suiteContext.getAuthServerInfo().getQualifier());
|
||||
reconnectAdminClient();
|
||||
@ -198,12 +178,10 @@ public class UserStorageFailureTest extends AbstractTestRealmKeycloakTest {
|
||||
// test that once user storage provider is available again we can still access the token.
|
||||
tokenResponse = oauth.doRefreshTokenRequest(offlineTokenString);
|
||||
Assert.assertNotNull(tokenResponse.getAccessToken());
|
||||
token = oauth.verifyToken(tokenResponse.getAccessToken());
|
||||
oauth.verifyToken(tokenResponse.getAccessToken());
|
||||
offlineTokenString = tokenResponse.getRefreshToken();
|
||||
offlineToken = oauth.parseRefreshToken(offlineTokenString);
|
||||
oauth.parseRefreshToken(offlineTokenString);
|
||||
events.clear();
|
||||
|
||||
|
||||
}
|
||||
|
||||
protected void evictUser(final String username) {
|
||||
@ -278,22 +256,6 @@ public class UserStorageFailureTest extends AbstractTestRealmKeycloakTest {
|
||||
|
||||
toggleForceFail(true);
|
||||
|
||||
testingClient.server().run(session -> {
|
||||
RealmModel realm = session.realms().getRealmByName(AuthRealm.TEST);
|
||||
|
||||
UserModel local = session.users().getUserByUsername(realm, LOCAL_USER);
|
||||
Assert.assertNotNull(local);
|
||||
// assert that lookup of user storage user fails
|
||||
try {
|
||||
UserModel user = session.users().getUserByUsername(realm, FailableHardcodedStorageProvider.username);
|
||||
Assert.fail();
|
||||
} catch (Exception e) {
|
||||
Assert.assertEquals("FORCED FAILURE", e.getMessage());
|
||||
|
||||
}
|
||||
|
||||
});
|
||||
|
||||
// test that we can still login to a user
|
||||
loginSuccessAndLogout("test-user@localhost", "password");
|
||||
|
||||
@ -381,11 +343,4 @@ public class UserStorageFailureTest extends AbstractTestRealmKeycloakTest {
|
||||
|
||||
events.clear();
|
||||
}
|
||||
|
||||
|
||||
//@Test
|
||||
public void testIDE() throws Exception {
|
||||
Thread.sleep(100000000);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user