Question

We are using securityTrimming in our ASP.NET web site and using site map to show/hide menus. But the problem is for every post back it keeps coming to this class and go through IsAccessibleToUser method.

Since we are using active directory groups it's really a performance issue. (I'm already caching the groups (a user is belongs to) when the first call comes when get the groups from AD, but still its taking time to execute this method.

It would be great if some body suggest me other ways to improve performance of this method, or not to call this method for each post back. As of now and as I understand this method automatically get executed from site map and the menu.

Web.config:

<siteMap defaultProvider="CustomSiteMapProvider" enabled="true">
      <providers>
        <clear/>
        <add siteMapFile="Web.sitemap" name="CustomSiteMapProvider" type="xxx.CustomSiteMapProvider"
                   description="Default SiteMap provider."  securityTrimmingEnabled="true"/>
      </providers>
    </siteMap>

Class file..

public class CustomSiteMapProvider : System.Web.XmlSiteMapProvider
    {

        public override bool IsAccessibleToUser(System.Web.HttpContext context,    System.Web.SiteMapNode node)
        {
          // return true false depend on user has access to menu or not.
          // return UserIsInRole(string role, string userName);
        }
    }

This is how we get the Roles from AD and cache them. ( I got the base of this code is from another article)

public class SecurityHelpler2 : WindowsTokenRoleProvider
    {
        /// <summary>
        /// Retrieve the list of roles (Windows Groups) that a user is a member of
        /// </summary>
        /// <remarks>
        /// Note that we are checking only against each system role because calling:
        /// base.GetRolesForUser(username);
        /// Is very slow if the user is in a lot of AD groups
        /// </remarks>
        /// <param name="username">The user to check membership for</param>
        /// <returns>String array containing the names of the roles the user is a member of</returns>
        public override string[] GetRolesForUser(string username)
        {
            // contain the list of roles that the user is a member of
            List<string> roles = null;


            // Create unique cache key for the user
            string key = username.RemoveBackSlash();

            // Get cache for current session
            Cache cache = HttpContext.Current.Cache;

             // Obtain cached roles for the user
             if (cache[key] != null)
             {
                roles = new List<string>(cache[key] as string[]);
             }

            // is the list of roles for the user in the cache?
            if (roles == null)
            {
                // create list for roles 
                roles = new List<string>();
                Dictionary<string, string> groupNames = new Dictionary<string, string>();


                // check the groups are available in cache
                if (cache[Common.XXX_SEC_GROUPS] != null)
                {
                    groupNames = new Dictionary<string, string>(cache[Common.XXX_SEC_GROUPS] as Dictionary<string, string>);
                }
                else
                {
                    // if groups are not available in the cache get again
            // here we are getting the valid group from web config  
                    // also add to the cache inside this method
                    groupNames = Utility.GetRetailSecurityGroups();
                }

                // For each  role, determine if the user is a member of that role
                foreach (KeyValuePair<String,String> entry in groupNames)
                {
                    if (base.IsUserInRole(username, entry.Value))
                    {
                        roles.Add(entry.Value);
                    }
                }

                // Cache the roles for 1 hour
                cache.Insert(key, roles.ToArray(), null, DateTime.Now.AddHours(1), Cache.NoSlidingExpiration);

            }

            // Return list of roles for the user
            return roles.ToArray();
        }
    }
}

And finally I call following method from IsAccessibleToUser method.

/// <summary>
    /// Get the usr role from the cache and check the role exists
    /// </summary>
    /// <param name="role"></param>
    /// <param name="userName"></param>
    /// <returns>return true if the user is in role</returns>
    public static bool UserIsInRole(string role, string userName)
    {
        // contains the list of roles that the user is a member of
        List<string> roles = null;

        // Get cache for current session
        Cache cache = HttpContext.Current.Cache;
        string key = userName.RemoveBackSlash();

        // Obtain cached roles for the user
        if (cache[key] != null)
        {
            roles = new List<string>(cache[key] as string[]);
        }
        else
        {
            // if the cache is null call the method and get the roles.
            roles = new List<string>(new SecurityHelpler2().GetRolesForUser(userName) as string[]);
        }

        if (roles.Count > 0)
        {
            return roles.Contains(role);
        }

        return false;
    }
Was it helpful?

Solution

By the design of SiteMapProvider, IsAccessibleToUser will always be called. If it were not to call it, it would have to cache the results of the previous call. A SiteMapProvider cannot decide if in your case it is correct to cache the results or not. That is your decision. Any caching you need will have to be inside your implementation.

I believe the function where you are fetching data from Active Directory is in SecurityHelpler2().GetRolesForUser

Any call to this function would be quite slow. The rest of the code where you are fetching from cache should be quite fast.

Since your cache is valid only for 1 hour, every hour one hit by the user would be very slow.

If you already know the users of your site(and it is not a very huge number), to speed it up you could pre load the Cache for all users. For active users, sliding expiration would be better. That way user would have the same roles till they are active. On next logon newer roles would be loaded from Active directory.

OTHER TIPS

I'd recommend implementing a custom IAclModule instead of overriding the provider. In this module you can write any logic for IsAccessibleToUser method. Same result, just more elegant.

See an example here: https://github.com/maartenba/MvcSiteMapProvider/wiki/Security-Trimming

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top