Did you order the code red?!
Posted by Dan Croak
Oct 05
On Fridays, we enjoy getting together for lunch and reviewing code which needs tender loving care. This week, our liveliest discussion surrounded responsibility for authorization logic.
The problem ~ where does the authorization logic go?
You’ve come across it before ~ you have a User model and a second model for which Users must be authorized to perform certain tasks:
1 2 3 4 5 6 7 8 9 10 |
def show @event = Event.find params[:id] unless (logged_in? && (@event.members.include?(current_user) || @event.organizer.include?(current_user)) || current_user.admin?) flash[:failure] = "You are not authorized to view this event." redirect_to home_url and return end end |
In discussion, we decided there is too much logic in the controller. We want to move the authorization into the correct place in our domain model. But where?
unless current_user.authorized_for_event?(@event) |
That reads nicely in English as “Unless the current user is authorized for the event” but logically, a User should not be responsible for its own authorization. If that were the case in the real world, we’d have societal chaos:
Kaffee: Did you order the code red?!
Col. Jessep: You’re goddamn right I did!

The solution ~ the non-User model authorizes the User
Whether your domain model has an Event or a CodeRed, the authorization logic should be on those models, not on User. In just about every case I can think of, that will remain true.

For example, I have a ticket to a Red Sox playoff game. When I arrive on Yawkey Way, the ticket-taker will authorize me to enter the event. He or she is the Controller in our MVC pattern (and I suppose the View is the world of sight, touch, taste, sound, and smell…). The ticket-taker doesn’t need to know that I paid with a MasterCard on September 20th or that my seat is in right field or any other details. All that matters is: “is this guy authorized?”
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 |
def attend @game = RedSoxGame.find_by_id params[:id] unless @game.authorized?(current_user) flash[:failure] = "You are not authorized for this game. Watch it over a pint of Sam Adams at the Cask n' Flagon." redirect_to home_url and return end end class RedSoxGame < ActiveRecord::Base def authorized?(user) return false if user.nil? self.doors_open? && self.not_rained_out? && user.ticket?(self) && user.not_drunk? end end |
The authorized method is noteworthy for two reasons. First, it uses the Composed Method:
Divide your program into methods that perform one identifiable task. Keep all of the operations in a method at the same level of abstraction. This will naturally result in programs with many small methods, each a few lines long.
The authorized? method is very easy to read because it focuses on simply authorizing the user for the game. It relies on a number of other methods whose purpose is clear and are similarly of narrow focus.
The second thing to note is that authorized? actually calls methods on the User model. It may seem contradictory to call User after saying that authorization logic does not belongs there. However, it isn’t authorization logic, it’s another level of abstraction.
Most importantly, the ticket-taker will let me into the Sox game and no one gets a code red.
Comments on this post
Oct 05
Joe Grossberg said,
Wouldn’t this sort of thing fit best in a
:before_filter?Oct 05
Jon Yurek said,
Yeah, sure, but that’s not the important part. Even in the before_filter, your controller should ask the event if the current_user can see it. The point was more that the controller shouldn’t care about the business rules concerning permissions and authorization, it’s just there to control the flow.
Oct 05
Michael Schuerig said,
In my opinion,
“logically, a User should not be responsible for its own authorization”
is a bad reason for not putting authorization logic in the user class. I agree that in the real world you may not want this, but you’re not developing a simulation of the real world, after all.
Putting authorization in the Event or RedSoxGame class has the drawback of making these classes unnecessarily aware of the existence of Users. In Ruby, that’s not too bad, of course.
OTOH, burdening the User class with authorization for a multitude of things doesn’t look like good modularization, either.
Of course, with Ruby, it would be easy to define authorization in a separate module and mix it in where needed. In a more pedestrian OOP vein, the whole authorization stuff could be extracted into separate classes.
Oct 05
Andrew Kuklewicz said,
It seems like you are conflating business rules with authorization.
Here’s wikipedia’s definition of authorization “In security engineering and computer security, authorization is a part of the operating system that protects computer resources by only allowing those resources to be used by resource consumers that have been granted authority to use them.”
Not all validations to see if an action should be allowed are authorization – authorization is a check on authority. In other words “For this given action, does this person have sufficient authority to do the action?”, not “Have all business rules been checked that would prevent this action from occurring”.
For example, the owner of 2 bank accounts may have the authority to transfer money from A to B, but if A has $0 balance, it ain’t happening – that is not stopped because of authority, but because of the business rule that you can’t transfer 0 dollars.
In your example, the answer to “Is this guy authorized?” depends on the state of doors, sobriety, and weather – when really, all a ticket taker is concerned with is if the ticket is valid for the game in question – as a ticket is a token granting authority.
Not that those aren’t all lovely things to check, but they are not about authorization.
Your earlier example, where you check things like membership to an event, and if a user is an organizer – these are business rules. When you check if the user is an admin – now we’re talking about an authorization check.
As for the ‘correct’ place to enforce authorization, since it is a check to see about authority to access a resource, if you consider a particular route to be a resource (the R in URL), then one mechanism of protecting a URL/route would be to enforce this at the controller level – if not even higher, at the level of dispatch before the controller is even called. This is a typical pattern in other security/authorization frameworks. Business rules, on the other hand, should definitely belong to the model – no argument there.
It could be that you want to view the model objects as the protected resource, not the URL/action requested by the user. That’s a very valid approach as well, in fact, they can both work togther – WebLogic has a security product (for example) that allows you to enforce authorization both at the http request level, and also at the object method level for just this reason. I have even seen such a thing enforced as a cross-cutting concern using AOP. As I recall, that is how the Acegi framework does things – it intercepts and checks authority without even having to make an explicit check in the code.
The approach of enforcing at the model level has the benefit of allowing mulitple UI/access points to the model (e.g. if all the authorization is in the controllers, what happens in my scripts that call the model directly, or if I create another UI that does not use the controllers?). Not a bad way to go, but the choice still all comes back to what you consider to be the resource needing protecting.
Cheers,
-Andrew Kuklewicz
Oct 05
Dan Croak said,
Joe and Andrew,
I agree that there should be a before_filter. It does what we want: protects the RedSoxGame resource being shown (changed the method name to be more RESTful) when certain conditions are not being met.
I still stand by the fact that the authorization conditions are business logic that belong in the Event or RedSoxGame models in these examples.
Here’s the example, re-written: (authenticate is implied to be in an external library such as AuthenticatedSystem in the restful_authentication plugin)
Oct 05
Dan Croak said,
authorize should be protected.
Oct 05
Dan Croak said,
Michael,
You mentioned:
I don’t think it’s desirable to have Event unaware of Users. The business logic dictates that it knows something about the Users:
That the user is in possession of a ticket and complying with the rules of behavior laid down by Fenway Park.
Oct 05
sandofsky said,
I’m glad I’m not the only one to realize authorization goes in the model.
As for business rules vs authorization, authorization falls under business rules. It’s part of your software’s behavior.
The difference between authorization and ability is “may” vs “can.” If you have a ticket to a game, you “may” enter. But if you have a wheelchair, they need a wheelchair-ramp before you “can.”
Which side should perform the check is debatable, but it’s more important to pick one side and stick to it.
Oct 05
Andrew Kuklewicz said,
I’m not saying don’t put authorization checks in the models, what I am saying is that some of the checks you are doing in your authorize methods are in fact business rules, and not authorization related at all. Authorization has a pretty particular meaning in software systems, and you are stretching it beyond its definition when you include all kinds of biz rules in an authorization check.
So I think it’s a category error – and one that I would find confusing if I was trying to maintain this code and understand what it is doing (I would not assume you were checking these other business rules in an ‘authorize’ method).
To put it another way – authorization asks if a person A can access a resource B. If nothing changes as regards to A’s authority over B, the answer should always be the same. In your authorize method, the result will vary on not only A’s authority over B, but all kinds of other mutable facts about B’s state, and A’s state that having nothing to do with A’s authority over B.
In Unix when I go to save an addition to a file, it fails based on authorization if I have read-only access; it is not an authorization failure if the disk is full, and would be odd to describe stopping me from adding to that file as an authorization failure. Both conditions have to be met (can the user write to this file? Is there enough disk space?), but both are not checking authorization. By your logic, I should plunk both checks in an authorize method, and that will ‘do what you want’, but that doesn’t seem right.
And while you are arguing effectively the benefits of putting authorization logic in the model (which is why this is an interesting blog post), I’m also saying that authorization is often only enforced at the controller level – there is even a recipe for doing just that in “Rails Recipes” (32 I think?).
This is coming from the perspective of treating an http requested resource as protected for authorization, not the model as resource. Is it a better way to do things? Not necessarily, but I don’t think you can discard it so easily. Your original point was to remove the bloated/misplaced logic from the controllers to the model, and for the biz rules, that’s right, move it. But a role based authorization check? Controlling who can access what resource sounds more like a controller issue than a model responsibility…that’s what contollers do, they access the model objects. Having them decide if a model can be accessed or not is well within their pervue – I don’ think your reasons for moving this out of the controller really hold up.
I would agree the real answer here is that security is best when it is enforced not only at the perimeter, but at every layer possible. Check both at the controller and model levels – just as it is good to protect access to a machine, and the database that runs on it – it’s not mutually exclusive. Your code with filters and model checks does this, so really my point of contention is the categorization of what consitutes an authorization check, and what is a business rule validation.
Oct 06
dream said,
Great post! But you could use before filer…
Oct 06
Aníbal Rojas said,
I think the discussion is even better than the post :-) +1 in “Authorizations is Business Logic” and yes, adding a authorized? predicate to the Model doesn’t prevent the use of a before filter.
But this gets better, because there are more interesting twists in this discussion: What about filtering the rows the user has access, or enhancing the predicate to authorized_to? an especific action…
Oct 06
Zach Dennis said,
The :before_filter does nothing to help the poster or his colleagues in solving this issue about where authorization logic goes.
If you hardcode real “authorization” logic into a :before_filter then you are writing or into a controller action you are coding yourself into a corner as that code is not re-usable.
If your business domain says, “a person can only attend a game if they have a ticket for the event” then that is domain logic. It is always a bad idea to put this kind of logic on a controller.
In regards to Andrew’s comment… you place an authorization check where you need it, but you put the authorization logic where it belongs. The authorization logic never belongs in the controller.
Oct 06
Andrew Kuklewicz said,
@Anibal – I did not argue against “Authorization is business logic” (as business logic is a very vague and broad category), but that not all business logic is authorization.
@Zach – It’s a good clarification to discriminate between where you make a check and where the logic resides. So yeah – don’t put the actual logic in a before_filter, but I think it’s a fine way to enforce authorization by making the call there, and then you have protection at the request/controller layer, not just the in the model.
For the record, I don’t put authorization logic in controllers – my point was that controllers/URLs can be treated as resources to be protected just as models can be – so not all authorization should be pushed into the model.
Of course I agree authorization checks should be made where needed (that was my concluding point). My favorite method of implementing is to have an authorization framework that can be called/applied in various ways – in the view, in making a request, and in the domain model. My favorite implementation pre-rails used pointcuts to keep the check entirely out of the mvc code, but check calls into the controllers, and services/model layer.
Sorry, comments are closed for this article.
© 2000 - 2009 by thoughtbot, inc.
written by a bushel of tiny robots
Come “ride the toad” on Hoptoad, the app error app.
Thunder Thimble: Brand monitoring for social media.
Widgetfinger: Simple content management for simple websites.
Tee-Bot, funny shirts your friends won't understand!
Umbrella Today: “It’s like totally the simplest weather report ever, Julie.”
Thoughtbot
thoughtbot is a technology consulting firm that provides web application development and design services. We focus on building modern systems, embracing good ideas and delivering elegant solutions.
Interested in learning Rails?
Sign up for our beginning or advanced training.
Archives