Hi everybody
I’m still unsure how to handle my authorisation checks, so here’s some
of my theory and I’d be thankful go get some opinions about it.
Let’s guess I have a news system where every logged in user can post
news items. Every user can edit his own news items, and if a user is
moderator he can also edit the other users’ news items.
My RHTML files should display edit links on each post only on the posts
that the user can edit (that’s an important feature to me!).
So where should the authorisation for this take place? After some
thoughts and experimentation I came to the conclusion that the model
object itself should authorize a user to edit itself or not. So my News
looks like that:
class News < ActiveRecord::Base
def allowed_to_edit?(user)
(self.user.id == user.id or user.id.is_moderator? or
user.is_administrator?) ? true : false
end
end
When iterating through the news items I can easily display the edit
links now:
<% @news.each do |n| -%>
<%= textilize @news.body %>
<%= link_to(“Edit”, :action => ‘edit’, :id => @news) if
@news.allowed_to_edit?(logged_in_user) %>
<% end -%>
(The helper method logged_in_user returns the currently logged in user.)
In the NewsController we could check the authorisation using a
before_filter (to prevent from malicious users that fake our poor
URL’s):
class NewsController < ApplicationController
before_filter :allowed_to_edit?(logged_in_user),
:only => :edit
def edit
# …
end
private
def allowed_to_edit?(user)
if News.exists?(params[:id]) # Does the news item exist?
if News.find_by_id(params[:id]).allowed_to_edit?(user) # Is the
user allowed?
return true
else # No he isn’t allowed!
flash[:warning] = “You are not allowed to edit the news item
##{news.id}!”
redirect_to_referer
end
else # No the news item does not exist!
flash[:message] = “A news item with the ID #{news.id} does not
exist”
end
end
end
One thing that isn’t very great is the fact that the news item to be
edited is instantiated two times: the first time in allowed_to_edit?()
and the second time in edit(). Is there maybe a non-hackish way to pass
the news item from allowed_to_edit?() to edit() to save some
performance? Or is that not necessary in your opinion?
Another way would be:
class NewsController < ApplicationController
def edit
news = allowed_to_edit?(logged_in_user) # Do the authorisation here
# …
end
private
# This version passes the instantiated news object back in case of
success
def allowed_to_edit?(user)
if News.exists?(params[:id]) # Does the news item exist?
news = News.find_by_id(params[:id])
if news.allowed_to_edit?(user) # Is the user allowed?
return news
else # No he isn’t allowed!
flash[:warning] = “You are not allowed to edit the news item
##{news.id}!”
redirect_to_referer
end
else # No the news item does not exist!
flash[:message] = “A news item with the ID #{news.id} does not
exist”
end
end
end
In some way I like the first version more, it looks cleaner or more
semantic to me; but obviously (?) the second is far better, isn’t it?
How do you think about that as a whole? Any suggestions? Improvements?
I saw some other ways of doing authorization, but this one is nearly the
only one that feels “right” to me: everything is in its place, it can be
easily extended or more abstracted (e.g. to use as a plugin).
Thank you very much for your time.
Joshua