PostReviewers: Fail if designated reviewer cannot see the change

If a user tries to add a user as reviewer to a change that is not
visible to that user, a proper error message is displayed now. Before
the request didn't fail, but the user was not added as reviewer. And
without feedback the calling user didn't know why the reviewer was not
added.

Change-Id: Ib364d1a862e75eda492c6bf3f80a0be0341072e6
Signed-off-by: Edwin Kempin <ekempin@google.com>

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -28,6 +28,7 @@ import static com.google.gerrit.server.project.Util.blockLabel;
 import static com.google.gerrit.server.project.Util.category;
 import static com.google.gerrit.server.project.Util.value;
 import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.junit.Assert.fail;
 
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableSet;
@@ -63,6 +64,7 @@ import com.google.gerrit.extensions.common.RevisionInfo;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Change;
@@ -535,6 +537,45 @@ public class ChangeIT extends AbstractDaemonTest {
         .rebase(ri);
   }
 
+  @Test
+  public void addReviewerThatCannotSeeChange() throws Exception {
+    // create hidden project that is only visible to administrators
+    Project.NameKey p = createProject("p");
+    ProjectConfig cfg = projectCache.checkedGet(p).getConfig();
+    Util.allow(cfg,
+        Permission.READ,
+        groupCache.get(new AccountGroup.NameKey("Administrators"))
+            .getGroupUUID(),
+        "refs/*");
+    Util.block(cfg, Permission.READ, REGISTERED_USERS, "refs/*");
+    saveProjectConfig(p, cfg);
+
+    // create change
+    TestRepository<InMemoryRepository> repo = cloneProject(p, admin);
+    PushOneCommit push = pushFactory.create(db, admin.getIdent(), repo);
+    PushOneCommit.Result result = push.to("refs/for/master");
+    result.assertOkStatus();
+
+    // check the user cannot see the change
+    setApiUser(user);
+    try {
+      gApi.changes().id(result.getChangeId()).get();
+      fail("Expected ResourceNotFoundException");
+    } catch (ResourceNotFoundException e) {
+      // Expected.
+    }
+
+    // try to add user as reviewer
+    setApiUser(admin);
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = user.email;
+    exception.expect(UnprocessableEntityException.class);
+    exception.expectMessage("Change not visible to " + user.email);
+    gApi.changes()
+        .id(result.getChangeId())
+        .addReviewer(in);
+  }
+
   @Test
   public void addReviewer() throws Exception {
     TestTimeUtil.resetWithClockStep(1, SECONDS);

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

@@ -161,10 +161,9 @@ public class PostReviewers
 
   public Addition prepareApplication(ChangeResource rsrc, AddReviewerInput input)
       throws OrmException, RestApiException, IOException {
+    Account.Id accountId;
     try {
-      Account.Id accountId = accounts.parse(input.reviewer).getAccountId();
-      return putAccount(input.reviewer, reviewerFactory.create(rsrc, accountId),
-          input.state());
+      accountId = accounts.parse(input.reviewer).getAccountId();
     } catch (UnprocessableEntityException e) {
       try {
         return putGroup(rsrc, input);
@@ -173,17 +172,19 @@ public class PostReviewers
             .format(ChangeMessages.get().reviewerNotFound, input.reviewer));
       }
     }
+    return putAccount(input.reviewer, reviewerFactory.create(rsrc, accountId),
+        input.state());
   }
 
   private Addition putAccount(String reviewer, ReviewerResource rsrc,
-      ReviewerState state) {
+      ReviewerState state) throws UnprocessableEntityException {
     Account member = rsrc.getReviewerUser().getAccount();
     ChangeControl control = rsrc.getReviewerControl();
     if (isValidReviewer(member, control)) {
       return new Addition(reviewer, rsrc.getChangeResource(),
           ImmutableMap.of(member.getId(), control), state);
     }
-    return new Addition(reviewer);
+    throw new UnprocessableEntityException("Change not visible to " + reviewer);
   }
 
   private Addition putGroup(ChangeResource rsrc, AddReviewerInput input)