Hi Eric,

Thanks for the detailed analysis. This is very helpful for our plans.
I'll address the issues inline.

On Thu, Nov 18, 2010 at 3:43 PM, Eric Johnson
<ericjohn...@alumni.brown.edu> wrote:
> *Principle*: Use corporate authentication mechanism.
> We don't want people to have to remember a gazillion passwords.
> /How does RB currently do?/  Decently.  I couldn't figure out how to get the
> "ActiveDirectory" approach to work, so I fell back to standard LDAP, which
> eventually worked - once I looked at the code to figure out what the
> settings were used for.  However, the name of the setting "Anonymous User
> Mask" didn't match what I eventually got to work.  Our company doesn't allow
> for anonymous connections to ActiveDirectory, so we need to have a "Lookup"
> user and password, but that's not what these fields are named.
> What would help make this even better?  Better field labels.  Sample
> configurations, diagnostic tips.  Even pointers to the code so that people
> can just go look to see what it is doing.

This is definitely an area that could be improved. I will admit, I
personally do not have a lot of LDAP experience. The names chosen were
based on the configuration fields provided when we were provided with
the initial LDAP module, but we can absolutely change them. I welcome
recommendations there to make things clearer, on both label names and
any help text.

> *Principle*: "Respect" access control permissions on the underlying version
> control system.
> We're setting this up against a Subversion server hosting multiple
> repositories.  Each repository has a different list of users that have
> permission to access the repo.  Mapping this to reviewboard, we do not want
> just anyone to be able to see patches against these repositories - only the
> people who already have access to the underlying Subversion server.
> To make this work with Subversion, we're grabbing access information from
> the Subversion server, and automatically updating reviewboard projects with
> the usernames allowed.
> /How does RB currently do/: So-So.  For the moment this amounts to a Custom
> Authentication method.  What I've currently done is simply hack the existing
> LDAP authentication method.  For us, this is both a custom authentication &
> authorization mechanism, at least insofar as authentication fails for people
> who should not have access.

One of the tasks currently underway for 1.6 is to allow objects to
return whether or not they're accessible by a given user. At the
moment, this is based on the per-group and per-repository permissions
set for the given user (another new feature in 1.6), but the intent is
to allow this to be extended. I had planned to allow them to be
extended through our upcoming extension mechanism, but perhaps we can
also ask the auth module.

ACLs for Subversion are something I would like to support in some
fashion, so I'm keeping that in mind.

A couple other things on my mind (hopefully for 1.6, maybe 1.7) is the
ability to more easily configure authentication modules. Much like
SCMTools, I would like third parties to be able to create an auth
module and have Review Board auto-discover it. Part of this would be
to move the configuration aspects into the module, which would allow
Review Board to ask the third-party module for the form data to
display during configuration. I imagine this would help a little in
your case (or would have if you were writing from scratch).

One thing we may want to do is have a new layer, something separate
from both extensions and the authentication module, which would be
used for specifying whether a user has permissions to some thing (a
repository, repository + path, review request, etc.) and go through
that. Then, you could keep using the standard LDAP module, and instead
provide one of these that could be configured through the admin UI.
This would be the thing that checks with Subversion to determine
whether they can see a particular file. I'll think about how best to
do this.

> Better docs on how to set up a custom authentication mechanism would be
> great.  This may not be much more than a pointer to the existing
> implementations, pointers to the Django docs, and pointers on how to update
> ReviewBoard configuration so that new auth mechanisms can be used.

I do want to write developer docs on writing custom auth modules and
SCMTools. Hopefully for 1.6. There's a lot to juggle.

> I've set up an automated process whereby user status (active/inactive)
> (staff/not staff) is updated by a cron job directly writing to the auth_user
> table in the database.  This works.  From what I know, I haven't figured out
> a better way to accomplish the same thing.  If my approach really is the
> best way, it would be nice to document, and if it isn't, I'd love to see
> docs on the best way.

What triggers the change? Is this just asking Subversion and updating
the data, in order to keep RB in sync with your Subversion ACL?

> With 1.5, as per previous discussion, I am going to set up a separate review
> board instance for each repository, as each repository has a separate access
> list.  Note that, even with the changes you've talked about for 1.6, I'm not
> sure we'd change this approach, as we're concerned about both leakage and
> interference.  Not even having the same underlying MySQL database is
> actually useful.
> One approach that might help here would be to follow something like the
> MoinMoin approach - a "farm" of RB instances, wherein I only have to
> configure Apache once, but I still get multiple instances of RB, all
> independent of each other.  I don't understand Django enough to see if this
> kind of approach is easy/possible with their framework.

This is sort of the plan for the new Local Site mechanism. You can
have one install that has several sites managed from one UI.

Using Django's new multi-database and "database router" support, we
may be able to allow storing each site into a separate database. I
still need to investigate this. It's something we need as well.

> *Principle*: Highly restricted access to the operating server.
> Only three people have access to remotely log into the server.  Since the
> server stores credentials that access other systems, access to the system
> must be severely limited.  Anything that even might potentially compromise
> the underlying system must not be exposed to others.
> /How does RB currently do?/ Unfortunately, it fails - although your tips
> below helped me fix it so that it works the way I need, with a minimal
> amount of code patching.  So improvement here looks like it might be easy.

> Settings tab:
> For some reason, the "siteconfig" permission, which restricts access to this
> information under the database tab, has no effect on whether you can see the
> settings if you're "Staff".  Specifically, under settings:

All the settings stuff will be under control for 1.6. You'll be able
to blanket allow/deny, or give them access to certain settings pages.

> Diff viewer - OK - but oddly these settings feel like they should be
> per-user.

It affects what gets cached, and making these per-user has the
potential to greatly increase cache needs. These are more advanced
settings that most places don't need, but some do. I'd actually like
to get rid of some of them.

> *Principle*: Delegated administration.
> With a large number of repositories, the small number of people who have
> access cannot possibly manage the details of "review" configuration.
>  Setting up of groups & default reviewers must be something we can delegate
> to a large number of people who closer to the specific projects, without
> risking compromise to the server as a whole.
> /How does ReviewBoard currently do?/ OK. Using the "staff" capability,
> currently available permissions, and a separate instance per underlying code
> repository as described above, this gets me exactly where I want to be.  To
> be fair, if you don't count the issues from above around restricting key
> things to just superuser, then ReviewBoard actually does quite well here.
> One minor complaint.  The distinction between "groups" and "review groups"
> is initially quite confusing.  Minimally changing the label to
> "authorization groups" would help.

This is a Django thing. On Django, there are Groups (authentication
groups). I don't know if we can change the name in the Admin UI or
not. Worth looking into.

> Also, it appears that if you don't set any permissions, you get them all.
>  Seems backwards.  Maybe I'm mistaken on this point, but that's what I
> recall from my poking around.

Also a Django thing. This one we have no say in.

> Insofar as RB currently has a lot of separate permissions fields, those are
> probably useful only to the extent that I provide the right capabilities to
> the staff & to the regular users.

These all come from Django. We can't really do anything about them, as
far as which are provided. We mostly ignore them outside the Admin UI,
since they're really intended for the Admin UI itself.

> Well, hopefully the above input is useful.  I'd be glad to answer any
> follow-up questions.

It was very helpful. Thanks!


Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com

Want to help the Review Board project? Donate today at 
Happy user? Let us know at http://www.reviewboard.org/users/
To unsubscribe from this group, send email to 
For more options, visit this group at 

Reply via email to