Closed
Bug 631508
Opened 14 years ago
Closed 14 years ago
authenticate_user broken with 'remote_user_original'
Categories
(Cloud Services :: Server: Core, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: tarek, Assigned: jrconlin)
Details
Attachments
(1 file)
(deleted),
patch
|
tarek
:
review+
|
Details | Diff | Splinter Review |
if authenticate_user() fails to authenticate the user, and the user does not have an '@', remote_user_original is not defined and the code breaks. It misses an "else" here: http://hg.mozilla.org/services/server-core/file/98dc4a02cc4c/services/wsgiauth.py#l110 We need an extra test w/ the fix here to cover this combination. Traceback (most recent call last): File "/home/tarek/dev/hg.mozilla.org/server-full-clean/deps/server-core/services/util.py", line 443, in __call__ return self.app(environ, start_response) File "/home/tarek/dev/hg.mozilla.org/server-full-clean/lib/python2.6/site-packages/Paste-1.7.5.1-py2.6.egg/paste/translogger.py", line 68, in __call__ return self.application(environ, replacement_start_response) File "/home/tarek/dev/hg.mozilla.org/server-full-clean/lib/python2.6/site-packages/WebOb-1.0.1-py2.6.egg/webob/dec.py", line 147, in __call__ resp = self.call_func(req, *args, **self.kwargs) File "/home/tarek/dev/hg.mozilla.org/server-full-clean/lib/python2.6/site-packages/WebOb-1.0.1-py2.6.egg/webob/dec.py", line 208, in call_func return self.func(req, *args, **kwargs) File "/home/tarek/dev/hg.mozilla.org/server-full-clean/deps/server-core/services/baseapp.py", line 247, in __call__ self.auth.check(request, match) File "/home/tarek/dev/hg.mozilla.org/server-full-clean/deps/server-core/services/wsgiauth.py", line 66, in check match.get('username')) File "/home/tarek/dev/hg.mozilla.org/server-full-clean/deps/server-core/services/wsgiauth.py", line 125, in authenticate_user if remote_user_original is not None and \ UnboundLocalError: local variable 'remote_user_original' referenced before assignment
Assignee | ||
Comment 1•14 years ago
|
||
Actually, removing the prior "if", and always setting remote_user_original will fix the undef (and apparently, there's no way to test for variable definition without trapping it in an exception handler, so correcting that mistake). I'm a bit concerned about the test, though, since this should have been triggered by the existing bad user exception trap. (userid == None && userid does not contain '@' should have triggered the error) Looking into why that may not have fired.
Reporter | ||
Comment 2•14 years ago
|
||
(In reply to comment #1) > Actually, removing the prior "if", and always setting remote_user_original will > fix the undef In that case, if remote_user_original is always equal to user_name, why do we have two variables ? (and apparently, there's no way to test for variable definition > without trapping it in an exception handler, so correcting that mistake). > > I'm a bit concerned about the test, though, since this should have been > triggered by the existing bad user exception trap. (userid == None && userid > does not contain '@' should have triggered the error) Looking into why that may > not have fired. Yeah, we need to make sure our test catch this bug :D
Assignee | ||
Comment 3•14 years ago
|
||
> In that case, if remote_user_original is always equal to user_name, why do we > have two variables ? Because user_name is potentially changed and lost at http://hg.mozilla.org/services/server-core/file/98dc4a02cc4c/services/wsgiauth.py#l113 (Obviously, the "is None" can be removed from the following test.)
Reporter | ||
Comment 4•14 years ago
|
||
Ok. Turns out hudson is broken in fact, but there's a problem with the email sendings
Assignee | ||
Comment 5•14 years ago
|
||
removed '@' conditional. added test to catch exception.
Attachment #510713 -
Flags: review?(tarek)
Reporter | ||
Updated•14 years ago
|
Attachment #510713 -
Flags: review?(tarek) → review+
Assignee | ||
Comment 6•14 years ago
|
||
changeset: 572:4a93662d2a88 tag: tip user: JR Conlin <jconlin@mozilla.com> date: Tue Feb 08 13:15:07 2011 -0800 summary: bz 631508: removed conditional and added test for remote_user_original
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•