views:

555

answers:

5

I have this showing up all over in my controllers:

if not session[:admin]
  flash[:notice] = "You don't have the rights to do #{:action}."
  redirect_to :action=>:index
  return
end

And its sibling:

if not session[:user] and not session[:admin]
  flash[:notice] = "You don't have the rights to do #{:action}."
  redirect_to :action=>:index
  return
end

I would like to reduce this all down to a single declarative line when I want to use it in a method:

def action_which_requires_rights
    require_rights :admin
    #or:
    #:require_rights :user_or_admin
end

Clearly, if require_rights fails, I don't want the rest of the method executed. I would swear that there was a way to do this, but I can't find where I read about it. Am I imagining this?

+6  A: 

Firstly you can do: unless session[:admin] instead of if not ...

Then you can have a before filter which calls your method, this method would do your redirect_to "url" and return.

I have a question, I hope you are not just storing the id of the admin in a session as your the only means of authentication, having an attribute on your user model and querying that might be a safer bet.

MatthewFord
+2  A: 

Look at before_filter. They can halt execuion and can be limited to certain actions.

Ben Alpert
+1  A: 

I wouldn't show an action to user if user is not permitted to execute this action (I would use helpers to accomplish that)

In controllers, as mentioned in other answers, imho the best approach is to use before filters to control access rights.

I would also suggest to use restful authentication plugin to manage user roles.

aivarsak
A: 

You could try something that throws an exception.

def action_for_admins
  require_rights :admin
end

begin 
  action_for_admins
rescue
  <%= You don't have the rights to do that %>
end

Then require_rights should look something like that

def require_rights(*rights)
  rights.each do |right|
    raise "Missing right #{right.to_s}" if not user.has_right?(right)
  end
end

Note that I am a beginner in Ruby or Rails so it may not be the way.

Vincent Robert
+3  A: 

As others have said, a before_filter seems like the right tool here. But I'll address the actual pattern you were asking about.

Unfortunately, a method can't cause its calling method to return. The two closest matches to the pattern you are looking for:

A block:

def require_rights(rights)
  if session[rights]
    yield
  else
    flash[:notice] = "You don't have the rights to do #{:action}."
    redirect_to :action=>:index
  end
end

So with that you'd do:

def action_which_requires_rights
  require_rights :admin do
    #do whatever here
  end
end

Or a return value:

def require_rights(rights)
  return true if session[rights]
  flash[:notice] = "You don't have the rights to do #{:action}."
  redirect_to :action=>:index
  false
end

So with that you'd do:

def action_which_requires_rights
  require_rights :admin or return
  #do whatever here
end

I like the block better because it fits in with similar methods, and making the caller do or return feels kind of unnatural to me.

Chuck
+1 for the first example as a general ruby pattern.
krusty.ar