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:
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:
On Mon, 2012-10-08 at 19:20 -0400, Chris McDonough wrote:
> 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:
Er, that diff logic is bogus. But you get the idea.
> 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:
Couldn't we just replace the logic with a hasattr and then if it does
run through the __acl__ like normal without exception handling?
On Mon, 2012-10-08 at 20:25 -0300, John Anderson wrote:
> On 10/08, Chris McDonough wrote:
> > 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:
> Couldn't we just replace the logic with a hasattr and then if it does
> run through the __acl__ like normal without exception handling?
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.
> 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')
