• 
      

    Fix validating and trusting Perforce repositories with Mirror Paths.

    Review Request #11007 — Created May 4, 2020 and submitted

    Information

    Review Board
    release-3.0.x

    Reviewers

    Perforce repositories work differently from all other types of
    repositories. Instead of all communication happening over the Path, it
    happens over the Mirror Path. This is done for very ancient historical
    reasons, centered around a very early need for tools like Review Board
    using a read-only Perforce mirror that was separate from the main
    read-write repository that developers had been using. This inversed
    logic is sadly probably here to stay long-term.

    The problem is, the repository form didn't know about Perforce's needs
    here, and was checking only the Path field, not Mirror Path, when
    validating a repository. This was generally fine for most usage, but if
    using an SSL-backed repository in Mirror Path that had to be explicitly
    trusted, Review Board would never have an opportunity to validate this
    and present it to the user. The only way it'd work is to have the SSL
    repository in Path, and a non-SSL in Mirror Path, at which point the
    communication wouldn't be encrypted.

    This change fixes this up by adding a new SCMTool.prefers_mirror_path
    flag, which instructs the form to check using the Mirror Path if set.
    Only Perforce has this enabled, and the documentation discourages others
    from ever setting it.

    In time, we can clean some of this up at a presentational level by
    creating a custom subform and making it clear which is used for which.
    This change does take a step toward this by adding new help text for
    Mirror Path that makes it clear it takes precedence for communication.

    Unit tests pass.

    Manually tested adding a local repository and verifying the paths it
    was using.

    Summary ID
    Fix validating and trusting Perforce repositories with Mirror Paths.
    Perforce repositories work differently from all other types of repositories. Instead of all communication happening over the Path, it happens over the Mirror Path. This is done for very ancient historical reasons, centered around a very early need for tools like Review Board using a read-only Perforce mirror that was separate from the main read-write repository that developers had been using. This inversed logic is sadly probably here to stay long-term. The problem is, the repository form didn't know about Perforce's needs here, and was checking only the Path field, not Mirror Path, when validating a repository. This was generally fine for most usage, but if using an SSL-backed repository in Mirror Path that had to be explicitly trusted, Review Board would never have an opportunity to validate this and present it to the user. The only way it'd work is to have the SSL repository in Path, and a non-SSL in Mirror Path, at which point the communication wouldn't be encrypted. This change fixes this up by adding a new `SCMTool.prefers_mirror_path` flag, which instructs the form to check using the Mirror Path if set. Only Perforce has this enabled, and the documentation discourages others from ever setting it. In time, we can clean some of this up at a presentational level by creating a custom subform and making it clear which is used for which. This change does take a step toward this by adding new help text for Mirror Path that makes it clear it takes precedence for communication.
    cec4eea031988bc5d4e4452ac32e1a5a1f40c136
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-3.0.x (74b3a44)