How we handle user permissions in DoneDone

When talking about software design, handling permissions seems straightforward: Make sure a user can only access things they have rights to. There’s nothing inherently complex about it. But, creating an elegant coding strategy for permissions is hard—particularly as an application grows in scope.

For one, the actual logic gets more complex. What may have started as searching for one associative record in a relational database now becomes a series of different cascading conditional checks.

For example, in DoneDone, a user has permission to view a project if:

  • The account that the project belongs to is in good standing.
  • The project hasn’t been archived.
  • The user is an admin in the account the project belongs to…
  • …or the user has membership to that project.

This is actually one of DoneDone’s simpler permission checks. When we start getting deeper into the model, the checks quickly get more elaborate.

The other hard part is where—exactly—to handle these permission checks. Which layer is responsible for which check? Do business objects check themselves? Can we prevent checking the same permissions multiple times in a single request?

Over nearly a decade of shaping DoneDone’s codebase, I’ve prioritized these questions differently. Like so many things in coding, I have yet to find an approach that answers everything perfectly; I concede some goals for others. For DoneDone, I’ve prioritized my goals in this order (from most important to least):

  1. Make it obvious where a permissions check lives.
  2. Keep a check as DRY as possible, so it’s easy to update.
  3. Rationalize why a particular layer owns a check.
  4. Prevent checking the same thing multiple times on a single request.

With this prioritization in mind, here are a few general strategies that have worked well for me (including one I just stumbled across very recently).

Consolidate business permissions within a single class

The bulk of business logic checks are done in our services layer that sits between the MVC-patterned web application and the repository layer (where data-store interactions occur). The actual methods live in a single permissions repository class. The repository class holds a myriad of public methods (e.g. UserCanAccessProject(), UserCanAdminAccount(), etc.) that encapsulate all the cascading conditional checks (like the one I outlined above).

This keeps service methods really clean and readable. A simple example:

public List GetTagsForProject(int project_id, VerifiedRequester requester)
{
  if (!_permissions_repository.UserCanAccessProject(requester.UserID, requester.AccountID, project_id))
  {
    throw new AccessDenied("You do not have access to this project.");
  }

  return _projects_repository.GetAllTagsForProject(project_id);
}

Inside a permissions repository method, the cascading checks looks something like this (with calls to other methods holding other conditional checks):

public bool UserCanAccessProject(int current_user_id, int account_id, int project_id)
{
  if (!isProjectActive(project_id, account_id))
  {
    return false;
  }

  if (UserCanAdminAccount(current_user_id, account_id))
  {
    return true;
  }

  if (UserCanAccessAccount(current_user_id, account_id))
  {
    if (getProjectMembershipRoleID(project_id, current_user_id) != null)
    {
      return true;
    }
  }

  return false;
}

By consolidating permissions checks in one class, it’s obvious where to look if I need to debug something. Within the permissions class, each method has a very specific responsibility, keeping checks DRY and easy to modify wholesale, if necessary.

Separate account standing from user permissions

This was a recent revelation I made while refactoring the permissions layer for an upcoming new release of DoneDone (you’ll hear more about this in the months to come!).  Through a series of other fundamental changes we’re making, it became more obvious to me that a particular aspect of the permissions class just didn’t belong there. Namely, the account’s standing.

In DoneDone, an account can be active, suspended (due to a late payment) or canceled (there’s a short period of time between a cancelation and physically removing account data from our data stores). Up until recently, the account’s standing was checked within the permissions layer, intermingled with all other user permission checks. So, a project was considered active if it was marked active and its account was active. This seems fine and dandy, but exceptions started to creep up.

For example, if an account is suspended, account owners should still be able to access billing information so they can update their credit card accordingly, even if the rest of the app is locked down. The same is true for a handful of other job processes that run behind-the-scenes but still interface with the services layer. They need access to account data regardless of its standing.

This required building in extra service methods that ran different permissions checks but otherwise identical processes. It also made it unclear whether a particular permissions method was considering account standing. All in all, making the services layer responsible for handling account standing introduced some extra unsightly duct tape to cover up a few leaks.

To resolve this, I removed the entire concept of an account’s standing from the permissions layer. Instead, I moved the ownership of this check to the service implementers. In other words, the DoneDone web app and public API own it, not the service. The account standing is still accessible via the service, but the implementing app is in charge of what to do with it.

On a web app request cycle, for instance, the account’s standing is handled just before the execution of an action:

protected override void OnActionExecuting(ActionExecutingContext context)
{
  int account_id = SiteHelper.GetAccountIDFromHttpContext(context.HttpContext);

  try
  {
    _CurrentAccount = _accounts_service.GetAccountForSite(account_id);

    if (_CurrentAccount.IsCanceled)
    {
      throw new AccountCanceled();
    }
       
    if (_CurrentAccount.IsSuspended)
    {
      throw new AccountSuspended();
    }
  }
  catch (AccountCanceled)
  {
    // Redirect to login page
  }
  catch (AccountSuspended)
  {
    if (_LoggedInUser.IsAccountOwner)
    {
      // Redirect to billing page
    }
    // Redirect to login page
  }
}

Each implementer decides how to handle account standing individually. For example, the underlying job processes or our internal admin tool doesn’t check standing at all. They can freely access all the services revolving around accounts. Doing this also helps remove the duct tape around those exception cases when the service was responsible for handling.

This also makes business sense as well. An account’s standing never changes the rights a user has to anything within the account. It’s a superficial concern that an implementing app can choose to honor or not. That responsibility should be a concern of the implementer, not the service.

Don’t sweat redundant checks

The goal I’ve prioritized the least is ensuring a check is made only once on a single request. As the architecture evolved, trying to solve this problem well meant sacrificing too much else. A typical request might require calling a handful of service methods. Each service method, in turn, handles its own permissions checks without knowledge of what other calls are being made in that request. Assuming that a request returns a successful result, it’s normal for the same permissions check to have been called multiple times. (On failures, the first failed check would throw an exception and short-circuit the request).

I could solve this by moving all permissions checking as close to the surface as possible. Have the controller action own the permissions checks entirely so they aren’t repeated by isolated service method calls. But, doing this would result in repeated code all over the implementing layer and the service layer itself would be letting go of a large component of the business domain. That’s too steep a price to pay. I could also solve this by some clever tracking of permission method calls during a request cycle. But, there seems to be all sorts of encapsulation violations and general ickiness there as well.

In my opinion, the major justifiable concern of multiple calls is a performance hit. Fortunately, this can be solved in other ways. Most of these calls are inherently lightweight when they are based on primary key or indexed foreign key lookups.  I can also cache permission results for a small period of time so subsequent calls in the same request cycle are even lighter-weight, or cache things indefinitely and invalidate when necessary (something which admittedly can get hairy).


As I mentioned earlier, these are just general strategies that have worked well for me. It will continue to evolve as the product does. If you’re developing an app, I hope this insight helps you think through your own strategies with more clarity.

 

Ka Wai Cheung is the original creator of DoneDone and author of The Developer’s Code. Follow him personally on Twitter via @developerscode and read more at Life Imitates Code.