Re: [jira] Commented: (JS2-21) Missing Security Feature: Check roles assigned to any group to user belongs

[David Sean Taylor]

On Nov 1, 2007, at 7:34 AM, prasana wrote:

Thanks for the reply.

I looked at those methods.

In my custom SecurityMappingHandler, for retrieving roles based on  
group
assigned to user, in the method of getRolePrincipals(String  
username), I am
getting all the groups for the user getGroupPrincipals(String  
username) and
for each group I am calling getRolesInGroup(String groupFullPathName)

The only issue that I am seeing currently is Roles that are mapped  
to Groups
assigned to user start showing up in User Detail Information Portlet
Ideally, there should not be a delete check box for these Roles as  
they are
not mapped directly to User. They are mapped to Groups that the  
user belongs
to.

When I tried to delete those Roles in User Detail Information  
Portlet, its
giving an error saying "The user 'user_name' does not exist. "

The 'Mapped principal' attribute needs to be made public

There is a InternalPrincipal.isMappingOnly, however there is not a  
public BasePrincipal equivalent (the base class for RolePrincipal,  
UserPrincipal, and GroupPrincipal)
I propose adding to the BasePrincipal interface:

    /**
     * <p>is this principal a security principal mapping or a real  
principal</p>
     * @return true if is a mapping
     */
    boolean isMapping();

and implementing it in BasePrincipalImpl.java:

    /** is this principal a mapping **/
    private boolean isMapping = false;


In looking at the transition from internal principals to the public  
facing principals, it appears that we sometimes lose the attributes  
isEnabled and isMapping
Looking at DefaultUserSecurityHandler.java, setEnabled is there, but  
not setIsMapping:

    public Principal getUserPrincipal(String username)
    {
        UserPrincipal userPrincipal = null;
        InternalUserPrincipal internalUser =  
securityAccess.getInternalUserPrincipal(username, false);
        if (null != internalUser)
        {
            userPrincipal = new UserPrincipalImpl 
(UserPrincipalImpl.getPrincipalNameFromFullPath 
(internalUser.getFullPath()));
            userPrincipal.setEnabled(internalUser.isEnabled());
        }
        return userPrincipal;
    }

so we can add a constructor  :

            userPrincipal = new UserPrincipalImpl 
(UserPrincipalImpl.getPrincipalNameFromFullPath 
(internalUser.getFullPath()), isMapping);


Looking at the DefaultRoleSecurityHandler.java, looks neither  
attribute is considered:


            rolePrincipal = new RolePrincipalImpl(RolePrincipalImpl
                    .getPrincipalNameFromFullPath 
(internalRole.getFullPath()));

and same for DefaultGroupSecurityHandler.java:

            groupPrincipal = new GroupPrincipalImpl(GroupPrincipalImpl
                    .getPrincipalNameFromFullPath 
(internalGroup.getFullPath()));

thus we would have:

            rolePrincipal = new RolePrincipalImpl(RolePrincipalImpl
                    .getPrincipalNameFromFullPath 
(internalRole.getFullPath()), internalGroup.isEnabled(),  
internalGroup.isMappingOnly());


and

            groupPrincipal = new GroupPrincipalImpl(GroupPrincipalImpl
                    .getPrincipalNameFromFullPath 
(internalGroup.getFullPath()), internalGroup.isEnabled(),  
internalGroup.isMappingOnly());

finally, in the administrative portlet, we can check the the  
isMapping method during iteration...

                    Iterator roles = roleManager.getRolesInGroup 
(selectedGroup).iterator();

Let me know if these proposed changes will help