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

Proposed by Mark Lee
Status: Work in progress
Proposed branch: lp://staging/~malept/loggerhead/standalone-auth
Merge into: lp://staging/loggerhead
Diff against target: 240 lines (+189/-0)
4 files modified
NEWS (+3/-0)
loggerhead/config.py (+11/-0)
loggerhead/htpasswd.py (+139/-0)
loggerhead/main.py (+36/-0)
To merge this branch: bzr merge lp://staging/~malept/loggerhead/standalone-auth
Reviewer Review Type Date Requested Status
Krisztian Eyssen (community) Approve
Max Kanat-Alexander (community) Needs Fixing
Michael Hudson-Doyle Approve
Review via email: mp+18828@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Mark Lee (malept) wrote :

Please see bug 518724 for description/rationale.

Revision history for this message
Matt Nordhoff (mnordhoff) wrote :

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. :)

Revision history for this message
Mark Lee (malept) wrote :

> 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.

405. By Mark Lee

NEWS: add bug number.

406. By Mark Lee

Replace single-character variable names; add comments.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

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
407. By Mark Lee

Move import statements to the top of the file.

Revision history for this message
Mark Lee (malept) wrote :

> 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.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Did you ever get to the contributor agreement?

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Ping?

Revision history for this message
Max Kanat-Alexander (mkanat) wrote :

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
Revision history for this message
John A Meinel (jameinel) wrote :

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.

Revision history for this message
Krisztian Eyssen (eyssen) :
review: Approve
Revision history for this message
Graham Binns (gmb) wrote :

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.

Revision history for this message
Richie Ward (richies) wrote :

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

Unmerged revisions

407. By Mark Lee

Move import statements to the top of the file.

406. By Mark Lee

Replace single-character variable names; add comments.

405. By Mark Lee

NEWS: add bug number.

404. By Mark Lee

Typo/PEP8 fixes.

403. By Mark Lee

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

402. By Mark Lee

Add support for HTTP digest authentication in standalone mode.

401. By Mark Lee

Add support for HTTP basic authentication in standalone mode.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-01-11 14:17:45 +0000
3+++ NEWS 2010-02-08 20:46:16 +0000
4@@ -12,6 +12,9 @@
5
6 - Support FastCGI, SCGI and AJP using flup. (Denis Martinez)
7
8+ - Add support for HTTP basic/digest authentication in standalone
9+ mode. (Mark Lee, #518724)
10+
11 1.17 [20Aug2009]
12 ---------------
13
14
15=== modified file 'loggerhead/config.py'
16--- loggerhead/config.py 2009-11-23 22:45:46 +0000
17+++ loggerhead/config.py 2010-02-08 20:46:16 +0000
18@@ -49,6 +49,17 @@
19 parser.add_option("--protocol", dest="protocol",
20 help=("Protocol to use: http, scgi, fcgi, ajp"
21 "(defaults to http)."))
22+ parser.add_option("--auth-realm", dest="auth_realm", metavar="REALM",
23+ help="The authentication realm used by Loggerhead "
24+ "when HTTP authentication is enabled.")
25+ parser.add_option("--digest-auth", dest="digest_auth", metavar="PATH",
26+ help="Enables HTTP Digest authentication support, "
27+ "PATH being the absolute path to the htdigest "
28+ "file (requires --auth-realm).")
29+ parser.add_option("--basic-auth", dest="basic_auth", metavar="PATH",
30+ help="Enables HTTP Basic authentication support, "
31+ "PATH being the absolute path to the htpasswd "
32+ "file (requires --auth-realm).")
33 parser.add_option("--memory-profile", action="store_true",
34 help="Profile the memory usage using Dozer.")
35 parser.add_option("--prefix", dest="user_prefix",
36
37=== added file 'loggerhead/htpasswd.py'
38--- loggerhead/htpasswd.py 1970-01-01 00:00:00 +0000
39+++ loggerhead/htpasswd.py 2010-02-08 20:46:16 +0000
40@@ -0,0 +1,139 @@
41+#!/usr/bin/python
42+#
43+# Originally from http://svn.edgewall.org/repos/trac/trunk/contrib/htpasswd.py
44+# (revision 6651)
45+#
46+# According to http://svn.edgewall.org/repos/trac/trunk/contrib/README, this
47+# is the license:
48+#
49+# Permission to use, copy, modify, and distribute this software and its
50+# documentation for any purpose and without fee is hereby granted, provided
51+# that the above copyright notice appears in all copies and that both the
52+# copyright notice and this permission notice appear in supporting
53+# documentation, and that the same name not be used in advertising or
54+# publicity pertaining to distribution of the software without specific,
55+# written prior permission. We make no representations about the
56+# suitability this software for any purpose. It is provided "as is"
57+# without express or implied warranty.
58+"""Replacement for htpasswd"""
59+# Original author: Eli Carter
60+
61+import os
62+import sys
63+import random
64+from optparse import OptionParser
65+
66+# We need a crypt module, but Windows doesn't have one by default. Try to find
67+# one, and tell the user if we can't.
68+try:
69+ import crypt
70+except ImportError:
71+ try:
72+ import fcrypt as crypt
73+ except ImportError:
74+ sys.stderr.write("Cannot find a crypt module. "
75+ "Possibly http://carey.geek.nz/code/python-fcrypt/\n")
76+ sys.exit(1)
77+
78+
79+def salt():
80+ """Returns a string of 2 random letters"""
81+ letters = 'abcdefghijklmnopqrstuvwxyz' \
82+ 'ABCDEFGHIJKLMNOPQRSTUVWXYZ' \
83+ '0123456789/.'
84+ return random.choice(letters) + random.choice(letters)
85+
86+
87+class HtpasswdFile:
88+ """A class for manipulating htpasswd files."""
89+
90+ def __init__(self, filename, create=False):
91+ self.entries = []
92+ self.filename = filename
93+ if not create:
94+ if os.path.exists(self.filename):
95+ self.load()
96+ else:
97+ raise Exception("%s does not exist" % self.filename)
98+
99+ def load(self):
100+ """Read the htpasswd file into memory."""
101+ lines = open(self.filename, 'r').readlines()
102+ self.entries = []
103+ for line in lines:
104+ username, pwhash = line.split(':')
105+ entry = [username, pwhash.rstrip()]
106+ self.entries.append(entry)
107+
108+ def save(self):
109+ """Write the htpasswd file to disk"""
110+ open(self.filename, 'w').writelines(["%s:%s\n" % (entry[0], entry[1])
111+ for entry in self.entries])
112+
113+ def update(self, username, password):
114+ """Replace the entry for the given user, or add it if new."""
115+ pwhash = crypt.crypt(password, salt())
116+ matching_entries = [entry for entry in self.entries
117+ if entry[0] == username]
118+ if matching_entries:
119+ matching_entries[0][1] = pwhash
120+ else:
121+ self.entries.append([username, pwhash])
122+
123+ def delete(self, username):
124+ """Remove the entry for the given user."""
125+ self.entries = [entry for entry in self.entries
126+ if entry[0] != username]
127+
128+
129+def main():
130+ """%prog [-c] -b filename username password
131+ Create or update an htpasswd file"""
132+ # For now, we only care about the use cases that affect tests/functional.py
133+ parser = OptionParser(usage=main.__doc__)
134+ parser.add_option('-b', action='store_true', dest='batch', default=False,
135+ help='Batch mode; password is passed on the command line IN THE '
136+ 'CLEAR.')
137+ parser.add_option('-c', action='store_true', dest='create', default=False,
138+ help='Create a new htpasswd file, overwriting any existing file.')
139+ parser.add_option('-D', action='store_true', dest='delete_user',
140+ default=False, help='Remove the given user from the password file.')
141+
142+ options, args = parser.parse_args()
143+
144+ def syntax_error(msg):
145+ """Utility function for displaying fatal error messages with usage
146+ help.
147+ """
148+ sys.stderr.write("Syntax error: " + msg)
149+ sys.stderr.write(parser.get_usage())
150+ sys.exit(1)
151+
152+ if not options.batch:
153+ syntax_error("Only batch mode is supported\n")
154+
155+ # Non-option arguments
156+ if len(args) < 2:
157+ syntax_error("Insufficient number of arguments.\n")
158+ filename, username = args[:2]
159+ if options.delete_user:
160+ if len(args) != 2:
161+ syntax_error("Incorrect number of arguments.\n")
162+ password = None
163+ else:
164+ if len(args) != 3:
165+ syntax_error("Incorrect number of arguments.\n")
166+ password = args[2]
167+
168+ passwdfile = HtpasswdFile(filename, create=options.create)
169+
170+ if options.delete_user:
171+ passwdfile.delete(username)
172+ else:
173+ passwdfile.update(username, password)
174+
175+ passwdfile.save()
176+
177+
178+if __name__ == '__main__':
179+ main()
180
181=== modified file 'loggerhead/main.py'
182--- loggerhead/main.py 2009-11-27 18:30:24 +0000
183+++ loggerhead/main.py 2010-02-08 20:46:16 +0000
184@@ -24,6 +24,8 @@
185 from bzrlib.plugin import load_plugins
186
187 from paste import httpserver
188+from paste.auth.basic import AuthBasicHandler
189+from paste.auth.digest import AuthDigestHandler
190 from paste.httpexceptions import HTTPExceptionHandler, HTTPInternalServerError
191 from paste.translogger import TransLogger
192
193@@ -31,6 +33,7 @@
194 from loggerhead.apps.transport import (
195 BranchesFromTransportRoot, UserBranchesFromTransportRoot)
196 from loggerhead.config import LoggerheadConfig
197+from loggerhead import htpasswd
198 from loggerhead.util import Reloader
199 from loggerhead.apps.error import ErrorHandlerApp
200
201@@ -151,6 +154,39 @@
202 else:
203 host = config.get_option('user_host')
204
205+ if config.get_option('auth_realm'):
206+ realm = config.get_option('auth_realm')
207+ if config.get_option('digest_auth'):
208+ path = config.get_option('digest_auth')
209+ users = {}
210+ # parse htdigest file
211+ for line in open(path):
212+ # format: $user:$realm:$hash
213+ htuser, htrealm, hthash = line.strip().split(':', 2)
214+ if htrealm not in users:
215+ users[htrealm] = {}
216+ users[htrealm][htuser] = hthash
217+
218+ def d_handler(environ, hrealm, huser):
219+ return users.get(hrealm, {}).get(huser, False)
220+ app = AuthDigestHandler(app, realm, d_handler)
221+ elif config.get_option('basic_auth'):
222+ path = config.get_option('basic_auth')
223+ htp = htpasswd.HtpasswdFile(path)
224+
225+ def b_handler(environ, username, password):
226+ result = [hthash for htuser, hthash in htp.entries
227+ if htuser == username]
228+ if len(result) == 0:
229+ return False
230+ hthash = result[0]
231+ return htpasswd.crypt.crypt(password, hthash[:2]) == hthash
232+ app = AuthBasicHandler(app, realm, b_handler)
233+ elif config.get_option('basic_auth') or config.get_option('digest_auth'):
234+ print 'Both the --basic-auth and --digest-auth flags require that', \
235+ 'the --auth-realm flag is set.'
236+ sys.exit(1)
237+
238 if not config.get_option('protocol'):
239 protocol = 'http'
240 else:

Subscribers

People subscribed via source and target branches