Merge lp:~malept/loggerhead/standalone-auth into lp:loggerhead

[Mark Lee]

[Mark Lee (malept)]

Please see bug 518724 for description/rationale.

[Matt Nordhoff (mnordhoff)]

You should add the bug number to NEWS (but someone can do that when landing it).

Also, the single-letter variable names are icky, though coming up with 3 different username variables is a pain.

Other than that, this looks good to me, but I'm not smart enough to know if it really is. It's great that you were able to leverage Paste and Trac to avoid writing any unnecessary code. :)

[Mark Lee (malept)]

> You should add the bug number to NEWS (but someone can do that when landing
> it).

Fixed in revision 405. I wasn't entirely sure whether to add the bug report number, as the dev entries in NEWS are a bit inconsistent in that respect.

> Also, the single-letter variable names are icky, though coming up with 3
> different username variables is a pain.

Yeah, it is :) I attempted to fix this in revision 406.

> Other than that, this looks good to me, but I'm not smart enough to know if it
> really is. It's great that you were able to leverage Paste and Trac to avoid
> writing any unnecessary code. :)

Thanks for the review.

[lp:~malept/loggerhead/standalone-auth]

405. By Mark Lee on 2010-02-08

NEWS: add bug number.

406. By Mark Lee on 2010-02-08

Replace single-character variable names; add comments.

[Michael Hudson-Doyle (mwhudson)]

It looks good to me, and very simple testing indicates it works. It's a pleasingly small amount of code!

The only thing I'd have you change is to move the import statements to the top of the file.

Thanks for your contribution!

Oh, can I get you to do the contributor agreement thing? http://www.canonical.com/contributors

review: Approve

[lp:~malept/loggerhead/standalone-auth]

407. By Mark Lee on 2010-02-08

Move import statements to the top of the file.

[Mark Lee (malept)]

> The only thing I'd have you change is to move the import statements to the top
> of the file.

OK. I wanted to avoid importing code when it's not being used, but this is OK too, as it's not really an extra dependency. Fixed in revision 407.

> Oh, can I get you to do the contributor agreement thing?
> http://www.canonical.com/contributors

I'll read + sign it later tonight.

[Michael Hudson-Doyle (mwhudson)]

Did you ever get to the contributor agreement?

[Jelmer Vernooij (jelmer)]

Ping?

[Max Kanat-Alexander (mkanat)]

I find it hard to believe that the code in the htpasswd module here doesn't already exist in some external python library. Isn't there already some standard method for storing passwords, using a python library, that we can use without having to write our own?

Also, the code that is currently in main.py should probably be mostly within a middleware class.

review: Needs Fixing

[John A Meinel (jameinel)]

AFAIK, we're still waiting on a contributor agreement, and Max has some concerns about the implementation. I'm thinking to kick this back to Work-in-Progress, but I'll try to wait a day or two to see if it just got forgotten.

[Krisztian Eyssen (eyssen)]

review: Approve

[Graham Binns (gmb)]

Since there doesn't seem to have been any change to this I'm going to move it back to Work In Progress as John suggests above.

[Richie Ward (richies)]

I have rebased this on master (rev 451):
https://code.launchpad.net/~richies/+junk/standalone_auth

Unmerged revisions

407. By Mark Lee on 2010-02-08

Move import statements to the top of the file.

406. By Mark Lee on 2010-02-08

Replace single-character variable names; add comments.

405. By Mark Lee on 2010-02-08

NEWS: add bug number.

404. By Mark Lee on 2010-02-07

Typo/PEP8 fixes.

403. By Mark Lee on 2010-02-07

Split out the authentication realm option into its own command-line flag.

402. By Mark Lee on 2010-02-07

Add support for HTTP digest authentication in standalone mode.

401. By Mark Lee on 2010-02-07

Add support for HTTP basic authentication in standalone mode.

=== modified file 'NEWS'
--- NEWS	2010-01-11 14:17:45 +0000
+++ NEWS	2010-02-08 20:46:16 +0000
@@ -12,6 +12,9 @@
 
     - Support FastCGI, SCGI and AJP using flup. (Denis Martinez)
 
+    - Add support for HTTP basic/digest authentication in standalone
+      mode. (Mark Lee, #518724)
+
 1.17 [20Aug2009]
 ---------------
 

=== modified file 'loggerhead/config.py'
--- loggerhead/config.py	2009-11-23 22:45:46 +0000
+++ loggerhead/config.py	2010-02-08 20:46:16 +0000
@@ -49,6 +49,17 @@
     parser.add_option("--protocol", dest="protocol",
                       help=("Protocol to use: http, scgi, fcgi, ajp"
                            "(defaults to http)."))
+    parser.add_option("--auth-realm", dest="auth_realm", metavar="REALM",
+                      help="The authentication realm used by Loggerhead "
+                           "when HTTP authentication is enabled.")
+    parser.add_option("--digest-auth", dest="digest_auth", metavar="PATH",
+                      help="Enables HTTP Digest authentication support, "
+                           "PATH being the absolute path to the htdigest "
+                           "file (requires --auth-realm).")
+    parser.add_option("--basic-auth", dest="basic_auth", metavar="PATH",
+                      help="Enables HTTP Basic authentication support, "
+                           "PATH being the absolute path to the htpasswd "
+                           "file (requires --auth-realm).")
     parser.add_option("--memory-profile", action="store_true",
                       help="Profile the memory usage using Dozer.")
     parser.add_option("--prefix", dest="user_prefix",

=== added file 'loggerhead/htpasswd.py'
--- loggerhead/htpasswd.py	1970-01-01 00:00:00 +0000
+++ loggerhead/htpasswd.py	2010-02-08 20:46:16 +0000
@@ -0,0 +1,139 @@
+#!/usr/bin/python
+#
+# Originally from http://svn.edgewall.org/repos/trac/trunk/contrib/htpasswd.py
+# (revision 6651)
+#
+# According to http://svn.edgewall.org/repos/trac/trunk/contrib/README, this
+# is the license:
+#
+# Permission to use, copy, modify, and distribute this software and its
+# documentation for any purpose and without fee is hereby granted, provided
+# that the above copyright notice appears in all copies and that both the
+# copyright notice and this permission notice appear in supporting
+# documentation, and that the same name not be used in advertising or
+# publicity pertaining to distribution of the software without specific,
+# written prior permission. We make no representations about the
+# suitability this software for any purpose. It is provided "as is"
+# without express or implied warranty.
+"""Replacement for htpasswd"""
+# Original author: Eli Carter
+
+import os
+import sys
+import random
+from optparse import OptionParser
+
+# We need a crypt module, but Windows doesn't have one by default.  Try to find
+# one, and tell the user if we can't.
+try:
+    import crypt
+except ImportError:
+    try:
+        import fcrypt as crypt
+    except ImportError:
+        sys.stderr.write("Cannot find a crypt module.  "
+                         "Possibly http://carey.geek.nz/code/python-fcrypt/\n")
+        sys.exit(1)
+
+
+def salt():
+    """Returns a string of 2 random letters"""
+    letters = 'abcdefghijklmnopqrstuvwxyz' \
+              'ABCDEFGHIJKLMNOPQRSTUVWXYZ' \
+              '0123456789/.'
+    return random.choice(letters) + random.choice(letters)
+
+
+class HtpasswdFile:
+    """A class for manipulating htpasswd files."""
+
+    def __init__(self, filename, create=False):
+        self.entries = []
+        self.filename = filename
+        if not create:
+            if os.path.exists(self.filename):
+                self.load()
+            else:
+                raise Exception("%s does not exist" % self.filename)
+
+    def load(self):
+        """Read the htpasswd file into memory."""
+        lines = open(self.filename, 'r').readlines()
+        self.entries = []
+        for line in lines:
+            username, pwhash = line.split(':')
+            entry = [username, pwhash.rstrip()]
+            self.entries.append(entry)
+
+    def save(self):
+        """Write the htpasswd file to disk"""
+        open(self.filename, 'w').writelines(["%s:%s\n" % (entry[0], entry[1])
+                                             for entry in self.entries])
+
+    def update(self, username, password):
+        """Replace the entry for the given user, or add it if new."""
+        pwhash = crypt.crypt(password, salt())
+        matching_entries = [entry for entry in self.entries
+                            if entry[0] == username]
+        if matching_entries:
+            matching_entries[0][1] = pwhash
+        else:
+            self.entries.append([username, pwhash])
+
+    def delete(self, username):
+        """Remove the entry for the given user."""
+        self.entries = [entry for entry in self.entries
+                        if entry[0] != username]
+
+
+def main():
+    """%prog [-c] -b filename username password
+    Create or update an htpasswd file"""
+    # For now, we only care about the use cases that affect tests/functional.py
+    parser = OptionParser(usage=main.__doc__)
+    parser.add_option('-b', action='store_true', dest='batch', default=False,
+        help='Batch mode; password is passed on the command line IN THE '
+             'CLEAR.')
+    parser.add_option('-c', action='store_true', dest='create', default=False,
+        help='Create a new htpasswd file, overwriting any existing file.')
+    parser.add_option('-D', action='store_true', dest='delete_user',
+        default=False, help='Remove the given user from the password file.')
+
+    options, args = parser.parse_args()
+
+    def syntax_error(msg):
+        """Utility function for displaying fatal error messages with usage
+        help.
+        """
+        sys.stderr.write("Syntax error: " + msg)
+        sys.stderr.write(parser.get_usage())
+        sys.exit(1)
+
+    if not options.batch:
+        syntax_error("Only batch mode is supported\n")
+
+    # Non-option arguments
+    if len(args) < 2:
+        syntax_error("Insufficient number of arguments.\n")
+    filename, username = args[:2]
+    if options.delete_user:
+        if len(args) != 2:
+            syntax_error("Incorrect number of arguments.\n")
+        password = None
+    else:
+        if len(args) != 3:
+            syntax_error("Incorrect number of arguments.\n")
+        password = args[2]
+
+    passwdfile = HtpasswdFile(filename, create=options.create)
+
+    if options.delete_user:
+        passwdfile.delete(username)
+    else:
+        passwdfile.update(username, password)
+
+    passwdfile.save()
+
+
+if __name__ == '__main__':
+    main()

=== modified file 'loggerhead/main.py'
--- loggerhead/main.py	2009-11-27 18:30:24 +0000
+++ loggerhead/main.py	2010-02-08 20:46:16 +0000
@@ -24,6 +24,8 @@
 from bzrlib.plugin import load_plugins
 
 from paste import httpserver
+from paste.auth.basic import AuthBasicHandler
+from paste.auth.digest import AuthDigestHandler
 from paste.httpexceptions import HTTPExceptionHandler, HTTPInternalServerError
 from paste.translogger import TransLogger
 
@@ -31,6 +33,7 @@
 from loggerhead.apps.transport import (
     BranchesFromTransportRoot, UserBranchesFromTransportRoot)
 from loggerhead.config import LoggerheadConfig
+from loggerhead import htpasswd
 from loggerhead.util import Reloader
 from loggerhead.apps.error import ErrorHandlerApp
 
@@ -151,6 +154,39 @@
     else:
         host = config.get_option('user_host')
 
+    if config.get_option('auth_realm'):
+        realm = config.get_option('auth_realm')
+        if config.get_option('digest_auth'):
+            path = config.get_option('digest_auth')
+            users = {}
+            # parse htdigest file
+            for line in open(path):
+                # format: $user:$realm:$hash
+                htuser, htrealm, hthash = line.strip().split(':', 2)
+                if htrealm not in users:
+                    users[htrealm] = {}
+                users[htrealm][htuser] = hthash
+
+            def d_handler(environ, hrealm, huser):
+                return users.get(hrealm, {}).get(huser, False)
+            app = AuthDigestHandler(app, realm, d_handler)
+        elif config.get_option('basic_auth'):
+            path = config.get_option('basic_auth')
+            htp = htpasswd.HtpasswdFile(path)
+
+            def b_handler(environ, username, password):
+                result = [hthash for htuser, hthash in htp.entries
+                          if htuser == username]
+                if len(result) == 0:
+                    return False
+                hthash = result[0]
+                return htpasswd.crypt.crypt(password, hthash[:2]) == hthash
+            app = AuthBasicHandler(app, realm, b_handler)
+    elif config.get_option('basic_auth') or config.get_option('digest_auth'):
+        print 'Both the --basic-auth and --digest-auth flags require that', \
+              'the --auth-realm flag is set.'
+        sys.exit(1)
+
     if not config.get_option('protocol'):
         protocol = 'http'
     else: