j.c.sackett has proposed merging
lp:~jcsackett/launchpad/apparently-we-support-basic-auth into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/apparently-we-support-basic-auth/+merge/55845
Summary
=======
Launchpad sort of supports basic auth, but sending badly encoded or unencoded
authentication headers results in a 500 error on the client side and an OOPS on
our side about base 64 encoding. If you send lousy authentication headers the
correct response is simply stating that your still unauthorized, which is an
exception conveniently ignored in our OOPS reporting.
Preimp
======
Spoke with Robert Collins about lp's basic auth support.
Implementation
==============
In authenticate(), we catch the base64 encoding error; since this is a result
of bad data, we raise Unauthorized. Unauthorized is on the list of exceptions
our oops reports ignore, as long as it's coming from an unauthenticated user.
Tests
=====
bin/test -m canonical.launchpad.webapp
No new tests were added, b/c to test this particular error case requires an
actual fully fledged independently running lp instance (.dev or otherwise).
None of the layers properly provide that, and out TestRequest machine can't
simulate what's actually going on. It's a small change though, so the risk is
minimal.
If my assertion is wrong and you can provide a way to test this, I'm happy to
implement it. More than happy, actually.
QA
==
Try to make a request with urllib2 and an uncoded email:password Basic auth
string. You should get an Unauthorized resonse instead of Server Error.
Lint
====
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/canonical/launchpad/webapp/authentication.py
--
https://code.launchpad.net/~jcsackett/launchpad/apparently-we-support-basic-auth/+merge/55845
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jcsackett/launchpad/apparently-we-support-basic-auth into lp:launchpad.
=== modified file 'lib/canonical/launchpad/webapp/authentication.py'
--- lib/canonical/launchpad/webapp/authentication.py 2011-02-04 14:41:18 +0000
+++ lib/canonical/launchpad/webapp/authentication.py 2011-03-31 22:08:39 +0000
@@ -30,6 +30,7 @@
from zope.event import notify
from zope.interface import implements
from zope.preference.interfaces import IPreferenceGroup
+from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
from zope.session.interfaces import ISession
@@ -121,10 +122,15 @@
# To avoid confusion (hopefully), basic auth trumps cookie auth
# totally, and all the time. If there is any basic auth at all,
# then cookie auth won't even be considered.
-
# XXX daniels 2004-12-14: allow authentication scheme to be put into
# a view; for now, use basic auth by specifying ILoginPassword.
- credentials = ILoginPassword(request, None)
+ try:
+ credentials = ILoginPassword(request, None)
+ except binascii.Error:
+ # We have probably been sent Basic auth credentials that aren't
+ # encoded properly. That's a client error, so we don't really
+ # care, and we're done.
+ raise Unauthorized("Bad Basic authentication.")
if credentials is not None and credentials.getLogin() is not None:
return self._authenticateUsingBasicAuth(credentials, request)
else:
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
