[Zope3-dev] Form framework, adapters and pau

[Dominik Huber]

Jim Fulton wrote:

>>>> So at the moment I do not see any another possibility to set those 
>>>> permissions than using an additional <class.. directive.
>>>>
>>>> All 'bugs' related to this issue (that I'm aware of) including the 
>>>> zwiki-bug that was reported uses the above pattern and breaks.
>>>> The reason for my branch was to solve this kind problem :)
>>>
>>> But your original fix caused other problems.
>>
>> Only if somebody is memorizing (and pickle) adapters. Please be 
>> honest, that's not the most ordinary application.
>> (at least there were definitely more application using the above 
>> pattern. ;)
>
> This isn't about the the specific pickling problem.  It's about
> the unexpected problems from implicitly proxying something.
> Proxies are a technology that should be used only when necessary.
>
> This whole discussion is about providing the convenience of
> not having to subclass Location in an adapter class when an adapter
> is going to be security proxied.  While I think this convenience has
> value, the value does not justify always adding the location proxy.

Yes and No. It's convience, but it's also fact that security lookups do 
need a location (unless zope.Public)
to invoke local security settings correctly.

So, we have two cases:
1. the developer is *fully aware* of the security and adapter machinery.
2. the developer is *not aware* of the security and adapter machinery.

Today dev 2  is going to struggle and he will have a hard time to find 
the bug.
The current branch prevents that dev 2 struggles. Dev 1 might be 
disturbed by an implicit location proxy, but he is able to handle the 
problem deriving his adapter class from ILocation.

> ...
>
>>>> 2. the resulting adapter requires the permission defined by <class..
>>>>
>>>>  <adapter
>>>>      factory=".wikipage.MailSubscriptions"
>>>>      provides=".interfaces.IMailSubscriptions"
>>>>      for=".interfaces.IWiki"
>>>>      *permission="any.Permission"*
>>>>      trusted="true"
>>>>      />
>>>>
>>>>  <class class=".wikipage.MailSubscriptions">
>>>>    <require
>>>>        permission="zwiki.EditWikiPage"
>>>>        attributes="getSubscriptions"
>>>>        />
>>>>    <require
>>>>        permission="zwiki.EditWikiPage"
>>>>        attributes="addSubscriptions removeSubscriptions"
>>>>        />
>>>>  </class>
>>>>
>>>> IMO case 2. happens (experimental verification only, I do not 
>>>> understand all magics within _protectedFactory).
>>>> The status-quo is pretty implicit too. I looking forward to explain 
>>>> such stuff to newbies ;)
>>>
>>>
>>>
>>> In this case, the designer needs to do one of:
>>>
>>> - Make their adapter class a location
>>>
>>> - Factor their adapter into separate adapters that each
>>>   do one thing and need a single permission.
>>
>
> I just thought of another alternative in the case of single adapters.
> The adapter directive lets you name multiple factories in the factory
> attribute.  You could list the location proxy constructor as a factory
> in the ZCML:
>
>   <adapter ...
>       factory=".MyOriginalFactory
>                zope.app.location.LocationProxy"
>
> Here, we are *explicitly* saying that we want to combine
> an aplication factory with a location proxy.
>
> This works for the example above.

Yup.
-1 That's might be *to much explicitness* for dev 1 and complecates the 
adapter directive too.

>> We missed us.
>>
>> Question: What should the precedence be if I use the sample zwiki 
>> registration (modified example above)?
>>
>> At the moment (trunk) the permission attribute of the <adapter... is 
>> ignored and the permission-set of the <class... is invoked
>> (experimental verification only).
>
>
> I couldn't tell you what the precedence should be because I didn't
> anticipate that someone would do both.
>
>>> Here's what I want:
>>>
>>>   The adapter directive grows a new feature.  If the adapter 
>>> directive has
>>>   a permission directive with a permission other than zope.Public 
>>> and the
>>>   adapter adapts one or more objects, then we provide a factory that:
>>>
>>>   - Adds a location proxy if it doesn't provide ILocation, and
>>>
>>>   - Sets the __parent__ if the existing value is not None
>>>
>>> Dis you implement this?
>>
>>
>>
>> I tried to implement your solution [Revision 30053], but then I 
>> noticed the following problems:
>>
>> 1. no permission (None) and zope.Public within a trusted adapter 
>> registration provokes different behavior (example below 
>> KeyReferenceToPersistent)
>
>
> OK, sounds like a bug.
>
>> 2. the zwiki bug and my related implementations bugs still exists, 
>> because regularly folks that registering trusted adapters using  
>> <adapter... and <class...do not set
>> any permission within <adapter.., but only within <class.... (That 
>> kind of permission declaration causes the invocation of the 
>> regular-trusted-adapter-factory.)
>>
>> Therefore I reverted 'your' solution back to the first implementation 
>> [Revision 30059, 30060].
>
>
> That's not acceptable
>
>> I assumed that it will be less evil
>> to do without two different trusted adapters factories (regular 
>> (zope.Public and None) and the locating one (other permission)).
>> + we can fix the zwiki bug and related implementations bugs easily
>> + we can omit the unclear permission-precedence if the <adapter... 
>> <class... pattern is used for trusted adapters
>> o the untrusted adapters with no location get only location-proxied 
>> if permission is not None or zope.Public
>> - we have to derive the KeyReferenceToPersistent adapter from 
>> Location to omit the pickle error
>
>
> I didn't follow all of that...
>
>> Just now I added some optimization [30067]:
>> Trusted adapters get regularly only protected if the adapted object 
>> is protected. Therefore we can omit the location proxy in cases where 
>> the trusted adapters get not protected.
>> I wrote an other adapter factory 
>> (PartiallyLocatingTrustedAdapterFactory) which is only using location 
>> proxies if the adapter is protected and does not provide ILocation.
>> If ILocation is provided the parent is still set if None.
>
>
> OK, this is an imporovement.
>
>> Within the current branch there are the three adapter factories:
>> - PartiallyLocatingTrustedAdapterFactory
>> - LocatingTrustedAdapterFactory
>> - TrustedAdapterFactory
>> You can easily switch them within adapter() directive handler and 
>> look for the optimum.
>>
>> After all I would prefer the current solution. But I know the 
>> decision is up to you.
>
>
> This is looking pretty good.  When we're done, I'd like to simplify
> the code quite a bit. You currently have two classes
> that aren't used, TrustedAdapterFactory and 
> LocatingTrustedAdapterFactory.
> If we get rod of these, then we can get rid of the mixin.

Yes, this is my intend too, those classes are only artefacts of our 
dispute :)

> BTW, from a style point of view, we've moved away from putting
> doctests in modules toward separate .txt files (e.g. adapter.txt).
> This makes both the documentation and the code easier to read. I
> realize you were following the existing style.

Ok, I'm going to move the tests.

> I'm happy with a simplified version fo what you have now.
>
> Here are 2 other alternatives to consider:
>
> 1. Add a "locate" option to the adapter directive, which
>    defaults to false.  If true (locate="1"), then a location
>    proxy is always added to the adapter.

Yup.
-1 That's might be *to much explicitness* for dev 1 and complecates the 
adapter directive too.

> 2. Option 1 but default to true if a non-public permission is
>    specified.
>
> These alternatives are explicit and hande the case where
> permissions are declared in a separate directive.

In both sugestions the problem is not solved, that the public permission 
declaration within the adapter directive cannot be adapted to all  
trusted adapters, because the 'valid' security-declartations might be 
registered within an addional <class...

So, I'm going to tidy up the code...

Regards,
Dominik