2121: post-review doesn't handle bad passwords nicely

gtu****@gmai***** (Google Code) (Is this you? Claim this profile.)
Jan. 20, 2014 
http://reviews.reviewboard.org/

What version are you running?
RBTools 0.3.2

What's the URL of the page this enhancement relates to, if any?
NA

Describe the enhancement and the motivation for it.
For end user tools (front end and CLI), errors should be informative and handled cleanly as not doing so creates noise for the admin.  When an incorrect password is provided to post-review, instead of getting an error message, the user is given a traceback.  A better user experience would catch the ValueError exception and give the user an error like "Login failed.  Please check your password or contact your ReviewBoard administrator."

What steps will reproduce the problem?
1. Remove $HOME/.post-review-cookies.txt
2. Create a change.
2. Execute post-review to post changes.
3. Enter an incorrect password.

What operating system are you using? What browser?
$ lsb_release -a
LSB Version:	:core-3.1-amd64:core-3.1-ia32:core-3.1-noarch:graphics-3.1-amd64:graphics-3.1-ia32:graphics-3.1-noarch
Distributor ID:	CentOS
Description:	CentOS release 5.2 (Final)
Release:	5.2
Codename:	Final

Please provide any additional information below.
We're using LDAP for authentication.  This is the traceback:

"""
Traceback (most recent call last):
  File "/usr/bin/post-review", line 7, in ?
    sys.exit(
  File "/usr/lib/python2.4/site-packages/RBTools-0.3.2-py2.4.egg/rbtools/postreview.py", line 3741, in main
  File "/usr/lib/python2.4/site-packages/RBTools-0.3.2-py2.4.egg/rbtools/postreview.py", line 434, in check_api_version
  File "/usr/lib/python2.4/site-packages/RBTools-0.3.2-py2.4.egg/rbtools/postreview.py", line 876, in api_get
  File "/usr/lib/python2.4/site-packages/RBTools-0.3.2-py2.4.egg/rbtools/postreview.py", line 846, in http_get
  File "/usr/lib64/python2.4/urllib2.py", line 130, in urlopen
    return _opener.open(url, data)
  File "/usr/lib64/python2.4/urllib2.py", line 364, in open
    response = meth(req, response)
  File "/usr/lib/python2.4/site-packages/RBTools-0.3.2-py2.4.egg/rbtools/postreview.py", line 317, in http_response
  File "/usr/lib64/python2.4/urllib2.py", line 396, in error
    result = self._call_chain(*args)
  File "/usr/lib64/python2.4/urllib2.py", line 337, in _call_chain
    result = func(*args)
  File "/usr/lib64/python2.4/urllib2.py", line 741, in http_error_401
    host, req, headers)
  File "/usr/lib64/python2.4/urllib2.py", line 720, in http_error_auth_reqed
    return self.retry_http_basic_auth(host, req, realm)
  File "/usr/lib/python2.4/site-packages/RBTools-0.3.2-py2.4.egg/rbtools/postreview.py", line 339, in retry_http_basic_auth
  File "/usr/lib64/python2.4/urllib2.py", line 730, in retry_http_basic_auth
    return self.parent.open(req)
  File "/usr/lib64/python2.4/urllib2.py", line 364, in open
    response = meth(req, response)
  File "/usr/lib/python2.4/site-packages/RBTools-0.3.2-py2.4.egg/rbtools/postreview.py", line 317, in http_response
  File "/usr/lib64/python2.4/urllib2.py", line 396, in error
    result = self._call_chain(*args)
  File "/usr/lib64/python2.4/urllib2.py", line 337, in _call_chain
    result = func(*args)
  File "/usr/lib64/python2.4/urllib2.py", line 916, in http_error_401
    host, req, headers)
  File "/usr/lib64/python2.4/urllib2.py", line 807, in http_error_auth_reqed
    raise ValueError("AbstractDigestAuthHandler doesn't know "
ValueError: AbstractDigestAuthHandler doesn't know about Basic
"""
chipx86
#1 chipx86 
This problem seems to be specific to Python 2.4. We normally do the right thing.
  • +Confirmed
  • -Priority-Medium
    +Priority-High
    +Milestone-RBTools-Release1.0
    +Component-RBTools
#2 mark.d******@gmai***** (Google Code) (Is this you? Claim this profile.) 
(for people Googling for this stack trace in the future)
It's worth noting that this same stack trace is displayed if the site administrator failed to add the "WSGIPassAuthorization On" setting to the Apache config file for the site when upgrading from an older version of ReviewBoard.

Without this flag the Authorization header isn't passed to ReviewBoard, and so the ReviewBoard server thinks the client isn't providing username and password.

This happens with python 2.4, at least.  With newer versions of python I'd expect the client to show the normal "incorrect password" error message.
#4 Chris******@actia***** (Google Code) (Is this you? Claim this profile.) 
This is the hack I'm using for older python machines:

    diff --git a/rbtools/postreview.py b/rbtools/postreview.py
    index df791aa..d8af51c 100755
    --- a/rbtools/postreview.py
    +++ b/rbtools/postreview.py
    @@ -12,6 +12,11 @@ from optparse import OptionParser
     import datetime
     
     try:
    +    from cStringIO import StringIO as FakeFile
    +except ImportError:
    +    from StringIO import StringIO as FakeFile
    +
    +try:
         # setuptools from http://peak.telecommunity.com/
         from pkg_resources import parse_version
     except ImportError:
    @@ -192,8 +197,18 @@ class ReviewBoardHTTPBasicAuthHandler(urllib2.HTTPBasicAuthHandler):
             if not self._retried:
                 self._retried = True
                 self.retried = 0
    -            response = urllib2.HTTPBasicAuthHandler.retry_http_basic_auth(
    -                self, *args, **kwargs)
    +            try:
    +                response = urllib2.HTTPBasicAuthHandler.retry_http_basic_auth(
    +                    self, *args, **kwargs)
    +            except ValueError, e:
    +                if e.args[0] == "AbstractDigestAuthHandler doesn't know about Basic" and sys.version_info >= (2, 4) and sys.version_info < (2, 5):
    +                    # Python 2.4 client and user gave bad username/password
    +                    # Hack to log a slightly more useful error message
    +                    # See http://code.google.com/p/reviewboard/issues/detail?id=2121
    +                    print 'ERROR: Suspect incorrect username and/or password'
    +                    fileptr = FakeFile('')
    +                    raise urllib2.HTTPError(None, 401, None, None, fileptr)
    +                raise
     
                 if response.code != 401:
                     self._retried = False
david
#5 david 
We'll be dropping support for python 2.4 in RBTools 0.6.
  • -Confirmed
    +WontFix