KEYCLOAK-15524 Cleanup user related interfaces

tknzk · Jan 18, 2021 · ba8e2fe · ba8e2fe
1 parent dae4a3e
commit ba8e2fe
Show file tree

Hide file tree

Showing 145 changed files with 1,612 additions and 1,368 deletions.
diff --git a/.../java/org/keycloak/authorization/policy/provider/permission/UMAPolicyProviderFactory.java b/.../java/org/keycloak/authorization/policy/provider/permission/UMAPolicyProviderFactory.java
@@ -349,7 +349,7 @@ public UmaPermissionRepresentation toRepresentation(Policy policy, Authorization
                 UserPolicyRepresentation rep = UserPolicyRepresentation.class.cast(associatedRep);
 
                 for (String user : rep.getUsers()) {
-                    representation.addUser(authorization.getKeycloakSession().users().getUserById(user, realm).getUsername());
+                    representation.addUser(authorization.getKeycloakSession().users().getUserById(realm, user).getUsername());
                 }
             }
         }

diff --git a/.../main/java/org/keycloak/authorization/policy/provider/user/UserPolicyProviderFactory.java b/.../main/java/org/keycloak/authorization/policy/provider/user/UserPolicyProviderFactory.java
@@ -119,7 +119,7 @@ public void onExport(Policy policy, PolicyRepresentation representation, Authori
             UserProvider userProvider = authorizationProvider.getKeycloakSession().users();
             RealmModel realm = authorizationProvider.getRealm();
 
-            config.put("users", JsonSerialization.writeValueAsString(userRep.getUsers().stream().map(id -> userProvider.getUserById(id, realm).getUsername()).collect(Collectors.toList())));
+            config.put("users", JsonSerialization.writeValueAsString(userRep.getUsers().stream().map(id -> userProvider.getUserById(realm, id).getUsername()).collect(Collectors.toList())));
         } catch (IOException cause) {
             throw new RuntimeException("Failed to export user policy [" + policy.getName() + "]", cause);
         }
@@ -142,12 +142,12 @@ private void updateUsers(Policy policy, AuthorizationProvider authorization, Set
                 UserModel user = null;
 
                 try {
-                    user = userProvider.getUserByUsername(userId, realm);
+                    user = userProvider.getUserByUsername(realm, userId);
                 } catch (Exception ignore) {
                 }
 
                 if (user == null) {
-                    user = userProvider.getUserById(userId, realm);
+                    user = userProvider.getUserById(realm, userId);
                 }
 
                 if (user == null) {

diff --git a/...n/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java b/...n/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java
@@ -48,7 +48,7 @@
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  */
 public class KerberosFederationProvider implements UserStorageProvider,
-        UserLookupProvider,
+        UserLookupProvider.Streams,
         CredentialInputValidator,
         CredentialInputUpdater.Streams,
         CredentialAuthentication,
@@ -83,7 +83,7 @@ public UserModel validate(RealmModel realm, UserModel user) {
     }
 
     @Override
-    public UserModel getUserByUsername(String username, RealmModel realm) {
+    public UserModel getUserByUsername(RealmModel realm, String username) {
         KerberosUsernamePasswordAuthenticator authenticator = factory.createKerberosUsernamePasswordAuthenticator(kerberosConfig);
         if (authenticator.isUserAvailable(username)) {
             // Case when method was called with username including kerberos realm like john@REALM.ORG . Authenticator already checked that kerberos realm was correct
@@ -98,12 +98,12 @@ public UserModel getUserByUsername(String username, RealmModel realm) {
     }
 
     @Override
-    public UserModel getUserByEmail(String email, RealmModel realm) {
+    public UserModel getUserByEmail(RealmModel realm, String email) {
         return null;
     }
 
     @Override
-    public UserModel getUserById(String id, RealmModel realm) {
+    public UserModel getUserById(RealmModel realm, String id) {
         return null;
     }
 
@@ -234,9 +234,9 @@ public void close() {
      * @return user if found or successfully created. Null if user with same username already exists, but is not linked to this provider
      */
     protected UserModel findOrCreateAuthenticatedUser(RealmModel realm, String username) {
-        UserModel user = session.userLocalStorage().getUserByUsername(username, realm);
+        UserModel user = session.userLocalStorage().getUserByUsername(realm, username);
         if (user != null) {
-            user = session.users().getUserById(user.getId(), realm);  // make sure we get a cached instance
+            user = session.users().getUserById(realm, user.getId());  // make sure we get a cached instance
             logger.debug("Kerberos authenticated user " + username + " found in Keycloak storage");
 
             if (!model.getId().equals(user.getFederationLink())) {

diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
@@ -79,6 +79,8 @@
 import org.keycloak.storage.user.UserQueryProvider;
 import org.keycloak.storage.user.UserRegistrationProvider;
 
+import static org.keycloak.utils.StreamsUtil.paginatedStream;
+
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
@@ -88,11 +90,12 @@ public class LDAPStorageProvider implements UserStorageProvider,
         CredentialInputValidator,
         CredentialInputUpdater.Streams,
         CredentialAuthentication,
-        UserLookupProvider,
+        UserLookupProvider.Streams,
         UserRegistrationProvider,
         UserQueryProvider.Streams,
         ImportedUserValidation {
     private static final Logger logger = Logger.getLogger(LDAPStorageProvider.class);
+    private static final int DEFAULT_MAX_RESULTS = Integer.MAX_VALUE >> 1;
 
     protected LDAPStorageProviderFactory factory;
     protected KeycloakSession session;
@@ -176,7 +179,7 @@ protected UserModel proxy(RealmModel realm, UserModel local, LDAPObject ldapObje
 
         // We need to avoid having CachedUserModel as cache is upper-layer then LDAP. Hence having CachedUserModel here may cause StackOverflowError
         if (local instanceof CachedUserModel) {
-            local = session.userStorageManager().getUserById(local.getId(), realm);
+            local = session.userStorageManager().getUserById(realm, local.getId());
 
             existing = userManager.getManagedProxiedUser(local.getId());
             if (existing != null) {
@@ -245,7 +248,7 @@ public boolean supportsCredentialAuthenticationFor(String type) {
     }
 
     @Override
-    public Stream<UserModel> searchForUserByUserAttributeStream(String attrName, String attrValue, RealmModel realm) {
+    public Stream<UserModel> searchForUserByUserAttributeStream(RealmModel realm, String attrName, String attrValue) {
     	 try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm)) {
              LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder();
 
@@ -256,7 +259,7 @@ public Stream<UserModel> searchForUserByUserAttributeStream(String attrName, Str
 
              return ldapObjects.stream().map(ldapUser -> {
                  String ldapUsername = LDAPUtils.getUsername(ldapUser, this.ldapIdentityStore.getConfig());
-                 UserModel localUser = session.userLocalStorage().getUserByUsername(ldapUsername, realm);
+                 UserModel localUser = session.userLocalStorage().getUserByUsername(realm, ldapUsername);
                  if (localUser == null) {
                      return importUserFromLDAP(session, realm, ldapUser);
                  } else {
@@ -323,12 +326,12 @@ public boolean removeUser(RealmModel realm, UserModel user) {
     }
 
     @Override
-    public UserModel getUserById(String id, RealmModel realm) {
+    public UserModel getUserById(RealmModel realm, String id) {
         UserModel alreadyLoadedInSession = userManager.getManagedProxiedUser(id);
         if (alreadyLoadedInSession != null) return alreadyLoadedInSession;
 
         StorageId storageId = new StorageId(id);
-        return getUserByUsername(storageId.getExternalId(), realm);
+        return getUserByUsername(realm, storageId.getExternalId());
     }
 
     @Override
@@ -342,29 +345,20 @@ public Stream<UserModel> getUsersStream(RealmModel realm) {
     }
 
     @Override
-    public Stream<UserModel> getUsersStream(RealmModel realm, int firstResult, int maxResults) {
+    public Stream<UserModel> getUsersStream(RealmModel realm, Integer firstResult, Integer maxResults) {
         return Stream.empty();
     }
 
     @Override
-    public Stream<UserModel> searchForUserStream(String search, RealmModel realm) {
-        return searchForUserStream(search, realm, 0, Integer.MAX_VALUE - 1);
-    }
-
-    @Override
-    public Stream<UserModel> searchForUserStream(String search, RealmModel realm, Integer firstResult, Integer maxResults) {
+    public Stream<UserModel> searchForUserStream(RealmModel realm, String search, Integer firstResult, Integer maxResults) {
         Map<String, String> attributes = new HashMap<String, String>();
         attributes.put(UserModel.SEARCH,search);
-        return searchForUserStream(attributes, realm, firstResult, maxResults);
+        return searchForUserStream(realm, attributes, firstResult, maxResults);
     }
 
-    @Override
-    public Stream<UserModel> searchForUserStream(Map<String, String> params, RealmModel realm) {
-        return searchForUserStream(params, realm, 0, Integer.MAX_VALUE - 1);
-    }
 
     @Override
-    public Stream<UserModel> searchForUserStream(Map<String, String> params, RealmModel realm, Integer firstResult, Integer maxResults) {
+    public Stream<UserModel> searchForUserStream(RealmModel realm, Map<String, String> params, Integer firstResult, Integer maxResults) {
         String search = params.get(UserModel.SEARCH);
         if(search!=null) {
             int spaceIndex = search.lastIndexOf(' ');
@@ -385,41 +379,34 @@ public Stream<UserModel> searchForUserStream(Map<String, String> params, RealmMo
         Stream<LDAPObject> stream = searchLDAP(realm, params).stream()
             .filter(ldapObject -> {
                 String ldapUsername = LDAPUtils.getUsername(ldapObject, this.ldapIdentityStore.getConfig());
-                return (session.userLocalStorage().getUserByUsername(ldapUsername, realm) == null);
+                return (session.userLocalStorage().getUserByUsername(realm, ldapUsername) == null);
             });
-        if (firstResult > 0)
-            stream = stream.skip(firstResult);
-        if (maxResults >= 0)
-            stream = stream.limit(maxResults);
-        return stream.map(ldapObject -> importUserFromLDAP(session, realm, ldapObject));
-    }
 
-    @Override
-    public Stream<UserModel> getGroupMembersStream(RealmModel realm, GroupModel group) {
-        return getGroupMembersStream(realm, group, 0, Integer.MAX_VALUE - 1);
+        return paginatedStream(stream, firstResult, maxResults).map(ldapObject -> importUserFromLDAP(session, realm, ldapObject));
     }
 
     @Override
     public Stream<UserModel> getGroupMembersStream(RealmModel realm, GroupModel group, Integer firstResult, Integer maxResults) {
+        int first = firstResult == null ? 0 : firstResult;
+        int max = maxResults == null ? DEFAULT_MAX_RESULTS : maxResults;
+
         return realm.getComponentsStream(model.getId(), LDAPStorageMapper.class.getName())
             .sorted(ldapMappersComparator.sortAsc())
             .map(mapperModel ->
-                mapperManager.getMapper(mapperModel).getGroupMembers(realm, group, firstResult, maxResults))
+                mapperManager.getMapper(mapperModel).getGroupMembers(realm, group, first, max))
             .filter(((Predicate<List>) List::isEmpty).negate())
             .map(List::stream)
             .findFirst().orElse(Stream.empty());
     }
 
-    @Override
-    public Stream<UserModel> getRoleMembersStream(RealmModel realm, RoleModel role) {
-        return getRoleMembersStream(realm, role, 0, Integer.MAX_VALUE - 1);
-    }
-
     @Override
     public Stream<UserModel> getRoleMembersStream(RealmModel realm, RoleModel role, Integer firstResult, Integer maxResults) {
+        int first = firstResult == null ? 0 : firstResult;
+        int max = maxResults == null ? DEFAULT_MAX_RESULTS : maxResults;
+
         return realm.getComponentsStream(model.getId(), LDAPStorageMapper.class.getName())
                 .sorted(ldapMappersComparator.sortAsc())
-                .map(mapperModel -> mapperManager.getMapper(mapperModel).getRoleMembers(realm, role, firstResult, maxResults))
+                .map(mapperModel -> mapperManager.getMapper(mapperModel).getRoleMembers(realm, role, first, max))
                 .filter(((Predicate<List>) List::isEmpty).negate())
                 .map(List::stream)
                 .findFirst().orElse(Stream.empty());
@@ -428,7 +415,7 @@ public Stream<UserModel> getRoleMembersStream(RealmModel realm, RoleModel role,
     public List<UserModel> loadUsersByUsernames(List<String> usernames, RealmModel realm) {
         List<UserModel> result = new ArrayList<>();
         for (String username : usernames) {
-            UserModel kcUser = session.users().getUserByUsername(username, realm);
+            UserModel kcUser = session.users().getUserByUsername(realm, username);
             if (kcUser == null) {
                 logger.warnf("User '%s' referenced by membership wasn't found in LDAP", username);
             } else if (model.isImportEnabled() && !model.getId().equals(kcUser.getFederationLink())) {
@@ -514,7 +501,7 @@ protected LDAPObject loadAndValidateUser(RealmModel realm, UserModel local) {
     }
 
     @Override
-    public UserModel getUserByUsername(String username, RealmModel realm) {
+    public UserModel getUserByUsername(RealmModel realm, String username) {
         LDAPObject ldapUser = loadLDAPUserByUsername(realm, username);
         if (ldapUser == null) {
             return null;
@@ -575,15 +562,15 @@ protected LDAPObject queryByEmail(RealmModel realm, String email) {
 
 
     @Override
-    public UserModel getUserByEmail(String email, RealmModel realm) {
+    public UserModel getUserByEmail(RealmModel realm, String email) {
         LDAPObject ldapUser = queryByEmail(realm, email);
         if (ldapUser == null) {
             return null;
         }
 
         // Check here if user already exists
         String ldapUsername = LDAPUtils.getUsername(ldapUser, ldapIdentityStore.getConfig());
-        UserModel user = session.userLocalStorage().getUserByUsername(ldapUsername, realm);
+        UserModel user = session.userLocalStorage().getUserByUsername(realm, ldapUsername);
 
         if (user != null) {
             LDAPUtils.checkUuid(ldapUser, ldapIdentityStore.getConfig());
@@ -771,7 +758,7 @@ public void close() {
      * @return finded or newly created user
      */
     protected UserModel findOrCreateAuthenticatedUser(RealmModel realm, String username) {
-        UserModel user = session.userLocalStorage().getUserByUsername(username, realm);
+        UserModel user = session.userLocalStorage().getUserByUsername(realm, username);
         if (user != null) {
             logger.debugf("Kerberos authenticated user [%s] found in Keycloak storage", username);
             if (!model.getId().equals(user.getFederationLink())) {
@@ -796,7 +783,7 @@ protected UserModel findOrCreateAuthenticatedUser(RealmModel realm, String usern
 
         // Creating user to local storage
         logger.debugf("Kerberos authenticated user [%s] not in Keycloak storage. Creating him", username);
-        return getUserByUsername(username, realm);
+        return getUserByUsername(realm, username);
     }
 
     public LDAPObject loadLDAPUserByUsername(RealmModel realm, String username) {

diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProviderFactory.java
@@ -593,7 +593,7 @@ public void run(KeycloakSession session) {
                         String username = LDAPUtils.getUsername(ldapUser, ldapFedProvider.getLdapIdentityStore().getConfig());
                         exists.value = true;
                         LDAPUtils.checkUuid(ldapUser, ldapFedProvider.getLdapIdentityStore().getConfig());
-                        UserModel currentUser = session.userLocalStorage().getUserByUsername(username, currentRealm);
+                        UserModel currentUser = session.userLocalStorage().getUserByUsername(currentRealm, username);
 
                         if (currentUser == null) {
 
@@ -649,7 +649,7 @@ public void run(KeycloakSession session) {
                             }
 
                             if (username != null) {
-                                UserModel existing = session.userLocalStorage().getUserByUsername(username, currentRealm);
+                                UserModel existing = session.userLocalStorage().getUserByUsername(currentRealm, username);
                                 if (existing != null) {
                                     UserCache userCache = session.userCache();
                                     if (userCache != null) {

diff --git a/.../ldap/src/main/java/org/keycloak/storage/ldap/mappers/UserAttributeLDAPStorageMapper.java b/.../ldap/src/main/java/org/keycloak/storage/ldap/mappers/UserAttributeLDAPStorageMapper.java
@@ -149,7 +149,7 @@ protected void checkDuplicateEmail(String userModelAttrName, String email, Realm
             // lowercase before search
             email = KeycloakModelUtils.toLowerCaseSafe(email);
 
-            UserModel that = session.userLocalStorage().getUserByEmail(email, realm);
+            UserModel that = session.userLocalStorage().getUserByEmail(realm, email);
             if (that != null && !that.getId().equals(user.getId())) {
                 session.getTransactionManager().setRollbackOnly();
                 String exceptionMessage = String.format("Can't import user '%s' from LDAP because email '%s' already exists in Keycloak. Existing user with this email is '%s'", user.getUsername(), email, that.getUsername());
@@ -166,7 +166,7 @@ protected void checkDuplicateUsername(String userModelAttrName, String username,
             }
             boolean usernameChanged = !username.equals(user.getUsername());
             if (realm.isEditUsernameAllowed() && usernameChanged) {
-                UserModel that = session.users().getUserByUsername(username, realm);
+                UserModel that = session.users().getUserByUsername(realm, username);
                 if (that != null && !that.getId().equals(user.getId())) {
                     throw new ModelDuplicateException(
                             String.format("Cannot change the username to '%s' because the username already exists in keycloak", username),

diff --git a/federation/sssd/src/main/java/org/keycloak/federation/sssd/SSSDFederationProvider.java b/federation/sssd/src/main/java/org/keycloak/federation/sssd/SSSDFederationProvider.java
@@ -44,7 +44,7 @@
  * @version $Revision: 1 $
  */
 public class SSSDFederationProvider implements UserStorageProvider,
-        UserLookupProvider,
+        UserLookupProvider.Streams,
         CredentialInputUpdater.Streams,
         CredentialInputValidator,
         ImportedUserValidation {
@@ -68,7 +68,7 @@ public SSSDFederationProvider(KeycloakSession session, UserStorageProviderModel
 
 
     @Override
-    public UserModel getUserByUsername(String username, RealmModel realm) {
+    public UserModel getUserByUsername(RealmModel realm, String username) {
         return findOrCreateAuthenticatedUser(realm, username);
     }
 
@@ -85,7 +85,7 @@ public UserModel validate(RealmModel realm, UserModel user) {
          * @return user if found or successfully created. Null if user with same username already exists, but is not linked to this provider
          */
     protected UserModel findOrCreateAuthenticatedUser(RealmModel realm, String username) {
-        UserModel user = session.userLocalStorage().getUserByUsername(username, realm);
+        UserModel user = session.userLocalStorage().getUserByUsername(realm, username);
         if (user != null) {
             logger.debug("SSSD authenticated user " + username + " found in Keycloak storage");
 
@@ -130,12 +130,12 @@ protected UserModel importUserToKeycloak(RealmModel realm, String username) {
     }
 
     @Override
-    public UserModel getUserById(String id, RealmModel realm) {
+    public UserModel getUserById(RealmModel realm, String id) {
         return null;
     }
 
     @Override
-    public UserModel getUserByEmail(String email, RealmModel realm) {
+    public UserModel getUserByEmail(RealmModel realm, String email) {
         return null;
     }
 

diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java
@@ -380,6 +380,6 @@ public int hashCode() {
     }
 
     private UserModel getUserModel() {
-        return userProviderCache.getDelegate().getUserById(cached.getId(), realm);
+        return userProviderCache.getDelegate().getUserById(realm, cached.getId());
     }
 }