views:

273

answers:

3

Is it bad design to mix code that deals with security logic in the model?

Example for editing a page in the before_save callback

  • The current user is grabbed from the current_user method in the Controller layer.
  • Throw exception if current_user.has_permission? :edit_page is false
  • The editor_id is set to current_user.id
  • The change is logged in a separate table

The model isn't the only security check in the application. The user interface checks for permission before display editing views. The model acts as a barrier against any bugs in the View/Controller level.

Note: The only breach between the Model and Controller levels is the current_user method. The application I'm working on will never allow anonymous users.

+1  A: 

I don't think it's bad design to put security logic in the model. You put business logic there and you could arguably view security logic as a sort of business logic. You certainly don't want it all in the controller or the view, you want to follow the skinny controller, fat model approach.

Your models should stand alone as a cohesive chunk of application logic. You should be able to completely drive your models from the Rails console. Also, having security logic in the model makes it easier to unit test.

John Topley
+1  A: 

I'd say it depends whether your models are intended to be accessible directly. If yes, then there should definitely be awareness of security concerns, probably via a mixin, since such concerns are likely to be somewhat orthogonal to the principal concerns of the model.

If the models are supposed to be invisible and you already have your security logic in your controllers, then I'd leave the models alone.

Mike Woodhouse
+2  A: 

The model in an MVC framework is supposed to completely contain all of your business logic. In a well designed MVC application, you should, in theory at least, be able to re-use your models in a different context without having to re implement any of your business logic.

In every case I can think of authorization, input validation, and audit logging are very definitely business logic, and so should be handled in your model.

On the other hand, I think authentication, encryption, cryptographic hashing, etc. are not part of the model. These aspects of security are not part of the core business logic, they are usually part of the application interface.

Bob McCormick