launchpad-dev team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

On Dec 22, 2010, at 2:45 AM, Henning Eggers wrote:

> Am 21.12.2010 19:51, schrieb Gary Poster:
>> On Dec 21, 2010, at 1:23 PM, Henning Eggers wrote:
>>> 
>>> It would be interesting to know if this behavior is intended and deterministic.
>> 
>> Yes.  The first is always regarded as the most important, in this and other aspects.
> 
> Aha, good to know. Thank you.
> 
>>> The IAuthorization objects in security.py get registered as adapters from
>>> "<usedfor>" to IAuthorization, named "<permission>". With the behavior I
>>> identified here, it follows that the selection of the security policy depends
>>> on the order in which the "implement" statement of the "Product" class lists
>>> interfaces. In this case, the Adapter for IHasCustomLangugeCodes shadows the
>>> adapter for IProduct because that list is sorted alphabetically AFAICT.
>> 
>> I'm not familiar with anything alphabetical being involved.
> 
> I was referring to the convention used in Launchpad code. AFAICT the
> interfaces in the "implements" list are sorted alphabetically but some
> implementation break that rule. I just did a quick survey and at least Person
> and ProjectGroup have IPerson and IProjectGroup first in their lists. With
> others I can't tell because the main interface comes alphabetically first
> anyway (Distribution, Archive, etc.) so it may be deliberate or not.
> 
> Putting the main interface first in this list seems to conform to the "most
> important" rule that you mentioned. I can only assume that it was done on
> purpose to deal with security.py issues. Maybe Curtis can confirm this.
> 
>> 
>>> 
>>> I was not aware of this. Is everybody else? Is this intended? Is this good?
>> 
>> I think it is good for the adapter registry: it is a simple rule that can be
>> relatively easily reasoned about, as opposed to something with more
>> heuristics.
> 
> Oh yeah, definitely. I am not questioning the way adapter look-up works in
> Zope. ;)
> 
>> 
>> In this context, I've never done a thoughtful survey of Launchpad's security
>> machinery, and don't have a concrete opinion as to its usage of the adapter
>> machinery.  It sounds like a "Gotcha" at best, in this context.
> 
> This is what I was questioning. Because the security machinery uses a simple
> "queryAdapter" to find the right security adapter, it is important to
> understand how the adapter machinery works. Putting the main interface first
> in the "implements" list sees like a good convention for me to achieve
> deterministic interaction with the security policy. If it already is a
> convention then it is not being followed and that lead to a "gotcha" for me.

I don't know what the historical intent was, but I suspect it should be a convention, or more aptly, a rule.  It appears to be, simply, how the tool we currently have works.

> 
> A more complete "fix" would be to have the security policy query all security
> adapters for the given situation and only allow access if all adapters allow
> it. I don't think that the adapter machinery can be used to do that, though.

It could, fwiw.  That would slow things down *a lot*.  I suspect that it is a bad idea, practically speaking, though I understand your reasoning.

> I
> thought something like the following might work but a casted obj is still an
> obj and the same adapter is returned every time.
> 
>    for iface in providedBy(obj):
>        adapted = queryAdapter(iface(obj), ITarget, "the_name")
> 
> Should I file a bug for this (improving LP security policy) or does anybody
> know of a fitting one?

I don't know of one off-hand.  I think this might be two bugs: one about the documentation of the "rule," and one about the fact that requiring devs to follow the rule is fragile, and identifying another approach might be nice.

Gary