26

Should user permission checks take place in the model or the controller? And who should handle the permission checks, the User object or some UserManagement helper?

Where should it happen?

Checking in the Controller:

class MyController {
  void performSomeAction() {
    if (user.hasRightPermissions()) {
      model.someAction();
    }
  }
  ...

Having the checks in the Controller helps making the Models simple actions, so we can keep all logic to the Controllers.

Checking in the Model:

class MyModel {
  void someAction() {
    if (user.hasRightPermissions()) {
      ...
    }
  }
  ...

By putting the checks in the Model, we complicate the Model, but also make sure we don't accidentally allow users to do stuff they aren't supposed to in the Controller.

And by who?

Once we've settled on the place, who should do the checks? The user?

Class User {
  bool hasPermissions(int permissionMask) {
    ...
  }
  ...

But it's not really the user's responsibility to know what he or she can acccess, so perhaps some helper class?

Class UserManagement {
  bool hasPermissions(User user, int permissionMask) {
    ...
  }
  ...

I know it's common to ask just a single question in, well, a question, but I think these can be answered nicely together.

20

As usual, "it depends"

  • permission checks will functionally work anywhere it's convenient to put them,
  • but if you're asking a technical question then the answer may be 'put the checks in the object that owns the data required to perform the check' (which is probably the controller).
  • but if you're asking a philosophical question, I suggest an alternate answer: don't show users actions that they are not permitted to perform.

So in the latter case you might have the permission check in the controller implemented as a boolean property, and bind that property to the Visible property of the button or panel in the user-interface that controls the action

as a user, it's frustrating to see buttons for actions that I cannot perform; feels like I'm being left out of the fun ;)

  • Our application implements the third scenario with the exception that we don't hide the controls, we disable them. Unfortunately it's all done in Winforms code-behind, so it's not really relevant to the OP question. – Dave Nay Dec 8 '13 at 1:36
  • 11
    "it's frustrating to see buttons for actions that I cannot perform" -> Try to upvote your own post :) – Rowan Freeman Sep 10 '14 at 7:03
  • 5
    It's not sufficient to simply hide buttons for actions that the user cannot perform, the server must check every request for permissions. The third bullet item is not "an alternate answer", it's something to do in addition to checking for permissions server-side. – Flimm Jul 27 '17 at 10:48
  • @Flimm agreed, if the requests are handled by a server; the specific question was about the Controller class – Steven A. Lowe Jul 28 '17 at 21:53
7

Security is a cross-cutting concern, therefore needs to be implemented in multiple layers. What follows is an example for MVC but the concept applies to other architectures and/or patterns, you just have to identify the points of enforcement.

Where should it happen?

Views might contain UI elements (widgets, buttons, menus etc.) that need to be displayed or not for some users, based on their permissions. This could be a responsibility of the view engine, since you don't want every view to handle this on its own. Depending on the type of elements you're doing authorization on you cold move this responsibility in another place. For example, think of a menu in which some items have to be displayed and some don't. The items can be implemented as a list somewhere and filter that list based on permissions then forward it to the view.

Controllers respond to requests, so if a user doesn't have the permission to execute an action it should be checked before the action is invoked, moving the responsibility to the action invoker instead of keeping it in the controller. This has the advantage of keeping your controller clean and if something changes in the permissions you don't have to sift through your controllers to apply those changes.

Resources are displayed based on permissions. This is normally done at database level, since you don't want to pull everything from the database and then apply permissions.

As you can see, depending on what you want to authorize there are different places where this should be done. The goal is to be as unobtrusive as possible, so that when your security policy changes you can easily apply it, preferably without altering your application's code. This might not be valid for small applications, where the permission set is fairly small and doesn't change very often. In enterprise applications though, the story is quite different.

Who should do it?

Clearly not the model. Each layer should have an enforcement point that handles the authorization. The italic text above highlights the possible enforcement point for each level.

Have a look at XACML. You don't have to implement it as is, but it will give you some directions that you could follow.

  • This is the best answer. For some reason the top one and others deal with the differences between controller and view, or view and model, which is not what OP is asking. Thanks! – redFur Mar 2 '18 at 14:21
1

I use following scheme. It worth to say that most user permissions checks can be divided into two general cases:

  • user access to controller action based on user role without checking parameters action is called with,
  • user access to model based on any logic or relations between particular user and particular model.

Access to controller action without checking of attributes are usually implemented in MVC frameworks. This is simple at all: you define rules, your users have role. You simply check that user has permission to action lookup up its role in rules.

User access to particular model should be defined in model. (Actor is base user class. Suppose it can be either customer, seller or guest.)

interface ICheckAccess
{
    public function checkAccess(Actor $actor, $role);
}

class SomeModel implements ICheckAccess
{
    public function checkAccess(Actor $actor, $role)
    {
        // Your permissions logic can be as sophisticated as you want.
    }
}

Placing that logic in model brings some profit. Access check method can be inherited, you don't need to create any extra classes, you can use general OOP advantages.

Next, to simplify access checking, we take some assumptions that are almost always implemented already for simplicity and good style:

  • usually controllers are related to some model class;
  • actions which are checked for access take single model id as parameter;
  • this parameter can always be accessed uniformly from method of base controller class;
  • action is placed in controller corresponding to model which id action takes.

With these assumptions, actions that use model id can be associated with particular model instance. In fact, most actions can easily be transformed and moved to fit assumptions stated above.

Then, some base abstract controller class should be defined and inherited.

abstract class ModelController
{
    // Retrieve model from database using id from action parameter.
    public abstract function loadModel($id);

    // Returns rules for user role to pass to SomeModel::checkAccess()
    // Something like array('view' => 'viewer', 'delete' => 'owner', 'update' => 'owner')
    public abstract function modelRules();

    public abstract fucntion getIdParameter();

    public function filterModelAccess()
    {
        $id = $this->getIdParameter();
        if(!$this->checkModelAccess($id))
            throw new HttpException(403);
    }

    public function checkModelAccess($id)
    {
        $model = $this->loadModel($id);
        $actor = My::app()->getActor();
        $rules = $this->modelRules();
        $role = $rules[My::app()->getActionName()];
        return $model->chechAccess($actor, $role);
    }
}

You can call SomeController::checkModelAccess($id) method when you construct your menus and decide whether to show some link.

1

In both Model and View

In the View - because the UI should not show the UI-elements that are restricted to the current user

(like, say, the "Delete" button should be shown to people with appropriate permissions)

In the Model - because your app probably has some kind of API, right? The API has to check permissions as well and probably re-uses the Model.

(like, say, you have the "Delete" button in the UI and the "http:/server/API/DeleteEntry/123" API method at the same time

  • Why did you choose the model over the controller? – Flimm Jul 27 '17 at 10:49
  • not sure why view, model and not in the controller, where most of the time it is done. – VP. Aug 16 '18 at 6:46
  • @VP the controller has no power to show/hide UI elements (other than passing a bool-var TO THE VIEW) – jitbit Aug 16 '18 at 17:42
  • I don't know, everywhere is normally done in the controller layer, that's why I was curious. – VP. Aug 17 '18 at 8:53
0

MVC is a presentation pattern. As such view and controller should only have responsibilities regarding presentation. Some permissions apply to the presentation, like an expert mode, experimental UI features or different designs. Those can be handled by the MVC-controller.

Many other kinds of permissions are relevant on several layers of the application. For example if you want to have users that can only view data and not change things:

  • the presentation layer has to hide the editing features
  • If an editing feature is called anyway this could/should be detected (by the application specific parts of the business layer, not the domain specific part of it - TrainEditor, not Train) and probably cause an exception
  • The data access layer can also check for writes too, but for more complex kinds of permissions that quickly requires too much knowledge of the business-layer to be a good idea.

There is some duplication in this approach. But as the presentation is usually volatile, one can make a good case for checking permission in the usually more stable part of application, even if that means some redundant checks in case the presentation layer works as intended.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.