[decl_auth] Taking it down to attribute level

[Jan Luehr]

Hello,

I started playing around with Declarative Authorization and noticed, that I
ACLs cannot be taken down to attribute level granularity.
Since this feature seemed to be requested before [1], I start hacking a few
lines of code.

If you're interested:
The good news is: There are no changes to declarative_authorization -
especially to it's dsl.
The bad news is:
- There is some ugly ActiveRecord hooking / patching involved
http://pastie.org/962758
- ACLs can be circumvented, if attrs are accessed rawly. (through
object.attributes[...] - blocking by proxing might be possible, but is out of
scope for me right now )

Imho the code speaks for itself: Permissions can be defined as usual:
role :editor do
has_permission_on :entries, :to =>
[:read_title,:read_content :write_content]
end

and alias-method chains are used protect the attributes. (ActiveRecord
style :-) ) - comments welcome. Feel free to maintain this snipplet as a part
of decl-auths codebase - it'll easy my pain of porting it to future versions
of ActiveRecord ;-)

Keep smiling
yanosz

[1]
http://groups.google.com/group/declarative_authorization/browse_thread/thread/a8ab427e6247d0b0/42eee1fef4704abd?lnk=gst&q=attribute#42eee1fef4704abd

--
You received this message because you are subscribed to the Google Groups "declarative_authorization" group.
To post to this group, send email to declarative_...@googlegroups.com.
To unsubscribe from this group, send email to declarative_author...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/declarative_authorization?hl=en.

[Jan Luehr]

Hello,

On Sunday 16 May 2010 20:41:02 Jan Luehr wrote:
> Hello,
>
> I started playing around with Declarative Authorization and noticed, that I
> ACLs cannot be taken down to attribute level granularity.
> Since this feature seemed to be requested before [1], I start hacking a few
> lines of code.
>
> If you're interested:
> The good news is: There are no changes to declarative_authorization -
> especially to it's dsl.
> The bad news is:
> - There is some ugly ActiveRecord hooking / patching involved
> http://pastie.org/962758
> - ACLs can be circumvented, if attrs are accessed rawly. (through
> object.attributes[...] - blocking by proxing might be possible, but is out
> of scope for me right now )

I continued playing around: In order to easy the AR-dependencies, I patched
decl_auths in_model.rb to inject the necessary getter / setter methods and
removed all AR-patching code.
Comments welcome:
http://pastie.org/963721

Keep smiling
yanosz

[vanboom]

Nice approach - I am trying to do the same thing - but using "if
has_role?(...)" tests instead. It makes my authorization rules dirty
- maybe EDEN can offer some more suggestions?
thanks,
Don

On May 18, 3:28 am, "Jan Luehr "

> For more options, visit this group athttp://groups.google.com/group/declarative_authorization?hl=en.- Hide quoted text -
>
> - Show quoted text -

[Jan Lühr]

Hello,

On Tuesday 08 June 2010 18:55:12 vanboom wrote:
> Nice approach - I am trying to do the same thing - but using "if
> has_role?(...)" tests instead. It makes my authorization rules dirty
> - maybe EDEN can offer some more suggestions?

Thanks - I'm wondering: Why do you use has_role?(...)?

Btw. I noticed a few bugs - a new version is here:
http://pastie.org/1000711

Keep smiling
yanosz

[Jan Lühr]

Hello,

On Friday 11 June 2010 20:48:39 Blake M wrote:
> Hello Jan,
>
> Thanks for the patch. It's exactly what I needed.
>
> I made a small modification to your code so that you can set
> permission on :read_all and :write_all for the times when the role
> will always have access. Code change can be found here
> http://pastie.org/1000711

That's what :write and :read are for (and it's already implemented ;-) )
I don't use :read_all, 'cause 'an attributed named all might exist.

Btw. I don't see any changes in your pastie-link.

Keep smiling
yanpo

[Blake McIntyre]

Turns out I posted too soon. After I did I noticed that the way I implemented the all did not actually work and that it was already done with write and read. I think I also messed up the pasties as well with some poor copy and pasting.

[Jan Lühr]

Helllo,

the code envolves and I'd like to share the current one - comments welcome:
http://pastie.org/1028758

A short note on what the patch actually does:
- It provides methods for injected ACL-Checks into the method chain
- If enabled, access to ActiveRecord attributes and association-proxies (new)
is restricted.
- It can be enabled via: http://pastie.org/1028767
- It allows querying of readable or writable attributes:
http://pastie.org/1028781 as a replacement for activerecord_base.attributes

and what it doesn't do
- Protect any instance or class-method from being called. (methods are
blacklists, not whitelisted. Changing it is trivial, but whitelisting
all "non-evil" methods is not (imho).
- Intercept ActiveRecord-read operations to protected data. The patch works on
attribute / model level and not on ActiveRecord / SQL level. This is useful
if data is needed elsewhere (ie if_attribute statements), but DANGEROUS if
ignored.

Hints:
- Building and checking attribute ACLs is expensive.
- Consider negative tests on every attribute / method you like to protect. The
code is under (heavy) development.

Keep smiling
yanosz

> > declarative_author...@googlegroups.com<declarative_author
> >ization%2Bunsu...@googlegroups.com> .

[Steffen Bartsch]

Am Samstag, 3. Juli 2010 schrieb Jan Lühr:
> the code envolves and I'd like to share the current one - comments welcome:
> http://pastie.org/1028758

Thanks for the interesting patch. At last I came around having an as deep
look as this complex topic deserves.

Some thoughts:

* If I understand correctly, you have to set read_[attribute] for every
potentially protected attribute in the policy file. Is this a practical
approach? Do you use this already in production and are your authorization
rules then still readable? One alternative could be to allow to define groups
of attributes (e.g. user_data, login_data) just as you are able to require a
specific privilege for controller actions.

* Do you already have tests for the patch? Tests are a must for a potential
integration into the main decl_auth branch.

* As this patch has quite a lot of complexity, it could be helpful if you
provided a fork at github for the time being, so that it would be easier for
others to try your patch and we have some experience before merging it into
the main branch. Is that an option for you?

Steffen

[Jan Lühr]

Hello,

On Thursday 08 July 2010 22:57:58 Steffen Bartsch wrote:
> Am Samstag, 3. Juli 2010 schrieb Jan Lühr:
> > the code envolves and I'd like to share the current one - comments
> > welcome: http://pastie.org/1028758
>
> Thanks for the interesting patch. At last I came around having an as deep
> look as this complex topic deserves.

thanks ;)

> Some thoughts:
>
> * If I understand correctly, you have to set read_[attribute] for every
> potentially protected attribute in the policy file. Is this a practical
> approach? Do you use this already in production and are your authorization
> rules then still readable? One alternative could be to allow to define
> groups of attributes (e.g. user_data, login_data) just as you are able to
> require a specific privilege for controller actions.

We're about to use it in production, but it isn't released yet ;-)
Furtheremore we're using privileges to define "attribute groups" (based on
permissions) and to keep the rules clean.
(Btw. :read implies :read_xyz and :write implies :write_xyz)
In order to keep the patch simple, I decided to implement attribute acls using
naming conventions instead of modifying decl auth's engine - but it may be
future option.

> * Do you already have tests for the patch? Tests are a must for a
> potential integration into the main decl_auth branch.

I must confess, that all tests I have are nailed against specific applcations
(integration tests for concrete model class / rule sets) - since you're
interested in including my patch into decl_auth, I can take a look at it, but
it will take some time

> * As this patch has quite a lot of complexity, it could be helpful if you
> provided a fork at github for the time being, so that it would be easier
> for others to try your patch and we have some experience before merging it
> into the main branch. Is that an option for you?

Yep - I agree. I alredy fixed some bugs again (moved some meta-progamming from
class to instance-level) and some refactoring is still to be done. [1]
However, I'm not familiar with git (I only used svn, cvs up to now :-( ), thus
it might take some days for getting into it.

Thanks,
Keep smiliing
yanosz

[1] Current one is: http://pastie.org/1041524

[byrnejb]

> However, I'm not familiar with git (I only used svn, cvs up to now :-( ), thus
> it might take some days for getting into it.
>

http://help.github.com/svn-importing/

[Sharagoz]

Any progress on this one, Jan Lühr?

[Jan Luehr]

Hello,

sorry for answering late, I've been off work for a while.
I uploaded my code to http://github.com/yanosz/declarative_authorization but no app-independent tests are implemented yet.
I plan to streamline the dsl (more CoC-style). Eric Vatour contributed some ideas / specs, but I'ven't manged to implement 'em yet.

Keep smiling
yanosz

Am 22.08.2010 um 23:35 schrieb Sharagoz:

> Any progress on this one, Jan Lühr?
>

> --
> You received this message because you are subscribed to the Google Groups "declarative_authorization" group.
> To post to this group, send email to declarative_...@googlegroups.com.

> To unsubscribe from this group, send email to declarative_author...@googlegroups.com.

[Eric Vautour]

Hello Jan,

Welcome back, we have been using this plugin for the last little bit, and just recently found a bug.  Unfortunately, the approach taken will not work when we start adding attributes to authorization, ie if_attribute :some_id => 1.  When this is a new record, the some_id is not set yet, and thus, the permitted_to?(:read/:write) will fail.

I think we will need to have the validation happen at the time of save, instead of at time of write, at least for new_record?

Please let me know what you think.

Thanks,

[Jan Luehr]

Hello,

Am 30.08.2010 15:11, schrieb Eric Vautour:
> Hello Jan,
>
> Welcome back,
thanks, (btw. I took a seasite-vacation)

> we have been using this plugin for the last little bit, and just
> recently found a bug. Unfortunately, the approach taken will not work

> when we start adding attributes to authorization, ie *if_attribute
> :some_id => 1*. When this is a new record, the some_id is not set

> yet, and thus, the permitted_to?(:read/:write) will fail.
>
> I think we will need to have the validation happen at the time of
> save, instead of at time of write, at least for new_record?

well' - I don't fully understand what you're trying to accomplish. Can
you provide a test exposing the bug?

Thanks,
Keep smiling
yanosz