[jira] Commented: (MODPYTHON-124) Improvements associated with the req.ap_auth_type attribute.
[ http://issues.apache.org/jira/browse/MODPYTHON-124?page=comments#action_12366957 ] Graham Dumpleton commented on MODPYTHON-124: Changes to add req.auth_name(), req.auth_type() and make req.ap_auth_type writable commited into mod_python SVN trunk in revsion 378864. Improvements associated with the req.ap_auth_type attribute. Key: MODPYTHON-124 URL: http://issues.apache.org/jira/browse/MODPYTHON-124 Project: mod_python Type: Improvement Components: core Versions: 3.3 Reporter: Graham Dumpleton The req.ap_auth_type attribute is set to the authentication type corresponding to the type of authentication processing successfully carried out in respect of a request. For example, if one has Apache configuration: AuthType Basic AuthName Restricted Files AuthUserFile /usr/local/apache/passwd/passwords Require valid-user it is expected that the request uses basic authentication header as appropriate. These headers will be dealt with by inbuilt Apache core module. Upon successful authentication, the Apache core module will set req.ap_auth_type attribute to be Basic and set req.user to the user ID of the logged in user. If instead Apache support for digest authentication was used, eg: AuthType Digest ... then req.ap_auth_type attribute will be set to Digest. If authentication was not requested, ie., no AuthType directive, the req.ap_auth_type is set to Python None. The intent is that you should be able to implement authentication handlers in mod_python using PythonAuthenHandler, but you can't actually do this correctly at the moment as there are a few things missing. Firstly, in order to trigger the PythonAuthenHandler, you must still define the AuthType/AuthName/Require directives. In order to ensure that our authentication handler is triggered and not the builtin ones or some other one, the AuthType directive should specify a string other than Basic or Digest. This would be a name we choose and can basically be anything. For example, you might choose a descriptive name like Python-Basic-DBM to denote basic authentication is used against a DBM database but using the Python authentication handler. AuthType Python-Basic-DBM AuthName Web Application Require valid-user PythonAuthenHandler basicdbmauth PythonOption basicdbmauth.UserDatabase /.../users.dbm When the authentication handler in basicdbmauth is called, the req.ap_auth_type field is still None. This is because authentication hasn't succeed yet. In terms of being able to implement the authentication handler correctly, the first problem is that there is no way to access the actual value associated with the AuthType directive. This needs to be consulted to determine if the authentication handler should actually do anything. Second is that the value associated with the AuthName directive can't be determined either, something which may influence against which database authentication should be done. Thus first lot of changes that need to be made are that req object needs to have two new methods called get_auth_type() and get_auth_name(). These will map to the Apache API functions called ap_auth_type() and ap_auth_name(). Note that ap_auth_type() is returning a different value to req.ap_auth_type. With those two functions, authentication handler can then be written as: def authenhandler(req): if req.get_auth_type() != Python-Basic-DBM: return apache.DECLINED realm = req.get_auth_name() # Do all the processing of Authorization header and # validate user etc. If not okay, return appropriate error # status. If okay, keep going. req.user = ... from header req.ap_auth_type = Python-Basic-DBM return apache.OK As well as returning apache.OK, convention is to set req.user and req.ap_auth_type. This is where the final problem occurs. That is that req.ap_auth_type is read only and cannot actually be set as necessary. Thus in addition to req.get_auth_type(), req.get_auth_name(), need to make req.ap_auth_type writable. Having made these changes it would then actually be possible to write authentication handlers correctly, ie., whereby they correctly look at AuthType etc to see whether they should be applied. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: [jira] Commented: (MODPYTHON-124) Improvements associated with the req.ap_auth_type attribute.
Graham, I don't think it's necessary to add an additional JIRA comment when you commit some code. JIRA will pickup the svn commit as long as the issue number is mentioned in the svn commit message. People can subscribe to python-cvs if they want notification of the commits. This should save you a bit of work. :) Jim Graham Dumpleton (JIRA) wrote: [ http://issues.apache.org/jira/browse/MODPYTHON-124?page=comments#action_12366957 ] Graham Dumpleton commented on MODPYTHON-124: Changes to add req.auth_name(), req.auth_type() and make req.ap_auth_type writable commited into mod_python SVN trunk in revsion 378864.
Re: What is test_req_get_basic_auth_pw meant to test.
Yes, this test seems to be broken. I'll create a JIRA issue for it. We need unit tests for the unit tests. :) Jim For my WTF moment, have a look at test_req_get_basic_auth_pw in the test suite. I guess it is supposed to be test req.get_basic_auth_pw (), but that function isn't even mentioned anywhere. Plus the authenhandler phase will not even be run since there is no Require directive in the configuration. Even if you add the Require directive, it seems to be testing the mod_auth module and not the ability to call req.get_basic_auth_pw(). Would it perhaps be better as: def test_req_get_basic_auth_pw_conf(self): c = VirtualHost(*, ServerName(test_req_get_basic_auth_pw), DocumentRoot(DOCUMENT_ROOT), Directory(DOCUMENT_ROOT, SetHandler(mod_python), AuthName(blah), AuthType(basic), #PythonAuthenHandler (tests::req_get_basic_auth_pw), PythonHandler (tests::req_get_basic_auth_pw), PythonDebug(On))) return str(c) def req_get_basic_auth_pw(req): req.get_basic_auth_pw() if req.user != spam: req.write(test failed) else: req.write(test ok) return apache.OK Graham
JIRA Housekeeping
Now that 3.2.7 is out, should we be changing the status resolved issues to closed in JIRA. Jim
Re: JIRA Housekeeping
Jim Gallacher wrote: Now that 3.2.7 is out, should we be changing the status resolved issues to closed in JIRA. Other JIRA thoughts: Should we have a unit test component for bugs in the actual unit test code? Since we plan on having 3.2.x bugfix releases, should create new JIRA versions starting with 3.2.7? Jim
Re: What is test_req_get_basic_auth_pw meant to test.
I already fixed the test and checked it in. :-) On 20/02/2006, at 5:15 AM, Jim Gallacher wrote: Yes, this test seems to be broken. I'll create a JIRA issue for it. We need unit tests for the unit tests. :) Jim For my WTF moment, have a look at test_req_get_basic_auth_pw in the test suite. I guess it is supposed to be test req.get_basic_auth_pw (), but that function isn't even mentioned anywhere. Plus the authenhandler phase will not even be run since there is no Require directive in the configuration. Even if you add the Require directive, it seems to be testing the mod_auth module and not the ability to call req.get_basic_auth_pw(). Would it perhaps be better as: def test_req_get_basic_auth_pw_conf(self): c = VirtualHost(*, ServerName(test_req_get_basic_auth_pw), DocumentRoot(DOCUMENT_ROOT), Directory(DOCUMENT_ROOT, SetHandler(mod_python), AuthName(blah), AuthType(basic), #PythonAuthenHandler (tests::req_get_basic_auth_pw), PythonHandler (tests::req_get_basic_auth_pw), PythonDebug(On))) return str(c) def req_get_basic_auth_pw(req): req.get_basic_auth_pw() if req.user != spam: req.write(test failed) else: req.write(test ok) return apache.OK Graham
Re: mod_python 3.2.8 available for testing
+1 Debian sid, apache 2.0.55 mpm-prefork, python 2.3.5
Re: JIRA Housekeeping
Jim Gallacher wrote .. Jim Gallacher wrote: Now that 3.2.7 is out, should we be changing the status resolved issues to closed in JIRA. If that is what closed implies. Is there somewhere which states what we should interprete the different status as meaning? I don't recollect seeing anything unless I am missing the obvious. BTW, as far as I know I still don't have full JIRA access otherwise I would help with closing them off. I noticed a few on the weekend that weren't even in resolved status yet, even though fixed in 3.2.7. Grisha, if you are reading this, what do I need to do to get admin JIRA access on mod_python stuff? I recollect asking you but can't remember what you said. I thought perhaps you were going to organise it, but also can't remember. Other JIRA thoughts: Should we have a unit test component for bugs in the actual unit test code? Since we plan on having 3.2.x bugfix releases, should create new JIRA versions starting with 3.2.7? No harm in doing so. Probably would only be used if reported by someone else or change is not simple. For the simple stuff, like basic auth, easier to just fix it on the spot, although I am tending towards thinking having a JIRA issue for all changes is a good goal. Graham
Re: JIRA Housekeeping
Graham Dumpleton wrote: Jim Gallacher wrote .. Jim Gallacher wrote: Now that 3.2.7 is out, should we be changing the status resolved issues to closed in JIRA. If that is what closed implies. Is there somewhere which states what we should interprete the different status as meaning? I don't recollect seeing anything unless I am missing the obvious. http://www.atlassian.com/software/jira/docs/v3.3.2/statuses.html The status of an issue cannot be changed once it has been marked as closed. Other JIRA thoughts: Should we have a unit test component for bugs in the actual unit test code? Since we plan on having 3.2.x bugfix releases, should create new JIRA versions starting with 3.2.7? No harm in doing so. Probably would only be used if reported by someone else or change is not simple. For the simple stuff, like basic auth, easier to just fix it on the spot, although I am tending towards thinking having a JIRA issue for all changes is a good goal. I guess I'm just looking for a bit of clarity. Let's say we make a 3.2.10 bugfix release. A JIRA issue which is fixed in 3.2.10 gets marked as fixed in 3.2. A user looks at the issue and says Great, I'm using 3.2.7, and it says it's fixed in 3.2, so I must be ok. Jim
Re: Maually adding notes about commits to JIRA.
Graham Dumpleton wrote: Jim Gallacher wrote .. Graham, I don't think it's necessary to add an additional JIRA comment when you commit some code. JIRA will pickup the svn commit as long as the issue number is mentioned in the svn commit message. People can subscribe to python-cvs if they want notification of the commits. This should save you a bit of work. :) Jim Looking at the bigger picture, I believe that adding the comment to JIRA explicitly saves me work down the track. Where I am coming from is the perspective of a user who has started using mod_python recently and is having a problem. For myself, I find it quite annoying when looking at other third party packages when investigating an issue and find the problem mentioned in a issue tracking system, but then find that it hasn't been updated in a while and thus looks like the software is still broken and isn't about to be fixed. Often one digs further though and finds that it was actually fixed some time back, but because the issue tracking system was never updated to at least mention that it was fixed in the repository you never actually know this. The average person is not going to want to subscribe to some special mailing list to find out about repository commits for a package they have only just started to use. Even if one did this, you aren't going to find out about what has happened in the past from the mailing list. Thus to me, the JIRA system is perfect for recording a full and complete history of what has happened that people can refer to. The only problem at the moment is that the JIRA system still isn't linked from the mod_python home page so people never find it in the first place. So overall I feel it saves me work as people will either find the information themselves and know that if they simply check out latest source code that they will get the fix, or at worst I can respond to questions on the mailing list with a quick link to the JIRA issue and not have to reiterate the same information all over again. I also feel that identifying the revision may be helpful when someone wants to perhaps backport changes to an older version. This may be us, or someone who needs to do it for themselves. All very interesting, except that JIRA does record the svn commit info, and with great detail. It just doesn't treat the commit as a comment. For example: http://issues.apache.org/jira/browse/MODPYTHON-124?page=all Personally I think the Subversion commit information should be included in the comments by default, rather than requiring a separate view, but I guess that's between JIRA and myself. :) Jim
Re: JIRA Housekeeping
Jim Gallacher wrote .. Other JIRA thoughts: Should we have a unit test component for bugs in the actual unit test code? Since we plan on having 3.2.x bugfix releases, should create new JIRA versions starting with 3.2.7? No harm in doing so. Probably would only be used if reported by someone else or change is not simple. For the simple stuff, like basic auth, easier to just fix it on the spot, although I am tending towards thinking having a JIRA issue for all changes is a good goal. I guess I'm just looking for a bit of clarity. Let's say we make a 3.2.10 bugfix release. A JIRA issue which is fixed in 3.2.10 gets marked as fixed in 3.2. A user looks at the issue and says Great, I'm using 3.2.7, and it says it's fixed in 3.2, so I must be ok. Hmmm, I can see why you might be confused by my response. I was only referring to the unit test question. Totally missed the 3.2.7 question. On the latter I'm not sure. This sort of confusion is why I like to add final comments to issues when I put stuff back in a repository. I was remiss this time in not mentioning that the fixes would be expected to first be available in 3.3.0. BTW, are we going to put up your developer guidelines anywhere for viewing? http://www.modpython.org/pipermail/mod_python/2005-December/019712.html Just spent a while trying to find out how to subscribe to python-cvs. Couldn't find it in an obvious spot unless I missed the obvious. Graham
Re: Maually adding notes about commits to JIRA.
Jim Gallacher wrote .. All very interesting, except that JIRA does record the svn commit info, and with great detail. It just doesn't treat the commit as a comment. For example: http://issues.apache.org/jira/browse/MODPYTHON-124?page=all Personally I think the Subversion commit information should be included in the comments by default, rather than requiring a separate view, but I guess that's between JIRA and myself. :) This is showing my ignorance then. That the commit appeared there was a fluke on my part. I was not aware that it was able to tie the commit message to the JIRA issue. I do not recollect seeing the commits there when I added the comment, perhaps because I had the issue up already before doing the commit. :-( What is the magic then? Is it purely because I had listed MODPYTHON-124 in the commit message. Where is that documented? Did a quick scan of dev.apache.org I haven't found mention of it yet. Graham
Re: Maually adding notes about commits to JIRA.
Graham Dumpleton wrote: Jim Gallacher wrote .. All very interesting, except that JIRA does record the svn commit info, and with great detail. It just doesn't treat the commit as a comment. For example: http://issues.apache.org/jira/browse/MODPYTHON-124?page=all Personally I think the Subversion commit information should be included in the comments by default, rather than requiring a separate view, but I guess that's between JIRA and myself. :) This is showing my ignorance then. That the commit appeared there was a fluke on my part. I was not aware that it was able to tie the commit message to the JIRA issue. I do not recollect seeing the commits there when I added the comment, perhaps because I had the issue up already before doing the commit. :-( There can be a bit of a time lag, so it may not have been there when you added your comment. What is the magic then? Is it purely because I had listed MODPYTHON-124 in the commit message. Yep. That's the magic. Where is that documented? Did a quick scan of dev.apache.org I haven't found mention of it yet. I have no idea where it's documented. Likely in the JIRA help pages, but it was Nicolas that first informed me of this feature. I'll add this information to my developer guidelines page. Jim
Re: mod_python 3.2.8 available for testing
+1 FreeBSD 6.0, apache 2.0.55 mpm-prefork, python 2.4.2
Re: How mod_python treats apache.OK/apache.DECLINED response fromhandlers.
Jim Gallacher wrote .. I don't have alot more to say on these last 2 emails. I think you are on the right path here. Okay, I think I have a good plan now. To summarise the whole issue, the way Apache treats multiple handlers in a single phase for non content handler phases is as follows: PostReadRequestHandler RUN_ALL TransHandler RUN_FIRST MapToStorageHandler RUN_FIRST InitHandler RUN_ALL HeaderParserHandler RUN_ALL AccessHandlerRUN_ALL AuthenHandlerRUN_FIRST AuthzHandler RUN_FIRST TypeHandler RUN_FIRST FixupHandler RUN_ALL LogHandler RUN_ALL RUN_ALL means run all handlers until one returns something other than OK or DECLINED. Thus, handler needs to return DONE or an error to have it stop processing for that phase. RUN_FIRST means run all handlers while they return DECLINED. Thus, needs handler to return OK, DONE or error to have it stop processing for that phase. Where multiple handlers are registered within mod_python for a single phase it doesn't behave like either of these. In mod_python it will keep running the handlers only while OK is returned. Returning DECLINED causes it to stop. This existing behaviour can be described (like mod_perl) as stacked handlers. Having non content handler phases behave differently to how Apache does it causes problems. For example things like a string of authentication handlers which only say OK when they handle the authentication type, can't be implemented properly. In Apache, it should stop the first time one returns OK, but in mod_python it will keep running the handlers in that phase. In summary, it needs to behave more like Apache for the non content handler phases. In respect of the content handler phase itself, in practice only one handler module is supposed to implement it. At the Apache level there is no concept of different Apache modules having goes at the content handler phase and returning DECLINED if they don't want to handle it. This is reflected in how in the type handler phase, selection of the module to deliver content is usually done by setting the single valued req.handler string. Although, when using mod_python this is done implicitly by setting the SetHandler/AddHandler directives and mod_negotiation then in turn setting req.handler to be mod_python for you. Because mod_python when executed for the content handler phase is the only thing generating the content, the existing mechanism of stacked handlers and how the status is handled is fine within just the content handler phase. Can thus keep that as is and no chance of stuffing up existing systems. Where another phase calls req.add_handler() to add a handler or multiple handlers for the PythonHandler (content) phase, these will be added in a stacked manner within that phase. This also is the same as it works now. There would be non need to have a new function to add stacked handlers as that behaviour would be dictated by phase being PythonHandler. For all the non content handler phases though, the current stacked handlers algorithm used by mod_python would be replaced with how Apache does it. That is, within multiple handlers registered with mod_python for non content handler phase, it would use RUN_FIRST or RUN_ALL algorithm as appropriate for the phase. For those which use RUN_ALL, this wouldn't be much different than what mod_python does now except that returning DECLINED would cause it to go to next mod_python handler in that phase instead of stopping. It is highly unlikely that this change would have an impact as returning DECLINED in RUN_ALL phases for how mod_python currently implements it, tends not to be useful and can't see that anyone would have been using it. For those which use RUN_FIRST, the difference would be significant as reurning OK will now cause it to stop instead of going to next mod_python handler in the phase. Personally I don't think this would be a drama as not many people would be using these phases and highly unlikely that someone would have listed multiple handlers for such phases. If they had and knew what they were doing, they should have long ago realised that the current behaviour was a bit broken and it even probably stopped them from doing what they wanted unless they fudged it. As to use of req.add_handler() for non content handler phases, each call would create a distinct handler, ie., no stacking of handlers at all. No separate function is required though, as slight change in behaviour determine form phase specified. To sum up, I think these changes would have minimal if no impact as where changes are significant, it isn't likely to overlap with existing code as shortcomings in current system would have mean't people wouldn't have been doing the sorts of things that may have been impacted. Therefore, I don't see a need for this to be switch enabled and the change could just be made and merely documented.
FieldStorage callback only works with FileType objects in 3.2.7
There is still trouble with the FieldStorage class. Looks like this one is not
really fixed:
http://issues.apache.org/jira/browse/MODPYTHON-40
See for a counter example:
http://modpython.org/pipermail/mod_python/2006-February/020248.html
(remove the import fmt)
It fails with the following traceback:
Mod_python error: PythonHandler mod_python.psp
Traceback (most recent call last):
File C:\Python24\Lib\site-packages\mod_python\apache.py, line 299, in
HandlerDispatch
result = object(req)
File C:\Python24\Lib\site-packages\mod_python\psp.py, line 302, in handler
p.run()
File C:\Python24\Lib\site-packages\mod_python\psp.py, line 213, in run
exec code in global_scope
File C:/source/archaic_web/upload.psp, line 47, in ?
for afile in frm.getlist('archivefile'):
File C:\Python24\Lib\site-packages\mod_python\util.py, line 354, in getlist
found.append(StringField(item.value))
File C:\Python24\Lib\site-packages\mod_python\util.py, line 74, in
__getattr__
value = self.file.read()
AttributeError: FileCounter instance has no attribute 'read'
The file is posted as file, and correctly writting into the FileCounter. However, the FieldStorage
class forgets about this, and in util.py it tries to determine whether this is a file or not by
looking whether it derives from FileType. Since any home made file object is not likely to derive
from that class, util.py concludes that this was a simple field, and does a file.read() to fetch it
into memory (which is what we were preventing in issue 40)
A quick way to solve it is to use this code:
http://issues.apache.org/jira/browse/MODPYTHON-93
this will remove the bad checks for field types alltogether.
--
Mike Looijmans
Philips Natlab / Topic Automation
