How do I implement role permissions in the service layer?

Question

In my current project I have a Report class, and I am going to implement a service layer for it. Every method will be allowed only for some roles. Like this:

public class ReportService : IReportService
{
    public void CreateReport(Report report, User user)
    {
        if(user.HasRole("Admin"))
        {
            throw new SecurityException("You do not have permissions to do it");
        }

        ...
    }

    public void ModifyReport(Report report, User user)
    {
        if(user.HasRole("Admin"))
        {
            throw new SecurityException("You do not have permissions to do it");
        }

        ...
    }

    public bool ValidateReport(Report report, User user)
    {
        if (!user.HasRole("Manager"))
        {
            throw new SecurityException("You do not have permissions to do it");
        }
    }

    public void DeleteReport(Report report, User user)
    {
        if (user.HasRole("Employee") || user.HasRole("Manager"))
        {
            throw new SecurityException("You do not have permissions to do it");
        }
        ...
    }
}

Is this straightforward approach OK? Do you have some better ideas?
I have just an idea to implement it as the decorator pattern, so the code will be like this:

public class ReportService : IReportService
{
    public void CreateReport(Report report, User user)
    {
      // Actual implementation
    }

    public void ModifyReport(Report report, User user)
    {
        // Actual implementation
    }

    public bool ValidateReport(Report report, User user)
    {
        // Actual implementation
    }

    public void DeleteReport(Report report, User user)
    {
        // Actual implementation
    }
}

public class PermissionsReportService : IReportService
{
    private IReportSerivce _service;
    public PermissionsReportService(IReportService service)
    {
        _service = service;
    }

    public void CreateReport(Report report, User user)
    {
        if (user.HasRole("Admin"))
        {
            throw new SecurityException("You do not have permissions to do it");
        }

        _service.CreateReport(report, user);
    }

    public void ModifyReport(Report report, User user)
    {
        if (user.HasRole("Admin"))
        {
            throw new SecurityException("You do not have permissions to do it");
        }
        _service.ModifyReport(report, user);
    }

    public bool ValidateReport(Report report, User user)
    {
        if (!user.HasRole("Manager"))
        {
            throw new SecurityException("You do not have permissions to do it");
        }

        _service.ValidateReport(report, user);
    }

    public void DeleteReport(Report report, User user)
    {
        if (user.HasRole("Employee") || user.HasRole("Manager"))
        {
            throw new SecurityException("You do not have permissions to do it");
        }

        _service.DeleteReport(report, user);
    }
}

It seems better because because role permissions checking and other logic now is splitted, doesn't it?

I also have another concern. Most likely this service will be used in ASP.NET MVC project and Microsoft proposes to use Authorize attributes for different role permissions, like

 public class ReportController : Controller
{
    [HttpGet]
    [Authorize(Roles="Admin")]
    public void DeleteReport(int reportId)
    {

    }
}

If I use this approach I will duplicate role permissions logic (inside the service layer and inside the controller). What should I do? Do I need to refuse Microsoft's approach? Or do I need to remove my role permissions checking in the service layer (but in this case what do I do if my service will be used in (for example) console application)? Or is it OK to have this code duplication and have permissions checks in the service and web layers?

Answer 1

I would say to ditch both in favor for a class which handles all permissions. Your classes would then pass a string identifier of the permission you want to check and your class would perform a lookup to see what roles are required to perform that action.

For instance, rather than have a method like:

public void CreateReport(Report report, User user)
{
    if(user.HasRole("Admin"))
    {
        throw new SecurityException("You have not permissions to do it");
    }

    ...
}

I would expect something more like:

public void CreateReport(Report report, UserAuth auth)
{
    auth.Validate("report.create");  // throws SecurityException if invalid

    ...
}

UserAuth class would take a User instance, and throw a SecurityException if user doesn't have the roles required to perform that action. Ideally the link between action and what roles are required would be dynamically loaded from a database, though you can start off with a simple Dictionary.

I would still use the facade to separate the actual permission checking and the actual action, because in my humble opinion, that is a separation of concerns and should be kept in a separate class just the same.

Your controller would do the same. What's important is that the logic behind the permissions is in its own class. You're still having to call it, but you don't have to care about anything other than what the permission you're trying to lookup is called. Checking if a user has employee or administrator role for every method call which merits such a check, for instance, is sort of an antipattern.

I hope that helps!