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 -- 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 http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~----------~----~----~----~------~----~------~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en