I think your proposal is great. My thoughts: 1.- the "restore defaults" method it's to avoid backward compatibility issues? If true, great idea, but i'll logger.warn that to the developer. 2.- you read my mind (or i read your post :D ). However, making custom actions to require base actions pass the check (update in order to approve), could make a little conflict: supose that you want to approve but not make the record editable. Of course you can exclude the "edit" action, but does it disables the update action from passing querystring params? 3.- I agree, too. But "most" of the times is posible to put the security checks on the model. But sometimes you need to rely not only on the user(who) and the model(what), but in the controller (where). You don't want to mix the same options (security checks) on Admin controllers and Browse controllers, for example. And sometimes is not good idea to use the same controller for both, and the admin don't want to see edit or destroy buttons when browsing. So, i would make security methods default to (column)_authorized_for_(action) as you said, but still allow to override that with a :security_method in the controller. That way you also solve a backward compatibility issue.
On Tue, Jul 7, 2009 at 4:26 PM, <[email protected]> wrote: > > On Tue, 7 Jul 2009 14:52:52 -0500, Kenny Ortmann <[email protected]> > wrote: > > The way ActiveScaffold works I don't see any reason for it to check any > > other type of action. > > All of the things you just listed can be classified as a crud type, > > activate - update, > > suspend - update, > > pdf - read, > > review - read, > > comment - create, > > diggit. - probably create > > > > You can set the crud type on your action_links to be one of these values. > > > > One thing that I could see being an issue. > > Say on my model I have: > > > > def id_authorized_for_update? > > return false > > end > > > > Which means any action with the crud type of update will not be allowed > to > > change my id column. > > I agree ActiveScaffold security is not enough for some cases. I wrote a > proposal, but I only get a reply, it was a positive reply. This is my > proposal: > > I'm working currently in a project which requires finer grained permissions > > than other projects I made before. Also bugs hard to find and fix were > reported > recently related to security code (like issue 21 in github). So I think > currently is confusing and it's not enough in some cases. > > I have some ideas to improve it: > > 1. authorized_for? called in a model should look for security methods at > class, instead of looking for instance methos in a new instance. This > change > can be backwards incompatible. Code which returns true for new records > won't > be affected, but code which hide links returning false for new records > would > need to add class methods. Maybe I could add a method to restore current > behaviour globally. > > Problems in issue 21 were caused by ActiveScaffold calling a security > method > twice, first to show or hide an action link in a new instance, second to > enable > or disable the action link with a instance loaded from DB. It wouldn't be a > > problem separating methods to show or hide an action link (using class > methods) from methods to enable or disable an action link (using instance > methods, like now). > > Another problem was links_for_association, which sets links in columns with > associations, calls each in columns object, and then authorized_for_read? > is checked, and it was causing some problems, first requestfailed to add > links because session was not started and user was not logged, second > request added the links (in development mode). > > 2. Using crud type to enable or disable an action link is not enough for > custom action links. I would look for security methods with action name and > > crud_type. > For example it would look for authorized_for_update? and > authorized_for_edit? > for edit action link. > > If I want to add an action link to approve an order, I would set crud type > to > update. Currently it would check permissions with authorized_for_update?. I > > can't disable editing but enable approving with current schema. Adding this > I > could enable approving with authorized_for_approve? and disable editing > with > authorized_for_update? > > 3. Remove the security method from action links. IMHO security checks > should > be at the model. So we can change security method calls in views or > controllers to direct calls to authorized_for? method in the model (class > or > instance). > > Security checks for nested forms don't use security method because we > doesn't > want to create a new controller for associated model (and I think it's not > easy and it would have high cost). People who relies on overridden security > > methods to implement security checks find it doesn't work for nested forms. > If > we remove security methods and force to set security checks in the model it > > would work for normal scaffolds and nested forms. > > What do you think about this changes? > > > > > > On Tue, Jul 7, 2009 at 1:03 PM, Hernan Astudillo <[email protected]> > wrote: > > > >> Hi all, > >> Is there a reason to block authorized_for? to only the 4 crud > >> actions? > >> i mean this line in active_scaffold_permissions.rb: > >> def authorized_for?(options = {}) > >> raise ArgumentError, "unknown action #{options[:action]}" if > >> options[:action] and ![:create, :read, :update, > >> :destroy].include?(options[:action]) > >> > >> It would be nice to allow any action to pass through this security > >> structure. For example, use security for many posible actions that apply > >> like activate, suspend, pdf, review, comment, diggit... > >> > >> I checked the code and i don't see anything that could be affected by > >> passing any other actions as parameter. However, in the view > >> (_list_actions.html.erb) it doesn't look so good to be using "crud_type" > >> as > >> the action to be checked against to, instead of the real :action > >> parameter, > >> but changing that might need double checking for backward compatibility > >> issues. I just say commenting out that raise ArgumentError which does > >> nothing. > >> > >> > >> > > >> > > > > > > > > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "ActiveScaffold : Ruby on Rails plugin" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/activescaffold?hl=en -~----------~----~----~----~------~----~------~--~---
