Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authorization Adapter redefines BaseController#authorize! method and clashes with CanCan #2077

Closed
RobWu opened this issue Apr 9, 2013 · 37 comments

Comments

@RobWu
Copy link

RobWu commented Apr 9, 2013

Get this error when using cancan.

My initializers/active_admin:

  # -*- encoding : utf-8 -*-
  ActiveAdmin.setup do |config|
    ...
    config.authorization_adapter = ActiveAdmin::CanCanAdapter
    ...
  end

My controller:

  ActiveAdmin.register Profile do
    menu :if => proc{ can?(:manage, Profile ) }
    controller.authorize_resource
    ...
  end
@RobWu
Copy link
Author

RobWu commented Apr 9, 2013

Worked fine in the old active_admin versions. (< 0.6.0)

@macfanatic
Copy link
Contributor

Our wiki pages are now out of date with the 0.6 release, read this blurb - you're mixing 0.5 & 0.6 ways of authorization.

https://github.com/gregbell/active_admin/blob/master/docs/13-authorization-adapter.md#checking-for-authorization-in-controllers-and-views

@lessless
Copy link

@macfanatic does this means that one can't use authorize_resource?
I have the same problem https://gist.github.com/dirty-hippie/5dd815bb9fbf72b3f94b

@macfanatic
Copy link
Contributor

I haven't tested, but I suspect that you wouldn't want to mix in two different authentication methods at the same time. The newly added authorization adapter will take care of that for you.

@lessless
Copy link

As I understand authorize_resource is an old method, right? Although I have removed the following line from each admin/*rb file the problem the problem persists. What I'm doing wrong and what should I do?

@macfanatic
Copy link
Contributor

authorize_resource is actually provided from CanCan, and not from ActiveAdmin at all.

Hmm, can you post a backtrace as a gist that I can review? Assuming you removed the controller.authorize_resource call, then I believe that's all you need to get this to work. I'm guessing that authorize! is being called somewhere in the backtrace itself.

@lessless
Copy link

@macfanatic
Copy link
Contributor

This is definitely a bug, we're overriding a method originally defined by CanCan and it is declared as private in our code.

https://github.com/ryanb/cancan/blob/master/lib/cancan/controller_additions.rb#L336
https://github.com/gregbell/active_admin/blob/master/lib/active_admin/base_controller/authorization.rb#L73

@lessless
Copy link

yeah, this is the fix, maybe just move directive from line 43 to 80 (right after authorize! method)?

@seanlinsley
Copy link
Contributor

@macfanatic, @dirty-hippie is that really a fix? From my reading of the code, if you set your own authorization_adapter then AA's AuthorizationAdapter class shouldn't be used (that's the one that defines authorize!).

@seanlinsley
Copy link
Contributor

@gregbell thoughts on this?

@ayamani
Copy link

ayamani commented Apr 19, 2013

I'm encountering the same issue. Any updates on a fix?

@ghost
Copy link

ghost commented Apr 20, 2013

also having the same issue even after removing all references of authorize_resource from active admin

@seanlinsley
Copy link
Contributor

@ayamani, @benjaminmort I'm going to try debugging this problem. What sort of configuration do you have with CanCan? Is it any different than @RobWu described when he opened this ticket?

@ghost
Copy link

ghost commented Apr 21, 2013

Thanks for the fast response daxter got to love your work

ActiveAdmin.setup do |config|
  config.authorization_adapter = ActiveAdmin::CanCanAdapter
  ...
end

ActiveAdmin.register Post, :namespace => :admin do
  ### CANCAN ABILITIES ###
  controller.authorize_resource
end

class Ability
  include CanCan::Ability
  def initialize(current_user)
    current_user ||= User.new
    can :manage, Post do |post|
      post.administration_users.include?(current_user)
    end 
  end 
end

note: post.administration_users.include?(current_user) = true

@ayamani
Copy link

ayamani commented Apr 22, 2013

@daxter I have the same configuration as @RobWu , and I also tried not utilizing the CanCanAdapter. Nevertheless, in both cases, I received the same error "protected method `authorize!' called for...."

@Jamevidi
Copy link

+1

2 similar comments
@ilyakatz
Copy link
Contributor

+1

@luca776
Copy link

luca776 commented May 1, 2013

+1

@macfanatic
Copy link
Contributor

I'm looking into this now, hopefully I can get a PR together.

@macfanatic
Copy link
Contributor

I was able to upgrade my application to the 0.6 release and use the new authorization system by doing the following:

  1. Add the new line to tell AA to use CanCanAuthorizationAdapter
  2. Remove all occurrences of controller.authorize_resource that I might have had in my AA resource files
  3. Any calls directly to #authorize! continue to work, since that is all the AA wrapper is doing under the hood
ActiveAdmin.setup do |config|
  config.authorization_adapter = ActiveAdmin::CanCanAdapter
  ...
end

ActiveAdmin.register Post, :namespace => :admin do

end

class Ability
  include CanCan::Ability
  def initialize(current_user)
    current_user ||= User.new
    can :manage, Post, administration_users: { user_id: current_user.id }
  end 
end

I don't believe that any code action is required.

@seanlinsley
Copy link
Contributor

I'm going to close this, then. Please feel free to comment if you're still encountering this problem.

@Malet
Copy link

Malet commented May 4, 2013

Still having the same error after trying the fix suggested by @macfanatic

@MattiHameister
Copy link

Having the same bug too. CanCan says, that the authorize_resource call in the controller is missing. If i (re)add it than this bug occurs.

@jakemauer
Copy link

Just wanted to throw my hat in here and say I have the same issue as MacNuke using ActiveAdmin 0.6.0 and CanCan 1.6.9

@luca776
Copy link

luca776 commented Jun 7, 2013

Same issue as MacNuke and jakemauer

@chagel
Copy link

chagel commented Jul 4, 2013

update to 0.6 and this bug shows up again

@seanlinsley
Copy link
Contributor

Okay, I'm going to re-open this. Anyone, please feel free to add any details about your situation that differ from what's already been said.

@seanlinsley seanlinsley reopened this Jul 4, 2013
@jkrall
Copy link

jkrall commented Jul 21, 2013

FWIW... I managed to get things working by monkey-patching ActiveAdmin::BaseController:

class ActiveAdmin::BaseController
  public :authorized?, :authorize!, :authorize_resource!
end

Not suggesting that anyone else do this, and I don't know what other side-effects it might cause... but it did get rid of this error for me. (I am using the CanCanAdapter configs suggested above)

@OscarBarrett
Copy link

The fix posted by @macfanatic works for me with ActiveAdmin 0.6.0 and CanCan 1.6.10.

@zellunit
Copy link

if you're still seeing this error with AA 0.6.0 and CC 1.6.10 make sure you don't have the gem 'activeadmin-cancan', '0.1.4' still installed that some of the guides recommended. that bit me. as soon as i removed, the @macfanatic fix worked perfect.

@chagel
Copy link

chagel commented Jul 28, 2013

I have time today to update activeadmin v0.6 and test again. Finally it all works and I have to say, it's all about messy configurations but not ActiveAdmin or CanCan problems.

I suggest to check these steps, and hopefully it fixes:

  1. Remove old patchs or gmes. ( initializers/activeadmin-cancan.rb, activeadmin-cancan gem etc.)
    I had one activeadmin-cancan patch before in initializers folder which made the mess. No need in current versions.
  2. Remove all controller.authorize_resource in views
  3. Follow official guide and add cancan adapter support(http://activeadmin.info/docs/13-authorization-adapter.html#using_the_cancan_adapter) .
    I have to add Dashboard in ability.rb to allow visit. This caused one of my roles went to endless loop.

@seanlinsley
Copy link
Contributor

So we've got a few people with success stories now. @Malet, @MacNuke, @jakemauer, @luca776 are you all still not able to get this working properly?

@Mesonyx
Copy link

Mesonyx commented Aug 10, 2013

the fix suggested by @macfanatic worked for me -- all errors vanished when I removed the activeadmin-cancan gem and fixed up the files (ActiveAdmin 0.6.0 and CanCan 1.6.10)

@chenghung
Copy link

fix suggested by @macfanatic worked for me, too

make sure

  1. no 'activeadmin-cancan' in your Gemfile
  2. no controller.authorize_resource in your admin page of resources

@seanlinsley
Copy link
Contributor

Okay, quite a few people have confirmed that @macfanatic's directions work. Re-closing this ticket.

@bramski
Copy link

bramski commented Nov 12, 2018

Now broken again...
activeadmin 2.0.0.alpha
rails 5.1.6
cancancan 2.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests