I also think the validations should be on the model. I was simply trying to state that any action can be narrowed down to a CRUD type.
Hernan's 3rd point is a very strong reason as to why it shouldn't be moved entirely to the model. ~Kenny On Tue, Jul 7, 2009 at 4:36 PM, Hernan Astudillo <[email protected]> wrote: > 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 -~----------~----~----~----~------~----~------~--~---
