[jira] [Comment Edited] (WW-4117) RolesInterceptor ignores disallowedRoles when allowedRoles are configured

["Dave Newton (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/WW-4117?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13688147#comment-13688147 ] 

Dave Newton edited comment on WW-4117 at 6/19/13 6:56 PM:
----------------------------------------------------------

{{allowedRoles}} is a whitelist; it *should* ignore disallowedRoles if configured, no? IMO configuring *both* would be more confusing and error-prone.

I'd think it'd look roughly like:

{code}
    protected boolean isAllowed(HttpServletRequest request, Object action) {
        if (allowedRoles.size() > 0) {
            return processAllowedRoles();
        }

        if (disallowedRoles.size() > 0) {
            return processDisallowedRoles();
        }

        return true;
    }

    protected boolean processAllowedRoles() {
        for (String role : allowedRoles) {
            if (request.isUserInRole(role)) {
                return true;
            }
        }
        return false;
    }

    protected boolean processDisallowedRoles() {
       for (String role : disallowedRoles) {
            if (request.isUserInRole(role)) {
                return false;
            }
        }
        return true;
    }
{code}

(Delta an additional refactor if it's important.)

                
      was (Author: newton_dave):
    It looks like the issue is the {{return true}} part, it should be {{return result}}.

{{allowedRoles}} is a whitelist; it *should* ignore disallowedRoles if configured, no? IMO configuring *both* would be more confusing and error-prone.

I'd think it'd look roughly like:

{code}
    protected boolean isAllowed(HttpServletRequest request, Object action) {
        if (allowedRoles.size() > 0) {
            return processAllowedRoles();
        }

        if (disallowedRoles.size() > 0) {
            return processDisallowedRoles();
        }

        return true;
    }

    protected boolean processAllowedRoles() {
        for (String role : allowedRoles) {
            if (request.isUserInRole(role)) {
                return true;
            }
        }
        return false;
    }

    protected boolean processDisallowedRoles() {
       for (String role : disallowedRoles) {
            if (request.isUserInRole(role)) {
                return false;
            }
        }
        return true;
    }
{code}

(Delta an additional refactor if it's important.)


                  
> RolesInterceptor ignores disallowedRoles when allowedRoles are configured
> -------------------------------------------------------------------------
>
>                 Key: WW-4117
>                 URL: https://issues.apache.org/jira/browse/WW-4117
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Interceptors
>            Reporter: Cam Morris
>         Attachments: patch.txt
>
>
> The isAllowed method of RolesInterceptor does not enforce the disallowedRoles when allowedRoles are configured.  ex:
> {code}    
> <interceptor-ref name="roles">
>   <param name="allowedRoles">authenticated</param>
>   <param name="disallowedRoles">restrictedUser</param>
> </interceptor-ref>
> {code}
> With the above configuration a user with the roles "authenticated", and "restrictedUser" would be granted access.

--
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