keycloak-aplcache
Changes
federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java 3(+0 -3)
services/src/main/java/org/keycloak/services/resources/admin/ClientRoleMappingsResource.java 19(+17 -2)
Details
diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java
index 4ddd142..d95abdb 100755
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java
@@ -229,7 +229,9 @@ public class LDAPFederationProvider implements UserFederationProvider {
List<UserModel> result = new ArrayList<>();
for (String username : usernames) {
UserModel kcUser = session.users().getUserByUsername(username, realm);
- if (!model.getId().equals(kcUser.getFederationLink())) {
+ if (kcUser == null) {
+ logger.warnf("User '%s' referenced by membership wasn't found in LDAP", username);
+ } else if (!model.getId().equals(kcUser.getFederationLink())) {
logger.warnf("Incorrect federation provider of user %s" + kcUser.getUsername());
} else {
result.add(kcUser);
diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java
index cd220b2..8713de7 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java
@@ -27,7 +27,6 @@ import java.util.Map;
import java.util.Set;
import org.jboss.logging.Logger;
-import org.keycloak.federation.ldap.LDAPConfig;
import org.keycloak.federation.ldap.LDAPFederationProvider;
import org.keycloak.federation.ldap.LDAPUtils;
import org.keycloak.federation.ldap.idm.model.LDAPDn;
@@ -42,11 +41,9 @@ import org.keycloak.federation.ldap.mappers.membership.LDAPGroupMapperMode;
import org.keycloak.federation.ldap.mappers.membership.MembershipType;
import org.keycloak.federation.ldap.mappers.membership.UserRolesRetrieveStrategy;
import org.keycloak.models.GroupModel;
-import org.keycloak.models.LDAPConstants;
import org.keycloak.models.ModelException;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserFederationMapperModel;
-import org.keycloak.models.UserFederationProviderModel;
import org.keycloak.models.UserFederationSyncResult;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.KeycloakModelUtils;
diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientRoleMappingsResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientRoleMappingsResource.java
index 45f51c3..7307d81 100755
--- a/services/src/main/java/org/keycloak/services/resources/admin/ClientRoleMappingsResource.java
+++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientRoleMappingsResource.java
@@ -20,11 +20,14 @@ import org.jboss.resteasy.annotations.cache.NoCache;
import org.jboss.resteasy.spi.NotFoundException;
import org.keycloak.events.admin.OperationType;
import org.keycloak.models.ClientModel;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.ModelException;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleMapperModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.representations.idm.RoleRepresentation;
+import org.keycloak.services.ErrorResponseException;
import org.keycloak.services.ServicesLogger;
import javax.ws.rs.Consumes;
@@ -34,11 +37,14 @@ import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo;
+import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
+import java.util.Properties;
import java.util.Set;
/**
@@ -48,6 +54,7 @@ import java.util.Set;
public class ClientRoleMappingsResource {
protected static final ServicesLogger logger = ServicesLogger.ROOT_LOGGER;
+ protected KeycloakSession session;
protected RealmModel realm;
protected RealmAuth auth;
protected RoleMapperModel user;
@@ -55,8 +62,9 @@ public class ClientRoleMappingsResource {
protected AdminEventBuilder adminEvent;
private UriInfo uriInfo;
- public ClientRoleMappingsResource(UriInfo uriInfo, RealmModel realm, RealmAuth auth, RoleMapperModel user, ClientModel client, AdminEventBuilder adminEvent) {
+ public ClientRoleMappingsResource(UriInfo uriInfo, KeycloakSession session, RealmModel realm, RealmAuth auth, RoleMapperModel user, ClientModel client, AdminEventBuilder adminEvent) {
this.uriInfo = uriInfo;
+ this.session = session;
this.realm = realm;
this.auth = auth;
this.user = user;
@@ -182,7 +190,14 @@ public class ClientRoleMappingsResource {
if (roleModel == null || !roleModel.getId().equals(role.getId())) {
throw new NotFoundException("Role not found");
}
- user.deleteRoleMapping(roleModel);
+
+ try {
+ user.deleteRoleMapping(roleModel);
+ } catch (ModelException me) {
+ Properties messages = AdminRoot.getMessages(session, realm, auth.getAuth().getToken().getLocale());
+ throw new ErrorResponseException(me.getMessage(), MessageFormat.format(messages.getProperty(me.getMessage(), me.getMessage()), me.getParameters()),
+ Response.Status.BAD_REQUEST);
+ }
}
}
adminEvent.operation(OperationType.DELETE).resourcePath(uriInfo).representation(roles).success();
diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RoleMapperResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RoleMapperResource.java
index b324b5c..5bd67d1 100644
--- a/services/src/main/java/org/keycloak/services/resources/admin/RoleMapperResource.java
+++ b/services/src/main/java/org/keycloak/services/resources/admin/RoleMapperResource.java
@@ -22,6 +22,7 @@ import org.keycloak.common.ClientConnection;
import org.keycloak.events.admin.OperationType;
import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.ModelException;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleMapperModel;
import org.keycloak.models.RoleModel;
@@ -29,6 +30,7 @@ import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.representations.idm.ClientMappingsRepresentation;
import org.keycloak.representations.idm.MappingsRepresentation;
import org.keycloak.representations.idm.RoleRepresentation;
+import org.keycloak.services.ErrorResponseException;
import org.keycloak.services.ServicesLogger;
import org.keycloak.services.managers.RealmManager;
@@ -42,11 +44,15 @@ import javax.ws.rs.Produces;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriInfo;
+
+import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Properties;
import java.util.Set;
/**
@@ -238,7 +244,14 @@ public class RoleMapperResource {
if (roleModel == null || !roleModel.getId().equals(role.getId())) {
throw new NotFoundException("Role not found");
}
- roleMapper.deleteRoleMapping(roleModel);
+
+ try {
+ roleMapper.deleteRoleMapping(roleModel);
+ } catch (ModelException me) {
+ Properties messages = AdminRoot.getMessages(session, realm, auth.getAuth().getToken().getLocale());
+ throw new ErrorResponseException(me.getMessage(), MessageFormat.format(messages.getProperty(me.getMessage(), me.getMessage()), me.getParameters()),
+ Response.Status.BAD_REQUEST);
+ }
adminEvent.operation(OperationType.DELETE).resourcePath(uriInfo, role.getId()).representation(roles).success();
}
@@ -253,7 +266,7 @@ public class RoleMapperResource {
throw new NotFoundException("Client not found");
}
- return new ClientRoleMappingsResource(uriInfo, realm, auth, roleMapper, clientModel, adminEvent);
+ return new ClientRoleMappingsResource(uriInfo, session, realm, auth, roleMapper, clientModel, adminEvent);
}
}
diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java
index 9b03535..ad7feec 100755
--- a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java
+++ b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java
@@ -939,7 +939,14 @@ public class UsersResource {
if (group == null) {
throw new NotFoundException("Group not found");
}
- if (user.isMemberOf(group)) user.leaveGroup(group);
+
+ try {
+ if (user.isMemberOf(group)) user.leaveGroup(group);
+ } catch (ModelException me) {
+ Properties messages = AdminRoot.getMessages(session, realm, auth.getAuth().getToken().getLocale());
+ throw new ErrorResponseException(me.getMessage(), MessageFormat.format(messages.getProperty(me.getMessage(), me.getMessage()), me.getParameters()),
+ Response.Status.BAD_REQUEST);
+ }
}
@PUT
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPGroupMapperTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPGroupMapperTest.java
index 45afc47..89abefc 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPGroupMapperTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPGroupMapperTest.java
@@ -31,6 +31,7 @@ import org.junit.runners.MethodSorters;
import org.keycloak.federation.ldap.LDAPFederationProvider;
import org.keycloak.federation.ldap.LDAPFederationProviderFactory;
import org.keycloak.federation.ldap.LDAPUtils;
+import org.keycloak.federation.ldap.idm.model.LDAPDn;
import org.keycloak.federation.ldap.idm.model.LDAPObject;
import org.keycloak.federation.ldap.mappers.membership.LDAPGroupMapperMode;
import org.keycloak.federation.ldap.mappers.membership.MembershipType;
@@ -300,6 +301,47 @@ public class LDAPGroupMapperTest {
}
}
+
+ // KEYCLOAK-2682
+ @Test
+ public void test04_groupReferencingNonExistentMember() {
+ KeycloakSession session = keycloakRule.startSession();
+ try {
+ RealmModel appRealm = session.realms().getRealmByName("test");
+
+ UserFederationMapperModel mapperModel = appRealm.getUserFederationMapperByName(ldapModel.getId(), "groupsMapper");
+ FederationTestUtils.updateGroupMapperConfigOptions(mapperModel, GroupMapperConfig.MODE, LDAPGroupMapperMode.LDAP_ONLY.toString());
+ appRealm.updateUserFederationMapper(mapperModel);
+
+ // 1 - Add some group to LDAP for testing
+ LDAPFederationProvider ldapProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
+ GroupLDAPFederationMapper groupMapper = FederationTestUtils.getGroupMapper(mapperModel, ldapProvider, appRealm);
+ LDAPObject group2 = FederationTestUtils.createLDAPGroup(session, appRealm, ldapModel, "group2", descriptionAttrName, "group2 - description");
+
+ // 2 - Add one existing user rob to LDAP group
+ LDAPObject robLdap = ldapProvider.loadLDAPUserByUsername(appRealm, "robkeycloak");
+ LDAPUtils.addMember(ldapProvider, MembershipType.DN, LDAPConstants.MEMBER, group2, robLdap, false);
+
+ // 3 - Add non-existing user to LDAP group
+ LDAPDn nonExistentDn = LDAPDn.fromString(ldapProvider.getLdapIdentityStore().getConfig().getUsersDn());
+ nonExistentDn.addFirst(robLdap.getRdnAttributeName(), "nonexistent");
+ LDAPObject nonExistentLdapUser = new LDAPObject();
+ nonExistentLdapUser.setDn(nonExistentDn);
+ LDAPUtils.addMember(ldapProvider, MembershipType.DN, LDAPConstants.MEMBER, group2, nonExistentLdapUser, true);
+
+ // 4 - Check group members. Just existing user rob should be present
+ groupMapper.syncDataFromFederationProviderToKeycloak();
+ GroupModel kcGroup2 = KeycloakModelUtils.findGroupByPath(appRealm, "/group2");
+ List<UserModel> groupUsers = session.users().getGroupMembers(appRealm, kcGroup2, 0, 5);
+ Assert.assertEquals(1, groupUsers.size());
+ UserModel rob = groupUsers.get(0);
+ Assert.assertEquals("robkeycloak", rob.getUsername());
+
+ } finally {
+ keycloakRule.stopSession(session, false);
+ }
+ }
+
private void deleteGroupMappingsInLDAP(GroupLDAPFederationMapper groupMapper, LDAPObject ldapUser, String groupName) {
LDAPObject ldapGroup = groupMapper.loadLDAPGroupByName(groupName);
groupMapper.deleteGroupMappingInLDAP(ldapUser, ldapGroup);
diff --git a/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js b/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js
index 4ca5bf8..33a5d2b 100755
--- a/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js
+++ b/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js
@@ -59,6 +59,12 @@ module.controller('UserRoleMappingCtrl', function($scope, $http, realm, user, cl
$scope.selectedClientMappings = [];
}
Notifications.success("Role mappings updated.");
+ }).error(function(response) {
+ if (response && response['error_description']) {
+ Notifications.error(response['error_description']);
+ } else {
+ Notifications.error("Failed to remove role mapping");
+ }
});
};
@@ -87,6 +93,12 @@ module.controller('UserRoleMappingCtrl', function($scope, $http, realm, user, cl
$scope.realmComposite = CompositeRealmRoleMapping.query({realm : realm.realm, userId : user.id});
$scope.realmRoles = AvailableRealmRoleMapping.query({realm : realm.realm, userId : user.id});
Notifications.success("Role mappings updated.");
+ }).error(function(response) {
+ if (response && response['error_description']) {
+ Notifications.error(response['error_description']);
+ } else {
+ Notifications.error("Failed to remove role mapping");
+ }
});
};
@@ -1170,6 +1182,12 @@ module.controller('UserGroupMembershipCtrl', function($scope, $route, realm, gro
UserGroupMapping.remove({realm: realm.realm, userId: user.id, groupId: $scope.selectedGroup.id}, function() {
Notifications.success('Removed group membership');
$route.reload();
+ }, function(response) {
+ if (response.data && response.data['error_description']) {
+ Notifications.error(response.data['error_description']);
+ } else {
+ Notifications.error("Failed to leave group");
+ }
});
};