cancan and activescaffold issue #203

Closed
danil-z opened this Issue Dec 4, 2010 · 14 comments

Projects

None yet

3 participants

@danil-z
danil-z commented Dec 4, 2010

Hi, i'm real noob in rails and try learn it.
I' stuck with cancan and activescaffold
i can't limit controller to show (index) only limited records with cancan (i'm success with limit manage records but not with read...)
could you review a little piece of my code and give me an advice, where am i wrong?...

class School < ActiveRecord::Base
  has_and_belongs_to_many :editors, :class_name => "User"
end

class User < ActiveRecord::Base

  has_and_belongs_to_many :roles
  has_and_belongs_to_many :schools
  def role?(role)
    return !!self.roles.find_by_name(role.to_s.camelize)
  end

end

class Ability
  include CanCan::Ability

  def initialize(user)
    user ||= User.new # guest user
    if user.role? :super_admin
      can :manage, :all
    elsif user.role? :operator
      can [:manage], School, ['schools.id in (select su.school_id
                                        from schools_users su
                                        where su.user_id = ?)', user.id] do |school|
        school.editors.include?(user)
      end
    end
  end

end

class SchoolsController < ApplicationController
  load_and_authorize_resource

  active_scaffold :school do |conf|
    conf.columns.exclude :editors
    conf.list.columns =  [:title, :address, :city, :editors]
  end

  def before_create_save(record)
    record.editors << current_user
  end

end

idea is simple user should see(and manage) only schools he has created....
atm if i try manage other shools i get an error cacan access denied but how to limit view.. :(

@ryanb
Owner
ryanb commented Dec 4, 2010

Is the index action the only one which is not working? I'm assuming active_scaffold is performing the search from scratch so it's not using the accessible_by method which CanCan provides to only select the schools which one has permission to access.

You'll need to somehow override ActiveScaffold's index action behavior. Unfortunately I'm not familiar with ActiveScaffold enough to help out here. Does anyone else know how to do this?

@danil-z
danil-z commented Dec 28, 2010

regarding API of activescaffold

beginning_of_chain controller method, v2.3+
If you want to use named scopes to limit the resultset of find(:all) used by List, then override this method. It may return a scoped model. Defeault is active_scaffold_config.model. And as an instance method on the controller, it has access to params and session and all the standard good stuff.

Examples:

def beginning_of_chain
  super.vegetarian.older_than(20)
end

for fetching records should i write this method like code below?

def beginning_of_chain
  super.accessible_by(current_ability)
end
@ryanb
Owner
ryanb commented Dec 28, 2010

Yes, that looks correct. If that works for you I can put this in an adapter similar to how I do for Inherited Resource so it works out o the box.

@danil-z
danil-z commented Dec 30, 2010

I confirm, at least it works for me.

@ryanb
Owner
ryanb commented Dec 31, 2010

Thanks for confirming. I'll try to get this in for the 1.5 release.

@danil-z
danil-z commented Jan 1, 2011

activescaffold for security level uses some methods and pass them current_user variable in model
like
def autorized_for_delete
false until current_user
current_user.role? :admin
end

but i would like define rules at ability class
could you hint a way of using ability class in this case?

@danil-z
danil-z commented Jan 1, 2011

some example from code

class Subject < ActiveRecord::Base
 validates_uniqueness_of :title, :case_sensitive => false
 validates_uniqueness_of :short_title, :case_sensitive => false
 validates_presence_of :title

 def authorized_for_delete?
  # anonymous users may never destroy these/this records
  return false unless current_user
  current_user.role? :admin
 end

 def authorized_for_update?
  # anonymous users may never destroy these/this records
  return false unless current_user
  current_user.role?(:admin) || self.created_at > 3.day.ago
end

end

@ryanb
Owner
ryanb commented Jan 1, 2011

Hmm, so with ActiveScaffold you define the authorization behavior in the model? How does it have access to the current_user there? If nothing else you could do this.

def authorized_for_delete?
  ability = Ability.new(current_user)
  ability.can? :destroy, self
end

But it would be better to pass in the current_ability from the controller so there aren't multiple instances of Ability laying around.

@danil-z
danil-z commented Jan 3, 2011

so to pass it in model i should make something like this?
class Subject < ActiveRecord::Base
cattr_accessor: current_ability

   def authorized_for_delete?
    current_ability.can? :destroy, self
   end
 end

 class SubjectsController < ApplicationController
  load_and_authorize_resource
  before_filter :set_current_ability
  def set_current_ability
   Subject.current_ability = current_ability
  end
@ryanb
Owner
ryanb commented Jan 3, 2011

It's not a good idea to set this as a class variable because that is global and the current_ability changes for each request. You will have to set it however ActiveScaffold is setting the current_user. I'm not really sure where or how that is happening so it may take some digging or overriding Active Scaffold.

I'm not a fan of setting request specific details on a model like this (such as current_user) which is why Can Can does not work this way.

@ryanb
Owner
ryanb commented Jan 4, 2011

This issue seems a bit bigger then I initially expected so I don't think I will be able to get it in for the 1.5 release, I'll shoot for the next release however.

@ryanb
Owner
ryanb commented Jan 28, 2011

Since I don't work with Active Scaffold, I am going to leave this up to someone else to solve who knows it better. Feel free to fork CanCan and submit a pull request with support for Active Scaffold. Try to keep all changes isolated into a single file like the Inherited Resources file works. If you are working on this, feel free to add comments here and we can discuss details further.

@clyfe
clyfe commented Mar 19, 2011

Hi, I'm the author of ActiveScaffold CanCan bridge. It should solve your problem. It's nicely documented and it works. Currently only Rails3 with Volker's AS fork.

https://github.com/vhochstein/active_scaffold/wiki/CanCan-bridge

@ryanb
Owner
ryanb commented Mar 22, 2011

Thanks for your work on this clyfe. I'm closing this for now. If there's something that can be made in CanCan to improve this please post a comment here and I'll reopen.

@ryanb ryanb closed this Mar 22, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment