nathanl / authority

*CURRENTLY UNMAINTAINED*. Authority helps you authorize actions in your Rails app. It's ORM-neutral and has very little fancy syntax; just group your models under one or more Authorizer classes and write plain Ruby methods on them.
MIT License
1.21k stars 67 forks source link

Allow for application controller level authorize_actions_for when using authority on controllers that don't have a resource #86

Closed mwmeyer closed 10 years ago

mwmeyer commented 10 years ago

I've followed the instructions here to use authority on controllers that don't have a resource but I want to do this for every controller in my app except the authentication pages ( I am using devise ).

Right now it seems I have to include an authorize file for every controller and add the authorize_actions_for filter to the top of every controller. To DRY things up it would be nice if I could do something like this in my application controller:

authorize_actions_for ApplicationAuthorizer :unless => :devise_controller?

Trying that at the moment just throws the error:

undefined method `ApplicationAuthorizer' for ApplicationController:Class
nathanl commented 10 years ago

What's the backtrace? I'm guessing it points here: https://github.com/nathanl/authority/blob/master/lib/authority/controller.rb#L141

... but it shouldn't get there if ApplicationAuthorizer is a class.

mwmeyer commented 10 years ago

Hi Nathan,

Here is the full trace:

app/controllers/application_controller.rb:6:in `<class:ApplicationController>'
app/controllers/application_controller.rb:1:in `<top (required)>'
activesupport (4.1.1) lib/active_support/dependencies.rb:443:in `load'
activesupport (4.1.1) lib/active_support/dependencies.rb:443:in `block in load_file'
activesupport (4.1.1) lib/active_support/dependencies.rb:633:in `new_constants_in'
activesupport (4.1.1) lib/active_support/dependencies.rb:442:in `load_file'
activesupport (4.1.1) lib/active_support/dependencies.rb:342:in `require_or_load'
activesupport (4.1.1) lib/active_support/dependencies.rb:480:in `load_missing_constant'
activesupport (4.1.1) lib/active_support/dependencies.rb:180:in `const_missing'
app/controllers/admins_controller.rb:1:in `<top (required)>'
activesupport (4.1.1) lib/active_support/dependencies.rb:443:in `load'
activesupport (4.1.1) lib/active_support/dependencies.rb:443:in `block in load_file'
activesupport (4.1.1) lib/active_support/dependencies.rb:633:in `new_constants_in'
activesupport (4.1.1) lib/active_support/dependencies.rb:442:in `load_file'
activesupport (4.1.1) lib/active_support/dependencies.rb:342:in `require_or_load'
activesupport (4.1.1) lib/active_support/dependencies.rb:480:in `load_missing_constant'
activesupport (4.1.1) lib/active_support/dependencies.rb:180:in `const_missing'
activesupport (4.1.1) lib/active_support/inflector/methods.rb:238:in `const_get'
activesupport (4.1.1) lib/active_support/inflector/methods.rb:238:in `block in constantize'
activesupport (4.1.1) lib/active_support/inflector/methods.rb:236:in `each'
activesupport (4.1.1) lib/active_support/inflector/methods.rb:236:in `inject'
activesupport (4.1.1) lib/active_support/inflector/methods.rb:236:in `constantize'
activesupport (4.1.1) lib/active_support/dependencies.rb:552:in `get'
activesupport (4.1.1) lib/active_support/dependencies.rb:583:in `constantize'
actionpack (4.1.1) lib/action_dispatch/routing/route_set.rb:76:in `controller_reference'
actionpack (4.1.1) lib/action_dispatch/routing/route_set.rb:66:in `controller'
actionpack (4.1.1) lib/action_dispatch/routing/route_set.rb:44:in `call'
actionpack (4.1.1) lib/action_dispatch/journey/router.rb:71:in `block in call'
actionpack (4.1.1) lib/action_dispatch/journey/router.rb:59:in `each'
actionpack (4.1.1) lib/action_dispatch/journey/router.rb:59:in `call'
actionpack (4.1.1) lib/action_dispatch/routing/route_set.rb:676:in `call'
bullet (4.9.0) lib/bullet/rack.rb:12:in `call'
meta_request (0.3.0) lib/meta_request/middlewares/app_request_handler.rb:13:in `call'
rack-contrib (1.1.0) lib/rack/contrib/response_headers.rb:17:in `call'
meta_request (0.3.0) lib/meta_request/middlewares/headers.rb:16:in `call'
meta_request (0.3.0) lib/meta_request/middlewares/meta_request_handler.rb:13:in `call'
newrelic_rpm (3.8.1.221) lib/new_relic/rack/error_collector.rb:55:in `call'
newrelic_rpm (3.8.1.221) lib/new_relic/rack/agent_hooks.rb:32:in `call'
newrelic_rpm (3.8.1.221) lib/new_relic/rack/browser_monitoring.rb:27:in `call'
warden (1.2.3) lib/warden/manager.rb:35:in `block in call'
warden (1.2.3) lib/warden/manager.rb:34:in `catch'
warden (1.2.3) lib/warden/manager.rb:34:in `call'
rack (1.5.2) lib/rack/etag.rb:23:in `call'
rack (1.5.2) lib/rack/conditionalget.rb:25:in `call'
rack (1.5.2) lib/rack/head.rb:11:in `call'
actionpack (4.1.1) lib/action_dispatch/middleware/params_parser.rb:27:in `call'
actionpack (4.1.1) lib/action_dispatch/middleware/flash.rb:254:in `call'
rack (1.5.2) lib/rack/session/abstract/id.rb:225:in `context'
rack (1.5.2) lib/rack/session/abstract/id.rb:220:in `call'
actionpack (4.1.1) lib/action_dispatch/middleware/cookies.rb:560:in `call'
activerecord (4.1.1) lib/active_record/query_cache.rb:36:in `call'
activerecord (4.1.1) lib/active_record/connection_adapters/abstract/connection_pool.rb:621:in `call'
activerecord (4.1.1) lib/active_record/migration.rb:380:in `call'
actionpack (4.1.1) lib/action_dispatch/middleware/callbacks.rb:29:in `block in call'
activesupport (4.1.1) lib/active_support/callbacks.rb:82:in `run_callbacks'
actionpack (4.1.1) lib/action_dispatch/middleware/callbacks.rb:27:in `call'
actionpack (4.1.1) lib/action_dispatch/middleware/reloader.rb:73:in `call'
actionpack (4.1.1) lib/action_dispatch/middleware/remote_ip.rb:76:in `call'
actionpack (4.1.1) lib/action_dispatch/middleware/debug_exceptions.rb:17:in `call'
actionpack (4.1.1) lib/action_dispatch/middleware/show_exceptions.rb:30:in `call'
railties (4.1.1) lib/rails/rack/logger.rb:38:in `call_app'
railties (4.1.1) lib/rails/rack/logger.rb:20:in `block in call'
activesupport (4.1.1) lib/active_support/tagged_logging.rb:68:in `block in tagged'
activesupport (4.1.1) lib/active_support/tagged_logging.rb:26:in `tagged'
activesupport (4.1.1) lib/active_support/tagged_logging.rb:68:in `tagged'
railties (4.1.1) lib/rails/rack/logger.rb:20:in `call'
quiet_assets (1.0.2) lib/quiet_assets.rb:18:in `call_with_quiet_assets'
actionpack (4.1.1) lib/action_dispatch/middleware/request_id.rb:21:in `call'
rack (1.5.2) lib/rack/methodoverride.rb:21:in `call'
rack (1.5.2) lib/rack/runtime.rb:17:in `call'
activesupport (4.1.1) lib/active_support/cache/strategy/local_cache_middleware.rb:26:in `call'
rack (1.5.2) lib/rack/lock.rb:17:in `call'
actionpack (4.1.1) lib/action_dispatch/middleware/static.rb:64:in `call'
rack (1.5.2) lib/rack/sendfile.rb:112:in `call'
sentry-raven (0.9.1) lib/raven/rack.rb:55:in `call'
railties (4.1.1) lib/rails/engine.rb:514:in `call'
railties (4.1.1) lib/rails/application.rb:144:in `call'
rack (1.5.2) lib/rack/lock.rb:17:in `call'
rack (1.5.2) lib/rack/content_length.rb:14:in `call'
rack (1.5.2) lib/rack/handler/webrick.rb:60:in `service'
/Users/mwm/.rvm/rubies/ruby-2.1.1/lib/ruby/2.1.0/webrick/httpserver.rb:138:in `service'
/Users/mwm/.rvm/rubies/ruby-2.1.1/lib/ruby/2.1.0/webrick/httpserver.rb:94:in `run'
/Users/mwm/.rvm/rubies/ruby-2.1.1/lib/ruby/2.1.0/webrick/server.rb:295:in `block in start_thread'

If this is already supported, I suspect the issue has to do with the fact that I am using a rails engine and essentially have two application controllers. I've tried adding include Authority::Controller to both the host app application controller and the engine application controller as described (here)[https://github.com/nathanl/authority/issues/64]. Would I need to add Authority as a dependency of the engine even though it is only concerned with the host app?

nathanl commented 10 years ago

I find that backtrace pretty confusing - the only thing I can guess about it is that maybe it has something to do with the order in which files are getting loaded. I don't see anything pointing back into the authority gem code.

What happens if you try giving it a method name instead? Eg, authorize_actions_for :the_authorizer, :unless => :devise_controller?, then def the_authorizer; "ApplicationAuthorizer".constantize; end. That way we wait as long as possible to evaluate that constant (though you're not getting a missing constant error, so this is just a wild guess).

I don't see any reason why you'd need to include Authority::Controller in your engine's ApplicationController unless you plan to call Authority's controller methods in the engine.

nathanl commented 10 years ago

Just had a thought. Maybe your missing comma is the issue? Maybe this is being intepreted as authorize_actions_for(ApplicationAuthorizer({:unless => :devise_controller?})), as a capital method like Array()?

mwmeyer commented 10 years ago

Hey Nathan,

I figured out that rather than defining a new authorizer for each controller, a much neater solution that worked for my use case is to just define a default ApplicationAuthorizer and then authorize my controller with a model with the authority abilities included:

# application_authorizer.rb
class ApplicationAuthorizer < Authority::Authorizer

  def self.default(able, user)
    user.role_allowed?
  end
end
# my_controller.rb
class MyController < ApplicationController
  ensure_authorization_performed
  # User model has include Authority::Abilities
  authorize_actions_for User
end
mwmeyer commented 10 years ago

@nathanl

Note, the above method only works for controller actions specified in the controller_action_map:

config.controller_action_map = {
 :index   => 'read',    # `index` controller action will check `readable_by?`
 :show    => 'read',
 :new     => 'create',  # `new` controller action will check `creatable_by?`
 :create  => 'create',  # ...etc
 :edit    => 'update',
 :update  => 'update',
 :destroy => 'delete'
}

If I want to authorize a custom controller action, based on some user resource, I also need to either manually, I either have to add every action to the controller_action_map, or use the authority_actions filter for every custom action in my controller like so:

# my_controller.rb
class MyController < ApplicationController
  before_filter :authenticate_user!
  ensure_authorization_performed
  # User model has include Authority::Abilities and has all verb abilities (read, write, etc.)
  authorize_actions_for User
  authority_actions :my_custom_action_1 => 'read'
  authority_actions :my_custom_action_2 => 'read'
end

This works, but its seems redundant. Is there some way to tell rails to add every action of a controller to the controller_action_map automatically?

nathanl commented 10 years ago

Is there some way to tell rails to add every action of a controller to the controller_action_map automatically?

I guess you could get a list of the controller's actions and iterate through them if you wanted.

authority_actions Hash[ action_methods.map{ |name| [name, 'read'] } ]

I'm honestly pretty confused about your use case. None of your controllers have a resource? And you want all actions of all controllers to be authorized by the same application controller as "read"s?

That seems very unusual. If you're sure that's what you want, maybe Authority isn't a good fit for what you're doing? Its goal is to give you a lot of granularity.

But whatever floats your boat. :)