Christian Ringele
created MGNLFORUM-270
DefaultForumManager#isModerator() checks only the directly attached roles of the user, but ignores his roles inherited from his groups
Issue Type:
Bug
Affects Versions:
3.4.1, 3.3.3
Assignee:
Unassigned
Attachments:
DefaultForumManager.patch
Components:
moderation
Created:
11/Aug/14 5:20 PM
Description:
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.
CollectionString allRoles = currentUser.getAllRoles();
for (IteratorString 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.
Project:
Magnolia Forum Module
Labels:
support
Priority:
Critical
Reporter:
Christian Ringele
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
Forlistdetails,see:http://www.magnolia-cms.com/community/mailing-lists.html
Alternatively,useourforums:http://forum.magnolia-cms.com/
Tounsubscribe,E-mailto:dev-list-unsubscr...@magnolia-cms.com
