Rails: Adding methods to models to perform checks based on current_user?

Question

I have a model that looks something like this:

class Comment < ActiveRecord::Base
  ...
  #allow editing comment if it is moderated and the user passed-in
  #is the one that owns the comment
  def can_edit?(user)
    moderated? and user.Type == User and user.id == self.user_id
  end
  ...
end

And a call in a view:

<%= link_to 'Show Comment', @comment if @comment.can_show?(current_user) %>

I need to write many such methods in many different models - sort of validation checks to see if current_user is allowed to do something on a model.

But it feels cumbersome - especially the need to check that the passed-in user is indeed a object of type User.

What's a clean, best-practice way to do this sort of thing? Am I on the right track? (i.e. should I be adding such methods to a model or somewhere else)

Note

I am using scoped queries to get the comments and other models, but in some cases I cannot scope the query so I have to use the can_xxxx? methods

Ps. Is what I'm doing considered a "fat model"?

Answer 1

I don't think what you're doing is necessarily wrong. I see three ways to simplify, though:

1) track self.user as well as self.user_id. Then you can say:

def can_show?(user)
  moderated ? and user == self.user
end

Note, this might add overhead either with DB lookup times and/or memory footprint.

2) Use #is_a? in order to check ancestry and not just class equality:

def can_show?(user)
  moderated ? and user.is_a?( User ) and user.id == self.user_id
end

3) If passing in a non-user is wrong, you might want to raise an error when this happens:

def can_show?(user)
  raise "expected User, not #{ user.class.to_s }" unless user.is_a?(User)
  moderated ? and user.id == self.user_id
end

As for Q2, I haven't heard the terminology "fat model." Is it referenced anywhere in particular?

Answer 2

Re: fat model and skinny controller

This is the idea of pushing logic into the model rather than having it in the controller (or worse, the view).

A big benefit is to help with testing; also the focus of placing more logic in the model rather than in the controller. Remember that it is not uncommon to have controllers work with multiple models.

Putting the logic into a model rather than a controller often means that the business rules are being baked into the model--which is exactly where they belong.

A possible downside is that any information available to the controller that is not available in the model needs to be explicitly passed into the model's methods or "set" using a model's instance variables.

Your example of needing to pass the current user into the model illustrates the issue.

Overall though, I and many others have found that fat models tend to work out better than not.

Answer 3

Create a module containing all the authorization methods and include the module to all the classes requiring authorization.

Add a file called authorization.rb to app/models directory.

module Authorization

  def can_edit?(user)
    moderated? and user.is_a?(User) and user.id == self.user_id
  end

  def self.included(base)
    base.send(:extend, ClassMethods)
  end

  module ClassMethods
    # add your class methods here.
  end
end

Add a file called authorization.rb to config/initializers directory.

%w(
 Comment
 Post
).each do |klass|
  klass.constantize.include(Authorization)
end

Now Comment and Post models will have all the authorization methods.

Other approach is to use your current named_scope.

class Post
  named_scope :accessible, lambda { |user|
    {
      :conditions => { :user_id => user.id, :moderated => true}
    }
  }
end

Post controller actions

class PostsController

  def index
    @posts = Post.acessible(current_user)
    # process data
  end

  def show
    # throws record not found when the record is not accessible.
    @post = Post.acessible(current_user).find(params[:id])
    # process data
  end

end

I like this approach as it uses the same logic for accessing an array of objects or a single object.

You can add the named_scope to the module to avoid repeated definitions:

module Authorization
  def self.included(base)
    base.named_scope :accessible, lambda { |user|
      {
        :conditions => { :user_id => user.id, :moderated => true}
      }
    }
  end

  module ClassMethods
    # add your class methods here.
  end
end

Make sure to include the module in required classes as suggested earlier.