Authenticate(): When authenticating AuthUser, wrap the

self.authenticateMember() call in a try/except catching and ignoring
NotAMemberErrors.  The effect of this is that other authcontexts being
check will then proceed as normal.  This fixes admin login to the
private archives, and non-public rosters.

Under the old code, if you tried to get into the private archives w/o
entering an email address, but using the admin password, you'd be
denied access.

WebAuthenticate(): Removed the wrapping of .Authenticate() in
try/except catching of NotAMemberError, since this should never
percolate out now.

Also, use True/False everywhere it's appropriate (but not in the
cookie code).

Original bug and patch by Stephan Berndts.  Closes SF bug # 864676 and
SF patch # 864674.

1 files changed, 40 insertions, 35 deletions

diff --git a/Mailman/SecurityManager.py b/Mailman/SecurityManager.py
index af8e6b07..62360feb 100644
--- a/Mailman/SecurityManager.py
+++ b/Mailman/SecurityManager.py
@@ -67,6 +67,12 @@ from Mailman import Utils
 from Mailman import Errors
 from Mailman.Logging.Syslog import syslog
 
+try:
+    True, False
+except NameError:
+    True = 1
+    False = 0
+
 
 
 class SecurityManager:
@@ -143,15 +149,15 @@ class SecurityManager:
                     try:
                         salt = secret[:2]
                         if crypt and crypt.crypt(response, salt) == secret:
-                            return 1
-                        return 0
+                            return True
+                        return False
                     except TypeError:
                         # BAW: Hard to say why we can get a TypeError here.
                         # SF bug report #585776 says crypt.crypt() can raise
                         # this if salt contains null bytes, although I don't
                         # know how that can happen (perhaps if a MM2.0 list
                         # with USE_CRYPT = 0 has been updated?  Doubtful.
-                        return 0
+                        return False
                 # The password for the list admin and list moderator are not
                 # kept as plain text, but instead as an sha hexdigest.  The
                 # response being passed in is plain text, so we need to
@@ -163,20 +169,18 @@ class SecurityManager:
                 if secret is None:
                     continue
                 sharesponse = sha.new(response).hexdigest()
-                upgrade = ok = 0
+                upgrade = ok = False
                 if sharesponse == secret:
-                    ok = 1
+                    ok = True
                 elif md5.new(response).digest() == secret:
-                    ok = 1
-                    upgrade = 1
+                    ok = upgrade = True
                 elif cryptmatchp(response, secret):
-                    ok = 1
-                    upgrade = 1
+                    ok = upgrade = True
                 if upgrade:
-                    save_and_unlock = 0
+                    save_and_unlock = False
                     if not self.Locked():
                         self.Lock()
-                        save_and_unlock = 1
+                        save_and_unlock = True
                     try:
                         self.password = sharesponse
                         if save_and_unlock:
@@ -192,8 +196,12 @@ class SecurityManager:
                 if secret and sha.new(response).hexdigest() == secret:
                     return ac
             elif ac == mm_cfg.AuthUser:
-                if self.authenticateMember(user, response):
-                    return ac
+                if user is not None:
+                    try:
+                        if self.authenticateMember(user, response):
+                            return ac
+                    except Errors.NotAMemberError:
+                        pass
             else:
                 # What is this context???
                 syslog('error', 'Bad authcontext: %s', ac)
@@ -205,22 +213,19 @@ class SecurityManager:
         # contains a matching authorization, falling back to checking whether
         # the response matches one of the passwords.  authcontexts must be a
         # sequence, and if it contains the context AuthUser, then the user
-        # argument must not be None.
+        # argument should not be None.
         #
         # Returns a flag indicating whether authentication succeeded or not.
-        try:
-            for ac in authcontexts:
-                ok = self.CheckCookie(ac, user)
-                if ok:
-                    return 1
-            # Check passwords
-            ac = self.Authenticate(authcontexts, response, user)
-            if ac:
-                print self.MakeCookie(ac, user)
-                return 1
-        except Errors.NotAMemberError:
-            pass
-        return 0
+        for ac in authcontexts:
+            ok = self.CheckCookie(ac, user)
+            if ok:
+                return True
+        # Check passwords
+        ac = self.Authenticate(authcontexts, response, user)
+        if ac:
+            print self.MakeCookie(ac, user)
+            return True
+        return False
 
     def MakeCookie(self, authcontext, user=None):
         key, secret = self.AuthContextInfo(authcontext, user)
@@ -269,7 +274,7 @@ class SecurityManager:
         # authentication.
         cookiedata = os.environ.get('HTTP_COOKIE')
         if not cookiedata:
-            return 0
+            return False
         # We can't use the Cookie module here because it isn't liberal in what
         # it accepts.  Feed it a MM2.0 cookie along with a MM2.1 cookie and
         # you get a CookieError. :(.  All we care about is accessing the
@@ -294,8 +299,8 @@ class SecurityManager:
             for user in [Utils.UnobscureEmail(u) for u in usernames]:
                 ok = self.__checkone(c, authcontext, user)
                 if ok:
-                    return 1
-            return 0
+                    return True
+            return False
         else:
             return self.__checkone(c, authcontext, user)
 
@@ -304,7 +309,7 @@ class SecurityManager:
         # combination.
         key, secret = self.AuthContextInfo(authcontext, user)
         if not c.has_key(key) or not isinstance(secret, StringType):
-            return 0
+            return False
         # Undo the encoding we performed in MakeCookie() above.  BAW: I
         # believe this is safe from exploit because marshal can't be forced to
         # load recursive data structures, and it can't be forced to execute
@@ -318,18 +323,18 @@ class SecurityManager:
             data = marshal.loads(binascii.unhexlify(c[key]))
             issued, received_mac = data
         except (EOFError, ValueError, TypeError, KeyError):
-            return 0
+            return False
         # Make sure the issued timestamp makes sense
         now = time.time()
         if now < issued:
-            return 0
+            return False
         # Calculate what the mac ought to be based on the cookie's timestamp
         # and the shared secret.
         mac = sha.new(secret + `issued`).hexdigest()
         if mac <> received_mac:
-            return 0
+            return False
         # Authenticated!
-        return 1
+        return True