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
_______________________________________________ Zope3-dev mailing list Zope3-dev@zope.org Unsub: http://mail.zope.org/mailman/options/zope3-dev/archive%40mail-archive.com
