Comments (19)
So I can picture this better, can you give a little bit more example code of how you'd do this without and and with the new method?
from authority.
Sure. It would look smth like this. Let's say there's a controller that lets user to administrate his photos. Currently, you'd need Authorizer for both user and user photos. Then use smth like this in each UserAdminPhotos controller action:
authorize_action_for(@photo)
In Photo authorizer, it would check for photo permission by making sure only user himself touches it when administrating pictures.
I think may be better in some cases is smth like this in UserAdminPhotos controller:
authorize_for(:update, @photo.user)
This leverages User authorizer, no need for Photo authorizer. If visitor can edit owner of the picture (e.g. he's the owner himself or admin of the system), he can edit his pictures too.
I know this is nitpicking, but I think it would make Authority much more flexible.
from authority.
@mantas - Sorry I'm so slow to respond.
I see what you're saying, but I don't think this is the best direction to go. If we add that controller method, you can avoid the need for a UserAuthorizer in the controller, but you would still want to be able to say in your views: "show this link if current_user.can_edit?(@photo)
, and to answer that, you'd need a PhotoAuthorizer.
Also, if the logic for photos is really that simple, you don't need a big class.
class PhotoAuthorizer < Authority::Authorizer
def crudable_by?(user)
user.can_edit?(resource.owner)
end
# Etc
[:creatable_by?, :deletable_by?].each {|m| alias_method m, :crudable_by? }
end
Does this make sense?
from authority.
Thought a bit more. I suppose you may say that you decide whether to show a link to edit a photo by asking current_user.can_edit?(@photo.owner)
, or something like that. So, again, you avoid needing a PhotoAuthorizer.
But here's the thing: you have, in your head, the knowledge that anyone who can edit a user can edit his/her phots. Where should you encode that knowledge in your application?
If you do current_user.can_edit?(@photo.owner)
in your views and authorize_for(:update, @photo.user)
in your controller, you're spreading that knowledge out all over the place. It's not self-explanatory for the next programmer who sees it, and if the rules change, you have a lot of code to update.
On the other hand, if you use current_user.can_edit?(@photo)
and authorize_action_for(@photo)
, you can encode the knowledge you have in exactly one place: the PhotoAuthorizer. Its whole reason for existence is to be the canonical place where you store your knowledge about who can do what with photos.
Yes, you have to create another class, but you can make the actual code for it minimal, and I think you get the benefit of better separation of concerns.
This is my opinion, of course, but it's really sort of the philosophy behind Authority.
What do you think?
from authority.
@mantas - I agree with @nathanl, it's more convenient method to store authorization logic in a separate authorizer class for photos.
from authority.
Hi,
I see what you mean.
Is crudable_by an existing method? I don't see any mentions of it neither in documention, nor in code itself. That way might sort of work, because one of the biggest pains using authorize_action_for was authorizing non-standard-rest methods.
I stand behind my solution though. I don't put authorize_for all around the code too though. I've a before_action that I run in all needed controllers. Same before_action takes care of both photos and albums access management as well.
Anyhow, I guess the best way is for me to pack it as an additional gem.
from authority.
Please look at the section "Defining Your Abilities" in README. You can define crudable
in the initializer. But I prefer word "manageable".
from authority.
I'm aware of defining abilities in initializer. But it seem to require to add each custom method to it. It would be nice if it would be possible to add catch-all method.
from authority.
Is crudable_by an existing method? I don't see any mentions of it neither in documention, nor in code itself.
No, sorry, I didn't mean for that to be confusing. It's just a random method I made up in that one authorizer so that I could alias all the normal methods to do the same thing. I could have called it is_made_of_cheese?
. 😄 The point was just to show that, if all the permissions work the same way, you don't need much code to declare that. And then if something changes and, for example, createable_by?
needs to be different, it's a tiny code change in the authorizer to make it so.
I don't put authorize_for all around the code too though.
What are you doing in your views so that you don't show a link to edit a photo unless the user is authorized to do so? Without a PhotoAuthorizer, aren't you forced to do something like current_user.can_edit?(@photo.owner)
?
from authority.
Ah. .. Adding each custom method there seems a wee overkill to me.
I'm using authorize_for in controller to satisfy ensure_authorization_performed. For views, current_user.can_edit?(@photo.owner)
is good enough.
from authority.
Right, but can_edit?()
what? What do you pass to that method?
from authority.
Updated previous comment for clarity
from authority.
Gotcha. In that case, all of your views have encoded in them the knowledge that, in order to edit a photo, you must be authorized to edit its owner. As opposed to that knowledge being encapsulated in a single authorizer.
That's what I was trying to say.
from authority.
I'm not using it much in views. If I were, I'd sure use a helper to dry up the code. Since I use it for mostly controller/action authorisation, this is not an issue for me. Catching every custom action would cause more code smell for me.
Anyhow, I got the answer I asked for.
from authority.
OK. 😄 If you want this feature enough to make a fork or monkey patch, I think it would be pretty simple; you could just break up authorize_action_for
into two methods, so that the call to enforce
is done in a method that takes the action as a parameter, and then you could call that directly when you want to.
authority/lib/authority/controller.rb
Line 99 in fa510fb
Anyway, thanks for using Authority!
from authority.
Thank you for the help. I've already made this patch and am using it for the past couple weeks. So far so good :)
Keep up the good work on Authority. Since CanCan seems to be abandoned, Authority is the only good authorisation library for Rails :)
from authority.
@mantas - you know what? After saying how simple this would be to do and seeing that it's really just a matter of breaking apart a method, I've changed my mind. It's not a bad refactor, it doesn't require any additional tests, and it will help you not have to maintain a fork.
Is this all you've done in your patch?
def authorize_action_for(authority_resource, *options)
# `action_name` comes from ActionController
authority_action = self.class.authority_action_map[action_name.to_sym]
if authority_action.nil?
raise MissingAction.new("No authority action defined for #{action_name}")
end
authorize_for(authority_action, authority_resource, *options)
end
def authorize_for(authority_action, authority_resource, *options)
Authority.enforce(authority_action, authority_resource, authority_user, *options)
self.authorization_performed = true
end
If so, submit a pull request and I'll merge it. I will probably leave the second method as an undocumented feature since I think most people are better off using the normal way, but if anyone lands on the PR, they will see that it's available.
from authority.
Keep up the good work on Authority.
Thanks for the kind words. I do want to keep Authority as a great option, but I'm doing non-Rails Ruby work lately, so it's a bit harder for me to stay on top of it. If you like the gem and have time, maybe consider helping keep it updated.
Of course, you have to deal with my "benevolent dictator" attitude if you work on it. 😆
from authority.
If I'll bump into any bugs, I'll sure submit a patch ;)
I appreciate people who take "benelovent dictator" status seriously. That's why I asked for your take instead of blindly submitting a pull request ;)
from authority.
Related Issues (20)
- authorize_action_for: wrong number of arguments when using Sequel gem HOT 11
- How can I silence Authority.logger in test environment? HOT 6
- Should be usable outside of Rails
- controller action that involves data from multiple resources HOT 4
- Update authorization flow diagram HOT 1
- Issue with rails 4.2.5.1 HOT 2
- Is it possible to define multiple authorizers for a single method? HOT 2
- Allow custom opts in `authorize_actions_for`
- Log all authorization attempts? HOT 5
- Authorization with Rails Admin Gem HOT 3
- Could we use authority with active model serializer ? HOT 1
- Anyone have a good implementation of Authority with JSONAPI::Resources? HOT 1
- undefined method can for custom action HOT 2
- Controller action authorization not checked unless specifically adding authorize_actions_for HOT 3
- authority can method not working
- help with ensure_authorization_performed HOT 2
- Help with maintenance? HOT 10
- Instance level rules should rely on instance level default HOT 5
- The workflow for multiple controllers working with one model. HOT 2
- Use of :all_actions and :actions simultaneously HOT 4
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from authority.