I have a server with many methods that can be requested by a user (or other service). I want to implement a role based access control. I can think of 2 ways to do it.
Inline: Each method has
checkRole(req.role, 'some role')
as its first line.Centralized: I have an object called
RoleMethods
that has, for each role, the list of allowed methods. Each request, before being routed to the method, will first callcheckRoleAndMethod(req.role, req.method)
.
(I'm guessing these are the 2 ways to do it, if they are bad and there's another better way please tell)
- Pros of inline: each method explicitly says what role it requires, making it clear and readable
- Cons of inline: devs have to include the line in every method, causing some repetition and they may forget it causing vulnerabilities.
- Pros of centralized: avoids the cons of inline by centralizing into one step
- Cons of centralized: the role for each method is defined outside of it, making it harder to grasp and follow the logic. You'll need extra middleware to extract and check the method name (instead of just routing using a lib like apollo server). Devs may forget to include a method name, leading to the method being unusable.
Which way is considered best practice among professionals? Are there any other pros and cons I'm missing?
I personally prefer inline because I think the permission level is something that belongs in the method itself, and having to keep track of an extra RoleMethods object is annoying.
-
Some frameworks use three levels: Methods check for attributes, roles have/grant attributes and roles are assigned to either Users or Groups of Users. Having the extra tier of indirection, allows roles to be remapped at run time / more flexibility in role design.DavidT– DavidT08/26/2024 15:04:21Commented Aug 26, 2024 at 15:04
-
Which programming language are you using?Doc Brown– Doc Brown08/26/2024 16:46:08Commented Aug 26, 2024 at 16:46
-
@DocBrown typescript in node.js. But my question is in generalJuan Perez– Juan Perez08/26/2024 20:04:15Commented Aug 26, 2024 at 20:04
2 Answers 2
The problem I see with variation #1 is the use of stringly-typed code. This isn't completely fatal to your application, though. Some application frameworks practically mandate a string. For example, the AuthorizeAttribute in C# allows you to decorate a controller method with the names of the roles allowed to execute that method. The C# language requires a compile-time constant, and the framework literally asks for a string. There are use cases for passing a string for the role.
Variation #2 is better. Presumably you are using enums or constants, but the problem I see here is how messy the code gets if all invocations of checkRoleAndMethod
are being passed hard-coded values. If you are not careful, passing constants or enums can allow a caller to accidentally pass invalid combinations of roles and methods when the intent is to allow authorization.
I've found it useful to create explicit methods for authorizing a user. These methods would be named after the operation you are authorizing, for example, "canEditBlogPost". The arguments you pass to these methods would be some minimal information about the user so you can access the claims or roles they have. The "canEditBlogPost" method can hard-code the names of the roles it requires inside the mthod body. Callers should remain largely ignorant of which roles are needed. They simply ask this authorizer whether the specified user can perform a specific action.
This structure allows you to organize your code into cohesive classes to encapsulate permission checks. It prevents callers from passing a bunch of constants and enums around, and certainly prevents blatantly invalid combinations of roles and actions. A good IDE will guide the code author using auto-complete. Pulling these permission checks into their own classes gives you a way to write proper unit tests, provided you inject interfaces for dependencies that require file system, network, or database access.
This does not work so well if a framework or use case requires the name of the role or method to be parameterized. Note that having a method with parameters doesn't mean it needs to be parameterized. If you are calling checkRoleAndMethod(RoleMethod.editBlogPost, Role.foo)
, then the code author has already made the decision about which role and method is being checked. It is much easier and more expressive to simply write canEditBlogPost(user)
.
An ideal solution should:
- Reduce or eliminate the need for hard-coding constants or enum values.
- Facilitate unit testing and mocking.
- Prevent code authors from accidentally specifying the wrong roles for a particular action.
- Make it easy and expressive for callers.
In the realm of authorisation (specifically policy based ones), there are a few entities that are typically used and I think it helps to think along those lines.
- PDP - Policy Decision Point - The place where the decision to
allow
ordeny
is made. - PEP - Policy Enforcement Point - The place where the decision is enforced, i.e. the request is either rejected or allowed. In a typical MVC web application, this tends to be the controller.
Usually, PEP is the one which receives incoming requests, identifies who is making the request (i.e. authentication) and checks with PDP whether this authenticated entity is allowed to make the current request or not. If yes, requests proceeds else it is rejected.
There are a couple more entities like PIP (Policy Information Point) and PAP (Policy Administration Point) but those are probably not needed in the current context.
In my experience, it is better to keep PDP and PEP separate, which effectively means centralising the decision making. The advantage is that the authorisation can evolve independent of the core business logic. Anytime a new role has to be added with permissions to allow a bunch of actions or permissions for some existing role needs to be updated, you do not have to update code for all the action methods affected by this policy change.
Taking it one step further, if at some point you were to move to dynamic policies instead of hardcoding them in the code, this separation becomes even more useful as it would be the place where you will have the code to fetch the dynamic policies and apply them to the current request. If needed the authorisation system can actually be an external system altogether.
While you might feel that having the authorisation code inline makes the code more readable in terms of sequential glance (and it is true to a large extent) it is better to change the mental model to think of authorisation as a separate step before your code is executed. Just like you do not do URL parsing or routing yourself and expect that some other piece of code does it for you, you expect that the decision to allow or disallow the current request will be done by someone else and you just have to honour it.
There is, however, one scenario where inline is definitely better. If your application is such that the roles and permissions are very much frozen and you do not foresee those changing for years, then the added complexity of externalising the authorisation will be an overkill and unnecessary. Like you said, inline is simple - to both implement and maintain. So for systems where the permissions model is not going to change for significant durations, inline will work just fine.