using zope.testbrowser to test for Unauthorized exceptions and updated zope.publisher with IReRaise exception support

[Jan-Wijbrand Kolman-3]

Hi,


As per https://bugs.launchpad.net/grok/+bug/332061 and the fix in
zope.publisher 3.4.9 and in grok itself...

http://svn.zope.org/grok/trunk/src/grok/configure.zcml?rev=101871&r1=94110&r2=10187

...the Unauthorized exception is now never exposed to "the outside"
world anymore. We have however several doctests in for our applications
that use the following pattern:

  >>> browser = Browser()
  >>> browser.handleErrors = False
  >>> # Anonymous access is not allowed:
  >>> browser.open('http://localhost/some/restricted/view')
  Traceback (most recent call last):
  ...
  Unauthorized...

This doesn't work anymore, since even with the handleErrors set to
False, the Unauthorized exception will not be exposed to the test browser.

So, I thought, I'll test for the status code in the repsonse headers
then, something like so:

  >>> browser.open('http://localhost/some/restricted/view')
  >>> print browser.headers
  Status: 401 Unauthorized
  Cache-Control: no-store, no-cache, must-revalidate
  Content-Length: ...
  Content-Type: text/html;charset=utf-8
  Expires: ...
  Pragma: no-cache
  WWW-Authenticate: basic realm="Zope"
  X-Powered-By: Zope (www.zope.org), Python (www.python.org)

This can only work for basic auth though. If you use form based login -
like we do - the response status code will just be a 200 Ok, since
*that* login page was handled just fine...

A couple of thoughts:

* This should be part of the upgrade notes.

* The IReRaise adapter is now registered in the configure.zcml of Grok.

I wonder if it instead shouldn't be registered in the wsgi-specific
grokcore.startup.startup.application_factory. Maybe even only for the
cases where we want to debug. In all other cases then (during testing,
non-wsgi deployment, using the deploy.ini profile for paste serve), the
error raising works as expected.

Any opinions?

regards,
jw

_______________________________________________
Grok-dev mailing list
Grok-dev@...
https://mail.zope.org/mailman/listinfo/grok-dev

[Jan-Wijbrand Kolman-3]

Jan-Wijbrand Kolman wrote:

> A couple of thoughts:
>
> * This should be part of the upgrade notes.
>
> * The IReRaise adapter is now registered in the configure.zcml of Grok.
>
> I wonder if it instead shouldn't be registered in the wsgi-specific
> grokcore.startup.startup.application_factory. Maybe even only for the
> cases where we want to debug. In all other cases then (during testing,
> non-wsgi deployment, using the deploy.ini profile for paste serve), the
> error raising works as expected.

Following this line of thought:

This could be implemented by having a something like a
"debug_application_factory()" function in grokcore.startup that would
just after setting up the application register the IReRaise adapter for
Unauthorized.

I can even imagine actually configuring what exceptions not to reraise
from the debug.ini file somehow, to make this more readable/findable.

Again, this would make all other deployments and testing situations
behave "normally" and only when using the exception middleware do
something different. I'd prefer that, now that I'm working with it.

regards,
jw

_______________________________________________
Grok-dev mailing list
Grok-dev@...
https://mail.zope.org/mailman/listinfo/grok-dev

[Uli Fouquet]

Hi JW,

Jan-Wijbrand Kolman wrote:
> Jan-Wijbrand Kolman wrote:
> > A couple of thoughts:
> >
> > * This should be part of the upgrade notes.

Right.
 

> > * The IReRaise adapter is now registered in the configure.zcml of Grok.
> >
> > I wonder if it instead shouldn't be registered in the wsgi-specific
> > grokcore.startup.startup.application_factory. Maybe even only for the
> > cases where we want to debug. In all other cases then (during testing,
> > non-wsgi deployment, using the deploy.ini profile for paste serve), the
> > error raising works as expected.
>
> Following this line of thought:
>
> This could be implemented by having a something like a
> "debug_application_factory()" function in grokcore.startup that would
> just after setting up the application register the IReRaise adapter for
> Unauthorized.
>
> I can even imagine actually configuring what exceptions not to reraise
> from the debug.ini file somehow, to make this more readable/findable.
>
> Again, this would make all other deployments and testing situations
> behave "normally" and only when using the exception middleware do
> something different. I'd prefer that, now that I'm working with it.

In general I'd agree, especially about the
'set-handled-exceptions-in-the-ini-file' point. It would really be nice
to be able to set these stuff in an .ini file. But three things come to
my mind:

* what you call "normally" here, is only the behaviour of the publisher
  for the last years. Changing behaviour only to protect people from
  setting up their tests differently is not a really strong argument to
  me, although I can really understand the trouble.

* I am undecided about having two different behaviours for grok when
  used in a WSGI-based debugger pipeline or not. Could that confuse
  developers in any way?

* There are ongoing thoughts about changing the general way of marking
  the exceptions you want to "keep inside". See

    https://mail.zope.org/pipermail/zope-dev/2009-September/037831.html

  I'd support the suggestions made there but this would mean that we
  had to change our IReRaiseException registration anyway (at least,
  probably, when it comes to a ZTK-KGS compatibility redesign of grok).
  So maybe we should wait until this zope.publisher change (which is
  likely to come) arrived?

  On the other hand: if we reintroduce the former default behaviour for
  non-WSGI-debugger environments for 1.0 we are less likely to have
  additional upgrade troubles afterwards, when IPublisherExceptionInfo
  is part of the zope.publisher we use.

Overall +0.5 for the change.

Best regards,

--
Uli



_______________________________________________
Grok-dev mailing list
Grok-dev@...
https://mail.zope.org/mailman/listinfo/grok-dev

[Martijn Faassen-2]

Hey,

JW, could you add a note to the upgrade notes about this?

Uli Fouquet wrote:
[snip]
> In general I'd agree, especially about the
> 'set-handled-exceptions-in-the-ini-file' point. It would really be nice
> to be able to set these stuff in an .ini file. But three things come to
> my mind:
>
> * what you call "normally" here, is only the behaviour of the publisher
>   for the last years. Changing behaviour only to protect people from
>   setting up their tests differently is not a really strong argument to
>   me, although I can really understand the trouble.

Well, the tests depended on such a behavior of the publisher too. :)

> * I am undecided about having two different behaviours for grok when
>   used in a WSGI-based debugger pipeline or not. Could that confuse
>   developers in any way?

I share your concern here. I think ideally in the future the browser
tests should be run against the WSGI app.

> * There are ongoing thoughts about changing the general way of marking
>   the exceptions you want to "keep inside". See
>
>     https://mail.zope.org/pipermail/zope-dev/2009-September/037831.html
>
>   I'd support the suggestions made there but this would mean that we
>   had to change our IReRaiseException registration anyway (at least,
>   probably, when it comes to a ZTK-KGS compatibility redesign of grok).
>   So maybe we should wait until this zope.publisher change (which is
>   likely to come) arrived?

I'm not sure this particular proposal would help much in fixing JW's
issue, though...

> Overall +0.5 for the change.

I'm on the fence on configuring things when the WSGI app is set up too.
It'll obscure the place of configuration as it won't be grokked or
coming through ZCML...

Regards,

Martijn

_______________________________________________
Grok-dev mailing list
Grok-dev@...
https://mail.zope.org/mailman/listinfo/grok-dev

[Jan-Wijbrand Kolman-3]

Martijn Faassen wrote:
> JW, could you add a note to the upgrade notes about this?

Yup.

> Uli Fouquet wrote:
> [snip]
>> In general I'd agree, especially about the
>> 'set-handled-exceptions-in-the-ini-file' point. It would really be nice
>> to be able to set these stuff in an .ini file. But three things come to
>> my mind:
>>
>> * what you call "normally" here, is only the behaviour of the publisher
>>   for the last years. Changing behaviour only to protect people from
>>   setting up their tests differently is not a really strong argument to
>>   me, although I can really understand the trouble.
>
> Well, the tests depended on such a behavior of the publisher too. :)

Added to this point is that the change in publisher behaviour is now
"universal" where it really is only needed in one very particular case:
wrapping zope in the exception catching middleware. Something you do not
need to do in (functional)tests nor when running "production".

>> * I am undecided about having two different behaviours for grok when
>>   used in a WSGI-based debugger pipeline or not. Could that confuse
>>   developers in any way?
>
> I share your concern here. I think ideally in the future the browser
> tests should be run against the WSGI app.

That's an interesting idea. But not available soon I suppose.

>> * There are ongoing thoughts about changing the general way of marking
>>   the exceptions you want to "keep inside". See
>>
>>     https://mail.zope.org/pipermail/zope-dev/2009-September/037831.html
>>
>>   I'd support the suggestions made there but this would mean that we
>>   had to change our IReRaiseException registration anyway (at least,
>>   probably, when it comes to a ZTK-KGS compatibility redesign of grok).
>>   So maybe we should wait until this zope.publisher change (which is
>>   likely to come) arrived?
>
> I'm not sure this particular proposal would help much in fixing JW's
> issue, though...
>
>> Overall +0.5 for the change.
>
> I'm on the fence on configuring things when the WSGI app is set up too.
> It'll obscure the place of configuration as it won't be grokked or
> coming through ZCML...

Semantics question: "On the fence" means "more or less undecided yet"
right? Or does it mean "Screaming on the fence not to ever ever ever do
this"?


I would not like to have the registration obscured in the
grokcore.startup code principly.

However, if the exceptions that will not be reraised can at least be
configured from the debug.ini then people will have a better idea what's
going on - maybe an even better idea then currently, since now the
adapter registration for IReRaise is done in Grok's configure.zcml which
is not immediately visible either (albeit still more visible than
grokcore.startup code of course).

Would people agree if I tried to implement the IReRaise configuration
from debug.ini on a branch of grokcore.startup? Or do people think that
would generally be a bad idea anyway?

regards,
jw



_______________________________________________
Grok-dev mailing list
Grok-dev@...
https://mail.zope.org/mailman/listinfo/grok-dev

[Jan-Wijbrand Kolman-3]

Jan-Wijbrand Kolman wrote:
> Would people agree if I tried to implement the IReRaise configuration
> from debug.ini on a branch of grokcore.startup? Or do people think that
> would generally be a bad idea anyway?

I sketched out something here:

http://svn.zope.org/grokcore.startup/branches/jw-configurable-ireraise-adaptation/?rev=104430&view=rev

What do people think about such an aproach?

BTW, in your project's debug.ini you would configure what exceptions to
exempt like so:



[app:zope]
use = egg:Debugging#debug
filter-with = translogger
exempt-exceptions = zope.security.interfaces.IUnauthorized,
                     some.other.interface.ISomething




regards,
jw

_______________________________________________
Grok-dev mailing list
Grok-dev@...
https://mail.zope.org/mailman/listinfo/grok-dev