On Friday 30 January 2009 16:41:17 Christian Hammond wrote:
> Hi Alexey.
> We have a patch up that I haven't reviewed yet (I'd like other
> LDAP-knowledgeable users to look into it if possible) that may address your
> concerns. Would you be able to give it a try?
Well, I would propose another way to solve this problem:
As the review description says this solution still allows to use complex LDAP
hierarchies. Suppose one has a big organization where all users divided into
different departments or "organization units" (ou's). So for Joe his "login"
string (I'm not sure how this entity is properly called in LDAP)
is 'uid=joe,ou=research_development,ou=users,dc=company,dc=com" while for
Jane her login looks
Searching prior to binding allow to determine an exact login string for a
given "shortcut" name.
Also I would like to propose another fix
When I first switched to LDAP based auth I discovered that I can't login
as 'admin' anymore. Short investigation showed that the problem is in
exception handling. django.contrib.auth.authenticate() (as of 1.0.2) catches
only TypeErrors from a backend authenticate() so if another error occures
(IndexError in my case) it simply stops processing of an authentication chain
(in case of LDAP in reviewboard this chain is 'LDAP, builtin'). Probably the
similar appoach should be applied to all auth backeds too.
While submitting these two patches I also stepped into few other issues :)
First I couldn't type certain letters (e.g. 'a' and 'A') into Summary and
Description fields when diffviewer is active (e.g. on
http://reviews.review-board.org/r/729/diff/ ) Probably these keys are
intercepted as hotkeys by diffviewer component. I use Firefox-3.0.4 on Linux.
Also I found that submitting a patch is somewhat hard and cumbersome . The
problem is how a patch is parsed upon upload. Since reviewboard repo type is
SVN the patch processing machinery assumes that the patch is made by svn (or
svntool), so it has additional fields in patch header, particularly file
revision. This obviously isn't always true, suppose the patch is made against
an installed file or it was made in another SCM (git in my case). I had to
look through reviewboard code to understand what it expects to see in the
patch and modify the patches accordingly.
Maybe it's worth to re-arrange the whole process. I would propose smth like
when a submitter uploads a patch he or she is presented with an interface
which allows to choose a place (either a branch/tag or a single revision)
where the patch should be applied. Since reviewboard knows exactly how to
exctract this information from the repository this shouldn't be too hard to
implement. Another option is to upload a modified file (or files) instead of
a patch. Probably this interface should have SCM-specific parts (or
callbacks) (yes I dream of git commits-based reviews :))
Unfortunately reviewboard is the first django app I deal with so I can't
implement it myself, at least right now.
You received this message because you are subscribed to the Google Groups
To post to this group, send email to email@example.com
To unsubscribe from this group, send email to
For more options, visit this group at