Authorization related issue in Pyramids

[artee]

Hi

I've spent a few hours trying to resolve security (authorization) related problem in a big Pyramid app.
The problem was on my side - an exception was thrown inside __acl__ property on RootFactory level, something like this:
class RootFactory(object):
    @property
    def __acl__(self):
        def load_ACL():
            acl = []
            for group in GroupFacade.all(): # 'all' function was not defined
                ...
            return acl
        return load_ACL()
In result access to all views with permission defined was denied.
After small investigation I've found that this exception was silently handled on Pyramids side:

for location in lineage(context):
try:
    acl = location.__acl__
except AttributeError:
    continue

I think that it should be a good idea to change this behavior or add proper trace here.
Any exception related to missing attribute here will cause hard to find error and misleading trace:

<No ACL found on any object in resource lineage>

Any ideas to handle it in a proper way?

--
Thanks
Artur Lew

[Chris McDonough]

On Mon, 2012-10-08 at 13:50 -0700, artee wrote:
> Hi

>
> In result access to all views with permission defined was denied.
> After small investigation I've found that this exception was silently
> handled on Pyramids side:
> for location in lineage(context):
> try:
> acl = location.__acl__
> except AttributeError:
> continue
>
> I think that it should be a good idea to change this behavior or add
> proper trace here.
> Any exception related to missing attribute here will cause hard to
> find error and misleading trace:
> <No ACL found on any object in resource lineage>
>
> Any ideas to handle it in a proper way?

I agree it is a problem. I'm not sure what the best way to handle it
is. Python is pretty bad at AttributeError introspection, so it might
be necessary to do something horrible like this inside Pyramid:

diff --git a/pyramid/authorization.py b/pyramid/authorization.py
index 943f8bd..33f03ac 100644
--- a/pyramid/authorization.py
+++ b/pyramid/authorization.py
@@ -75,11 +75,21 @@ class ACLAuthorizationPolicy(object):
acl = '<No ACL found on any object in resource lineage>'

for location in lineage(context):
+
try:
acl = location.__acl__
- except AttributeError:
+ except AttributeError as e:
+ # We are trying to catch only the AttributeError
+ # raised as the result of the location w/o __acl__
+ # attribute. But often __acl__ is defined as a prop
+ # which has logic that itself may raise an unrelated
+ # AttributeError. Below we make sure that we don't
+ # catch those. Only way to do that I know of.
+ args = e.args
+ if args and '__acl__' in str(args[0]):
+ raise
continue
-
+
for ace in acl:
ace_action, ace_principal, ace_permissions = ace
if ace_principal in principals:

[Chris McDonough]

Er, that diff logic is bogus. But you get the idea.

- C

[John Anderson]

Couldn't we just replace the logic with a hasattr and then if it does
run through the __acl__ like normal without exception handling?

[Chris McDonough]

No. Both hasattr and getattr hide AttributeError.

- C

[Chris Withers]

On 09/10/2012 09:57, Chris McDonough wrote:
> No. Both hasattr and getattr hide AttributeError.

I knew about hasattr, but how does getattr hide AttributeError?

cheers,

Chris

--
Simplistix - Content Management, Batch Processing & Python Consulting
- http://www.simplistix.co.uk

[Chris McDonough]

On Tue, 2012-10-09 at 10:04 +0930, Chris Withers wrote:
> On 09/10/2012 09:57, Chris McDonough wrote:
> > No. Both hasattr and getattr hide AttributeError.
>
> I knew about hasattr, but how does getattr hide AttributeError?

What you want is for "real" AttributeErrors to bubble up through the
getattr but they don't. For example, if you do this:

class Foo(object):
@property
def __acl__(self):
raise AttributeError('abc')

ob = Foo()

result = getattr(ob, '__acl__', None)

An AttributeError will not be raised, and result will be None.

- C

[Chris Withers]

On 09/10/2012 10:12, Chris McDonough wrote:
> On Tue, 2012-10-09 at 10:04 +0930, Chris Withers wrote:
>> On 09/10/2012 09:57, Chris McDonough wrote:
>>> No. Both hasattr and getattr hide AttributeError.
>>
>> I knew about hasattr, but how does getattr hide AttributeError?
>
> What you want is for "real" AttributeErrors to bubble up through the
> getattr but they don't. For example, if you do this:

Yeah, that is annoying..

If it were me, I'd do this:

# nb: does not subclass AttributeError
class InnerAttributeError(Exception):
pass

class Foo(object):
@property
def __acl__(self):

try:
...
raise AttributeError('abc')
except AttributeError as e:
raise InnerAttributeError(e)

If I used it more than once, I'd probably turn it into a decorator:

class Foo(object):
@property
@raise_inner_attribute_error
def __acl__(self):
...
raise AttributeError('abc')