Namespaced policies
Closed this issue ยท 44 comments
The app I'm working on has two sections: user and admin. There are multiple types of users and multiple types of admins, and different authentication solutions are used for each of them.
Although all the application models are used by both admins and users, policies are very different. Handling both users and admins and all distinct types within single policy was not so clear and full of if
s.
The way I handled this was to dynamically define policy_class
on every ActiveRecord
descendant in before filter. Something like this:
def set_policy_namespace
m = self.class.name.split("::").first
active_record_descendants.each do |ar|
ar.class.instance_eval do
define_method(:policy_class) do
"::Policies::#{m}::#{self.name}Policy"
end
end
end
end
def active_record_descendants
Rails.application.eager_load! unless Rails.application.config.cache_classes
ActiveRecord::Base.descendants
end
So, if I'm in Admin::HomeController
, Project
model will have policy_class
redefined to return ::Policies::Admin::ProjectPolicy
.
I've tried to demonstrate valid use case for this. Do you have any thoughts on how can this be made easier? I'm not sure whether or not it should be a part of gem (since it is somewhat rare).. I'm just asking for opinion.
thanks
I don't really understand what you're trying to do.
I realised that it was design issue with my application. I had multiple "sub applications" inside one large rails application, and each one required completely different policies. The solution was to break the big app into smaller ones.
I think this is still a valid issue. I'm about to create a system that has controllers for the same resource namespaced under two user classes (User and Admin). What's the recommended practice for doing so other than filling up policy files with tons of conditionals.
Example:
admins/widgets
users/widgets
Could you be trying to do what @assembler described as fitting multiple sub apps into one large app?
They're not distinct apps, just namespaced areas of the web app for different users types.
I see, like a web interface and an admin backend in one app. They might share models but have a completely different set of controllers / workflows and might thus need completely different policies for one model, right? Any suggestions how Pundit can help you do what you want to do?
Exactly. Off the top of my mind, I'd assume namespacing the policies themselves, such as
/policies/widget_policy.rb -> WidgetPolicy
/policies/admins/widget_policy.rb -> Admins::WidgetPolicy
I guess the alternative would be to check the class of current_user and include different policies subclasses manually?
# Currently this works
authorize @widget, :update?
# which is equivalent to
raise Pundit::NotAuthorizedError unless WidgetPolicy.new(current_user, @widget).update?
# However, if you need a custom namespaced policy you can use
raise Pundit::NotAuthorizedError unless Admins::WidgetPolicy.new(current_user, @widget).update?
Would that be enough to get started? When you're further into your app, we'll see if a custom solution is still needed and how it can look like.
Are you suggesting perhaps creating an authorize_admin
method on my end that would infer the class and make the second call? That could work. I'll update this thread as I get deeper. Cheers.
How about an authorize @widget, :update?, policy: Admin::WidgetPolicy
option in an options hash on the Pundit side? Or a policy_namespace: 'Admin'
option for writing a general helper.
# app/helpers/authorization_helper.rb
module AuthorizationHelper
def authorize_admin(*args)
record, query = args[0], args[1]
options = args[2].merge({policy_namespace: 'Admin'}) # Will look for an Admin::WidgetPolicy when @widget is passed in
authorize record, query, options
end
end
That being said, it's all hypothetical currently with no app to use it, so it won't go into Pundit yet.
I think that this could work out of the box.
If you are in Admin module:
module Admin
class WidgetsController
# referencing WidgetPolicy
end
end
Referencing WidgetPolicy
from that controller would see if Admin::WidgetPolicy
exists, and if it is not found, it would try to find ::WidgetPolicy
. This is default behavior of ruby, and pundit should confront to it.
We're currently not inferring policy names from the controller, but from the record passed in. (Source).
I know. My point was that any policy you reference from within controller within specific module, should naturally search inside that module for policy definition. My example would be the same for e.g. UserPolicy
within WidgetsController
.
I would also like to have separate policies for different needs (eg, admins), it would be nice for keeping the policies focussed and descriptive, and for returning different error messages.
@thomasklemm More than passing in a namespace, I like your idea of being able to do
authorize @widget, :update?, policy: Admin::WidgetPolicy
Controller namespaces for the same model is a common use case for us as well. I think the simple policy:
option makes a lot of sense.
I'd love to see how people are wiring together their authorization layer without namespaces on non-trivial apps. For example, many people use active_admin
which is commonly integrated into the same Rails app. So how are you supposed to build policies for UsersController
vs Admin::UsersController
? Currently they both refer to the same UserPolicy
.
Here's why I consider it an insecure default:
/users => UserPolicy.index?
/admin/users => UserPolicy.index?
With that in mind, I'd consider automatic namespacing essential.
We never use the index/resolve component of Pundit. It's not useful in apps with any kind of user role system.
Here's the controller concern I created for our admin namespace for anybody trying to do something similar. It works as a temporary workaround but I'd rather have a solution built into pundit.
module AdminPolicies
extend ActiveSupport::Concern
included do
helper_method :admin_authorize
helper_method :admin_policy_scope
end
# These are workarounds for the lack of support for namespacing in pundit
# https://github.com/elabs/pundit/issues/12
def admin_authorize(record, query=nil)
klass = "Admin::#{record.model_name}Policy".constantize
policy = klass.new(current_user, record)
query ||= "#{params[:action]}?"
@_policy_authorized = true
unless policy.public_send(query)
error = Pundit::NotAuthorizedError.new("not allowed to #{query} this #{record}")
error.query, error.record, error.policy = query, record, policy
raise error
end
true
end
def admin_policy_scope(scope)
klass = "Admin::#{scope.model_name}Policy::Scope".constantize
policy = klass.new(current_user, scope)
@_policy_scoped = true
policy.resolve
end
end
Very handy. Cheers.
I'm not opposed to inferring the namespace from the controller. If we do a const lookup on a particular constant, it will look in it first, and then in the global namespace, which sounds like exactly the behaviour we want. And it does seem sane to do something like Admin.const_get(...)
. Can anyone provide a reasonably clean PR for this?
I'm taking a stab at this. Hopefully I'll have a PR tomorrow.
I'm running into a frustrating problem though. const_defined? returns false for the policies because they have not yet been autoloaded. Does anybody know any way around that other than a rescue block?
@adamcrown Do you have code anywhere to look at? I was looking into using Module.parent
provided from the controller but was running into problems since all lookups are done via the Pundit
module. It's an initial pass and I was trying to avoid significant refactors. I was thinking it would be reasonable to have the Pundit.policy*
defined on the controller to make this simpler. @jnicklas @thomasklemm, any thoughts?
ecbypi/pundit@71db8c35d18aa0ceaaf8403388257cd384ef105f
@ecbypi, here's what I have so far: adamcrown@d3092ec
I had to pass the controller around as an argument a bit because the finder is always called from the Pundit module, as you said. I'm hoping I can improve on that but I wanted to get something working initially before attempting some bigger refactor.
No tests yet, and one test still red. But it does seem to be working in the app I'm using it with. Feel free to take whatever you want from my code and take a closer look at yours when I have some more time.
I like the code that @ecbypi linked. I'm against moving Pundit.policy
though. It's nice that the code as is does not break any APIs, and I'm cool with having the namespace
parameter, so I think moving it would just break people's code unnecessarily.
@adamcrown's suggestion was maybe a bit convoluted? Are all these changes really necessary? The resulting code is pretty hard to read.
My code is definitely a work in progress. It works. But I'm not happy with it yet either. If @ecbypi can get a PR up soon, I'm happy to have his code merged.
I'm adding a test to make sure that it still finds policies in the global namespace if there isn't one in the controller's namespace. After I get that working I'll submit a PR.
Implemented in #152.
I'm still having issue with namespaces.
I have model Question.
app/models/question.rb
class Question < ActiveRecord::Base
end
Created two different policies for simple user and admin:
app/policies/question_policy.rb
class QuestionPolicy < ApplicationPolicy
end
app/policies/admin/question_policy.rb
class Admin::QuestionPolicy < Admin::ApplicationPolicy
end
app/controllers/admin/questions_controller.rb
class Admin::QuestionsController < Admin::ApplicationController
def index
authorize @questions.first
end
end
In my admin controller app/policies/question_policy.rb is used, not app/policies/admin/question_policy.rb
@jizak, const_get
looks through namespace
's ancestry to find a constant. It might be that if Admin::QuestionPolicy
hasn't been loaded, then when it gets to Object
and looks for QuestionPolicy
it loads QuestionPolicy
through the Rails' constant autoloading.
Does it work if you add require_dependency 'admin/question_policy'
in the controller?
I just released a gem the other day. It is compatible with Pundit's policies and allows you to namespace based on controller, model or both. Check it out if you still need this functionality -> Regulator
Thank you!
Hope you can add "Use" section with a few examples to the readme... The
tests don't speak clearly to me.
On Fri, Jul 24, 2015 at 12:47 PM Cory ODaniel notifications@github.com
wrote:
I just released a gem the other day. It is compatible with Pundit's
policies and allows you to namespace based on controller, model or both.
Check it out if you still need this functionality. Regulator
https://github.com/coryodaniel/regulatorโ
Reply to this email directly or view it on GitHub
#12 (comment).
Will do this Sunday. No problem!
On Jul 24, 2015, at 6:31 PM, Andrei Andreev notifications@github.com wrote:
Thank you!
Hope you can add "Use" section with a few examples to the readme... The
tests don't speak clearly to me.
On Fri, Jul 24, 2015 at 12:47 PM Cory ODaniel notifications@github.com
wrote:I just released a gem the other day. It is compatible with Pundit's
policies and allows you to namespace based on controller, model or both.
Check it out if you still need this functionality. Regulator
https://github.com/coryodaniel/regulatorโ
Reply to this email directly or view it on GitHub
#12 (comment).โ
Reply to this email directly or view it on GitHub.
And -- I appreciate your releasing. It. Model-based permissions feel
strange for me as most of my apps have Admin-namespaced controllers. I have
an app ready to use this !
On Fri, Jul 24, 2015 at 8:04 PM Cory ODaniel notifications@github.com
wrote:
Will do this Sunday. No problem!
On Jul 24, 2015, at 6:31 PM, Andrei Andreev notifications@github.com
wrote:Thank you!
Hope you can add "Use" section with a few examples to the readme... The
tests don't speak clearly to me.
On Fri, Jul 24, 2015 at 12:47 PM Cory ODaniel notifications@github.com
wrote:I just released a gem the other day. It is compatible with Pundit's
policies and allows you to namespace based on controller, model or
both.
Check it out if you still need this functionality. Regulator
https://github.com/coryodaniel/regulatorโ
Reply to this email directly or view it on GitHub
#12 (comment).โ
Reply to this email directly or view it on GitHub.โ
Reply to this email directly or view it on GitHub
#12 (comment).
how do I implement the policy:
option from here?
I have this working:
raise Pundit::NotAuthorizedError unless Admin::HomePolicy.new(current_user, :home).index?
but not this:
authorize :home, :index?, policy: Admin::HomePolicy
is this still the best approach?
Still do not know how to use, where is the documentation?
@thomasklemm, actually I figure it from older issues, please look at #382 #216
TL;DR: override policy(record)
method
It took me a long time to figure out how to add namespace and do a controller based authorization, after tracing down the source code, it turns out very easy, I just need to override policy(record)
from Pundit.
My code:
I'll just post my solution as reference here, hope I can save someone's time.
In my base controller:
# app/controllers/api/v1/base_controller.rb
class Api::V1::BaseController < ApplicationController
include Pundit
after_action :verify_authorized
private
def policy(record)
policies[record] ||= "#{controller_path.classify}Policy".constantize.new(pundit_user, record)
end
end
In my controller
# app/controllers/api/v1/auth/offers_controller.rb
class Api::V1::Auth::OffersController < Api::V1::Auth::BaseController
def index
authorize Offer
# ...
end
end
My policy
# app/policies/api/v1/auth/offer_policy.rb
class Api::V1::Auth::OfferPolicy < Api::V1::BasePolicy
def index?
true
end
end
I came up with a different approach to the problem. Since Pundit does model-based authorization, no point fighting it.
Given a Widget
model and the need for a Admin
namespace policy, I created a new model Admin::Widget
which just inherit Widget
:
# app/models/admin/widget.rb
class Admin::Widget < Widget
end
On the controller side, instead of using the Widget
model for admin-related tasks, I use Admin::Widget
:
@widget = Admin::Widget.first
Policy-wise, I can have both WidgetPolicy
and Admin::WidgetPolicy
without any problem, keeping both authorization rules separate and neat.
Yet another option, in cases where defining policy_class
on the model is undesirable:
authorize OpenStruct.new(policy_class: ArbitraryPolicy, widget: @widget)
Thanks @adamcrown for his great work around.
Below is my modified base on his solution. Link to gist
module ScopedPolicies
extend ActiveSupport::Concern
included do
helper_method :authorize
helper_method :policy_scope
end
# These are workarounds for the lack of support for namespacing in pundit
# https://github.com/elabs/pundit/issues/12
def controller_namespace
@controller_namespace ||= self.class.to_s.sub(/::[A-z]*Controller/, '')
end
def authorize(record, query = nil)
klass = "#{controller_namespace}::#{record.model_name}Policy".constantize
policy = klass.new(current_user, record)
query ||= "#{params[:action]}?"
@_policy_authorized = true
unless policy.public_send(query)
error = Pundit::NotAuthorizedError.new("not allowed to #{query} this #{record}")
error.query, error.record, error.policy = query, record, policy
raise error
end
true
end
def policy_scope(scope)
klass = "#{controller_namespace}::#{scope.model_name}Policy::Scope".constantize
policy = klass.new(current_user, scope)
@_policy_scoped = true
policy.resolve
end
end