From aa8c1ddeface5f39b4eaf06f774e0cd20ee6e004 Mon Sep 17 00:00:00 2001
From: Patrick Hiesel <hiesel@google.com>
Date: Thu, 10 Aug 2017 16:03:10 +0200
Subject: [PATCH] Factor RefControl#canCreate(Repository, RevCommit) out

This commit factors this method out into its own class and delegates all
RefPermission checks to the PermissionBackend.

Change-Id: I005ac6c5b68ba4b3887cfba09ecf89567d98c45f
---
 .../server/git/receive/ReceiveCommits.java    |  17 +-
 .../gerrit/server/project/CreateBranch.java   |  10 +-
 .../server/project/CreateRefControl.java      | 179 ++++++++++++++++++
 .../gerrit/server/project/RefControl.java     | 114 -----------
 4 files changed, 198 insertions(+), 122 deletions(-)
 create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/project/CreateRefControl.java

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 7f909d4a04..73bbf789f7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -128,7 +128,9 @@ import com.google.gerrit.server.permissions.GlobalPermission;
 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.CreateRefControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.ProjectState;
@@ -320,6 +322,7 @@ class ReceiveCommits {
   private final String canonicalWebUrl;
   private final SubmoduleOp.Factory subOpFactory;
   private final TagCache tagCache;
+  private final CreateRefControl createRefControl;
 
   // Assisted injected fields.
   private final AllRefsWatcher allRefsWatcher;
@@ -402,6 +405,7 @@ class ReceiveCommits {
       SshInfo sshInfo,
       SubmoduleOp.Factory subOpFactory,
       TagCache tagCache,
+      CreateRefControl createRefControl,
       @Assisted ProjectControl projectControl,
       @Assisted ReceivePack rp,
       @Assisted AllRefsWatcher allRefsWatcher,
@@ -440,6 +444,7 @@ class ReceiveCommits {
     this.sshInfo = sshInfo;
     this.subOpFactory = subOpFactory;
     this.tagCache = tagCache;
+    this.createRefControl = createRefControl;
 
     // Assisted injected fields.
     this.allRefsWatcher = allRefsWatcher;
@@ -524,7 +529,7 @@ class ReceiveCommits {
 
     try {
       parseCommands(commands);
-    } catch (PermissionBackendException err) {
+    } catch (PermissionBackendException | NoSuchProjectException | IOException err) {
       for (ReceiveCommand cmd : actualCommands) {
         if (cmd.getResult() == NOT_ATTEMPTED) {
           cmd.setResult(REJECTED_OTHER_REASON, "internal server error");
@@ -772,7 +777,7 @@ class ReceiveCommits {
   }
 
   private void parseCommands(Collection<ReceiveCommand> commands)
-      throws PermissionBackendException {
+      throws PermissionBackendException, NoSuchProjectException, IOException {
     List<String> optionList = rp.getPushOptions();
     if (optionList != null) {
       for (String option : optionList) {
@@ -977,7 +982,8 @@ class ReceiveCommits {
     }
   }
 
-  private void parseCreate(ReceiveCommand cmd) throws PermissionBackendException {
+  private void parseCreate(ReceiveCommand cmd)
+      throws PermissionBackendException, NoSuchProjectException, IOException {
     RevObject obj;
     try {
       obj = rp.getRevWalk().parseAny(cmd.getNewId());
@@ -994,8 +1000,8 @@ class ReceiveCommits {
       return;
     }
 
-    RefControl ctl = projectControl.controlForRef(cmd.getRefName());
-    String rejectReason = ctl.canCreate(rp.getRepository(), obj);
+    Branch.NameKey branch = new Branch.NameKey(project.getName(), cmd.getRefName());
+    String rejectReason = createRefControl.canCreateRef(rp.getRepository(), obj, user, branch);
     if (rejectReason != null) {
       reject(cmd, "prohibited by Gerrit: " + rejectReason);
       return;
@@ -1006,6 +1012,7 @@ class ReceiveCommits {
       return;
     }
 
+    RefControl ctl = projectControl.controlForRef(cmd.getRefName());
     validateNewCommits(ctl, cmd);
     actualCommands.add(cmd);
   }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
index 4e2e327091..77fb86b86b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
@@ -26,6 +26,7 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.GitRepositoryManager;
 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.util.MagicBranch;
 import com.google.inject.Inject;
@@ -55,6 +56,7 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
   private final GitRepositoryManager repoManager;
   private final GitReferenceUpdated referenceUpdated;
   private final RefValidationHelper refCreationValidator;
+  private final CreateRefControl createRefControl;
   private String ref;
 
   @Inject
@@ -64,18 +66,21 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
       GitRepositoryManager repoManager,
       GitReferenceUpdated referenceUpdated,
       RefValidationHelper.Factory refHelperFactory,
+      CreateRefControl createRefControl,
       @Assisted String ref) {
     this.identifiedUser = identifiedUser;
     this.permissionBackend = permissionBackend;
     this.repoManager = repoManager;
     this.referenceUpdated = referenceUpdated;
     this.refCreationValidator = refHelperFactory.create(ReceiveCommand.Type.CREATE);
+    this.createRefControl = createRefControl;
     this.ref = ref;
   }
 
   @Override
   public BranchInfo apply(ProjectResource rsrc, BranchInput input)
-      throws BadRequestException, AuthException, ResourceConflictException, IOException {
+      throws BadRequestException, AuthException, ResourceConflictException, IOException,
+          PermissionBackendException, NoSuchProjectException {
     if (input == null) {
       input = new BranchInput();
     }
@@ -100,7 +105,6 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
     }
 
     final Branch.NameKey name = new Branch.NameKey(rsrc.getNameKey(), ref);
-    final RefControl refControl = rsrc.getControl().controlForRef(name);
     try (Repository repo = repoManager.openRepository(rsrc.getNameKey())) {
       ObjectId revid = RefUtil.parseBaseRevision(repo, rsrc.getNameKey(), input.revision);
       RevWalk rw = RefUtil.verifyConnected(repo, revid);
@@ -117,7 +121,7 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
         }
       }
 
-      String rejectReason = refControl.canCreate(repo, object);
+      String rejectReason = createRefControl.canCreateRef(repo, object, identifiedUser.get(), name);
       if (rejectReason != null) {
         throw new AuthException("Cannot create \"" + ref + "\": " + rejectReason);
       }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateRefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateRefControl.java
new file mode 100644
index 0000000000..7607373d39
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateRefControl.java
@@ -0,0 +1,179 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.project;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.RefPermission;
+import java.io.IOException;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevObject;
+import org.eclipse.jgit.revwalk.RevTag;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/** Manages access control for creating Git references (aka branches, tags). */
+@Singleton
+public class CreateRefControl {
+  private static final Logger log = LoggerFactory.getLogger(CreateRefControl.class);
+
+  private final PermissionBackend permissionBackend;
+  private final ProjectCache projectCache;
+
+  @Inject
+  CreateRefControl(PermissionBackend permissionBackend, ProjectCache projectCache) {
+    this.permissionBackend = permissionBackend;
+    this.projectCache = projectCache;
+  }
+
+  /**
+   * Determines whether the user can create a new Git ref.
+   *
+   * @param repo repository on which user want to create
+   * @param object the object the user will start the reference with
+   * @param user the current identified user
+   * @param branch the branch the new {@link RevObject} should be created on
+   * @return {@code null} if the user specified can create a new Git ref, or a String describing why
+   *     the creation is not allowed.
+   * @throws PermissionBackendException on failure of permission checks
+   */
+  @Nullable
+  public String canCreateRef(
+      Repository repo, RevObject object, IdentifiedUser user, Branch.NameKey branch)
+      throws PermissionBackendException, NoSuchProjectException, IOException {
+    ProjectState ps = projectCache.checkedGet(branch.getParentKey());
+    if (ps == null) {
+      throw new NoSuchProjectException(branch.getParentKey());
+    }
+    if (!ps.getProject()
+        .getState()
+        .equals(com.google.gerrit.extensions.client.ProjectState.ACTIVE)) {
+      return "project state does not permit write";
+    }
+
+    PermissionBackend.ForRef perm = permissionBackend.user(user).ref(branch);
+    if (object instanceof RevCommit) {
+      if (!testAuditLogged(perm, RefPermission.CREATE)) {
+        return user.getAccountId() + " lacks permission: " + Permission.CREATE;
+      }
+      return canCreateCommit(repo, (RevCommit) object, ps, user, perm);
+    } else if (object instanceof RevTag) {
+      final RevTag tag = (RevTag) object;
+      try (RevWalk rw = new RevWalk(repo)) {
+        rw.parseBody(tag);
+      } catch (IOException e) {
+        String msg =
+            String.format("RevWalk(%s) for pushing tag %s:", branch.getParentKey(), tag.name());
+        log.error(msg, e);
+
+        return "I/O exception for revwalk";
+      }
+
+      // If tagger is present, require it matches the user's email.
+      //
+      final PersonIdent tagger = tag.getTaggerIdent();
+      if (tagger != null) {
+        boolean valid;
+        if (user.isIdentifiedUser()) {
+          final String addr = tagger.getEmailAddress();
+          valid = user.asIdentifiedUser().hasEmailAddress(addr);
+        } else {
+          valid = false;
+        }
+        if (!valid && !testAuditLogged(perm, RefPermission.FORGE_COMMITTER)) {
+          return user.getAccountId() + " lacks permission: " + Permission.FORGE_COMMITTER;
+        }
+      }
+
+      RevObject tagObject = tag.getObject();
+      if (tagObject instanceof RevCommit) {
+        String rejectReason = canCreateCommit(repo, (RevCommit) tagObject, ps, user, perm);
+        if (rejectReason != null) {
+          return rejectReason;
+        }
+      } else {
+        String rejectReason = canCreateRef(repo, tagObject, user, branch);
+        if (rejectReason != null) {
+          return rejectReason;
+        }
+      }
+
+      // If the tag has a PGP signature, allow a lower level of permission
+      // than if it doesn't have a PGP signature.
+      //
+      RefControl refControl = ps.controlFor(user).controlForRef(branch);
+      if (tag.getFullMessage().contains("-----BEGIN PGP SIGNATURE-----\n")) {
+        return refControl.canPerform(Permission.CREATE_SIGNED_TAG)
+            ? null
+            : user.getAccountId() + " lacks permission: " + Permission.CREATE_SIGNED_TAG;
+      }
+      return refControl.canPerform(Permission.CREATE_TAG)
+          ? null
+          : user.getAccountId() + " lacks permission " + Permission.CREATE_TAG;
+    }
+
+    return null;
+  }
+
+  /**
+   * Check if the user is allowed to create a new commit object if this introduces a new commit to
+   * the project. If not allowed, returns a string describing why it's not allowed. The userId
+   * argument is only used for the error message.
+   */
+  @Nullable
+  private String canCreateCommit(
+      Repository repo,
+      RevCommit commit,
+      ProjectState projectState,
+      IdentifiedUser user,
+      PermissionBackend.ForRef forRef)
+      throws PermissionBackendException {
+    if (projectState.controlFor(user).isReachableFromHeadsOrTags(repo, commit)) {
+      // If the user has no push permissions, check whether the object is
+      // merged into a branch or tag readable by this user. If so, they are
+      // not effectively "pushing" more objects, so they can create the ref
+      // even if they don't have push permission.
+      return null;
+    } else if (testAuditLogged(forRef, RefPermission.UPDATE)) {
+      // If the user has push permissions, they can create the ref regardless
+      // of whether they are pushing any new objects along with the create.
+      return null;
+    }
+    return user.getAccountId()
+        + " lacks permission "
+        + Permission.PUSH
+        + " for creating new commit object";
+  }
+
+  private boolean testAuditLogged(PermissionBackend.ForRef forRef, RefPermission p)
+      throws PermissionBackendException {
+    try {
+      forRef.check(p);
+    } catch (AuthException e) {
+      return false;
+    }
+    return true;
+  }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index 16c820f767..3786fc988e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -16,7 +16,6 @@ package com.google.gerrit.server.project;
 
 import static com.google.common.base.Preconditions.checkArgument;
 
-import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.common.data.PermissionRange;
 import com.google.gerrit.common.data.PermissionRule;
@@ -34,7 +33,6 @@ import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.RefPermission;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.OrmException;
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -44,19 +42,9 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevObject;
-import org.eclipse.jgit.revwalk.RevTag;
-import org.eclipse.jgit.revwalk.RevWalk;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /** Manages access control for Git references (aka branches, tags). */
 public class RefControl {
-  private static final Logger log = LoggerFactory.getLogger(RefControl.class);
-
   private final ProjectControl projectControl;
   private final String refName;
 
@@ -230,108 +218,6 @@ public class RefControl {
     return canForcePerform(Permission.PUSH);
   }
 
-  /**
-   * Determines whether the user can create a new Git ref.
-   *
-   * @param repo repository on which user want to create
-   * @param object the object the user will start the reference with.
-   * @return {@code null} if the user specified can create a new Git ref, or a String describing why
-   *     the creation is not allowed.
-   */
-  @Nullable
-  public String canCreate(Repository repo, RevObject object) {
-    if (!isProjectStatePermittingWrite()) {
-      return "project state does not permit write";
-    }
-
-    String userId =
-        getUser().isIdentifiedUser() ? "account " + getUser().getAccountId() : "anonymous user";
-
-    if (object instanceof RevCommit) {
-      if (!canPerform(Permission.CREATE)) {
-        return userId + " lacks permission: " + Permission.CREATE;
-      }
-      return canCreateCommit(repo, (RevCommit) object, userId);
-    } else if (object instanceof RevTag) {
-      final RevTag tag = (RevTag) object;
-      try (RevWalk rw = new RevWalk(repo)) {
-        rw.parseBody(tag);
-      } catch (IOException e) {
-        String msg =
-            String.format(
-                "RevWalk(%s) for pushing tag %s:",
-                projectControl.getProject().getNameKey(), tag.name());
-        log.error(msg, e);
-
-        return "I/O exception for revwalk";
-      }
-
-      // If tagger is present, require it matches the user's email.
-      //
-      final PersonIdent tagger = tag.getTaggerIdent();
-      if (tagger != null) {
-        boolean valid;
-        if (getUser().isIdentifiedUser()) {
-          final String addr = tagger.getEmailAddress();
-          valid = getUser().asIdentifiedUser().hasEmailAddress(addr);
-        } else {
-          valid = false;
-        }
-        if (!valid && !canForgeCommitter()) {
-          return userId + " lacks permission: " + Permission.FORGE_COMMITTER;
-        }
-      }
-
-      RevObject tagObject = tag.getObject();
-      if (tagObject instanceof RevCommit) {
-        String rejectReason = canCreateCommit(repo, (RevCommit) tagObject, userId);
-        if (rejectReason != null) {
-          return rejectReason;
-        }
-      } else {
-        String rejectReason = canCreate(repo, tagObject);
-        if (rejectReason != null) {
-          return rejectReason;
-        }
-      }
-
-      // If the tag has a PGP signature, allow a lower level of permission
-      // than if it doesn't have a PGP signature.
-      //
-      if (tag.getFullMessage().contains("-----BEGIN PGP SIGNATURE-----\n")) {
-        return canPerform(Permission.CREATE_SIGNED_TAG)
-            ? null
-            : userId + " lacks permission: " + Permission.CREATE_SIGNED_TAG;
-      }
-      return canPerform(Permission.CREATE_TAG)
-          ? null
-          : userId + " lacks permission " + Permission.CREATE_TAG;
-    }
-
-    return null;
-  }
-
-  /**
-   * Check if the user is allowed to create a new commit object if this introduces a new commit to
-   * the project. If not allowed, returns a string describing why it's not allowed. The userId
-   * argument is only used for the error message.
-   */
-  @Nullable
-  private String canCreateCommit(Repository repo, RevCommit commit, String userId) {
-    if (canUpdate()) {
-      // If the user has push permissions, they can create the ref regardless
-      // of whether they are pushing any new objects along with the create.
-      return null;
-    } else if (projectControl.isReachableFromHeadsOrTags(repo, commit)) {
-      // If the user has no push permissions, check whether the object is
-      // merged into a branch or tag readable by this user. If so, they are
-      // not effectively "pushing" more objects, so they can create the ref
-      // even if they don't have push permission.
-      return null;
-    }
-    return userId + " lacks permission " + Permission.PUSH + " for creating new commit object";
-  }
-
   /**
    * Determines whether the user can delete the Git ref controlled by this object.
    *