996408 - bouncer API does not work with passwords containing :

Status: RESOLVED FIXED

(Webtools :: Bouncer, defect)

Description

[Robert Helmer [:rhelmer]]

If the password (or username presumably) contains : then parsing the basic auth header will fail:

Traceback (most recent call last):

  File "/data/www/download.allizom.org/tuxedo/vendor/lib/python/django/core/handlers/base.py", line 111, in get_response
    response = callback(request, *callback_args, **callback_kwargs)

  File "/data/www/download.allizom.org/tuxedo/vendor/lib/python/django/views/decorators/http.py", line 41, in inner
    return func(request, *args, **kwargs)

  File "/data/www/download.allizom.org/tuxedo/apps/api/decorators.py", line 42, in wrapper
    realm, *args, **kwargs)

  File "/data/www/download.allizom.org/tuxedo/apps/api/decorators.py", line 100, in _view_or_basicauth
    uname, passwd = base64.b64decode(auth[1]).split(':')

ValueError: too many values to unpack

Comment 1

[Fred Wenzel [:wenzel]]

hah, .split(':', 1)

Comment 2

[Fred Wenzel [:wenzel]]

unless the username can have a colon too o_O

Comment 3

[Robert Helmer [:rhelmer]]

(In reply to Fred Wenzel [:wenzel] from comment #1)
> hah, .split(':', 1)

Yep that's what I did :) I mentioned in the PR that I looked at https://pypi.python.org/pypi/basicauth/0.2 but from inspecting the code I think they have the same problem(!)

(In reply to Fred Wenzel [:wenzel] from comment #2)
> unless the username can have a colon too o_O

Good point. I am OK with being strict about what's acceptable for usernames, just not with passwords.
I'll take a look and open a new bug if this is needed.

Comment 4

[Robert Helmer [:rhelmer]]

https://github.com/mozilla/tuxedo/commit/2a603b2a2dcca5788e53b1b403f0cc2ae909ce46

Comment 5

[Robert Helmer [:rhelmer]]

(In reply to Fred Wenzel [:wenzel] from comment #2)
> unless the username can have a colon too o_O

Just tried via the django admin interface, doesn't allow it:

    This value may contain only letters, numbers and @/./+/-/_ characters.

Comment 6

[Robert Helmer [:rhelmer]]

Also opened an issue and filed a PR for python-basicauth package - might be nice to switch to it assuming a) I am correct a fix is needed and b) it is accepted :) https://github.com/rdegges/python-basicauth/issues/1

Comment 7

[Robert Helmer [:rhelmer]]

(In reply to Robert Helmer [:rhelmer] from comment #5)
> (In reply to Fred Wenzel [:wenzel] from comment #2)
> > unless the username can have a colon too o_O
> 
> Just tried via the django admin interface, doesn't allow it:
> 
>     This value may contain only letters, numbers and @/./+/-/_ characters.

Going deeper - according to the RFC user_id can contain anything *except* colon:

      userid      = *<TEXT excluding ":">

http://www.ietf.org/rfc/rfc2617.txt

Comment 8

[Fred Wenzel [:wenzel]]

Awesome, thanks for digging this up and fixing the bug :)

Comment 9

[Fred Wenzel [:wenzel]]

(In reply to Robert Helmer [:rhelmer] from comment #6)
> Also opened an issue and filed a PR for python-basicauth package - might be
> nice to switch to it assuming a) I am correct a fix is needed and b) it is
> accepted :) https://github.com/rdegges/python-basicauth/issues/1

I was just about to be a good open source citizen and do exactly that, but you were faster. Awesome!