0

in my app I'd like to add functionality for admins to go to the specific screen and make certain controllers/methods available for certain roles. Right now I'm using a build-in role check like

[Authorize(Roles = "APUL_Admin")]

So I changed that to be [AuthorizeExtended()] and I'm implementing it like that:

public class AuthorizeExtended : AuthorizeAttribute
    {
        protected override bool AuthorizeCore(HttpContextBase httpContext)
        {
            var isAuthorized = base.AuthorizeCore(httpContext);
            if (!isAuthorized)
            {
                return false;
            }
            // Point of interest **HERE**

            return true;
        }
    }

which is all pretty standard.

At this moment (HERE see above) from HttpContextBase I know user's roles, and controller and method. And I can go to the DB and make sure those roles has access to this controller/action.

Here is my problem: I don't want to go to the database for every request since it is slow and it is a lot of overhead for DB. What's the best way to deal with that? Cache it? I'm looking for implementation details.

Any help is appreciated. Thank you!

Pavel Kovalev
  • 5,405
  • 2
  • 40
  • 55

1 Answers1

2

Yes, the cache is what you need to avoid duplicated requests to the DB. Here is the basic implementation:

internal class CacheKey
{
    public string Role { get; set; }

    public string Controller { get; set; }

    public string Method { get; set; }

    public override bool Equals(Object obj)
    {
        CacheKey cmp = obj as CacheKey;
        if (cmp == null)
        {
            return false;
        }

        return Role == cmp.Role && Controller == cmp.Controller && Method == cmp.Method;
    }

    public override int GetHashCode()
    {
        // Overflow is fine, just wrap
        unchecked
        {
            int hash = 17;
            hash = hash * 23 + Role.GetHashCode();
            hash = hash * 23 + Controller.GetHashCode();
            hash = hash * 23 + Method.GetHashCode();
            return hash;
        }
    }
}

public class AuthorizeExtended : AuthorizeAttribute
{
    private static ConcurrentDictionary<CacheKey, bool> cache = new ConcurrentDictionary<CacheKey, bool>();

    protected override bool AuthorizeCore(HttpContextBase httpContext)
    {
        var isAuthorized = base.AuthorizeCore(httpContext);
        if (!isAuthorized)
        {
            return false;
        }
        // Point of interest **HERE**

        //  Looking up in the cache
        var cacheKey = new CacheKey
        {
            Role = role,
            Controller = controller,
            Method = method,
        };

        bool authorized;
        if (cache.TryGetValue(cacheKey, out authorized))
        {
            return authorized;
        }

        //  Make DB call and get value for authorized
        //  ...

        //  Store 'authorized' value in the cache
        cache.TryAdd(cacheKey, authorized);

        return authorized;
    }
}
CodeFuller
  • 27,446
  • 2
  • 46
  • 69
  • The only limitation is that this won't allow *admins to go to the specific screen and make certain controllers/methods available for certain roles* because the cache is not exposed. Since there is no real read/write contention a ConcurrentDictionary is kinda over kill. I would just use a Dictionary and store the value in `ControllerContext.HttpContext.Application.Item`. – Erik Philips Nov 08 '17 at 20:00
  • @CodeFuller Hey guys! Thank you for prompt response! I have a few questions here: 1. Do I need to use CacheKey? Can I just use string key = Role+Controller + Method? 2. It is a silly question, but I felt like I need a singleton to store that Dictionary. Am I missing something about lifecycle of a AuthorizeExtended attribute? 3. Last one. What if I changed permission for a user (not allowed to go to the method A anymore), how I'm suppose to purge this Dictionary? Should I use timestamp or something of that nature? Thanks! – Pavel Kovalev Nov 08 '17 at 20:35
  • Is it a good idea to use HttpRuntime.Cache for those purposes instead of private static member of AuthorizeExtended class? – Pavel Kovalev Nov 08 '17 at 23:36
  • Pavel, here are answers on your questions: 1. You can simplify it a bit by using string string key but it's usually better to have composite key as in my sample. Otherwise you have to consider some separator between key parts to avoid same result key for different input parts. And life will be easier if later you add some other non-string fields to the key. 2. cache is a static field that is shared between all instances of AuthorizeExtended class. – CodeFuller Nov 09 '17 at 04:39
  • 3. You could either introduce expiration for the cache or add method for deleting/updating cache entries and call it when actual permission change happens. In both cases I suggest extraction of cache functionality to separate class as it becomes more complicated. – CodeFuller Nov 09 '17 at 04:40
  • 1
    And yes, at this point you could use cache provided by ASP.NET. It has both expiration and remove capabilities. – CodeFuller Nov 09 '17 at 04:48
  • @CodeFuller Thank you sir! – Pavel Kovalev Nov 09 '17 at 22:53