-
Bug
-
Resolution: Fixed
-
Critical
-
3.3.3, 3.4.1
-
-
Yes
-
Empty show more show less
When trying to edit a form message, not only the ACL is checked, but also the method isModerator() is called on the DefaultForumManager.
This method only checks if the user has the roles "forum_ALL-admin" and "forum_ALL-moderator" directly assigned to the user. (currentUser.hasRole()).
But it is not checking, if the user has this role "inherited" be a group he is part of.
This means, if you have the role "forum_ALL-admin" only as a role of a group, you won't be able to edit a message, even if you have the content access rights to the data.
This is a big problem for all AD/LDap users. As AD/LDap users are matched by their user name or user group, one can not directly assign a role to a ad user, only groups. So even if the logged in AD user has the role by one if its group, he can not edit a message.
Former code:
@Override
public void isModerator() throws AccessDeniedException{
User currentUser = MgnlContext.getUser();
if (!currentUser.hasRole(ROLE_FORUM_ALL_MODERATOR) && !currentUser.hasRole(ROLE_FORUM_ALL_ADMIN)) {
throw new AccessDeniedException("User not allowed to perform that action.");
}
}
Should be changed to:
@Override
public void isModerator() throws AccessDeniedException{
User currentUser = MgnlContext.getUser();
boolean hasRole = false;
// Needs to use getAllRoles() instead of .hasRole() because .hasRole() will only check for the roles directly attached to the user, but not the ones inherited from the group.
// As roles can not directly be attached to a AD user, it is crucial to be able to define it over its group.
Collection<String> allRoles = currentUser.getAllRoles();
for (Iterator<String> iterator = allRoles.iterator(); iterator.hasNext();) {
String roleName = iterator.next();
if (roleName.equals(ROLE_FORUM_ALL_MODERATOR) || roleName.equals(ROLE_FORUM_ALL_ADMIN)) {
hasRole = true;
}
}
if (!hasRole) {
throw new AccessDeniedException("User not allowed to perform that action.");
}
}
The "currentUser.getAllRoles();" returns all roles also the ones form the user's groups.
I added the patch of the class.
But tests are failing because the mock user returns a empty list on .getAllRoles();
Test should be fixed accordingly.
