[magnolia-dev] [JIRA] (MGNLFORUM-270) DefaultForumManager#isModerator() checks only the directly attached roles of the user, but ignores his roles inherited from his groups

Mon, 11 Aug 2014 08:25:46 -0700

Issue Type: Bug 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.
        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.
Project: Magnolia Forum Module
Labels: support
Priority: Critical 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


----------------------------------------------------------------
For list details, see: http://www.magnolia-cms.com/community/mailing-lists.html
Alternatively, use our forums: http://forum.magnolia-cms.com/
To unsubscribe, E-mail to: <[email protected]>
----------------------------------------------------------------

Reply via email to