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

add stand-alone authorizer #136

Closed
wants to merge 1 commit into from

Conversation

billychan
Copy link

I found a stand alone authorizer is desired sometimes, when there is no involvement of controller.

I've wrote such class in my app's lib and being quite happy to use it for days. I guess the main repo may need this as well to provide more support to service object.

Test and doc can be added later if the idea is okay.

@thomasklemm
Copy link
Collaborator

Thanks Billy. I'll add a note to the Readme pointing people who want to use Pundit outside of a controller here, so they can have a look at your example. It seems to me that this requirement is not so common though that we should include this authorizer into pundit itself.

@jdickey
Copy link

jdickey commented Jul 15, 2014

It's not uncommon to people writing traditional monorail (Rails Way) apps, but once you start dealing with any sort of service layer, controllers are likely to not be where you want to have your authorisation live. See, for example, ActiveInteraction and other, similar service-layer implementations.

(edited to use the word I meant, highlighted, rather than its nonsense-in-context opposite.)

@jdickey
Copy link

jdickey commented Jul 22, 2014

Trying to make this work; @billychan or all, any ideas where I'm going wrong? This Gist shows me trying to move use of Pundit from direct-within-the-controller to a service layer. I'm mortally confused here...

@billychan
Copy link
Author

@thomasklemm Thanks! And yes I agree Pundit core can be an independent authorize at first by itself.

@jdickey I've checked your code. Did you mean problem comes from here: https://github.com/jdickey/new_poc/blob/pundit-1/app/controllers/sessions_controller.rb#L18

I'm not familiar with ActiveInteraction, but it looks you havn't implemented my pattern, and the logic around Pundit seems too complicated. I can't figure it out at first look, it just shouldn't be that complex.

To implement the pattern in this pull request, you need to add the module by yourself at first because it's not in Pundit core.

  1. Create a file pudit_authorizer.rb in /lib under Rails root. Copy the code to that file.
  2. Define the polices normally as per Pundit doc.
  3. Call this independent authorizer at any place you need it, say in #execute
authorizer = PunditAuthorizer.new(user, post) 
# 'user' can be 'current_user' in your code, 
# and 'post' is the object you want to authorize.

authorizer.authorize_on 'create' # 
# Send the action you want to authorize

That's all and quite straightforward. Then you need to rescue unauthorized error as before.

@paulccarey
Copy link

@billychan @jdickey I have a very similar use case right now. I'm using the mutations gem and currently just calling my Policy like `ThingPolicy.new(user,resource).create?`` its less than ideal.

I have a few ideas on how to make this better but ideally I'd also like a catch all in my controllers or service objects that would raise an error if authorization has not been performed or explicitly skipped.

I'll update with any progress I make on the topic, in the meantime thanks for the above.

@openface
Copy link

@paulccarey Any progress to report on your findings here? We're too looking to integrate Mutations gem (service objects) with a simple Authorization routine. I'm keen on using Pundit, but we have a mixed case of needing to perform authorization from both controllers and from the service objects, so a stand-alone authorization solution would be ideal.

@paulccarey
Copy link

@openface this slipped my mind if I'm honest due to other pressing priorities, however thanks for the reminder, I'll try to find some time over the coming days and put it on my todo list as this could become a real issue for us in the near future.

@openface
Copy link

@paulccarey Does #227 address this fully?

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

Successfully merging this pull request may close these issues.

5 participants