Commit 23ccba04 authored by Michael Hanselmann's avatar Michael Hanselmann
Browse files

http.auth: Fix bug with checking hashed passwords



When username and password were sent for a resource not requiring
authentication, it wouldn't be accepted if the user in question had a
hashed password. The reason was that the function GetAuthRealm used to
return None if no authentication was necessary. However, the
authentication realm is necessary to verify hashed passwords. This is
fixed by requiring GetAuthRealm to always return a realm and separating
the decision whether to require authentication or not to a separate
function.
Signed-off-by: default avatarMichael Hanselmann <hansmi@google.com>
Reviewed-by: default avatarIustin Pop <iustin@google.com>
parent e721c742
......@@ -118,17 +118,18 @@ class RemoteApiHttpServer(http.auth.HttpServerRequestAuthentication,
req.private = ctx
# Check for expected attributes
assert req.private.handler
assert req.private.handler_fn
assert req.private.handler_access is not None
return req.private
def GetAuthRealm(self, req):
"""Override the auth realm for queries.
def AuthenticationRequired(self, req):
"""Determine whether authentication is required.
"""
ctx = self._GetRequestContext(req)
if ctx.handler_access:
return self.AUTH_REALM
else:
return None
return bool(self._GetRequestContext(req).handler_access)
def Authenticate(self, req, username, password):
"""Checks whether a user can access a resource.
......
......@@ -78,7 +78,7 @@ def _FormatAuthHeader(scheme, params):
class HttpServerRequestAuthentication(object):
# Default authentication realm
AUTH_REALM = None
AUTH_REALM = "Unspecified"
# Schemes for passwords
_CLEARTEXT_SCHEME = "{CLEARTEXT}"
......@@ -87,13 +87,12 @@ class HttpServerRequestAuthentication(object):
def GetAuthRealm(self, req):
"""Returns the authentication realm for a request.
MAY be overridden by a subclass, which then can return different realms for
different paths. Returning "None" means no authentication is needed for a
request.
May be overridden by a subclass, which then can return different realms for
different paths.
@type req: L{http.server._HttpServerRequest}
@param req: HTTP request context
@rtype: str or None
@rtype: string
@return: Authentication realm
"""
......@@ -102,6 +101,18 @@ class HttpServerRequestAuthentication(object):
# pylint: disable-msg=W0613
return self.AUTH_REALM
def AuthenticationRequired(self, req):
"""Determines whether authentication is required for a request.
To enable authentication, override this function in a subclass and return
C{True}. L{AUTH_REALM} must be set.
@type req: L{http.server._HttpServerRequest}
@param req: HTTP request context
"""
return False
def PreHandleRequest(self, req):
"""Called before a request is handled.
......@@ -109,15 +120,16 @@ class HttpServerRequestAuthentication(object):
@param req: HTTP request context
"""
realm = self.GetAuthRealm(req)
# Authentication not required, and no credentials given?
if realm is None and http.HTTP_AUTHORIZATION not in req.request_headers:
if not (self.AuthenticationRequired(req) or
(req.request_headers and
http.HTTP_AUTHORIZATION in req.request_headers)):
return
if realm is None: # in case we don't require auth but someone
# passed the crendentials anyway
realm = "Unspecified"
realm = self.GetAuthRealm(req)
if not realm:
raise AssertionError("No authentication realm")
# Check "Authorization" header
if self._CheckAuthorization(req):
......@@ -255,7 +267,7 @@ class HttpServerRequestAuthentication(object):
realm = self.GetAuthRealm(req)
if not realm:
# There can not be a valid password for this case
return False
raise AssertionError("No authentication realm")
expha1 = md5()
expha1.update("%s:%s:%s" % (username, realm, password))
......
......@@ -154,8 +154,8 @@ class TestAuth(unittest.TestCase):
self.assert_(tvbap("This is only a test", "user", "pw",
"{HA1}92ea58ae804481498c257b2f65561a17"))
self.failIf(tvbap(None, "user", "pw",
"{HA1}92ea58ae804481498c257b2f65561a17"))
self.failUnlessRaises(AssertionError, tvbap, None, "user", "pw",
"{HA1}92ea58ae804481498c257b2f65561a17")
self.failIf(tvbap("Admin area", "user", "pw",
"{HA1}92ea58ae804481498c257b2f65561a17"))
self.failIf(tvbap("This is only a test", "someone", "pw",
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment