[Zope3-dev] Re: Problem with zope.app.security._protections for tests

[Philipp von Weitershausen]

Gary Poster wrote:
> Have you ever written functional tests and been surprised that they  get
> security proxies around i18n Messages and MessageIDs in the  tests, but
> not in the server?  I have. :-)
> 
> I'm not sure how often people encounter this, but I think I've heard 
> one other person describe the symptom.  I believe I've tracked down  the
> cause.  Here's a description of the problem AIUI, and then my  suggested
> solution.
> 
> Problem:
> 
> zope/app/security/_protections.py is the place in which zope.app
> makes some fundamental security declarations: MessageIDs should not
> get security proxies (a known security hole); Messages should not get
>  security proxies (just fine, and the intended solution to the 
> aforementioned security hole); and the __name__ and __parent__ 
> attributes should be available by default.
> 
> These settings are put in place in zope/app/security/__init__.py, 
> rather than in zcml.
> 
> When running the server, this is fine.  The declarations are made  once,
> and remembered: Messages and MessageIDs don't get proxies.
> 
> When running tests, however, (all?  most?) test cleanups run zope/
> security/checker.py _clear, which cleans out the mutable that held  the
> NoProxy declaration for Messages and MessageIDs and resets it.   The
> __name__ and __parent__ additions remain, not affected by the  _clear. 
> The end result is that subsequent tests give proxies to  Messages and
> MessageIDs, but everything else remains the same.   Surprise!
> 
> This is generally only an issue for functional tests.
> 
> Suggested solution:
> 
> Have zope.app.security define a new zcml tag in the main zope namespace:
> 
>     <defineChecker
>         class='standard.zcml.import.path'
>         checker='standard.zcml.import.path' />
> 
> where "standard.zcml.import.path" indicates that a standard zcml  import
> path type goes there.
> 
> Have zope/app/security/_protections.zcml use this new tag to set the 
> checker for MessageID and Message, removing the parallel code from 
> _protections.py.

+1

I've always found the protections stuff in zope.app.security
particularly icky, though never thought that it would cause that much
trouble.

For compatability reasons, zope.app.security._protections and the
protect() function inside (though empty) should probably still exist for
at least another release because people might be using it in their own
tests (even though it's not public API). A deprecation warning could
tell them it's going a way -- should be a piece of cake with Stephan's
deprecation framework.

> The __name__ and __parent__ setting smells like it ought to go in  zcml
> to me as well, but it is not currently causing a problem because  the
> mutable it modifies is not reset for tests (which also perhaps  should
> be revisited).  I'm not proposing to change any of that for now.

Ok. Maybe there should be a collector issue about this so that we
remember it for upcoming releases.

> Thoughts?  Alternate suggestions (I have a few, but this is my 
> favorite)?  Stephan, do you want this fix in 3.1, assuming it goes 
> quickly and smoothly?

I share Fred's concerns, but given the ickiness of the problem, I
wouldn't mind if this was fixed for 3.1, assuming the hold up for the
second release candidate wasn't more than just another day or two.

Philipp