[Zope3-dev] Form framework, adapters and pau

[Dominik Huber]

Jim Fulton wrote:

>>> It is not generally the case that you need to use separate security
>>> declarations with trusted adapters.
>>
>> I declare those additional class directive all the time if I'm using 
>> trusted adapters.
>> IMO this kind of registration is the common pattern
>>
>> For example stephan richter uses this pattern within the wiki:
>>
>>  <adapter
>>      factory=".wikipage.WikiPageHierarchyAdapter"
>>      provides=".interfaces.IWikiPageHierarchy"
>>      for=".interfaces.IWikiPage"
>>      trusted="true" />
>>
>>  <class class=".wikipage.WikiPageHierarchyAdapter">
>>    <require
>>        permission="zwiki.ViewWikiPage"
>>        attributes="parents path findChildren"
>>        />
>>    <require
>>        permission="zwiki.ReparentWikiPage"
>>        attributes="reparent"
>>        set_attributes="parents"
>>        />
>>  </class>
>
>
> This isn't a pattern I've used.  I think that there are
> lots of other, and perhaps better ways to do things.

We have the use case that we adapt a lot of additional functionalities 
to a pretty stupid content.
Each additional functionality has differentiated permission-sets 
comparable to regular content objects. For example different permissions 
for reading and setting attributes of the provided interface.

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 the permission declaration within the adapter directive defines 
>> the permission that is nessecary to invoke the adapter factory.
>
> >
> > It's implict to asume that the  instance by a certain factory desires
> > the same permissions like the invocationg of the factory.
> >
> > It's implict to asume that the  instance by a certain factory desires
> > the same permissions like the invocationg of the factory.
>
> Aaaaah.
>
> You missunderstand.  The permission is not the permission to
> to create the adapter. It's the permission necessary to use
> the provided interface on the adapter.

Ok, missleading naming _protectedFactory within the adapter handler ;)
But what happens in the following case?
1. the resulting adapter requires any.Permission
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 ;)

>>>> - How should we handle NullAdapters correctly?
>>>
>>> All the same I implemented your sugestion. The following question 
>>> appears:
>>>
>>> What do you mean by NullAdapters?  Are you talking about factories that
>>> return None?
>>>
>>> Id so, then, obviously, you should not proxy None.
>>
>> No, 'NullAdapter' are parameterless factories, for example x = IX()
>
> OK, so these cannot be locations.

Yup, that my interpretation too. So those null adapters are global 
object with or without dedicated permission
that can't be invoke by any local principal. What's the use cases for 
such object?

>>>> IMO this solution is pretty implicit and I do not like it at all.
>>>
>>> What solution?  If you are talking about my suggestion to
>>> add a location to any adapter for whoch a non-public permission
>>> is specified, then I consider this far less impleicit than what you
>>> had before.  It doesn't guess. If it knows a permission is required,
>>> it adds a location.
>>
>> Yes, the current solution in the branch.
>
>
> OK, I'll take a look at that.

Excuse me, but under those circumstances I have to retract my solution. 
IMO it will be not that what you are looking for... :(

>> 2. if we register a trusted adapter this correlation breaks because 
>> the adapter instance itself might have highly differented 
>> security-permission
>> (compare <class...)
>
> In a complex situation like this, I suggest it's better to have people
> make their adapters locations in Python.

It might more complex than a single permission, but its every day life. 
I only try to simplify independently if its complex or not :)

>> So 1 is easy to handle in the adapter directive, but 2 not. The 
>> suggestion above tries to respect your requirments, but also allow to 
>> use.
>> the registration pattern <adapter... and <class.. which is failry 
>> common and explicit.
>>
>> The current solution in the branch is not able to handle 2. correctly.
>> Stephan's zwiki example:
>> - The adapter directive has no permission (which is ok, because he 
>> doesn't need to protect the invocation of the adapter factory itself).
>> - In that case the locating trusted adapter factory is not invoked 
>> within the registration.
>> - Therefore the adapter instance will not have any location.
>> - Therefore we can't invoke the local authentication.
>>
>> My suggestion:
>> IMO the intid and keyrefernce stuff is an exceptional case. Nobody 
>> else using the zope framework tries to memorize adapters persistently.
>
> The problem is that we were being too intrusive.
>
> Most adapters are never security proxied. We shouldn't implicitly
> add location proxies. We should only add them if we know we need them.

Maybe I was focused to much to the existing implementation.
We location proxy the trusted adapters only if they get protected by 
adapter factory.

if an protected object is adapted:
  we assert a location (if no location is provided we use location proxy)
else:
  we only set the __parent__ attribute if it is None

>> Everyone is aware that adapters regularly are very transient.
>> If somebody needs a pickable adapter he should be aware that those 
>> adapter must provide ILocation and must be trusted.
>
> That's very intrusive and unacceptable.  KeyReferences have no reason
> whatsoever to be locations. They are publically available.  The only
> reason they "needed" to be locations was because of your implicit
> addtion of the location proxy.

I only partially agree. The intid-stuff we regularly implement once. So 
we could
handle that problem fairly painless.

All the other adapter have no reason whatsoever to be locations too. But 
the authentication
mechanism needs the location information. An that information can be 
provided by location proxies.

I no that addition is implicit but we can tell *one rule* that a 
developer may understand.
In that respect the status quo is very unacceptable for me.

Regards,
Dominik



P.S. Also the key reference uses the 
common-trusted-adapter-registration-pattern :)

  <adapter
      for="persistent.interfaces.IPersistent"
      provides=".interfaces.IKeyReference"
      factory=".persistent.KeyReferenceToPersistent"
      trusted="y"
      />

  <class class=".persistent.KeyReferenceToPersistent">
    <require permission="zope.Public" 
interface=".interfaces.IKeyReference" />
  </class>

If we would set zope.Public explicitly within the adapter directive an 
exception is raised:

  <adapter
      for="persistent.interfaces.IPersistent"
      provides=".interfaces.IKeyReference"
      factory=".persistent.KeyReferenceToPersistent"
      permission="zope.Public"
      trusted="y"
      />

  <class class=".persistent.KeyReferenceToPersistent">
    <require permission="zope.Public" 
interface=".interfaces.IKeyReference" />
  </class>

  File "E:\dev\amadeus_trunk\src\ZODB\serialize.py", line 330, in serialize
    return self._dump(meta, obj.__getstate__())
  File "E:\dev\amadeus_trunk\src\ZODB\serialize.py", line 339, in _dump
    self._p.dump(state)
  File "D:\Python23\lib\copy_reg.py", line 69, in _reduce_ex
    raise TypeError, "can't pickle %s objects" % base.__name__
TypeError: can't pickle Checker objects

----------------------------------------------------------------------
Ran 8 tests in 3.766s

FAILED (failures=1, errors=1)