Convert forge author, committer, server to PermissionBackend

Change-Id: I081392237957d3dc4afc703ca2256e27954d3774

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java

@@ -53,6 +53,7 @@ import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
+import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectControl;
@@ -90,6 +91,7 @@ public class ChangeInserter implements InsertChangeOp {
 
   private static final Logger log = LoggerFactory.getLogger(ChangeInserter.class);
 
+  private final PermissionBackend permissionBackend;
   private final ProjectControl.GenericFactory projectControlFactory;
   private final IdentifiedUser.GenericFactory userFactory;
   private final ChangeControl.GenericFactory changeControlFactory;
@@ -138,6 +140,7 @@ public class ChangeInserter implements InsertChangeOp {
 
   @Inject
   ChangeInserter(
+      PermissionBackend permissionBackend,
       ProjectControl.GenericFactory projectControlFactory,
       IdentifiedUser.GenericFactory userFactory,
       ChangeControl.GenericFactory changeControlFactory,
@@ -154,6 +157,7 @@ public class ChangeInserter implements InsertChangeOp {
       @Assisted Change.Id changeId,
       @Assisted ObjectId commitId,
       @Assisted String refName) {
+    this.permissionBackend = permissionBackend;
     this.projectControlFactory = projectControlFactory;
     this.userFactory = userFactory;
     this.changeControlFactory = changeControlFactory;
@@ -543,6 +547,8 @@ public class ChangeInserter implements InsertChangeOp {
       return;
     }
 
+    PermissionBackend.ForRef perm =
+        permissionBackend.user(ctx.getUser()).project(ctx.getProject()).ref(refName);
     try {
       RefControl refControl =
           projectControlFactory.controlFor(ctx.getProject(), ctx.getUser()).controlForRef(refName);
@@ -555,7 +561,7 @@ public class ChangeInserter implements InsertChangeOp {
               commitId,
               ctx.getIdentifiedUser())) {
         commitValidatorsFactory
-            .forGerritCommits(refControl, new NoSshInfo(), ctx.getRevWalk())
+            .forGerritCommits(perm, refControl, new NoSshInfo(), ctx.getRevWalk())
             .validate(event);
       }
     } catch (CommitValidationException e) {

gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java

@@ -320,6 +320,9 @@ public class PatchSetInserter implements BatchUpdateOp {
       return;
     }
 
+    PermissionBackend.ForRef perm =
+        permissionBackend.user(ctx.getUser()).ref(origCtl.getChange().getDest());
+
     String refName = getPatchSetId().toRefName();
     try (CommitReceivedEvent event =
         new CommitReceivedEvent(
@@ -333,7 +336,7 @@ public class PatchSetInserter implements BatchUpdateOp {
             commitId,
             ctx.getIdentifiedUser())) {
       commitValidatorsFactory
-          .forGerritCommits(origCtl.getRefControl(), new NoSshInfo(), ctx.getRevWalk())
+          .forGerritCommits(perm, origCtl.getRefControl(), new NoSshInfo(), ctx.getRevWalk())
           .validate(event);
     } catch (CommitValidationException e) {
       throw new ResourceConflictException(e.getFullMessage());

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -112,7 +112,6 @@ import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.permissions.RefPermission;
-import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectControl;
@@ -1210,6 +1209,7 @@ public class ReceiveCommits {
     private final boolean defaultPublishComments;
     Branch.NameKey dest;
     RefControl ctl;
+    PermissionBackend.ForRef perm;
     Set<Account.Id> reviewer = Sets.newLinkedHashSet();
     Set<Account.Id> cc = Sets.newLinkedHashSet();
     Map<String, Short> labels = new HashMap<>();
@@ -1496,6 +1496,7 @@ public class ReceiveCommits {
 
     magicBranch.dest = new Branch.NameKey(project.getNameKey(), ref);
     magicBranch.ctl = projectControl.controlForRef(ref);
+    magicBranch.perm = permissions.ref(ref);
     if (projectControl.getProject().getState()
         != com.google.gerrit.extensions.client.ProjectState.ACTIVE) {
       reject(cmd, "project is read only");
@@ -1837,7 +1838,7 @@ public class ReceiveCommits {
           logDebug("Creating new change for {} even though it is already tracked", name);
         }
 
-        if (!validCommit(rp.getRevWalk(), magicBranch.ctl, magicBranch.cmd, c)) {
+        if (!validCommit(rp.getRevWalk(), magicBranch.perm, magicBranch.ctl, magicBranch.cmd, c)) {
           // Not a change the user can propose? Abort as early as possible.
           newChanges = Collections.emptyList();
           logDebug("Aborting early due to invalid commit");
@@ -2415,8 +2416,9 @@ public class ReceiveCommits {
         }
       }
 
-      ChangeControl changeCtl = projectControl.controlFor(notes);
+      PermissionBackend.ForRef perm = permissions.ref(change.getDest().get());
-      if (!validCommit(rp.getRevWalk(), changeCtl.getRefControl(), inputCommand, newCommit)) {
+      RefControl refctl = projectControl.controlForRef(change.getDest());
+      if (!validCommit(rp.getRevWalk(), perm, refctl, inputCommand, newCommit)) {
         return false;
       }
       rp.getRevWalk().parseBody(priorCommit);
@@ -2714,12 +2716,13 @@ public class ReceiveCommits {
 
   private void validateNewCommits(RefControl ctl, ReceiveCommand cmd)
       throws PermissionBackendException {
+    PermissionBackend.ForRef perm = permissions.ref(ctl.getRefName());
     if (!RefNames.REFS_CONFIG.equals(cmd.getRefName())
         && !(MagicBranch.isMagicBranch(cmd.getRefName())
             || NEW_PATCHSET.matcher(cmd.getRefName()).matches())
         && pushOptions.containsKey(BYPASS_REVIEW)) {
       try {
-        permissions.ref(cmd.getRefName()).check(RefPermission.BYPASS_REVIEW);
+        perm.check(RefPermission.BYPASS_REVIEW);
         if (!Iterables.isEmpty(rejectCommits)) {
           throw new AuthException("reject-commits prevents " + BYPASS_REVIEW);
         }
@@ -2747,7 +2750,7 @@ public class ReceiveCommits {
         i++;
         if (existing.keySet().contains(c)) {
           continue;
-        } else if (!validCommit(walk, ctl, cmd, c)) {
+        } else if (!validCommit(walk, perm, ctl, cmd, c)) {
           break;
         }
 
@@ -2773,7 +2776,8 @@ public class ReceiveCommits {
     }
   }
 
-  private boolean validCommit(RevWalk rw, RefControl ctl, ReceiveCommand cmd, ObjectId id)
+  private boolean validCommit(
+      RevWalk rw, PermissionBackend.ForRef perm, RefControl ctl, ReceiveCommand cmd, ObjectId id)
       throws IOException {
 
     if (validCommits.contains(id)) {
@@ -2791,8 +2795,8 @@ public class ReceiveCommits {
               && magicBranch.merged;
       CommitValidators validators =
           isMerged
-              ? commitValidatorsFactory.forMergedCommits(ctl)
+              ? commitValidatorsFactory.forMergedCommits(perm, ctl)
-              : commitValidatorsFactory.forReceiveCommits(ctl, sshInfo, repo, rw);
+              : commitValidatorsFactory.forReceiveCommits(perm, ctl, sshInfo, repo, rw);
       messages.addAll(validators.validate(receiveEvent));
     } catch (CommitValidationException e) {
       logDebug("Commit validation failed on {}", c.name());

gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidationException.java

@@ -22,6 +22,11 @@ public class CommitValidationException extends ValidationException {
   private static final long serialVersionUID = 1L;
   private final ImmutableList<CommitValidationMessage> messages;
 
+  public CommitValidationException(String reason, CommitValidationMessage message) {
+    super(reason);
+    this.messages = ImmutableList.of(message);
+  }
+
   public CommitValidationException(String reason, List<CommitValidationMessage> messages) {
     super(reason);
     this.messages = ImmutableList.copyOf(messages);

gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java

@@ -26,6 +26,7 @@ import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.PageLinks;
 import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
 import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.GerritPersonIdent;
@@ -39,7 +40,11 @@ import com.google.gerrit.server.events.CommitReceivedEvent;
 import com.google.gerrit.server.git.BanCommit;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.git.ValidationError;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.RefPermission;
 import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.project.RefControl;
 import com.google.gerrit.server.ssh.SshInfo;
 import com.google.gerrit.server.util.MagicBranch;
@@ -96,38 +101,45 @@ public class CommitValidators {
     }
 
     public CommitValidators forReceiveCommits(
-        RefControl refControl, SshInfo sshInfo, Repository repo, RevWalk rw) throws IOException {
+        PermissionBackend.ForRef perm,
+        RefControl refctl,
+        SshInfo sshInfo,
+        Repository repo,
+        RevWalk rw)
+        throws IOException {
       NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(repo, rw);
+      IdentifiedUser user = refctl.getUser().asIdentifiedUser();
       return new CommitValidators(
           ImmutableList.of(
-              new UploadMergesPermissionValidator(refControl),
+              new UploadMergesPermissionValidator(refctl),
-              new AmendedGerritMergeCommitValidationListener(refControl, gerritIdent),
+              new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
-              new AuthorUploaderValidator(refControl, canonicalWebUrl),
+              new AuthorUploaderValidator(user, perm, canonicalWebUrl),
-              new CommitterUploaderValidator(refControl, canonicalWebUrl),
+              new CommitterUploaderValidator(user, perm, canonicalWebUrl),
-              new SignedOffByValidator(refControl),
+              new SignedOffByValidator(user, perm, refctl.getProjectControl().getProjectState()),
-              new ChangeIdValidator(
+              new ChangeIdValidator(refctl, canonicalWebUrl, installCommitMsgHookCommand, sshInfo),
-                  refControl, canonicalWebUrl, installCommitMsgHookCommand, sshInfo),
+              new ConfigValidator(refctl, rw, allUsers),
-              new ConfigValidator(refControl, rw, allUsers),
               new BannedCommitsValidator(rejectCommits),
               new PluginCommitValidationListener(pluginValidators),
               new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker)));
     }
 
-    public CommitValidators forGerritCommits(RefControl refControl, SshInfo sshInfo, RevWalk rw) {
+    public CommitValidators forGerritCommits(
+        PermissionBackend.ForRef perm, RefControl refctl, SshInfo sshInfo, RevWalk rw) {
+      IdentifiedUser user = refctl.getUser().asIdentifiedUser();
       return new CommitValidators(
           ImmutableList.of(
-              new UploadMergesPermissionValidator(refControl),
+              new UploadMergesPermissionValidator(refctl),
-              new AmendedGerritMergeCommitValidationListener(refControl, gerritIdent),
+              new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
-              new AuthorUploaderValidator(refControl, canonicalWebUrl),
+              new AuthorUploaderValidator(user, perm, canonicalWebUrl),
-              new SignedOffByValidator(refControl),
+              new SignedOffByValidator(user, perm, refctl.getProjectControl().getProjectState()),
-              new ChangeIdValidator(
+              new ChangeIdValidator(refctl, canonicalWebUrl, installCommitMsgHookCommand, sshInfo),
-                  refControl, canonicalWebUrl, installCommitMsgHookCommand, sshInfo),
+              new ConfigValidator(refctl, rw, allUsers),
-              new ConfigValidator(refControl, rw, allUsers),
               new PluginCommitValidationListener(pluginValidators),
               new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker)));
     }
 
-    public CommitValidators forMergedCommits(RefControl refControl) {
+    public CommitValidators forMergedCommits(PermissionBackend.ForRef perm, RefControl refControl) {
+      IdentifiedUser user = refControl.getUser().asIdentifiedUser();
       // Generally only include validators that are based on permissions of the
       // user creating a change for a merged commit; generally exclude
       // validators that would require amending the change in order to correct.
@@ -144,8 +156,8 @@ public class CommitValidators {
       return new CommitValidators(
           ImmutableList.of(
               new UploadMergesPermissionValidator(refControl),
-              new AuthorUploaderValidator(refControl, canonicalWebUrl),
+              new AuthorUploaderValidator(user, perm, canonicalWebUrl),
-              new CommitterUploaderValidator(refControl, canonicalWebUrl)));
+              new CommitterUploaderValidator(user, perm, canonicalWebUrl)));
     }
   }
 
@@ -441,37 +453,50 @@ public class CommitValidators {
   }
 
   public static class SignedOffByValidator implements CommitValidationListener {
-    private final RefControl refControl;
+    private final IdentifiedUser user;
+    private final PermissionBackend.ForRef perm;
+    private final ProjectState state;
 
-    public SignedOffByValidator(RefControl refControl) {
+    public SignedOffByValidator(
-      this.refControl = refControl;
+        IdentifiedUser user, PermissionBackend.ForRef perm, ProjectState state) {
+      this.user = user;
+      this.perm = perm;
+      this.state = state;
     }
 
     @Override
     public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
         throws CommitValidationException {
-      IdentifiedUser currentUser = refControl.getUser().asIdentifiedUser();
+      if (!state.isUseSignedOffBy()) {
-      final PersonIdent committer = receiveEvent.commit.getCommitterIdent();
+        return Collections.emptyList();
-      final PersonIdent author = receiveEvent.commit.getAuthorIdent();
+      }
-      final ProjectControl projectControl = refControl.getProjectControl();
 
-      if (projectControl.getProjectState().isUseSignedOffBy()) {
+      RevCommit commit = receiveEvent.commit;
-        boolean sboAuthor = false;
+      PersonIdent committer = commit.getCommitterIdent();
-        boolean sboCommitter = false;
+      PersonIdent author = commit.getAuthorIdent();
-        boolean sboMe = false;
+
-        for (final FooterLine footer : receiveEvent.commit.getFooterLines()) {
+      boolean sboAuthor = false;
-          if (footer.matches(FooterKey.SIGNED_OFF_BY)) {
+      boolean sboCommitter = false;
-            final String e = footer.getEmailAddress();
+      boolean sboMe = false;
-            if (e != null) {
+      for (FooterLine footer : commit.getFooterLines()) {
-              sboAuthor |= author.getEmailAddress().equals(e);
+        if (footer.matches(FooterKey.SIGNED_OFF_BY)) {
-              sboCommitter |= committer.getEmailAddress().equals(e);
+          String e = footer.getEmailAddress();
-              sboMe |= currentUser.hasEmailAddress(e);
+          if (e != null) {
-            }
+            sboAuthor |= author.getEmailAddress().equals(e);
+            sboCommitter |= committer.getEmailAddress().equals(e);
+            sboMe |= user.hasEmailAddress(e);
           }
         }
-        if (!sboAuthor && !sboCommitter && !sboMe && !refControl.canForgeCommitter()) {
+      }
+      if (!sboAuthor && !sboCommitter && !sboMe) {
+        try {
+          perm.check(RefPermission.FORGE_COMMITTER);
+        } catch (AuthException denied) {
           throw new CommitValidationException(
               "not Signed-off-by author/committer/uploader in commit message footer");
+        } catch (PermissionBackendException e) {
+          log.error("cannot check FORGE_COMMITTER", e);
+          throw new CommitValidationException("internal auth error");
         }
       }
       return Collections.emptyList();
@@ -480,56 +505,69 @@ public class CommitValidators {
 
   /** Require that author matches the uploader. */
   public static class AuthorUploaderValidator implements CommitValidationListener {
-    private final RefControl refControl;
+    private final IdentifiedUser user;
+    private final PermissionBackend.ForRef perm;
     private final String canonicalWebUrl;
 
-    public AuthorUploaderValidator(RefControl refControl, String canonicalWebUrl) {
+    public AuthorUploaderValidator(
-      this.refControl = refControl;
+        IdentifiedUser user, PermissionBackend.ForRef perm, String canonicalWebUrl) {
+      this.user = user;
+      this.perm = perm;
       this.canonicalWebUrl = canonicalWebUrl;
     }
 
     @Override
     public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
         throws CommitValidationException {
-      IdentifiedUser currentUser = refControl.getUser().asIdentifiedUser();
+      PersonIdent author = receiveEvent.commit.getAuthorIdent();
-      final PersonIdent author = receiveEvent.commit.getAuthorIdent();
+      if (user.hasEmailAddress(author.getEmailAddress())) {
-
+        return Collections.emptyList();
-      if (!currentUser.hasEmailAddress(author.getEmailAddress()) && !refControl.canForgeAuthor()) {
+      }
-        List<CommitValidationMessage> messages = new ArrayList<>();
+      try {
-
+        perm.check(RefPermission.FORGE_AUTHOR);
-        messages.add(
+        return Collections.emptyList();
-            getInvalidEmailError(
+      } catch (AuthException e) {
-                receiveEvent.commit, "author", author, currentUser, canonicalWebUrl));
+        throw new CommitValidationException(
-        throw new CommitValidationException("invalid author", messages);
+            "invalid author",
+            invalidEmail(receiveEvent.commit, "author", author, user, canonicalWebUrl));
+      } catch (PermissionBackendException e) {
+        log.error("cannot check FORGE_AUTHOR", e);
+        throw new CommitValidationException("internal auth error");
       }
-      return Collections.emptyList();
     }
   }
 
   /** Require that committer matches the uploader. */
   public static class CommitterUploaderValidator implements CommitValidationListener {
-    private final RefControl refControl;
+    private final IdentifiedUser user;
+    private final PermissionBackend.ForRef perm;
     private final String canonicalWebUrl;
 
-    public CommitterUploaderValidator(RefControl refControl, String canonicalWebUrl) {
+    public CommitterUploaderValidator(
-      this.refControl = refControl;
+        IdentifiedUser user, PermissionBackend.ForRef perm, String canonicalWebUrl) {
+      this.user = user;
+      this.perm = perm;
       this.canonicalWebUrl = canonicalWebUrl;
     }
 
     @Override
     public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
         throws CommitValidationException {
-      IdentifiedUser currentUser = refControl.getUser().asIdentifiedUser();
+      PersonIdent committer = receiveEvent.commit.getCommitterIdent();
-      final PersonIdent committer = receiveEvent.commit.getCommitterIdent();
+      if (user.hasEmailAddress(committer.getEmailAddress())) {
-      if (!currentUser.hasEmailAddress(committer.getEmailAddress())
+        return Collections.emptyList();
-          && !refControl.canForgeCommitter()) {
+      }
-        List<CommitValidationMessage> messages = new ArrayList<>();
+      try {
-        messages.add(
+        perm.check(RefPermission.FORGE_COMMITTER);
-            getInvalidEmailError(
+        return Collections.emptyList();
-                receiveEvent.commit, "committer", committer, currentUser, canonicalWebUrl));
+      } catch (AuthException e) {
-        throw new CommitValidationException("invalid committer", messages);
+        throw new CommitValidationException(
+            "invalid committer",
+            invalidEmail(receiveEvent.commit, "committer", committer, user, canonicalWebUrl));
+      } catch (PermissionBackendException e) {
+        log.error("cannot check FORGE_COMMITTER", e);
+        throw new CommitValidationException("internal auth error");
       }
-      return Collections.emptyList();
     }
   }
 
@@ -539,25 +577,30 @@ public class CommitValidators {
    */
   public static class AmendedGerritMergeCommitValidationListener
       implements CommitValidationListener {
+    private final PermissionBackend.ForRef perm;
     private final PersonIdent gerritIdent;
-    private final RefControl refControl;
 
     public AmendedGerritMergeCommitValidationListener(
-        final RefControl refControl, final PersonIdent gerritIdent) {
+        PermissionBackend.ForRef perm, PersonIdent gerritIdent) {
-      this.refControl = refControl;
+      this.perm = perm;
       this.gerritIdent = gerritIdent;
     }
 
     @Override
     public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
         throws CommitValidationException {
-      final PersonIdent author = receiveEvent.commit.getAuthorIdent();
+      PersonIdent author = receiveEvent.commit.getAuthorIdent();
-
       if (receiveEvent.commit.getParentCount() > 1
           && author.getName().equals(gerritIdent.getName())
-          && author.getEmailAddress().equals(gerritIdent.getEmailAddress())
+          && author.getEmailAddress().equals(gerritIdent.getEmailAddress())) {
-          && !refControl.canForgeGerritServerIdentity()) {
+        try {
-        throw new CommitValidationException("do not amend merges not made by you");
+          perm.check(RefPermission.FORGE_SERVER);
+        } catch (AuthException denied) {
+          throw new CommitValidationException("do not amend merges not made by you");
+        } catch (PermissionBackendException e) {
+          log.error("cannot check FORGE_SERVER", e);
+          throw new CommitValidationException("internal auth error");
+        }
       }
       return Collections.emptyList();
     }
@@ -629,7 +672,7 @@ public class CommitValidators {
     }
   }
 
-  private static CommitValidationMessage getInvalidEmailError(
+  private static CommitValidationMessage invalidEmail(
       RevCommit c,
       String type,
       PersonIdent who,

gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java

@@ -391,7 +391,7 @@ public class RefControl {
   }
 
   /** @return true if this user can forge the author line in a commit. */
-  public boolean canForgeAuthor() {
+  private boolean canForgeAuthor() {
     if (canForgeAuthor == null) {
       canForgeAuthor = canPerform(Permission.FORGE_AUTHOR);
     }
@@ -399,7 +399,7 @@ public class RefControl {
   }
 
   /** @return true if this user can forge the committer line in a commit. */
-  public boolean canForgeCommitter() {
+  private boolean canForgeCommitter() {
     if (canForgeCommitter == null) {
       canForgeCommitter = canPerform(Permission.FORGE_COMMITTER);
     }
@@ -407,7 +407,7 @@ public class RefControl {
   }
 
   /** @return true if this user can forge the server on the committer line. */
-  public boolean canForgeGerritServerIdentity() {
+  private boolean canForgeGerritServerIdentity() {
     return canPerform(Permission.FORGE_SERVER);
   }