[PEAK] peak.security weirdness

[Phillip J. Eby]

At 03:00 PM 01/18/2006 +0100, Simon Belak wrote:
>from dispatch import generic, strategy
>from peak.api import security
>
>
>class SecurityContext(security.Context):
>         pass
>
>security_context = SecurityContext()
>
>def hasPermission_adaptor(func):
>         def hasPermission(self, user, perm, subject):
>                 return func()
>         return hasPermission
>
>class Bar:
>         """ Poor-man's hasPermission(). """
>
>         @generic()
>         def foo(self, user, perm, subject): pass
>
>         @foo.when(strategy.default)     def 
> __denyByDefault(self,user,perm,subject):            return 
> security.Denial("Access 
> denied.")        @foo.when("perm==security.Nobody")      def 
> __nobodyGetsNobody(self,user,perm,subject):                 return 
> security.Denial("Access 
> forbidden")      @foo.when("perm==security.Anybody")     def 
> __anybodyGetsAnybody(self,user,perm,subject):               return True
>foo = Bar.foo.im_func

There's a bug in this code.  When you define a generic function method in a 
class, it implicitly adds a criterion to the rule that 'self' must be a 
subclass of the containing class.  In other words, you're adding a 
strategy.default that *only* takes effect if 'self' (the security context) 
is an instance of Bar!  So, the rules being used in your 
pseudo-hasPermission() are *not* the same as the ones used by 
peak.security, which is why there's no ambiguity in your duplicate.  In 
this code, the context is *never* a Bar instance, so this default rule 
never applies, and so it never conflicts with the GroupA rule.


>class SecurityGroupMeta(security.PermissionType):
>
>         def __init__ (cls, name, bases, attr):
>                 super(SecurityGroupMeta, cls).__init__(name, bases, attr)
>                 if "require" in attr:
>                         foo.when(
>                                 "getattr(perm, '__name__', '') == '%s'" % 
> name)(
>                                 hasPermission_adaptor(attr["require"]))
>                         security.hasPermission.when(
>                                 "getattr(perm, '__name__', '') == '%s'" % 
> name)(
>                                 hasPermission_adaptor(attr["require"]))

Just FYI, the above is *way* more complex than it needs to be.  A simpler 
and more robust way:

    security.hasPermission.when("perm is cls")(lambda *args: attr["require"]())

Remember that any symbol used in a "when" expression is *immediately* 
evaluated unless it is an argument name, so 'cls' is treated as a constant 
in the rule.  Also, this way of doing the definition is less likely to be 
ambiguous, since if you define two permissions with the same __name__, the 
function will not be able to distinguish them, but the permission objects 
themselves are unique.  Also, this way will run faster because it does not 
need to do a getattr() operation on each hasPermission() call.  Instead, 
the object's id() is just looked up in a dictionary.


>Traceback (most recent call last):
>   File "C:\Documents and Settings\Simon\My 
> Documents\hruska\vsemogoce\dispatch\weird_security.py", line 69, in ?
>     print security_context.hasPermission(user, GroupA, subject)
>   File "<string>", line 5, in hasPermission
>   File "_speedups.pyx", line 362, in _speedups.BaseDispatcher.__getitem__
>   File 
> "c:\python24\lib\site-packages\RuleDispatch-0.5a0.dev_r2100-py2.4-win32.egg\dispatch\interfaces.py", 
> line 15, in __call__
>     raise self.__class__(*self.args+(args,kw))
>dispatch.interfaces.AmbiguousMethod: ([(Signature((0, <class 
>'dispatch.strategy.Node'>)=Context), <function __denyByDefault at 
>0x00B58970>), (Signature((6, <class 
>'dispatch.strategy.node_type'>)=Inequality("..",(('GroupA', 'GroupA'),))), 
><function hasPermission at 0x00C0B9B0>)], (<__main__.SecurityContext 
>instance at 0x00B2AE40>, <object object at 0x009EC450>, <class 
>'__main__.GroupA'>, <object object at 0x009EC458>),

If you read this closely, you'll see that the conflict is between a test 
for __name__='GroupA', and the deny-by-default rule.  The deny-by-default 
rule applies to any security.Context instance, but your rule does not 
specify what class the security context must be.  That means that both 
rules apply, and neither is more specific, since they can be true or false 
independently of each other.  In other words, they are ambiguous.

The trivial fix is to slightly modify my suggestion for defining the rules:

    security.hasPermission.when(
       "isinstance(self,SecurityContext) and perm is cls"
    )(lambda *args: attr["require"]())

This would also work with security.Context, since it's still more specific 
than the default rule (which is just "isinstance(self,security.Context)").

By the way, when you define a rule A, and want it to be more specific than 
some rule B, there must never be a time when A could apply, but B does 
not.  In the specific problem you're having, rule A is the GroupA rule, and 
rule B is deny-by-default.  It's possible for rule B to not apply when rule 
A does, because rule B requires 'self' to be a security.Context, and rule A 
does not.  Therefore, rule A is ambiguous with respect to rule B; the 
system cannot conclude that whenever A is true, B is also true, and 
therefore it cannot tell if A is more specific than B.  If the reverse also 
applies (B is not more specific than A), then the two rules are ambiguous.

The way to fix the ambiguity is to add all of rule B's conditions (or more 
specific ones) to rule A.  In this case, by adding an isinstance() check on 
'self' for either security.Context or a subclass.

One of the things I'd like to do in future versions of RuleDispatch is have 
a more explicit way to override rules when there's an ambiguity.  For 
example, a way to say "this method overrides that one", and have the 
conditions automatically AND-ed for you, instead of you having to spell 
them out.  Also, sometimes there are rules that need to be prioritized, 
where you first check for one thing, and then another.  These situations 
can be specified using "and" and "not" conditions in a relatively simple 
way, but it tends to be verbose and error prone.  In the "Cecil" language 
that inspired RuleDispatch, there is a concept called a "classifier" that 
allows you to list a series of priority-ordered rules and the language 
automatically does the and's and not's for you.  So, I'd like to add some 
syntax sugar for that, too.