cancan + config.actions << :mark = bork?

[RobertLowe]

Was just testing features, cancan seems to bust with :mark?

I'll have a test app up tomorrow...

Somehow cancan or something isn't loading but does on further requests...

  Started GET "/admin/territories" for 127.0.0.1 at 2011-02-20 22:50:21 -0500

  ActionController::RoutingError (undefined method `can?' for nil:NilClass):
    app/controllers/admin/territories_controller.rb:4:in `<class:TerritoriesController>'
    app/controllers/admin/territories_controller.rb:1:in `<top (required)>'

class Ability
  include CanCan::Ability

  def initialize(user)
    # Define abilities for the passed in user here. For example:
    #
    user ||= User.new # guest user (not logged in)

    if user.new_record?
      can :read, :all # anonymous users can list/read
    else
      if user.admin?
        can :manage, :all
      else
        # gotta be signed up to "create new"
        can :read,    :all

        # Troll permissions:
        #
        can :delete, :all
        # but cannot destroy :P :(
        #
        # can :destroy, :all
      end
    end
  end
end

class Admin::TerritoriesController < ApplicationController
  before_filter :authenticate_admin!

  active_scaffold :territory do |config|
    config.actions << :mark
  end
end

[clyfe]

The current_ability (just like current_user) is available inside models only during request/response cycle, because is inserted there by means of a controller before_filter. AS/lib/AS/actions/mark.rb +19 queryes the CanCan ACL at configure time ( when the controller class body executes ) http://pastie.org/1588763.

At configure time we have nothing: no current_user, no current_ability ...

Not sure how to solve this. Sidenote: The whole security layer is so not thread safe ..

[clyfe]

Maybe AS/lib/AS/actions/mark.rb +19 should directly query the @core.list.columns#@set instead of calling `.names' that chains to collect that chains to each that queries ACL

[clyfe]

One way to get rid of this is: Since at configure time we know nothing of who we're authorizing, we can return whatever authorized_for_without_cancan? returns (default old behavior). In this case because of the default "paranoic" false security reply, you'll have to override the security methods as per AS#security wiki.

But that tastes bad ..

         def authorized_for_with_cancan?(options = {})
           raise InvalidArgument if options[:crud_type].blank? and options[:action].blank?
-          crud_type_result = options[:crud_type].nil? ? true : current_ability.can?(options[:crud_type], self)
-          action_result = options[:action].nil? ? true : current_ability.can?(options[:action].to_sym, self)
+          if current_ability.present?
+            crud_type_result = options[:crud_type].nil? ? true : current_ability.can?(options[:crud_type], self)
+            action_result = options[:action].nil? ? true : current_ability.can?(options[:action].to_sym, self)
+          else
+            crud_type_result, action_result = false, false
+          end
           default_result = authorized_for_without_cancan?(options)
           result = (crud_type_result and action_result) or default_result
           return result
         end
       end

[RobertLowe]

Thanks for this fix, do you want to do the pull request?

Also, IMO, I think the security layer, action links being passed as strings and test suite being rails 2.3.x should be the 'big' issues right now, just my two cents

[RobertLowe]

Update, everything loads fine but columns are not render as current_ability is not present during configuration...

Ideas?

    [2011-02-21 10:06:17] INFO  WEBrick::HTTPServer#start: pid=93317 port=3002
    current_ability.present?: false
    {:action=>nil, :crud_type=>:read, :column=>:status}
    current_ability.present?: false
    {:action=>nil, :crud_type=>:read, :column=>:subscriptions}
    current_ability.present?: false
    {:action=>nil, :crud_type=>:read, :column=>:territories}
    current_ability.present?: true
    {:crud_type=>:read}
    current_ability.present?: true
    {:action=>nil, :crud_type=>:read, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:create}
    current_ability.present?: true
    {:crud_type=>:read}
    current_ability.present?: true
    {:crud_type=>:create}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:crud_type=>:delete}
    current_ability.present?: true
    {:crud_type=>:delete}
    current_ability.present?: true
    {:action=>nil, :crud_type=>:read, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:action=>:update, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:read, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:action=>:update, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:action=>:update, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:read, :action=>"charge"}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:crud_type=>:update, :action=>"edit"}
    current_ability.present?: true
    {:crud_type=>:delete}
    current_ability.present?: true
    {:crud_type=>:delete, :action=>"destroy"}
    current_ability.present?: true
    {:crud_type=>:read}
    current_ability.present?: true
    {:crud_type=>:read, :action=>"show"}
    current_ability.present?: true
    {:crud_type=>:read, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:action=>:update, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:action=>:update, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:read, :action=>"charge"}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:crud_type=>:update, :action=>"edit"}
    current_ability.present?: true
    {:crud_type=>:delete}
    current_ability.present?: true
    {:crud_type=>:delete, :action=>"destroy"}
    current_ability.present?: true
    {:crud_type=>:read}
    current_ability.present?: true
    {:crud_type=>:read, :action=>"show"}
    current_ability.present?: true
    {:crud_type=>:read, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:action=>:update, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:action=>:update, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:read, :action=>"charge"}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:crud_type=>:update, :action=>"edit"}
    current_ability.present?: true
    {:crud_type=>:delete}
    current_ability.present?: true
    {:crud_type=>:delete, :action=>"destroy"}
    current_ability.present?: true
    {:crud_type=>:read}
    current_ability.present?: true
    {:crud_type=>:read, :action=>"show"}
    current_ability.present?: true
    {:crud_type=>:read, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:action=>:update, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:action=>:update, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:read, :action=>"charge"}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:crud_type=>:update, :action=>"edit"}
    current_ability.present?: true
    {:crud_type=>:delete}
    current_ability.present?: true
    {:crud_type=>:delete, :action=>"destroy"}
    current_ability.present?: true
    {:crud_type=>:read}
    current_ability.present?: true
    {:crud_type=>:read, :action=>"show"}
    current_ability.present?: true
    {:crud_type=>:read, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:action=>:update, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:action=>:update, :column=>:marked}
    current_ability.present?: true
    {:crud_type=>:read, :action=>"charge"}
    current_ability.present?: true
    {:crud_type=>:update}
    current_ability.present?: true
    {:crud_type=>:update, :action=>"edit"}
    current_ability.present?: true
    {:crud_type=>:delete}
    current_ability.present?: true
    {:crud_type=>:delete, :action=>"destroy"}
    current_ability.present?: true
    {:crud_type=>:read}
    current_ability.present?: true
    {:crud_type=>:read, :action=>"show"}

    Started GET "/admin/campaigns" for 127.0.0.1 at 2011-02-21 10:06:21 -0500
      Processing by Admin::CampaignsController#index as HTML
      SQL (0.5ms)  SHOW TABLES
      User Load (0.3ms)  SELECT `users`.* FROM `users` WHERE `users`.`id` = 2 LIMIT 1
      SQL (0.3ms)  SELECT COUNT(*) FROM `campaigns`
      Campaign Load (0.1ms)  SELECT `campaigns`.* FROM `campaigns` ORDER BY `campaigns`.`id` ASC LIMIT 15 OFFSET 0
    Rendered ./active-scaffoldfrontends/default/views/_action_group.html.erb (64.5ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_header.html.erb (69.7ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_column_headings.html.erb (6.4ms)
    Rendered ./active-scaffoldfrontends/default/views/_messages.html.erb (0.6ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_messages.html.erb (5.4ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_record_columns.html.erb (12.6ms)
      Territory Load (0.3ms)  SELECT `territories`.* FROM `territories` INNER JOIN `subscriptions` ON `territories`.id = `subscriptions`.territory_id WHERE ((`subscriptions`.campaign_id = 1))
    Rendered ./active-scaffoldfrontends/default/views/_action_group.html.erb (38.0ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_actions.html.erb (43.0ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_record_columns.html.erb (13.4ms)
      Territory Load (0.4ms)  SELECT `territories`.* FROM `territories` INNER JOIN `subscriptions` ON `territories`.id = `subscriptions`.territory_id WHERE ((`subscriptions`.campaign_id = 2))
    Rendered ./active-scaffoldfrontends/default/views/_action_group.html.erb (22.5ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_actions.html.erb (27.3ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_record_columns.html.erb (12.6ms)
      Territory Load (0.5ms)  SELECT `territories`.* FROM `territories` INNER JOIN `subscriptions` ON `territories`.id = `subscriptions`.territory_id WHERE ((`subscriptions`.campaign_id = 3))
    Rendered ./active-scaffoldfrontends/default/views/_action_group.html.erb (25.6ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_actions.html.erb (30.1ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_record_columns.html.erb (12.3ms)
      Territory Load (0.4ms)  SELECT `territories`.* FROM `territories` INNER JOIN `subscriptions` ON `territories`.id = `subscriptions`.territory_id WHERE ((`subscriptions`.campaign_id = 4))
    Rendered ./active-scaffoldfrontends/default/views/_action_group.html.erb (22.7ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_actions.html.erb (27.3ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_record_columns.html.erb (48.4ms)
      Territory Load (0.3ms)  SELECT `territories`.* FROM `territories` INNER JOIN `subscriptions` ON `territories`.id = `subscriptions`.territory_id WHERE ((`subscriptions`.campaign_id = 5))
    Rendered ./active-scaffoldfrontends/default/views/_action_group.html.erb (21.7ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_actions.html.erb (26.3ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_record.html.erb (291.3ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_pagination.html.erb (1.0ms)
    Rendered ./active-scaffoldfrontends/default/views/_list.html.erb (321.6ms)
    Rendered ./active-scaffoldfrontends/default/views/_list_with_header.html.erb (399.1ms)
    Rendered ./active-scaffoldfrontends/default/views/list.html.erb within layouts/application (425.8ms)
    Completed 200 OK in 522ms (Views: 432.7ms | ActiveRecord: 3.2ms)

[clyfe]

In this case because of the default "paranoic" false security reply, you'll have to override the security methods as per AS#security wiki.

That is:

can't ask CanCan, because there's no current_ability, then I ask AS#security ( https://github.com/activescaffold/active_scaffold/wiki/Security ), if I didn't write no overrides I return config.security.default_permission and that you probably configured to false, as such the rely is "NO" and nothing is shown.

Follow the rules in https://github.com/activescaffold/active_scaffold/wiki/Security to reply "YES" for the "mark" actions, that is, override authorized_for in some model's class.

PS. please use http://pastie.org/ or https://gist.github.com/

[RobertLowe]

Ah, well said and makes sense;

It'd just be a nice-to-have to keep security dry-erish, it feels weird to define permissions with cancan and have to supply overrides for columns because no current_ability

Specifically, why is configuration running before the request/response cycle?

Link issue in docs? @ https://github.com/vhochstein/active_scaffold/wiki/CanCan-bridge

PS: Will do

[RobertLowe]

Weird false positives fix:

https://gist.github.com/837318

[RobertLowe]

I hope this is amicable but I wanted to use cancan in combination with authorized_for?. So I created a hacked version of cancan_bridge. The hack seems to be happily working for which columns are shown via authorized_for?

git clone git://github.com/RobertLowe/active_scaffold-cancan-devise-simple_navigation-WIP.git

git checkout test

Let me know what you guys think, I'm still new to a lot of activescaffold internals. There are prolly better ways to handle this...

[clyfe]

current_ability is dependent on current_user

@current_ability = Ability.new(current_user)

We cannot talk about authorization unless we're in a context of a user. The "configuration" I'm talking about is the code injection that AS injects into the controller when the application code is loaded. At the point of app loading there is no user logged in, we are not even in a req/resp cycle.

Because there is no user to be authorized, it is meaningless to do authorization at that point. This is a flaw of AS for trying to do that and I'm not sure now how AS should be refactored.

You can pull request the fix for "false positives".

[clyfe]

You can authorize all columns at once like https://gist.github.com/838359

[vhochstein]

I ve just added a commit which avoids calling authorization methods during mark configuration

[clyfe]

Great!