Layer-Design: where to check permissions for database reads/updates?

Question

In most scenarios, you want a user only to be able to access entities in a database that was created by the user themselves. For example if there is a calendar that was created by User1, then only User1 should be able to read, update or delete this particular calendar and its contents in the database. This is not about authorization in general - in my project, there already is a role-based authorization component which checks if users belong to the "calendar-editor" role, but it doesn't check if a specific user is allowed to access a specific calendar.

So eventually there has to be a comparison between the user-id of the current request and the user-id that represents the owner of the calendar that is requested. But I'm wondering where to do this. My thoughts:

• I could do it on the the DAO-level. But then every DAO-method needs an additional parameter that represents the user-id which makes these methods more verbose and decreases re-usability.

  E.g.

  def findCalById(id: Int): Future[Option[Calendar]]

  becomes

  def findCalById(id: Int, ownerId: Int ): Future[Option[Calendar]]

  An advantage would be that the permission check is basically done on the query-level which means that if a user has no access to a calendar, no calendar is returned from the database. But then again: if no calendar is returned in some cases, how do you distinguish between a calendar that does not exist and an existing calendar that cannot be accessed by the current user? Two different scenarios, producing the same outcome.

• Another option could be to keep the DAO out of this and to do the check in a service layer or something similar. This would mean that the check is performed AFTER the requested calendar is returned by the DAO. This approach sounds more flexible than the other one, but it also means that in case a user has no access to the requested calendar, the requested calendar still consumes bandwidth and memory since it is fetched from the Database in either case.

Maybe there are additional options I didn't even consider. Are there any best practices?

Btw: there are no calendars in my web application, this was just an example to illustrate the issue.

Answer 1

I think, the key is to think about what exactly you mean when you say that the DAO approach "decreases reusability". If your requirement to enforce user access rights is universal to all applications of your DAO, then doing this at the DAO level actually increases reusability rather than reducing it: everybody using the DAO would be able to benefit from these checks rather than having to implement them on their own.

You can make the user id an implicit parameter to make these methods more friendly to upstream user. You could also make it return a Try (or, perhaps, an Either) to address your concern about distinguishing between a missing and an inaccessible object cases:

case class UserId(id: Int)
def findCalById(id: Int)(implicit user: UserId): Future[Try[Option[Calendar]]] = ???

Then, the caller could do something like this:

implicit val currentUser = UserId(request.getUserId)
dao.findCalById(request.getCalendarId).map { 
    case Failure(IllegalAccessException()) => "You are not allowed to use this calendar"
    case Return(None) => "Calendar not found"
    case Return(Some(cal)) => calendarToString(cal)
}

On the other hand, if there are possible cases where the DAO would be used without a user context (an "admin" application perhaps), then you might consider either subclassing it to provide the access control to your "regular applications", or, perhaps, simply making an additional role that would allow a user access to all calendars regarding of ownership, and then using that "superuser" in your admin application.

I would not worry about the cost of having to load the object before checking the access (even if the object is really expensive to load, it should be a rare enough occurrence that someone tries to access an object he does not own). I think, a stronger argument against the service layer approach is exactly reusability and modularity of the code: the very existence of the DAO class with a public interface suggests that it can, at least potentially, be reused by more than one component. It seems silly having to require all such components implement their own access checks, especially, considering that such contract would not be enforceable: there is no way to make sure that somebody, who decides to use your DAO class a couple of years from now, will remember about this requirement (or care to read the comment). If you are producing a layer for accessing the database anyway, you might as well make it useful for something.

Answer 2

I use the second approach. I will go only from an architectural point of view which I feel is more important than a usually small cost of query.

Some reasons include:

1. It is cleaner to have all validations/verifications in a single place. The code has a structure. There will be cases when some validations can not be performed in the DAO layer. And then it becomes ad-hoc, what are the validations go into the service layer and what in the DAO layer.

2. The method findCalById should only return Calendar by id using an id. It is more reusable. What if tomorrow you need a functionality that an admin can see all the calendars irrespective of the owner. You will end up writing another query for this feature. It will be more easy to add this check in a service layer.

3. Assuming someday you have another data store which returns the record, then you end up having validations in multiple places. It will not happen if there is a service layer to do the validations. The service layer will not change as it will not care from where the record comes.

4. Onboarding a new colleague becomes easier. Assuming a new guy who specializes in the DB domain starts to work with you. He will be more productive if he is only concerned with the records that the DB should return forgetting about how the application uses this data. (separation of concern applies in real life too :)).