[MGNLFORUM-270] DefaultForumManager#isModerator() checks only the directly attached roles of the user, but ignores his roles inherited from his groups Created: 11/Aug/14 Updated: 28/Aug/14 Resolved: 12/Aug/14 |
|
| Status: | Closed |
| Project: | Forum (closed) |
| Component/s: | moderation |
| Affects Version/s: | 3.3.3, 3.4.1 |
| Fix Version/s: | 3.3.4, 3.4.3 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Christian Ringele | Assignee: | Milan Divilek |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | maintenance, next, support | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||
| Issue Links: |
|
||||
| Template: |
|
||||
| Patch included: |
Yes
|
||||
| Acceptance criteria: |
Empty
|
||||
| Date of First Response: | |||||
| 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()). 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. |
| Comments |
| Comment by Peili Liang [ 13/Aug/14 ] |
|
The master fixVersion should be 3.4.3, it will be created when we release 3.4.2. |