Avoiding magic strings in ASP.NET MVC Authorize filters

Using the standard [Authorize] filter in ASP.NET MVC results in "magic strings"; comma-separated role names to define which roles are authorised to access that action. Take an example of a typical Forms Authentication setup, where you want to restrict an action to users in either the "Administrator" or "Assistant" role:

[Authorize(Roles = "Administrator,Assistant")]
public ActionResult AdminOrAssistant()
{                        
    return View();
}

This isn't ideal, we might land up peppering our controllers with these string constants, meaning any change to the role names can't be easily refactored. We should at least declare constants for the role names and re-use these in the [Authorize] filters so that we can use our refactoring tools:

public class MyController : Controller
{
    private const string AdministratorRole = "Administrator";
    private const string AssistantRole = "Assistant";

    [Authorize(Roles = AdministratorRole + "," + AssistantRole)]
    public ActionResult AdminOrAssistant()
    {                        
        return View();
    }
}

Whilst this is better as we are avoiding magic strings, it just doesn't look clean with the string concatenation, especially if we have a lot of roles. It's tempting to try set the "Roles" property to a value returned from a static method, but remember attributes can only contain constant expressions (which is why we have to use const strings for "AdministratorRole" and "AssistantRole"). We can do better.

If we only need to restrict access by role, as opposed to restricting access to particular users, we can use a simple and elegant solution by customising the standard [Authorise] filter with a new constructor that takes in a variable number of role name arguments, which we convert into the comma-separated string that the base class uses:

public class AuthorizeRolesAttribute : AuthorizeAttribute
{
    public AuthorizeRolesAttribute(params string[] roles) : base()
    {
        Roles = string.Join(",", roles);
    }
}

public class MyController : Controller
{
    private const string AdministratorRole = "Administrator";
    private const string AssistantRole = "Assistant";

    [AuthorizeRoles(AdministratorRole, AssistantRole)]
    public ActionResult AdminOrAssistant()
    {                        
        return View();
    }
}

I reckon this is much cleaner, what do you think?

Comments (4)

Hassan
Great solution man, thanks a bunch
Friday, 21 February 2014 23:44
Francis
Hi, thanks for a great solution. I made a separate static class for the role names to be used across several controllers.

Like;

public static class AppRoles {
public const string Admin = "Admin";
public const string Dev = "Dev;
}

And to seed the database with the roles, I used this nice bit to get all the roles to loop through:

var rolenames = typeof (AppRoles).GetFields(BindingFlags.Public | BindingFlags.Static).Where(fi => fi.IsLiteral && !fi.IsInitOnly).Select(f => f.Name).ToList();
Thursday, 30 October 2014 08:47
Olena
neat! It is 2014 now, and I still was in need of this solution.
Jono, thanks a lot!
Friday, 14 November 2014 00:47
Gelo32k
Thanks for this :) I used it as a reference for my own custom authorize attribute.
Monday, 24 November 2014 07:45
Add a Comment