From 7adf83355f1e7fac1fd7f49e6c41c0af422b6a6d Mon Sep 17 00:00:00 2001
From: Shawn Pearce <sop@google.com>
Date: Thu, 23 Feb 2017 22:55:38 -0800
Subject: [PATCH] Convert SuggestChangeReviewers to use PermissionBackend

Use testOrFalse(RefPermission.READ) to determine if it
is likely the candidate reviewer can read the destination
reference of the change.

Later on when adding the user to the change check(READ)
can be used to ensure the new reviewer has permission.

Change-Id: I3be18a9e2a8e81c895f1ca953290524acf1a2ba8
---
 .../server/change/SuggestChangeReviewers.java     | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestChangeReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestChangeReviewers.java
index 5260730f43..1daa7e352d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestChangeReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SuggestChangeReviewers.java
@@ -27,6 +27,8 @@ import com.google.gerrit.server.ReviewersUtil;
 import com.google.gerrit.server.ReviewersUtil.VisibilityControl;
 import com.google.gerrit.server.account.AccountVisibility;
 import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.RefPermission;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -45,6 +47,7 @@ public class SuggestChangeReviewers extends SuggestReviewers
   )
   boolean excludeGroups;
 
+  private final PermissionBackend permissionBackend;
   private final Provider<CurrentUser> self;
 
   @Inject
@@ -52,10 +55,12 @@ public class SuggestChangeReviewers extends SuggestReviewers
       AccountVisibility av,
       GenericFactory identifiedUserFactory,
       Provider<ReviewDb> dbProvider,
+      PermissionBackend permissionBackend,
       Provider<CurrentUser> self,
       @GerritServerConfig Config cfg,
       ReviewersUtil reviewersUtil) {
     super(av, identifiedUserFactory, dbProvider, cfg, reviewersUtil);
+    this.permissionBackend = permissionBackend;
     this.self = self;
   }
 
@@ -73,7 +78,7 @@ public class SuggestChangeReviewers extends SuggestReviewers
         excludeGroups);
   }
 
-  private VisibilityControl getVisibility(final ChangeResource rsrc) {
+  private VisibilityControl getVisibility(ChangeResource rsrc) {
     if (rsrc.getControl().getRefControl().isVisibleByRegisteredUsers()) {
       return new VisibilityControl() {
         @Override
@@ -82,13 +87,15 @@ public class SuggestChangeReviewers extends SuggestReviewers
         }
       };
     }
+
+    // Use the destination reference, not the change, as drafts may deny
+    // anyone who is not already a reviewer.
+    PermissionBackend.ForRef perm = permissionBackend.user(self).ref(rsrc.getChange().getDest());
     return new VisibilityControl() {
       @Override
       public boolean isVisibleTo(Account.Id account) throws OrmException {
         IdentifiedUser who = identifiedUserFactory.create(account);
-        // we can't use changeControl directly as it won't suggest reviewers
-        // to drafts
-        return rsrc.getControl().forUser(who).isRefVisible();
+        return perm.user(who).testOrFalse(RefPermission.READ);
       }
     };
   }