Acl module and multiple roles

Oct 18, 2009 at 8:45 PM

I've been working on one project and I've faced an incorrect behavior in acl module.

If sitemap node has defined "roles" attribute with multiple groups, then user will see this node only when he is a member of all of them. For example this node: 

<mvcSiteMapNode title="$resources:SiteMapLocalizations,AboutUsTitle" controller="Home" action="About" roles="Admins,Users,Other"/>

will be seen only by person who is a member of Admins, Users, Other all at the same time. This happens because of this code(DefaultMvcSiteMapAclModule.cs):

// check explicitly specified roles
if (mvcNode.Roles != null && mvcNode.Roles.Count > 0 && !mvcNode.Roles.Contains("*"))
{
  if (httpContext.User == null || mvcNode.Roles.OfType<string>().Any(role => role != "*" && !httpContext.User.IsInRole(role)))
  {
    return false;
  }
}

When user is just a member of "Admins" group, he won't see this node because of Any function, which returns true for "Users" and "Other" roles from mvcNode.Roles.

Is this a bug or some kind of feature and specific security policy?

P.S. I've changed call of "Any" function to "All" in my project and this seem to help.

Nov 4, 2009 at 2:39 PM

I'm experiencing the same issue.

Nov 23, 2009 at 9:31 PM

We had the same issue.  We did as Mongi suggested and changed line 57 of DefaultMvcSiteMapAclModule.cs to "All" instead of "Any", reversing the roles logic.  Now our pages correctly show the menu node if the user is in any of the roles listed in the Web.sitemap rather than all of them.

Dec 1, 2009 at 3:19 AM

 

In the same acl module, in the same method, if the node parameter passed in is not a MvcSiteMapNode type, the logic for authorization is different in the below section of code vs the code in the original post.  Is there a reason for this?

                    if (node.Roles != null && node.Roles.Count > 0)
                    {
                        foreach (string role in node.Roles)
                        {
                            // Grant access if one of the roles is a "*".
                            if (String.Equals(role, "*", StringComparison.InvariantCultureIgnoreCase))
                            {
                                return true;
                            }
                            else if (context.User != null && context.User.IsInRole(role))
                            {
                                return true;
                            }
                            else
                            {
                                return false;
                            }
                        }
                    }
                    else
                    {
                        return false;
                    }

 

 

 

 if (node.Roles != null && node.Roles.Count > 0)
                    {
                        foreach (string role in node.Roles)
                        {
                            // Grant access if one of the roles is a "*".
                            if (String.Equals(role, "*", StringComparison.InvariantCultureIgnoreCase))
                            {
                                return true;
                            }
                            else if (context.User != null && context.User.IsInRole(role))
                            {
                                return true;
                            }
                            else
                            {
                                return false;
                            }
                        }
                    }
                    else
                    {
                        return false;
                    }

 

 

Coordinator
Dec 14, 2009 at 4:18 PM
This discussion has been copied to a work item. Click here to go to the work item and continue the discussion.