[mod_python] PythonAuthzHandler not working

On 05/05/2006, at 10:21 PM, Jim Gallacher wrote:

> Graham Dumpleton wrote:
>> On 24/04/2006, at 1:50 PM, Graham Dumpleton wrote:
>>> Jim Gallacher wrote ..
>>>> Ultimately however it seems to me there is a bug in mod_python.c  
>>>> related
>>>> to the whole AuthAthoritative business. Consider the following code
>>>> pulled from the python_handler function. (mod_python.c line 1412
>>>> revision 396250):
>>>>
>>>>     if (strcmp(phase, "PythonAuthenHandler") == 0) {
>>>>           ... snip ...
>>>>
>>>>           if (result == HTTP_UNAUTHORIZED)
>>>>           {
>>>>                if   (! conf->authoritative)
>>>>                      result = DECLINED;
>>>>
>>>>
>>>> conf->authoritative is initialized to 1, but we don't have an  
>>>> Apache
>>>> directive to set the value. I wonder if the assumption was that  
>>>> this was
>>>> set by AuthAuthoritative, or if it there was an oversight in not  
>>>> adding
>>>> a new directive? Either way it's a bug. Mod_python should not  
>>>> concern
>>>> itself with AuthAuthoritative, as that is for use by mod_auth,  
>>>> so we
>>>> really need our on directive.
>>>>
>>>> As confirmation I modified python_handler to log conf- 
>>>> >authoritative and
>>>> indeed it's value is unaffected by the AuthAthoritative setting.  
>>>> In it's
>>>> current state, PythonAuthenHander will *always* be authoritative.
>>>>
>>>> Other mod_auth_* modules define their own  authoritative  
>>>> directives, for
>>>> example: AuthDBMAuthoritative, AuthLDAPAuthoritative,
>>>> AuthMySQLAuthoritative and Anonymous_Authoritative. Following  
>>>> the most
>>>> common pattern I would suggest we add AuthPythonAuthoritative.
>>>>
>>>> This issue may also be important to
>>>> http://issues.apache.org/jira/browse/MODPYTHON-129
>>>
>>> I've noted the PythonAuthenHandler code in python_handler many  
>>> times and
>>> although I need to go back and look at it again I have been thinking
>>> that that section of code may possibly be partly bogus and  
>>> shouldn't be
>>> in there. The warning message about req.user not being set is  
>>> possibly
>>> helpful, but why should mod_python be making a decision to change an
>>> unauthorized response back to a declined and why should it be  
>>> generating
>>> a WWW-Authenticate header with an assumption that Basic  
>>> authorisation
>>> is being used when it may well not be. What is going to happen if  
>>> I write
>>> an authenhandler for Digest authentication and it returns  
>>> unauthorised,
>>> mod_python will obliterate any WWW-Authenticate header I may have
>>> placed there specific to Digest authentication.
>>>
>>> Thus I don't necessarily think it is a case of amending it some  
>>> way, it
>>> may be a case of obliterating it and make people do the correct  
>>> thing
>>> in their handlers to begin with rather than providing a crutch to  
>>> fix their
>>> omissions. This may mean adding means of calling further auth  
>>> related
>>> functions through the req object if there is something missing now,
>>> such as access to ap_note_basic_auth_failure().
>> FWIW, my concerns about something being a bit wrong with the code  
>> are a
>> baseless in as much as ap_note_basic_auth_failure() only adds the  
>> header
>> is AuthType is set to Basic. Thus it cant obliterate a Digest header.
>
> That is incorrect. ap_note_basic_auth_failure() checks if the auth  
> type is "Basic". If so it sets the headers, otherwise it calls  
> ap_note_auth_failure(). ap_note_auth_failure() also checks the auth  
> type and will either call ap_note_basic_auth_failure if the type is  
> "Basic", ap_note_digest_auth_failure() if the type is "Digest" or  
> else just log an error.
>
> ap_note_digest_auth_failure will sets the headers appropriately for  
> digest authentication.
>
> This doesn't mean we are correct in calling  
> ap_note_basic_auth_failure, but doing so will not cause any  
> problems as long as the AuthType is set properly.

Me wrong. If this call is going to be made then, it may as well call  
ap_note_auth_failure()
then instead of ap_note_basic_auth_failure(). At least then it is  
obvious.

Anyway, makes sense now as I was seeing warning in error log of:

   need AuthType to note auth failure

which is generated by ap_note_auth_failure() when AuthType not set. I  
think I
somehow convinced myself at one point that I didn't see it and it  
must have been
from something else I was doing.

Too tired. Which makes me think I better not commit this code I was  
about to commit
for something else. :-)

BTW, what do you think of:

   http://issues.apache.org/jira/browse/MODPYTHON-169

Time for sleep me thinks. I'll see what you say tomorrow.

Graham