Provide more feedback for 'prohibited by Gerrit'.

This code path continues to be a source of support requests for our team.

Change-Id: I14b8cd6d492095d9e7ff5d65d7efad785cc6b4c4

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

@@ -1070,15 +1070,19 @@ public class ReceiveCommits {
     }
 
     RefControl ctl = projectControl.controlForRef(cmd.getRefName());
-    if (ctl.canCreate(rp.getRepository(), obj)) {
-      if (!validRefOperation(cmd)) {
-        return;
-      }
-      validateNewCommits(ctl, cmd);
-      actualCommands.add(cmd);
-    } else {
-      reject(cmd, "prohibited by Gerrit: create access denied for " + cmd.getRefName());
+    String rejectReason = ctl.canCreate(rp.getRepository(), obj);
+    if (rejectReason != null) {
+      reject(cmd, "prohibited by Gerrit: " + rejectReason);
+      return;
     }
+
+    if (!validRefOperation(cmd)) {
+      // validRefOperation sets messages, so no need to provide more feedback.
+      return;
+    }
+
+    validateNewCommits(ctl, cmd);
+    actualCommands.add(cmd);
   }
 
   private void parseUpdate(ReceiveCommand cmd) throws PermissionBackendException {

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

@@ -117,8 +117,9 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
         }
       }
 
-      if (!refControl.canCreate(repo, object)) {
-        throw new AuthException("Cannot create \"" + ref + "\"");
+      String rejectReason = refControl.canCreate(repo, object);
+      if (rejectReason != null) {
+        throw new AuthException("Cannot create \"" + ref + "\": " + rejectReason);
       }
 
       try {
@@ -155,7 +156,7 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
               }
               refPrefix = RefUtil.getRefPrefix(refPrefix);
             }
-            //$FALL-THROUGH$
+            // $FALL-THROUGH$
           case FORCED:
           case IO_FAILURE:
           case NOT_ATTEMPTED:

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

@@ -16,6 +16,7 @@ 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;
@@ -259,17 +260,18 @@ public class RefControl {
    *
    * @param repo repository on which user want to create
    * @param object the object the user will start the reference with.
-   * @return {@code true} if the user specified can create a new Git ref
+   * @return {@code null} if the user specified can create a new Git ref, or a String describing why
+   *     the creation is not allowed.
    */
-  public boolean canCreate(Repository repo, RevObject object) {
+  @Nullable
+  public String canCreate(Repository repo, RevObject object) {
     if (!isProjectStatePermittingWrite()) {
-      return false;
+      return "project state does not permit write";
     }
 
     if (object instanceof RevCommit) {
       if (!canPerform(Permission.CREATE)) {
-        // No create permissions.
-        return false;
+        return "lacks permission: " + Permission.CREATE;
       }
       return canCreateCommit(repo, (RevCommit) object);
     } else if (object instanceof RevTag) {
@@ -277,7 +279,14 @@ public class RefControl {
       try (RevWalk rw = new RevWalk(repo)) {
         rw.parseBody(tag);
       } catch (IOException e) {
-        return false;
+        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.
@@ -292,18 +301,20 @@ public class RefControl {
           valid = false;
         }
         if (!valid && !canForgeCommitter()) {
-          return false;
+          return "lacks permission: " + Permission.FORGE_COMMITTER;
         }
       }
 
       RevObject tagObject = tag.getObject();
       if (tagObject instanceof RevCommit) {
-        if (!canCreateCommit(repo, (RevCommit) tagObject)) {
-          return false;
+        String rejectReason = canCreateCommit(repo, (RevCommit) tagObject);
+        if (rejectReason != null) {
+          return rejectReason;
         }
       } else {
-        if (!canCreate(repo, tagObject)) {
-          return false;
+        String rejectReason = canCreate(repo, tagObject);
+        if (rejectReason != null) {
+          return rejectReason;
         }
       }
 
@@ -311,27 +322,34 @@ public class RefControl {
       // than if it doesn't have a PGP signature.
       //
       if (tag.getFullMessage().contains("-----BEGIN PGP SIGNATURE-----\n")) {
-        return canPerform(Permission.CREATE_SIGNED_TAG);
+        return canPerform(Permission.CREATE_SIGNED_TAG)
+            ? null
+            : "lacks permission: " + Permission.CREATE_SIGNED_TAG;
       }
-      return canPerform(Permission.CREATE_TAG);
-    } else {
-      return false;
+      return canPerform(Permission.CREATE_TAG) ? null : "lacks permission " + Permission.CREATE_TAG;
     }
+
+    return null;
   }
 
-  private boolean canCreateCommit(Repository repo, RevCommit commit) {
+  /**
+   * 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.
+   */
+  @Nullable
+  private String canCreateCommit(Repository repo, RevCommit commit) {
     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 true;
+      return null;
     } else if (isMergedIntoBranchOrTag(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 true;
+      return null;
     }
-    return false;
+    return "lacks permission " + Permission.PUSH  + " for creating new commit object";
   }
 
   private boolean isMergedIntoBranchOrTag(Repository repo, RevCommit commit) {