def verify_admin!
unless ((!params[:account_id].blank?) && (params[:account_id].to_i == current_account.id) && (params[:id].to_i == current_member.id)) || current_member.is_admin?
flash[:alert] = "Can't access this page"
redirect_to account_member_path(current_member.account, current_member) and return
end
end
Refactorings
No refactoring yet !
steved
May 14, 2010, May 14, 2010 18:03, permalink
- push logic into model (it's fine to pass params into a model method)
- changed name from verify_admin!; you're verifying more than admin, right?
class MyController < ApplicationController
before_filter :verify_access
# action methods
private
def verify_access
unless current_member.can_access_page?(params)
flash[:alert] = "Can't access this page"
redirect_to account_member_path(current_member.account, current_member) and return
end
end
end
class Member < ActiveRecord::Base
belongs_to :account
def can_access_page?(params)
is_admin? || (account_id == params[:account_id].to_i && id == params[:id].to_i)
end
end
I have this method as part of a security module included in my ApplicationController to help verify whether or not a member can access a page or not. It's based on an app with two models, an Account model that has many Members, and a Member model that belongs to an Account. current_member is a method from the built in Authentication gem I'm using (Devise), and current_account is a separate method that let's the application always know what account is being accessed.
I can't help but think I could refactor the unless condition in a more concise way. It's essentially saying that if a member does not belong to an account, and is not an admin, don't allow them to access it, and redirect them.
I'd love to hear any refactoring ideas!