class PropertiesController < ApplicationController
before_filter :authorise_action
private
def authorise_action
case action_name
when 'index', 'show', 'map_info_window'
# Allowed by anyone.
when 'new', 'create', 'edit', 'update', 'destroy'
if is_logged_out?
deny_access :notice => Phrases.login_required, :redirect_to => login_url
elsif is_landlord? or is_admin?
# Allowed.
else
deny_access :notice => Phrases.access_denied, :store_location => false
end
else
deny_access :notice => Phrases.not_public_yet, :store_location => false, :redirect_to => :back
end
end
end
Refactorings
No refactoring yet !
Adam
November 11, 2008, November 11, 2008 23:43, permalink
No rating.
Login to rate!
I'm not sure what deny_access does exactly. But I will assume it halts the filter chain.
class PropertiesController < ApplicationController
before_filter :authorize_user_is_logged_in, :only => [ :new, :create, :edit, :update, :destroy ]
before_filter :authorize_user_has_permission, :only => [ :new, :create, :edit, :update, :destroy ]
private
def authorized?
is_landlord? || is_admin?
end
def authorize_user_is_logged_in
deny_access :notice => Phrases.login_requred, :redirect_to => login_url if is_logged_out?
end
def authorize_user_has_permission
deny_access :notice => Phrases.access_denied, :store_location => false unless authorized?
end
endr
Nick
November 12, 2008, November 12, 2008 00:40, permalink
No rating.
Login to rate!
Hi Adam. You're correct about #deny_access; it uses #redirect_to to halt the filter chain. Your refactoring is great, though it dropped one bit of functionality that's in the original post: redirecting the browser to :back if the requested action doesn't require authentication and hasn't been made public. Any thoughts on how to incorporate that into your suggestion?
Thanks,
Nick
Adam
November 12, 2008, November 12, 2008 03:55, permalink
No rating.
Login to rate!
class PropertiesController < ApplicationController
PUBLIC_ACTIONS = %w[index show map_info_window]
LANDORD_ACTIONS = %w[new edit create update destroy]
before_filter :not_yet_public, :except => PUBLIC_ACTIONS + LANDLORD_ACTIONS
before_filter :authorize_user_is_logged_in, :only => LANDLORD_ACTIONS
before_filter :authorize_user_has_permission, :only => LANDLORD_ACTIONS
private
def not_yet_public
deny_access :notice => Phrases.not_public_yet, :store_location => false, :redirect_to => :back
end
end
The code below works, but feels a bit overly complex, and looks ugly. Any suggestions on how to condense it?