Merge "Explicitly check READ permission when processing a git push"

java/com/google/gerrit/server/git/receive/ReceiveCommits.java

@@ -1839,7 +1839,9 @@ class ReceiveCommits {
       magicBranch.perm = permissions.ref(ref);
 
       Optional<AuthException> err =
-          checkRefPermission(magicBranch.perm, RefPermission.CREATE_CHANGE);
+          checkRefPermission(magicBranch.perm, RefPermission.READ)
+              .map(Optional::of)
+              .orElse(checkRefPermission(magicBranch.perm, RefPermission.CREATE_CHANGE));
       if (err.isPresent()) {
         rejectProhibited(cmd, err.get());
         return;

javatests/com/google/gerrit/acceptance/api/change/QueryChangesIT.java

@@ -153,16 +153,16 @@ public class QueryChangesIT extends AbstractDaemonTest {
 
     // Create hidden project.
     Project.NameKey hiddenProject = projectOperations.newProject().create();
+    TestRepository<InMemoryRepository> hiddenRepo = cloneProject(hiddenProject, admin);
+    // Create 2 hidden changes.
+    createChange(hiddenRepo);
+    createChange(hiddenRepo);
+    // Actually hide project
     projectOperations
         .project(hiddenProject)
         .forUpdate()
         .add(block(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
         .update();
-    TestRepository<InMemoryRepository> hiddenRepo = cloneProject(hiddenProject, admin);
-
-    // Create 2 hidden changes.
-    createChange(hiddenRepo);
-    createChange(hiddenRepo);
 
     // Create a change query that matches all changes (visible and hidden changes).
     // The index returns the changes ordered by last updated timestamp:

javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java

@@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.git;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
 import static com.google.gerrit.git.testing.PushResultSubject.assertThat;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static java.util.stream.Collectors.toList;
@@ -145,6 +146,22 @@ public class PushPermissionsIT extends AbstractDaemonTest {
     assertThat(r).hasProcessed(ImmutableMap.of("refs", 1));
   }
 
+  @Test
+  public void createDeniedIfUserCantRead() throws Exception {
+    projectOperations
+        .project(project)
+        .forUpdate()
+        .add(block(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
+        .add(allow(Permission.PUSH).ref("refs/*").group(REGISTERED_USERS))
+        .update();
+    testRepo.branch("HEAD").commit().create();
+    PushResult r = push("HEAD:refs/for/master");
+    assertThat(r)
+        .onlyRef("refs/for/master")
+        .isRejected("prohibited by Gerrit: not permitted: read on refs/heads/master");
+    assertThat(r).hasProcessed(ImmutableMap.of("refs", 1));
+  }
+
   @Test
   public void groupRefsByMessage() throws Exception {
     try (Repository repo = repoManager.openRepository(project);