On Tue, 7 Jul 2009 17:36:31 -0400, 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?
It doesn't, so you should forbid update crud type and authorize approve
action, authorized_for_#{action}? will have higher priority, like
#{column}_authorized_for_#{crud_type} has higher priority so you can forbid
update (disable the edit link) but authorize update in a column (for
inplace_edit).
> 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.
I think you can check it in the model anyway, admin controller will be use
login and browse controller doesn't, or admin controller will be used by an
admin user. But I have no problem leaving security_method, it's a feature I
don't use. There is a reported bug, which it's not a bug IMO, about
security method not being used for association links, but it's not possible
(or it's bad) use another controller for a security check, and in that case
authorized_for? is called directly in the controller.
>
> 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
-~----------~----~----~----~------~----~------~--~---
