Merge "Cleanup CreateRefControl API"

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

@@ -1004,9 +1004,10 @@ class ReceiveCommits {
     }
 
     Branch.NameKey branch = new Branch.NameKey(project.getName(), cmd.getRefName());
-    String rejectReason = createRefControl.canCreateRef(rp.getRepository(), obj, user, branch);
+    try {
-    if (rejectReason != null) {
+      createRefControl.checkCreateRef(rp.getRepository(), branch, obj);
-      reject(cmd, "prohibited by Gerrit: " + rejectReason);
+    } catch (AuthException denied) {
+      reject(cmd, "prohibited by Gerrit: " + denied.getMessage());
       return;
     }
 

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

@@ -121,10 +121,7 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
         }
       }
 
-      String rejectReason = createRefControl.canCreateRef(repo, object, identifiedUser.get(), name);
+      createRefControl.checkCreateRef(repo, name, object);
-      if (rejectReason != null) {
-        throw new AuthException("Cannot create \"" + ref + "\": " + rejectReason);
-      }
 
       try {
         final RefUpdate u = repo.updateRef(ref);

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

@@ -14,14 +14,14 @@
 
 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.CurrentUser;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.RefPermission;
+import com.google.inject.Provider;
 import java.io.IOException;
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -41,137 +41,103 @@ public class CreateRefControl {
 
   private final PermissionBackend permissionBackend;
   private final ProjectCache projectCache;
+  private final Provider<CurrentUser> user;
 
   @Inject
-  CreateRefControl(PermissionBackend permissionBackend, ProjectCache projectCache) {
+  CreateRefControl(
+      PermissionBackend permissionBackend, ProjectCache projectCache, Provider<CurrentUser> user) {
     this.permissionBackend = permissionBackend;
     this.projectCache = projectCache;
+    this.user = user;
   }
 
   /**
-   * Determines whether the user can create a new Git ref.
+   * Checks whether the {@link CurrentUser} 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
+   * @param object the object the user will start the reference with
-   *     the creation is not allowed.
+   * @throws AuthException if creation is denied; the message explains the denial.
-   * @throws PermissionBackendException on failure of permission checks
+   * @throws PermissionBackendException on failure of permission checks.
    */
-  @Nullable
+  public void checkCreateRef(Repository repo, Branch.NameKey branch, RevObject object)
-  public String canCreateRef(
+      throws AuthException, PermissionBackendException, NoSuchProjectException, IOException {
-      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().permitsWrite()) {
-      return "project state does not permit write";
+      throw new AuthException("project state does not permit write");
     }
 
     PermissionBackend.ForRef perm = permissionBackend.user(user).ref(branch);
     if (object instanceof RevCommit) {
-      if (!testAuditLogged(perm, RefPermission.CREATE)) {
+      perm.check(RefPermission.CREATE);
-        return user.getAccountId() + " lacks permission: " + Permission.CREATE;
+      checkCreateCommit(repo, (RevCommit) object, ps, perm);
-      }
-      return canCreateCommit(repo, (RevCommit) object, ps, user, perm);
     } else if (object instanceof RevTag) {
-      final RevTag tag = (RevTag) object;
+      RevTag tag = (RevTag) object;
       try (RevWalk rw = new RevWalk(repo)) {
         rw.parseBody(tag);
       } catch (IOException e) {
-        String msg =
+        log.error(String.format("RevWalk(%s) parsing %s:", branch.getParentKey(), tag.name()), e);
-            String.format("RevWalk(%s) for pushing tag %s:", branch.getParentKey(), tag.name());
+        throw e;
-        log.error(msg, e);
-
-        return "I/O exception for revwalk";
       }
 
       // If tagger is present, require it matches the user's email.
-      //
+      PersonIdent tagger = tag.getTaggerIdent();
-      final PersonIdent tagger = tag.getTaggerIdent();
+      if (tagger != null
-      if (tagger != null) {
+          && (!user.get().isIdentifiedUser()
-        boolean valid;
+              || !user.get().asIdentifiedUser().hasEmailAddress(tagger.getEmailAddress()))) {
-        if (user.isIdentifiedUser()) {
+        perm.check(RefPermission.FORGE_COMMITTER);
-          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();
+      RevObject target = tag.getObject();
-      if (tagObject instanceof RevCommit) {
+      if (target instanceof RevCommit) {
-        String rejectReason = canCreateCommit(repo, (RevCommit) tagObject, ps, user, perm);
+        checkCreateCommit(repo, (RevCommit) target, ps, perm);
-        if (rejectReason != null) {
-          return rejectReason;
-        }
       } else {
-        String rejectReason = canCreateRef(repo, tagObject, user, branch);
+        checkCreateRef(repo, branch, target);
-        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.get()).controlForRef(branch);
-      RefControl refControl = ps.controlFor(user).controlForRef(branch);
       if (tag.getFullMessage().contains("-----BEGIN PGP SIGNATURE-----\n")) {
-        return refControl.canPerform(Permission.CREATE_SIGNED_TAG)
+        if (!refControl.canPerform(Permission.CREATE_SIGNED_TAG)) {
-            ? null
+          throw new AuthException(Permission.CREATE_SIGNED_TAG + " not permitted");
-            : user.getAccountId() + " lacks permission: " + Permission.CREATE_SIGNED_TAG;
+        }
+      } else if (!refControl.canPerform(Permission.CREATE_TAG)) {
+        throw new AuthException(Permission.CREATE_TAG + " not permitted");
       }
-      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
+   * Check if the user is allowed to create a new commit object if this creation would introduce a
-   * the project. If not allowed, returns a string describing why it's not allowed. The userId
+   * new commit to the repository.
-   * argument is only used for the error message.
    */
-  @Nullable
+  private void checkCreateCommit(
-  private String canCreateCommit(
+      Repository repo, RevCommit commit, ProjectState projectState, PermissionBackend.ForRef forRef)
-      Repository repo,
+      throws AuthException, PermissionBackendException {
-      RevCommit commit,
+    try {
-      ProjectState projectState,
+      // If the user has update (push) permission, they can create the ref regardless
-      IdentifiedUser user,
+      // of whether they are pushing any new objects along with the create.
-      PermissionBackend.ForRef forRef)
+      forRef.check(RefPermission.UPDATE);
-      throws PermissionBackendException {
+      return;
-    if (projectState.controlFor(user).isReachableFromHeadsOrTags(repo, commit)) {
+    } catch (AuthException denied) {
+      // Fall through to check reachability.
+    }
+
+    if (projectState.controlFor(user.get()).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;
-    } 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)
+    throw new AuthException(
-      throws PermissionBackendException {
+        String.format(
-    try {
+            "%s for creating new commit object not permitted",
-      forRef.check(p);
+            RefPermission.UPDATE.describeForException()));
-    } catch (AuthException e) {
-      return false;
-    }
-    return true;
   }
 }