POC for perforce integration (very ugly code)

Review Request #3247 — Created July 23, 2012 and discarded


Review Board



This codes needs to be rewritten VERY WELL but I thought I would first publish the ugly one so that I know you guys can or ever will accept this. If not, I need to create custom reviewboard version (based on what your help pages say) which I am NOT AT ALL looking forward to :(

The idea is to give option to the user in repository form, at least for perforce, to use either logged in user's credentials OR one centralized account. If you guys are going to be okay with this, I will rewrite the code based on your suggestions\comments and get this in 1.7 or whichever next release for this code is.
ONLY test - logged in using user's individual credentials and verified they were working 
  1. Can you clarify the motivation for this?
    1. David, thanks for the comment. Organization I work for is not ready to have one centralized account that can access the whole repository. They want logged in user's credentials used while authenticating to perforce. I thought that giving an option to user should be okay. I imagine this makes sense.
    2. It may not make as much sense as you'd hope.
      We wouldn't log in per-user. The way it works is that we have to fetch some data from the repository the first time a diff is viewed, and then that data is cached. The user who visits the page triggering that may be the review request owner, a reviewer, or just someone else on the network hitting that page. We need to have that data regardless of who's viewing it. So there needs to be a pre-defined user who can access that repository.
      Something that some companies do is to have a dedicated read-only user only usable by Review Board that can access those files. Some smaller companies just use the credentials of a developer at the company.
      If your concern is users viewing diffs that they're not supposed to view, then we have support for that by way of making repositories private and defining a whitelist of users who can access it.
    3. Christian, thanks for the comment. I don't have any concern using centralized account; it's the organization I work for. They already have Crucible for post-checkin reviews and I am trying to bring in reviewboard for pre-checkin reviews. All is well except for the one user being able to read the whole repository part. Ideally, if I give reviewboard vm to network admins and ask them to manage it instead of I, everything should be fine but only it isn't (again because of whatever reasoning they have). I have not looked at the first time data you mentioned about but I do want to. Does this mean if we have one account that is used ONLY for fetching that data but had per user log in for reviewing the diff it is okay (I am only trying to make sense of this all)? Do we have any other reasons for not giving the option to the user to use per user log-in. Again, only checking the possibility; I like to think that if user really wanted to we could and should give them that option if we are not losing anything in the process. But of course, I must be missing something because this may have come up earlier and was rejected because of some other reason as well?
    4. ping!
    5. Sorry, been busy with release prep.
      Per-user login doesn't make any sense for a variety of reasons. Things aren't necessarily generated when that user visits the page, meaning you can't ask for login credentials. Things also stick around in cache, so you wouldn't really be fetching new data when asking for credentials, just confirming they have access. It's sort of messy. When do you ask? When viewing the diff, probably. What about when you need to get the pieces of a diff for the comments on the review page? What about when generating e-mails?
      Also keep in mind that just because you've protected repository access doesn't mean users aren't going to see valuable information in the discussions of a review request. So really, you didn't protect everything.
      The only thing that makes sense, really, is to use the setup we currently support, which is to have one official account associated with the repository, and to make that repository accessible only by certain users or groups. That way, you can restrict access to only those users who should have any ability to log in to that repository, but you wouldn't have to ask for credentials. Other users couldn't access the files, or view diffs, or even view the review requests.
Review request changed

Status: Discarded