Credential validation shouldn't invalidate the user in the cache

Instead create a new instance of LegacyUserCredentialManager to ensure all calls are routed via the CacheAdapter and its SubjectCredentialManagerCacheAdapter.

Closes #14309
This commit is contained in:
Alexander Schwartz 2022-09-09 08:18:39 -03:00 committed by Pedro Igor
parent aa5a4e3d84
commit 621da7b803
2 changed files with 27 additions and 83 deletions

View file

@ -19,118 +19,68 @@ package org.keycloak.models.cache.infinispan;
import org.keycloak.credential.CredentialInput; import org.keycloak.credential.CredentialInput;
import org.keycloak.credential.CredentialModel; import org.keycloak.credential.CredentialModel;
import org.keycloak.models.SubjectCredentialManager; import org.keycloak.credential.LegacyUserCredentialManager;
import org.keycloak.models.KeycloakSession;
import java.util.List; import org.keycloak.models.RealmModel;
import java.util.stream.Stream; import org.keycloak.models.UserModel;
/** /**
* @author Alexander Schwartz * @author Alexander Schwartz
*/ */
public abstract class SubjectCredentialManagerCacheAdapter implements SubjectCredentialManager { public abstract class SubjectCredentialManagerCacheAdapter extends LegacyUserCredentialManager {
private final SubjectCredentialManager subjectCredentialManager; public SubjectCredentialManagerCacheAdapter(KeycloakSession session, RealmModel realm, UserModel user) {
super(session, realm, user);
protected SubjectCredentialManagerCacheAdapter(SubjectCredentialManager subjectCredentialManager) {
this.subjectCredentialManager = subjectCredentialManager;
} }
public abstract void invalidateCacheForEntity(); public abstract void invalidateCacheForEntity();
@Override
public boolean isValid(List<CredentialInput> inputs) {
// validating a password might still update its hashes, similar logic might apply to OTP logic
// instead of having each
invalidateCacheForEntity();
return subjectCredentialManager.isValid(inputs);
}
@Override @Override
public boolean updateCredential(CredentialInput input) { public boolean updateCredential(CredentialInput input) {
invalidateCacheForEntity(); invalidateCacheForEntity();
return subjectCredentialManager.updateCredential(input); return super.updateCredential(input);
} }
@Override @Override
public void updateStoredCredential(CredentialModel cred) { public void updateStoredCredential(CredentialModel cred) {
invalidateCacheForEntity(); invalidateCacheForEntity();
subjectCredentialManager.updateStoredCredential(cred); super.updateStoredCredential(cred);
} }
@Override @Override
public CredentialModel createStoredCredential(CredentialModel cred) { public CredentialModel createStoredCredential(CredentialModel cred) {
invalidateCacheForEntity(); invalidateCacheForEntity();
return subjectCredentialManager.createStoredCredential(cred); return super.createStoredCredential(cred);
} }
@Override @Override
public boolean removeStoredCredentialById(String id) { public boolean removeStoredCredentialById(String id) {
invalidateCacheForEntity(); invalidateCacheForEntity();
return subjectCredentialManager.removeStoredCredentialById(id); return super.removeStoredCredentialById(id);
}
@Override
public CredentialModel getStoredCredentialById(String id) {
return subjectCredentialManager.getStoredCredentialById(id);
}
@Override
public Stream<CredentialModel> getStoredCredentialsStream() {
return subjectCredentialManager.getStoredCredentialsStream();
}
@Override
public Stream<CredentialModel> getStoredCredentialsByTypeStream(String type) {
return subjectCredentialManager.getStoredCredentialsByTypeStream(type);
}
@Override
public CredentialModel getStoredCredentialByNameAndType(String name, String type) {
return subjectCredentialManager.getStoredCredentialByNameAndType(name, type);
} }
@Override @Override
public boolean moveStoredCredentialTo(String id, String newPreviousCredentialId) { public boolean moveStoredCredentialTo(String id, String newPreviousCredentialId) {
invalidateCacheForEntity(); invalidateCacheForEntity();
return subjectCredentialManager.moveStoredCredentialTo(id, newPreviousCredentialId); return super.moveStoredCredentialTo(id, newPreviousCredentialId);
} }
@Override @Override
public void updateCredentialLabel(String credentialId, String userLabel) { public void updateCredentialLabel(String credentialId, String userLabel) {
invalidateCacheForEntity(); invalidateCacheForEntity();
subjectCredentialManager.updateCredentialLabel(credentialId, userLabel); super.updateCredentialLabel(credentialId, userLabel);
} }
@Override @Override
public void disableCredentialType(String credentialType) { public void disableCredentialType(String credentialType) {
invalidateCacheForEntity(); invalidateCacheForEntity();
subjectCredentialManager.disableCredentialType(credentialType); super.disableCredentialType(credentialType);
}
@Override
public Stream<String> getDisableableCredentialTypesStream() {
return subjectCredentialManager.getDisableableCredentialTypesStream();
}
@Override
public boolean isConfiguredFor(String type) {
return subjectCredentialManager.isConfiguredFor(type);
}
@Override
public boolean isConfiguredLocally(String type) {
return subjectCredentialManager.isConfiguredLocally(type);
}
@Override
public Stream<String> getConfiguredUserStorageCredentialTypesStream() {
return subjectCredentialManager.getConfiguredUserStorageCredentialTypesStream();
} }
@Override @Override
public CredentialModel createCredentialThroughProvider(CredentialModel model) { public CredentialModel createCredentialThroughProvider(CredentialModel model) {
invalidateCacheForEntity(); invalidateCacheForEntity();
return subjectCredentialManager.createCredentialThroughProvider(model); return super.createCredentialThroughProvider(model);
} }
} }

View file

@ -18,6 +18,7 @@
package org.keycloak.models.cache.infinispan; package org.keycloak.models.cache.infinispan;
import org.keycloak.credential.CredentialModel; import org.keycloak.credential.CredentialModel;
import org.keycloak.credential.LegacyUserCredentialManager;
import org.keycloak.models.ClientModel; import org.keycloak.models.ClientModel;
import org.keycloak.models.GroupModel; import org.keycloak.models.GroupModel;
import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSession;
@ -53,7 +54,6 @@ public class UserAdapter implements CachedUserModel.Streams {
protected final KeycloakSession keycloakSession; protected final KeycloakSession keycloakSession;
protected final RealmModel realm; protected final RealmModel realm;
protected volatile UserModel updated; protected volatile UserModel updated;
private boolean userRegisteredForInvalidation;
public UserAdapter(CachedUser cached, UserCacheSession userProvider, KeycloakSession keycloakSession, RealmModel realm) { public UserAdapter(CachedUser cached, UserCacheSession userProvider, KeycloakSession keycloakSession, RealmModel realm) {
this.cached = cached; this.cached = cached;
@ -101,12 +101,8 @@ public class UserAdapter implements CachedUserModel.Streams {
public UserModel getDelegateForUpdate() { public UserModel getDelegateForUpdate() {
if (updated == null) { if (updated == null) {
userProviderCache.registerUserInvalidation(realm, cached); userProviderCache.registerUserInvalidation(realm, cached);
userRegisteredForInvalidation = true;
updated = modelSupplier.get(); updated = modelSupplier.get();
if (updated == null) throw new IllegalStateException("Not found in database"); if (updated == null) throw new IllegalStateException("Not found in database");
} else if (!userRegisteredForInvalidation) {
userProviderCache.registerUserInvalidation(realm, cached);
userRegisteredForInvalidation = true;
} }
return updated; return updated;
} }
@ -288,14 +284,13 @@ public class UserAdapter implements CachedUserModel.Streams {
@Override @Override
public SubjectCredentialManager credentialManager() { public SubjectCredentialManager credentialManager() {
if (updated == null) { // Instantiate a new LegacyUserCredentialManager that points to the instance that is wrapped by the cache
updated = modelSupplier.get(); // this way it the cache will know if any of the credentials are modified during validation of CredentialInputs.
if (updated == null) throw new IllegalStateException("Not found in database"); // This assumes that each implementation in the legacy world implements the LegacyUserCredentialManager and not something else.
} return new SubjectCredentialManagerCacheAdapter(keycloakSession, realm, this) {
return new SubjectCredentialManagerCacheAdapter(updated.credentialManager()) {
@Override @Override
public CredentialModel getStoredCredentialById(String id) { public CredentialModel getStoredCredentialById(String id) {
if (!userRegisteredForInvalidation) { if (updated == null) {
return cached.getStoredCredentials(modelSupplier).stream().filter(credential -> return cached.getStoredCredentials(modelSupplier).stream().filter(credential ->
Objects.equals(id, credential.getId())) Objects.equals(id, credential.getId()))
.findFirst().orElse(null); .findFirst().orElse(null);
@ -305,7 +300,7 @@ public class UserAdapter implements CachedUserModel.Streams {
@Override @Override
public Stream<CredentialModel> getStoredCredentialsStream() { public Stream<CredentialModel> getStoredCredentialsStream() {
if (!userRegisteredForInvalidation) { if (updated == null) {
return cached.getStoredCredentials(modelSupplier).stream(); return cached.getStoredCredentials(modelSupplier).stream();
} }
return super.getStoredCredentialsStream(); return super.getStoredCredentialsStream();
@ -313,7 +308,7 @@ public class UserAdapter implements CachedUserModel.Streams {
@Override @Override
public Stream<CredentialModel> getStoredCredentialsByTypeStream(String type) { public Stream<CredentialModel> getStoredCredentialsByTypeStream(String type) {
if (!userRegisteredForInvalidation) { if (updated == null) {
return cached.getStoredCredentials(modelSupplier).stream().filter(credential -> Objects.equals(type, credential.getType())); return cached.getStoredCredentials(modelSupplier).stream().filter(credential -> Objects.equals(type, credential.getType()));
} }
return super.getStoredCredentialsByTypeStream(type); return super.getStoredCredentialsByTypeStream(type);
@ -321,7 +316,7 @@ public class UserAdapter implements CachedUserModel.Streams {
@Override @Override
public CredentialModel getStoredCredentialByNameAndType(String name, String type) { public CredentialModel getStoredCredentialByNameAndType(String name, String type) {
if (!userRegisteredForInvalidation) { if (updated == null) {
return cached.getStoredCredentials(modelSupplier).stream().filter(credential -> return cached.getStoredCredentials(modelSupplier).stream().filter(credential ->
Objects.equals(type, credential.getType()) && Objects.equals(name, credential.getUserLabel())) Objects.equals(type, credential.getType()) && Objects.equals(name, credential.getUserLabel()))
.findFirst().orElse(null); .findFirst().orElse(null);
@ -331,10 +326,9 @@ public class UserAdapter implements CachedUserModel.Streams {
@Override @Override
public void invalidateCacheForEntity() { public void invalidateCacheForEntity() {
if (!userRegisteredForInvalidation) { // This implies invalidation of the cached entry,
userProviderCache.registerUserInvalidation(realm, cached); // and all future calls in this session for the user will go to the store instead of the cache.
userRegisteredForInvalidation = true; getDelegateForUpdate();
}
} }
}; };
} }