-
-
-
/trunk/reviewboard/accounts/backends.py (Diff revision 1) I know the others don't do it, but this should inherit from object. The others all should too, eventually. Also, please add a doc block here describing the purpose and use of this backend.
-
/trunk/reviewboard/accounts/backends.py (Diff revision 1) This should make use of the siteconfig stuff. Only grab this value as needed.
-
/trunk/reviewboard/accounts/backends.py (Diff revision 1) Need to wrap this in a try/catch. If the user specifies an invalid regular expression, this will fail.
-
/trunk/reviewboard/accounts/backends.py (Diff revision 1) This should be prefixed with the backend name. Also, no "!" is needed.
-
-
-
-
/trunk/reviewboard/accounts/views.py (Diff revision 1) Can you change this to "can_change_password"?
-
/trunk/reviewboard/accounts/views.py (Diff revision 1) Can collapse this to: if can_change_password and form.cleaned_data['password1']:
-
-
-
/trunk/reviewboard/admin/forms.py (Diff revision 1) "extracted." should align with the beginning of the string on the previous line. Also, "will be extracted," rather than "is to be extracted."
-
/trunk/reviewboard/admin/forms.py (Diff revision 1) "Username Regex" Do we really need to expose this to the user? How often will they need to do something custom? If it's generally in e-mail form or something, I'd rather keep it simple.
-
-
-
-
-
-
/trunk/reviewboard/admin/middleware.py (Diff revision 1) "TODO:" Also, I'd like to bail from this as early as possible. We should check the auth backend immediately and determine whether we should execute any of this code.
-
/trunk/reviewboard/admin/middleware.py (Diff revision 1) I'd prefer getattr(settings, "X509_USERNAME_FIELD", None), so that we don't break if this setting isn't at all in there. Also, this can be combined with the line below.
-
-
Adds ability to use x.509 certificates for authenticating users.
Review Request #938 — Created Aug. 2, 2009 and discarded
Information | |
---|---|
nheijermans | |
Review Board SVN (deprecated) | |
Reviewers | |
reviewboard | |
These changes enable Review Board to use x.509 certificates to be used alongside the builtin username/password authentication backend. This functionality will be useful for deployments in environments with a Public Key Infrastructure set up. I deliberately avoided modifying post-review to support client certificates due to a lack of decent support for them in Python's high-level libraries. Instead, I modified the user preferences view to permit authenticated users to set their Django password. Django's built-in support for multiple authentication backends will then permit them to use the builtin auth backend with post-review.
Tested on my test Review Board server, running Apache/mod_python,mod_ssl/Django 1.1.
NH
Change Summary:
Fixed all issues from Christian's review except for bailing early from the process_request() function in the middleware.
Diff: |
Revision 2 (+162 -10) |
---|
TH
-
-
/trunk/reviewboard/admin/forms.py (Diff revision 2) Just a minor remark, but I think the correct spelling is "X.509", not "x.509"
-
-
-
/trunk/reviewboard/accounts/backends.py (Diff revision 2) Same here. No need for str(e), though, as the "%s" covers that.
-
-
/trunk/reviewboard/accounts/views.py (Diff revision 2) We should store form.cleaned_data['password1'] in a variable and then check that variable here and use it in the sha1 code below.
-
-
-
-
-
-
/trunk/reviewboard/admin/middleware.py (Diff revision 2) str(None) isn't going to work so well... We should do: x509_settings_field = getattr(settings, "X509_USERNAME_FIELD") if x509_settings_field: x509_field = env.get(x509_settings_field) ....
NH
Change Summary:
This change adds the implementation of the code review suggestions from Thilo and Christian.
Diff: |
Revision 3 (+164 -10) |
---|