Make PermissionBackend#ForRef authoritative

This change fixes a misconception that leads to data being accessible
through Gerrit APIs that should be locked down.

Gerrit had two components for determining if a Git ref is visible to a
user: (Default)RefFilter and PermissionBackend#ForRef (ex RefControl).
The former was always capable of providing correct results for all refs.
The latter only had logic to decide if a Git ref is visible according to
the Gerrit READ permissions. This includes all refs under refs/heads as
well as any other ref that isn't a database ref or a Git tag. This
component was unware of Git tags and database references. Hence, when
asked for a database reference such as refs/changes/xx/yyyyxx/meta the
logic would allow access if the user has READ permissions on any of the
ref prefixes, such as the default "read refs/* Anonymous Users".

That is problematic, because it bypasses documented behavior [1] where
a user should only have access to a change if they can see the destination
ref. The same goes for other database references.

This change fixes the problem. It is intentionally kept to a minimally
invasive code change so that it's easier to backport it.

Add tests to assert the correct behavior. These tests would fail before
this fix. We have included them in this change to be able to backport
just a single commit.

[1] https://gerrit-review.googlesource.com/Documentation/access-control.html

Change-Id: Ice3a756cf573dd9b38e3f198ccc44899ccf65f75

10 files changed, 818 insertions, 83 deletions

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java
index cea907d941..101a9affe4 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java
@@ -157,6 +157,29 @@ public abstract class AbstractPushTag extends AbstractDaemonTest {
     pushTagDeletion(tagName, Status.OK);
   }
 
+  @Test
+  public void createTagForExistingCommit_withoutGlobalReadPermissions() throws Exception {
+    removeReadAccessOnRefsStar();
+    grantReadAccessOnRefsHeadsStar();
+    createTagForExistingCommit();
+  }
+
+  @Test
+  public void createTagForNewCommit_withoutGlobalReadPermissions() throws Exception {
+    removeReadAccessOnRefsStar();
+    grantReadAccessOnRefsHeadsStar();
+    createTagForNewCommit();
+  }
+
+  private void removeReadAccessOnRefsStar() throws Exception {
+    removePermission(allProjects, "refs/heads/*", Permission.READ);
+    removePermission(project, "refs/heads/*", Permission.READ);
+  }
+
+  private void grantReadAccessOnRefsHeadsStar() throws Exception {
+    grant(project, "refs/heads/*", Permission.READ, false, REGISTERED_USERS);
+  }
+
   private String pushTagForExistingCommit(Status expectedStatus) throws Exception {
     return pushTag(null, false, false, expectedStatus);
   }
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/GetBranchIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/GetBranchIT.java
new file mode 100644
index 0000000000..bd93a44150
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/GetBranchIT.java
@@ -0,0 +1,384 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.rest.project;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.testutil.GerritJUnit.assertThrows;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.api.changes.DraftInput;
+import com.google.gerrit.extensions.api.projects.BranchInfo;
+import com.google.gerrit.extensions.api.projects.TagInfo;
+import com.google.gerrit.extensions.api.projects.TagInput;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.testutil.NoteDbMode;
+import com.google.inject.Inject;
+import org.junit.Test;
+
+public class GetBranchIT extends AbstractDaemonTest {
+  @Inject protected AllUsersName allUsers;
+
+  @Test
+  public void cannotGetNonExistingBranch() {
+    assertBranchNotFound(project, RefNames.fullName("non-existing"));
+  }
+
+  @Test
+  public void getBranch() throws Exception {
+    setApiUser(user);
+    assertBranchFound(project, RefNames.fullName("master"));
+  }
+
+  @Test
+  public void getBranchByShortName() throws Exception {
+    setApiUser(user);
+    assertBranchFound(project, "master");
+  }
+
+  @Test
+  public void cannotGetNonVisibleBranch() throws Exception {
+    String branchName = "master";
+
+    // block read access to the branch
+    block(project, RefNames.fullName(branchName), Permission.READ, ANONYMOUS_USERS);
+
+    setApiUser(user);
+    assertBranchNotFound(project, RefNames.fullName(branchName));
+  }
+
+  @Test
+  public void cannotGetNonVisibleBranchByShortName() throws Exception {
+    String branchName = "master";
+
+    // block read access to the branch
+    block(project, RefNames.fullName(branchName), Permission.READ, ANONYMOUS_USERS);
+
+    setApiUser(user);
+    assertBranchNotFound(project, branchName);
+  }
+
+  @Test
+  public void getChangeRef() throws Exception {
+    // create a change
+    Change.Id changeId = createChange("refs/for/master").getPatchSetId().changeId;
+
+    // a user without the 'Access Database' capability can see the change ref
+    setApiUser(user);
+    String changeRef = RefNames.patchSetRef(new PatchSet.Id(changeId, 1));
+    assertBranchFound(project, changeRef);
+  }
+
+  @Test
+  public void getChangeRefOfNonVisibleChange() throws Exception {
+    // create a change
+    String branchName = "master";
+    Change.Id changeId = createChange("refs/for/" + branchName).getPatchSetId().changeId;
+
+    // block read access to the branch
+    block(project, RefNames.fullName(branchName), Permission.READ, ANONYMOUS_USERS);
+
+    // a user without the 'Access Database' capability cannot see the change ref
+    setApiUser(user);
+    String changeRef = RefNames.patchSetRef(new PatchSet.Id(changeId, 1));
+    assertBranchNotFound(project, changeRef);
+
+    // a user with the 'Access Database' capability can see the change ref
+    testGetRefWithAccessDatabase(project, changeRef);
+  }
+
+  @Test
+  public void getChangeEditRef() throws Exception {
+    TestAccount user2 = accountCreator.user2();
+
+    // create a change
+    Change.Id changeId = createChange("refs/for/master").getPatchSetId().changeId;
+
+    // create a change edit by 'user'
+    setApiUser(user);
+    gApi.changes().id(changeId.get()).edit().create();
+
+    // every user can see their own change edit refs
+    String changeEditRef = RefNames.refsEdit(user.id, changeId, new PatchSet.Id(changeId, 1));
+    assertBranchFound(project, changeEditRef);
+
+    // a user without the 'Access Database' capability cannot see the change edit ref of another
+    // user
+    setApiUser(user2);
+    assertBranchNotFound(project, changeEditRef);
+
+    // a user with the 'Access Database' capability can see the change edit ref of another user
+    testGetRefWithAccessDatabase(project, changeEditRef);
+  }
+
+  @Test
+  public void cannotGetChangeEditRefOfNonVisibleChange() throws Exception {
+    // create a change
+    String branchName = "master";
+    Change.Id changeId = createChange("refs/for/" + branchName).getPatchSetId().changeId;
+
+    // create a change edit by 'user'
+    setApiUser(user);
+    gApi.changes().id(changeId.get()).edit().create();
+
+    // make the change non-visible by blocking read access on the destination
+    block(project, RefNames.fullName(branchName), Permission.READ, ANONYMOUS_USERS);
+
+    // user cannot see their own change edit refs if the change is no longer visible
+    String changeEditRef = RefNames.refsEdit(user.id, changeId, new PatchSet.Id(changeId, 1));
+    assertBranchNotFound(project, changeEditRef);
+
+    // a user with the 'Access Database' capability can see the change edit ref
+    testGetRefWithAccessDatabase(project, changeEditRef);
+  }
+
+  @Test
+  public void getChangeMetaRef() throws Exception {
+    // create a change
+    Change.Id changeId = createChange("refs/for/master").getPatchSetId().changeId;
+
+    // A user without the 'Access Database' capability can see the change meta ref.
+    // This may be surprising, as 'Access Database' guards access to meta refs and the change meta
+    // ref is a meta ref, however change meta refs have been always visible to all users that can
+    // see the change and some tools rely on seeing these refs, so we have to keep the current
+    // behaviour.
+    setApiUser(user);
+    String changeMetaRef = RefNames.changeMetaRef(changeId);
+    if (NoteDbMode.get().ordinal() >= NoteDbMode.WRITE.ordinal()) {
+      // Branch is there when we write to NoteDb
+      assertBranchFound(project, changeMetaRef);
+    }
+  }
+
+  @Test
+  public void getRefsMetaConfig() throws Exception {
+    // a non-project owner cannot get the refs/meta/config branch
+    setApiUser(user);
+    assertBranchNotFound(project, RefNames.REFS_CONFIG);
+
+    // a non-project owner cannot get the refs/meta/config branch even with the 'Access Database'
+    // capability
+    allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
+    try {
+      assertBranchNotFound(project, RefNames.REFS_CONFIG);
+    } finally {
+      removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
+    }
+
+    setApiUser(user);
+
+    // a project owner can get the refs/meta/config branch
+    allow(project, "refs/*", Permission.OWNER, REGISTERED_USERS);
+    assertBranchFound(project, RefNames.REFS_CONFIG);
+  }
+
+  @Test
+  public void getUserRefOfOtherUser() throws Exception {
+    String userRef = RefNames.refsUsers(admin.id);
+
+    // a user without the 'Access Database' capability cannot see the user ref of another user
+    setApiUser(user);
+    assertBranchNotFound(allUsers, userRef);
+
+    // a user with the 'Access Database' capability can see the user ref of another user
+    testGetRefWithAccessDatabase(allUsers, userRef);
+  }
+
+  @Test
+  public void getOwnUserRef() throws Exception {
+    // every user can see the own user ref
+    setApiUser(user);
+    assertBranchFound(allUsers, RefNames.refsUsers(user.id));
+
+    // TODO: every user can see the own user ref via the magic ref/users/self ref
+    //  setApiUser(user);
+    // assertBranchFound(allUsers, RefNames.REFS_USERS_SELF);
+  }
+
+  @Test
+  public void getExternalIdsRefs() throws Exception {
+    // a user without the 'Access Database' capability cannot see the refs/meta/external-ids ref
+    setApiUser(user);
+    assertBranchNotFound(allUsers, RefNames.REFS_EXTERNAL_IDS);
+
+    // a user with the 'Access Database' capability can see the refs/meta/external-ids ref
+    testGetRefWithAccessDatabase(allUsers, RefNames.REFS_EXTERNAL_IDS);
+  }
+
+  @Test
+  public void getGroupRef() throws Exception {
+    // Groups were not yet in NoteDb in this release.
+  }
+
+  @Test
+  public void getGroupNamesRef() throws Exception {
+    // Groups were not yet in NoteDb in this release.
+  }
+
+  @Test
+  public void getDeletedGroupRef() throws Exception {
+    // Groups were not yet in NoteDb in this release.
+  }
+
+  @Test
+  public void getDraftCommentsRef() throws Exception {
+    TestAccount user2 = accountCreator.user2();
+
+    // create a change
+    String fileName = "a.txt";
+    Change change = createChange("A Change", fileName, "content").getChange().change();
+
+    // create a draft comment by the by 'user'
+    setApiUser(user);
+    DraftInput draftInput = new DraftInput();
+    draftInput.path = fileName;
+    draftInput.line = 0;
+    draftInput.message = "Some Comment";
+    gApi.changes().id(change.getChangeId()).current().createDraft(draftInput);
+
+    // every user can see their own draft comments refs
+    // TODO: is this a bug?
+    String draftCommentsRef = RefNames.refsDraftComments(change.getId(), user.id);
+    if (NoteDbMode.get().ordinal() >= NoteDbMode.WRITE.ordinal()) {
+      // Branch is there when we write to NoteDb
+      assertBranchFound(allUsers, draftCommentsRef);
+    }
+
+    // a user without the 'Access Database' capability cannot see the draft comments ref of another
+    // user
+    setApiUser(user2);
+    assertBranchNotFound(allUsers, draftCommentsRef);
+
+    // a user with the 'Access Database' capability can see the draft comments ref of another user
+    if (NoteDbMode.get().ordinal() >= NoteDbMode.WRITE.ordinal()) {
+      // Branch is there when we write to NoteDb
+      testGetRefWithAccessDatabase(allUsers, draftCommentsRef);
+    }
+  }
+
+  @Test
+  public void getStarredChangesRef() throws Exception {
+    TestAccount user2 = accountCreator.user2();
+
+    // create a change
+    Change change = createChange().getChange().change();
+
+    // let user star the change
+    setApiUser(user);
+    gApi.accounts().self().starChange(Integer.toString(change.getChangeId()));
+
+    // every user can see their own starred changes refs
+    // TODO: is this a bug?
+    String starredChangesRef = RefNames.refsStarredChanges(change.getId(), user.id);
+    assertBranchFound(allUsers, starredChangesRef);
+
+    // a user without the 'Access Database' capability cannot see the starred changes ref of another
+    // user
+    setApiUser(user2);
+    assertBranchNotFound(allUsers, starredChangesRef);
+
+    // a user with the 'Access Database' capability can see the starred changes ref of another user
+    testGetRefWithAccessDatabase(allUsers, starredChangesRef);
+  }
+
+  @Test
+  public void getTagRef() throws Exception {
+    // create a tag
+    TagInput input = new TagInput();
+    input.message = "My Tag";
+    input.revision = gApi.projects().name(project.get()).head();
+    TagInfo tagInfo = gApi.projects().name(project.get()).tag("my-tag").create(input).get();
+
+    // any user who can see the project, can see the tag
+    setApiUser(user);
+    assertBranchFound(project, tagInfo.ref);
+  }
+
+  @Test
+  public void cannotGetTagRefThatPointsToNonVisibleBranch() throws Exception {
+    // create a tag
+    TagInput input = new TagInput();
+    input.message = "My Tag";
+    input.revision = gApi.projects().name(project.get()).head();
+    TagInfo tagInfo = gApi.projects().name(project.get()).tag("my-tag").create(input).get();
+
+    // block read access to the branch
+    block(project, RefNames.fullName("master"), Permission.READ, ANONYMOUS_USERS);
+
+    // if the user cannot see the project, the tag is not visible
+    setApiUser(user);
+    assertBranchNotFound(project, tagInfo.ref);
+  }
+
+  @Test
+  public void getSymbolicRef() throws Exception {
+    // 'HEAD' is visible since it points to 'master' that is visible
+    setApiUser(user);
+    assertBranchFound(project, "HEAD");
+  }
+
+  @Test
+  public void cannotGetSymbolicRefThatPointsToNonVisibleBranch() throws Exception {
+    // block read access to the branch to which HEAD points by default
+    block(project, RefNames.fullName("master"), Permission.READ, ANONYMOUS_USERS);
+
+    // since 'master' is not visible, 'HEAD' which points to 'master' is also not visible
+    setApiUser(user);
+    assertBranchNotFound(project, "HEAD");
+  }
+
+  @Test
+  public void getAccountSequenceRef() throws Exception {
+    // Sequences were not yet in NoteDb in this release.
+  }
+
+  @Test
+  public void getGroupSequenceRef() throws Exception {
+    // Sequences were not yet in NoteDb in this release.
+  }
+
+  private void testGetRefWithAccessDatabase(Project.NameKey project, String ref) throws Exception {
+    allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
+    try {
+      setApiUser(user);
+      assertBranchFound(project, ref);
+    } finally {
+      removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
+    }
+  }
+
+  private void assertBranchNotFound(Project.NameKey project, String ref) {
+    ResourceNotFoundException exception =
+        assertThrows(
+            ResourceNotFoundException.class,
+            () -> gApi.projects().name(project.get()).branch(ref).get());
+    assertThat(exception).hasMessageThat().isEqualTo("Not found: " + ref);
+  }
+
+  private void assertBranchFound(Project.NameKey project, String ref) throws RestApiException {
+    BranchInfo branchInfo = gApi.projects().name(project.get()).branch(ref).get();
+    assertThat(branchInfo.ref).isEqualTo(RefNames.fullName(ref));
+  }
+}
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java
index 89de9dcd64..16896aab96 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java
@@ -101,11 +101,32 @@ public class RefNames {
     return ref;
   }
 
+  /**
+   * Whether the ref is managed by Gerrit. Covers all Gerrit-internal refs like refs/cache-automerge
+   * and refs/meta as well as refs/changes. Does not cover user-created refs like branches or custom
+   * ref namespaces like refs/my-company.
+   */
+  public static boolean isGerritRef(String ref) {
+    return ref.startsWith(REFS_CHANGES)
+        || ref.startsWith(REFS_EXTERNAL_IDS)
+        || ref.startsWith(REFS_CACHE_AUTOMERGE)
+        || ref.startsWith(REFS_DRAFT_COMMENTS)
+        || ref.startsWith(REFS_SEQUENCES)
+        || ref.startsWith(REFS_USERS)
+        || ref.startsWith(REFS_STARRED_CHANGES)
+        || ref.startsWith(REFS_REJECT_COMMITS);
+  }
+
   public static String changeMetaRef(Change.Id id) {
     StringBuilder r = newStringBuilder().append(REFS_CHANGES);
     return shard(id.get(), r).append(META_SUFFIX).toString();
   }
 
+  public static String patchSetRef(PatchSet.Id id) {
+    StringBuilder r = newStringBuilder().append(REFS_CHANGES);
+    return shard(id.changeId.get(), r).append('/').append(id.get()).toString();
+  }
+
   public static String robotCommentsRef(Change.Id id) {
     StringBuilder r = newStringBuilder().append(REFS_CHANGES);
     return shard(id.get(), r).append(ROBOT_COMMENTS_SUFFIX).toString();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java
index c16c195d3a..9b368beeae 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java
@@ -14,7 +14,6 @@
 
 package com.google.gerrit.server.git;
 
-import static com.google.gerrit.reviewdb.client.RefNames.REFS_CACHE_AUTOMERGE;
 import static com.google.gerrit.reviewdb.client.RefNames.REFS_CHANGES;
 import static com.google.gerrit.reviewdb.client.RefNames.REFS_CONFIG;
 import static com.google.gerrit.reviewdb.client.RefNames.REFS_USERS_SELF;
@@ -38,6 +37,7 @@ import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.permissions.RefPermission;
+import com.google.gerrit.server.permissions.RefVisibilityControl;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.query.change.ChangeData;
@@ -80,6 +80,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
   private final PermissionBackend.ForProject perm;
   private final ProjectState projectState;
   private final Repository git;
+  private final RefVisibilityControl refVisibilityControl;
   private ProjectControl projectCtl;
   private boolean showMetadata = true;
   private String userEditPrefix;
@@ -93,6 +94,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
       Provider<ReviewDb> db,
       Provider<CurrentUser> user,
       PermissionBackend permissionBackend,
+      RefVisibilityControl refVisibilityControl,
       @Assisted ProjectState projectState,
       @Assisted Repository git) {
     this.tagCache = tagCache;
@@ -105,6 +107,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
         permissionBackend.user(user).database(db).project(projectState.getProject().getNameKey());
     this.projectState = projectState;
     this.git = git;
+    this.refVisibilityControl = refVisibilityControl;
   }
 
   /** Show change references. Default is {@code true}. */
@@ -128,68 +131,54 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
       }
     }
 
-    Account.Id userId;
-    boolean viewMetadata;
+    boolean hasAccessDatabase;
     if (user.get().isIdentifiedUser()) {
-      viewMetadata = withUser.testOrFalse(GlobalPermission.ACCESS_DATABASE);
+      hasAccessDatabase = withUser.testOrFalse(GlobalPermission.ACCESS_DATABASE);
       IdentifiedUser u = user.get().asIdentifiedUser();
-      userId = u.getAccountId();
+      Account.Id userId = u.getAccountId();
       userEditPrefix = RefNames.refsEditPrefix(userId);
     } else {
-      userId = null;
-      viewMetadata = false;
+      hasAccessDatabase = false;
     }
 
-    Map<String, Ref> result = new HashMap<>();
+    Map<String, Ref> resultRefs = new HashMap<>();
     List<Ref> deferredTags = new ArrayList<>();
 
     projectCtl = projectState.controlFor(user.get());
     for (Ref ref : refs.values()) {
-      String name = ref.getName();
+      String refName = ref.getName();
       Change.Id changeId;
-      Account.Id accountId;
-      if (name.startsWith(REFS_CACHE_AUTOMERGE) || (!showMetadata && isMetadata(name))) {
-        continue;
-      } else if (RefNames.isRefsEdit(name)) {
-        // Edits are visible only to the owning user, if change is visible.
-        if (viewMetadata || visibleEdit(name)) {
-          result.put(name, ref);
-        }
-      } else if ((changeId = Change.Id.fromRef(name)) != null) {
-        // Change ref is visible only if the change is visible.
-        if (viewMetadata || visible(changeId)) {
-          result.put(name, ref);
-        }
-      } else if ((accountId = Account.Id.fromRef(name)) != null) {
-        // Account ref is visible only to corresponding account.
-        if (viewMetadata || (accountId.equals(userId) && canReadRef(name))) {
-          result.put(name, ref);
-        }
+      if (!showMetadata && isMetadata(refName)) {
+        log.debug("Filter out metadata ref %s", refName);
       } else if (isTag(ref)) {
         // If its a tag, consider it later.
         if (ref.getObjectId() != null) {
+          log.debug("Defer tag ref %s", refName);
           deferredTags.add(ref);
+        } else {
+          log.debug("Filter out tag ref %s that is not a tag", refName);
         }
-      } else if (name.startsWith(RefNames.REFS_SEQUENCES)) {
-        // Sequences are internal database implementation details.
-        if (viewMetadata) {
-          result.put(name, ref);
+      } else if ((changeId = Change.Id.fromRef(refName)) != null) {
+        // This is a mere performance optimization. RefVisibilityControl could determine the
+        // visibility of these refs just fine. But instead, we use highly-optimized logic that
+        // looks only on the last 10k most recent changes using the change index and a cache.
+        if (hasAccessDatabase) {
+          resultRefs.put(refName, ref);
+        } else if (!visible(changeId)) {
+          log.debug("Filter out invisible change ref %s", refName);
+        } else if (RefNames.isRefsEdit(refName) && !visibleEdit(refName)) {
+          log.debug("Filter out invisible change edit ref %s", refName);
+        } else {
+          // Change is visible
+          resultRefs.put(refName, ref);
         }
-      } else if (projectState.isAllUsers() && name.equals(RefNames.REFS_EXTERNAL_IDS)) {
-        // The notes branch with the external IDs of all users must not be exposed to normal users.
-        if (viewMetadata) {
-          result.put(name, ref);
-        }
-      } else if (canReadRef(ref.getLeaf().getName())) {
-        // Use the leaf to lookup the control data. If the reference is
-        // symbolic we want the control around the final target. If its
-        // not symbolic then getLeaf() is a no-op returning ref itself.
-        result.put(name, ref);
-      } else if (isRefsUsersSelf(ref)) {
-        // viewMetadata allows to see all account refs, hence refs/users/self should be included as
-        // well
-        if (viewMetadata) {
-          result.put(name, ref);
+      } else {
+        try {
+          if (refVisibilityControl.isVisible(projectCtl, ref.getLeaf().getName())) {
+            resultRefs.put(refName, ref);
+          }
+        } catch (PermissionBackendException e) {
+          log.warn("could not evaluate ref permission", e);
         }
       }
     }
@@ -197,22 +186,22 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
     // If we have tags that were deferred, we need to do a revision walk
     // to identify what tags we can actually reach, and what we cannot.
     //
-    if (!deferredTags.isEmpty() && (!result.isEmpty() || filterTagsSeparately)) {
+    if (!deferredTags.isEmpty() && (!resultRefs.isEmpty() || filterTagsSeparately)) {
       TagMatcher tags =
           tagCache
               .get(projectState.getNameKey())
               .matcher(
                   tagCache,
                   git,
-                  filterTagsSeparately ? filter(git.getAllRefs()).values() : result.values());
+                  filterTagsSeparately ? filter(git.getAllRefs()).values() : resultRefs.values());
       for (Ref tag : deferredTags) {
         if (tags.isReachable(tag)) {
-          result.put(tag.getName(), tag);
+          resultRefs.put(tag.getName(), tag);
         }
       }
     }
 
-    return result;
+    return resultRefs;
   }
 
   private Map<String, Ref> fastHideRefsMetaConfig(Map<String, Ref> refs) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/RefVisibilityControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/RefVisibilityControl.java
new file mode 100644
index 0000000000..d75bc7dbc1
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/RefVisibilityControl.java
@@ -0,0 +1,215 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.permissions;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Throwables;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.OneOffRequestContext;
+import com.google.gwtorm.server.OrmException;
+import javax.inject.Inject;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import org.eclipse.jgit.lib.Constants;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This class is a component that is internal to {@link DefaultPermissionBackend}. It can
+ * authoritatively tell if a ref is accessible by a user.
+ */
+@Singleton
+public class RefVisibilityControl {
+  private static final Logger logger = LoggerFactory.getLogger(RefVisibilityControl.class);
+
+  private final Provider<ReviewDb> dbProvider;
+  private final OneOffRequestContext oneOffRequestContext;
+  private final PermissionBackend permissionBackend;
+  private final ChangeData.Factory changeDataFactory;
+
+  @Inject
+  RefVisibilityControl(
+      Provider<ReviewDb> dbProvider,
+      OneOffRequestContext oneOffRequestContext,
+      PermissionBackend permissionBackend,
+      ChangeData.Factory changeDataFactory) {
+    this.dbProvider = dbProvider;
+    this.oneOffRequestContext = oneOffRequestContext;
+    this.permissionBackend = permissionBackend;
+    this.changeDataFactory = changeDataFactory;
+  }
+
+  /**
+   * Returns an authoritative answer if the ref is visible to the user. Does not have support for
+   * tags and will throw a {@link PermissionBackendException} if asked for tags visibility.
+   */
+  public boolean isVisible(ProjectControl projectControl, String refName)
+      throws PermissionBackendException {
+    if (refName.startsWith(Constants.R_TAGS)) {
+      throw new PermissionBackendException(
+          "can't check tags through RefVisibilityControl. Use PermissionBackend#filter instead.");
+    }
+    if (!RefNames.isGerritRef(refName)) {
+      // This is not a special Gerrit ref and not a NoteDb ref. Likely, it's just a ref under
+      // refs/heads or another ref the user created. Apply the regular permissions with inheritance.
+      return projectControl.controlForRef(refName).hasReadPermissionOnRef(false);
+    }
+
+    if (refName.startsWith(RefNames.REFS_CACHE_AUTOMERGE)) {
+      // Internal cache state that is accessible to no one.
+      return false;
+    }
+
+    boolean hasAccessDatabase =
+        permissionBackend
+            .user(projectControl.getUser())
+            .testOrFalse(GlobalPermission.ACCESS_DATABASE);
+    if (hasAccessDatabase) {
+      return true;
+    }
+
+    // Change and change edit visibility
+    Change.Id changeId;
+    if ((changeId = Change.Id.fromRef(refName)) != null) {
+      // Change ref is visible only if the change is visible.
+      try (CloseableOneTimeReviewDb ignored = new CloseableOneTimeReviewDb()) {
+        ChangeData cd;
+        try {
+          cd =
+              changeDataFactory.create(
+                  dbProvider.get(), projectControl.getProject().getNameKey(), changeId);
+          checkState(cd.change().getId().equals(changeId));
+        } catch (OrmException e) {
+          if (Throwables.getCausalChain(e).stream()
+              .anyMatch(e2 -> e2 instanceof NoSuchChangeException)) {
+            // The change was deleted or is otherwise not accessible anymore.
+            // If the caller can see all refs and is allowed to see private changes on refs/, allow
+            // access. This is an escape hatch for receivers of "ref deleted" events.
+            PermissionBackend.ForProject forProject = projectControl.asForProject();
+            return forProject.test(ProjectPermission.READ);
+          }
+          throw new PermissionBackendException(e);
+        }
+        if (RefNames.isRefsEdit(refName)) {
+          // Edits are visible only to the owning user, if change is visible.
+          return visibleEdit(refName, projectControl, cd);
+        }
+        return isVisible(projectControl.controlFor(getNotes(cd)).setChangeData(cd));
+      }
+    }
+
+    // Account visibility
+    CurrentUser user = projectControl.getUser();
+    Account.Id currentUserAccountId = user.isIdentifiedUser() ? user.getAccountId() : null;
+    Account.Id accountId;
+    if ((accountId = Account.Id.fromRef(refName)) != null) {
+      // Account ref is visible only to the corresponding account.
+      if (accountId.equals(currentUserAccountId)
+          && projectControl.controlForRef(refName).hasReadPermissionOnRef(true)) {
+        return true;
+      }
+      return false;
+    }
+
+    // We are done checking all cases where we would allow access to Gerrit-managed refs. Deny
+    // access in case we got this far.
+    logger.debug(
+        "Denying access to %s because user doesn't have access to this Gerrit ref", refName);
+    return false;
+  }
+
+  private boolean visibleEdit(String refName, ProjectControl projectControl, ChangeData cd)
+      throws PermissionBackendException {
+    Change.Id id = Change.Id.fromEditRefPart(refName);
+    if (id == null) {
+      throw new IllegalStateException("unable to parse change id from edit ref " + refName);
+    }
+
+    if (!isVisible(projectControl.controlFor(getNotes(cd)).setChangeData(cd))) {
+      // The user can't see the change so they can't see any edits.
+      return false;
+    }
+
+    if (projectControl.getUser().isIdentifiedUser()
+        && refName.startsWith(
+            RefNames.refsEditPrefix(projectControl.getUser().asIdentifiedUser().getAccountId()))) {
+      logger.debug("Own change edit ref is visible: %s", refName);
+      return true;
+    }
+
+    return false;
+  }
+
+  private ChangeNotes getNotes(ChangeData cd) throws PermissionBackendException {
+    try {
+      return cd.notes();
+    } catch (OrmException e) {
+      throw new PermissionBackendException(e);
+    }
+  }
+
+  private boolean isVisible(ChangeControl changeControl) throws PermissionBackendException {
+    try {
+      return changeControl.isVisible(dbProvider.get());
+    } catch (OrmException e) {
+      throw new PermissionBackendException(e);
+    }
+  }
+
+  private Optional<ReviewDb> getReviewDb() {
+    try {
+      return Optional.of(dbProvider.get());
+    } catch (Exception e) {
+      return Optional.absent();
+    }
+  }
+
+  /** Helper to establish a database connection. */
+  private class CloseableOneTimeReviewDb implements AutoCloseable {
+    @Nullable private final ManualRequestContext ctx;
+
+    CloseableOneTimeReviewDb() throws PermissionBackendException {
+      if (!getReviewDb().isPresent()) {
+        try {
+          ctx = oneOffRequestContext.open();
+        } catch (OrmException e) {
+          throw new PermissionBackendException(e);
+        }
+      } else {
+        ctx = null;
+      }
+    }
+
+    @Override
+    public void close() {
+      if (ctx != null) {
+        ctx.close();
+      }
+    }
+  }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
index 1582d43958..a605a7d240 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java
@@ -50,7 +50,7 @@ import java.util.Map;
 import java.util.Set;
 
 /** Access control management for a user accessing a single change. */
-class ChangeControl {
+public class ChangeControl {
   @Singleton
   static class Factory {
     private final ChangeData.Factory changeDataFactory;
@@ -87,6 +87,8 @@ class ChangeControl {
   private final ChangeNotes notes;
   private final PatchSetUtil patchSetUtil;
 
+  private ChangeData cd;
+
   ChangeControl(
       ChangeData.Factory changeDataFactory,
       ApprovalsUtil approvalsUtil,
@@ -128,17 +130,20 @@ class ChangeControl {
     return notes;
   }
 
-  /** Can this user see this change? */
-  private boolean isVisible(ReviewDb db, @Nullable ChangeData cd) throws OrmException {
-    if (getChange().isPrivate() && !isPrivateVisible(db, cd)) {
-      return false;
+  public ChangeControl setChangeData(@Nullable ChangeData cd) {
+    if (cd != null) {
+      this.cd = cd;
     }
-    return isRefVisible();
+    return this;
   }
 
-  /** Can the user see this change? Does not account for draft status */
-  private boolean isRefVisible() {
-    return getRefControl().isVisible();
+  /** Can this user see this change? */
+  public boolean isVisible(ReviewDb db) throws OrmException {
+    if (getChange().isPrivate() && !isPrivateVisible(db, changeData(db))) {
+      return false;
+    }
+    // Does the user have READ permission on the destination?
+    return refControl.asForRef().testOrFalse(RefPermission.READ);
   }
 
   /** Can this user abandon this change? */
@@ -237,7 +242,7 @@ class ChangeControl {
   /** Is this user a reviewer for the change? */
   private boolean isReviewer(ReviewDb db, @Nullable ChangeData cd) throws OrmException {
     if (getUser().isIdentifiedUser()) {
-      Collection<Account.Id> results = changeData(db, cd).reviewers().all();
+      Collection<Account.Id> results = setChangeData(cd).changeData(db).reviewers().all();
       return results.contains(getUser().getAccountId());
     }
     return false;
@@ -282,8 +287,8 @@ class ChangeControl {
         || getProjectControl().isAdmin();
   }
 
-  private ChangeData changeData(ReviewDb db, @Nullable ChangeData cd) {
-    return cd != null ? cd : changeDataFactory.create(db, getNotes());
+  private ChangeData changeData(ReviewDb db) {
+    return this.cd != null ? cd : changeDataFactory.create(db, getNotes());
   }
 
   private boolean isPrivateVisible(ReviewDb db, ChangeData cd) throws OrmException {
@@ -294,15 +299,13 @@ class ChangeControl {
   }
 
   ForChange asForChange(@Nullable ChangeData cd, @Nullable Provider<ReviewDb> db) {
-    return new ForChangeImpl(cd, db);
+    return new ForChangeImpl(db);
   }
 
   private class ForChangeImpl extends ForChange {
-    private ChangeData cd;
     private Map<String, PermissionRange> labels;
 
-    ForChangeImpl(@Nullable ChangeData cd, @Nullable Provider<ReviewDb> db) {
-      this.cd = cd;
+    ForChangeImpl(@Nullable Provider<ReviewDb> db) {
       this.db = db;
     }
 
@@ -370,7 +373,7 @@ class ChangeControl {
       try {
         switch (perm) {
           case READ:
-            return isVisible(db(), changeData());
+            return isVisible(db());
           case ABANDON:
             return canAbandon(db());
           case DELETE:
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
index 76e9e9816f..a0b1f678b8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
@@ -41,6 +41,8 @@ import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.account.GroupMembership;
 import com.google.gerrit.server.config.GitReceivePackGroups;
 import com.google.gerrit.server.config.GitUploadPackGroups;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.VisibleRefFilter;
 import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.permissions.FailedPermissionBackend;
@@ -51,6 +53,7 @@ import com.google.gerrit.server.permissions.PermissionBackend.ForProject;
 import com.google.gerrit.server.permissions.PermissionBackend.ForRef;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.permissions.RefVisibilityControl;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -134,6 +137,9 @@ public class ProjectControl {
   private final CommitsCollection commits;
   private final ChangeControl.Factory changeControlFactory;
   private final PermissionCollection.Factory permissionFilter;
+  private final RefVisibilityControl refVisibilityControl;
+  private final VisibleRefFilter.Factory visibleRefFilterFactory;
+  private final GitRepositoryManager gitRepositoryManager;
 
   private List<SectionMatcher> allSections;
   private Map<String, RefControl> refControls;
@@ -147,6 +153,9 @@ public class ProjectControl {
       CommitsCollection commits,
       ChangeControl.Factory changeControlFactory,
       PermissionBackend permissionBackend,
+      RefVisibilityControl refVisibilityControl,
+      GitRepositoryManager gitRepositoryManager,
+      VisibleRefFilter.Factory visibleRefFilterFactory,
       @Assisted CurrentUser who,
       @Assisted ProjectState ps) {
     this.changeControlFactory = changeControlFactory;
@@ -155,6 +164,9 @@ public class ProjectControl {
     this.permissionFilter = permissionFilter;
     this.commits = commits;
     this.perm = permissionBackend.user(who);
+    this.refVisibilityControl = refVisibilityControl;
+    this.gitRepositoryManager = gitRepositoryManager;
+    this.visibleRefFilterFactory = visibleRefFilterFactory;
     user = who;
     state = ps;
   }
@@ -186,7 +198,14 @@ public class ProjectControl {
     RefControl ctl = refControls.get(refName);
     if (ctl == null) {
       PermissionCollection relevant = permissionFilter.filter(access(), refName, user);
-      ctl = new RefControl(this, refName, relevant);
+      ctl =
+          new RefControl(
+              visibleRefFilterFactory,
+              refVisibilityControl,
+              this,
+              gitRepositoryManager,
+              refName,
+              relevant);
       refControls.put(refName, ctl);
     }
     return ctl;
@@ -428,11 +447,11 @@ public class ProjectControl {
     }
   }
 
-  ForProject asForProject() {
+  public ForProject asForProject() {
     return new ForProjectImpl();
   }
 
-  private class ForProjectImpl extends ForProject {
+  public class ForProjectImpl extends ForProject {
     @Override
     public ForProject user(CurrentUser user) {
       return forUser(user).asForProject().database(db);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index cde2c8098e..2ba5e0f6d2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/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.common.collect.ImmutableMap;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.common.data.PermissionRange;
 import com.google.gerrit.common.data.PermissionRule;
@@ -24,15 +25,20 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.VisibleRefFilter;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.permissions.FailedPermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
 import com.google.gerrit.server.permissions.PermissionBackend.ForRef;
 import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.permissions.RefPermission;
+import com.google.gerrit.server.permissions.RefVisibilityControl;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.util.Providers;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -42,10 +48,16 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
 
 /** Manages access control for Git references (aka branches, tags). */
 public class RefControl {
+  private final VisibleRefFilter.Factory visibleRefFilterFactory;
+  private final RefVisibilityControl refVisibilityControl;
   private final ProjectControl projectControl;
+  private final GitRepositoryManager repositoryManager;
   private final String refName;
 
   /** All permissions that apply to this reference. */
@@ -57,10 +69,19 @@ public class RefControl {
   private Boolean owner;
   private Boolean canForgeAuthor;
   private Boolean canForgeCommitter;
-  private Boolean isVisible;
-
-  RefControl(ProjectControl projectControl, String ref, PermissionCollection relevant) {
+  private Boolean hasReadPermissionOnRef;
+
+  RefControl(
+      VisibleRefFilter.Factory visibleRefFilterFactory,
+      RefVisibilityControl refVisibilityControl,
+      ProjectControl projectControl,
+      GitRepositoryManager repositoryManager,
+      String ref,
+      PermissionCollection relevant) {
+    this.visibleRefFilterFactory = visibleRefFilterFactory;
+    this.refVisibilityControl = refVisibilityControl;
     this.projectControl = projectControl;
+    this.repositoryManager = repositoryManager;
     this.refName = ref;
     this.relevant = relevant;
     this.effective = new HashMap<>();
@@ -83,7 +104,13 @@ public class RefControl {
     if (relevant.isUserSpecific()) {
       return newCtl.controlForRef(getRefName());
     }
-    return new RefControl(newCtl, getRefName(), relevant);
+    return new RefControl(
+        visibleRefFilterFactory,
+        refVisibilityControl,
+        newCtl,
+        repositoryManager,
+        getRefName(),
+        relevant);
   }
 
   /** Is this user a ref owner? */
@@ -99,14 +126,24 @@ public class RefControl {
     return owner;
   }
 
-  /** Can this user see this reference exists? */
-  boolean isVisible() {
-    if (isVisible == null) {
-      isVisible =
+  /**
+   * Returns {@code true} if the user has permission to read the ref. This method evaluates {@link
+   * RefPermission#READ} only. Hence, it is not authoritative. For example, it does not tell if the
+   * user can see NoteDb refs such as {@code refs/meta/external-ids} which requires {@link
+   * GlobalPermission#ACCESS_DATABASE} and deny access in this case.
+   */
+  public boolean hasReadPermissionOnRef(boolean allowNoteDbRefs) {
+    // Don't allow checking for NoteDb refs unless instructed otherwise.
+    if (!allowNoteDbRefs
+        && (refName.startsWith(Constants.R_TAGS) || RefNames.isGerritRef(refName))) {
+      return false;
+    }
+    if (hasReadPermissionOnRef == null) {
+      hasReadPermissionOnRef =
           (getUser().isInternalUser() || canPerform(Permission.READ))
               && isProjectStatePermittingRead();
     }
-    return isVisible;
+    return hasReadPermissionOnRef;
   }
 
   /** Can this user see other users change edits? */
@@ -552,7 +589,10 @@ public class RefControl {
     private boolean can(RefPermission perm) throws PermissionBackendException {
       switch (perm) {
         case READ:
-          return isVisible();
+          if (refName.startsWith(Constants.R_TAGS)) {
+            return isTagVisible();
+          }
+          return refVisibilityControl.isVisible(projectControl, refName);
         case CREATE:
           // TODO This isn't an accurate test.
           return canPerform(perm.permissionName().get());
@@ -588,4 +628,34 @@ public class RefControl {
       throw new PermissionBackendException(perm + " unsupported");
     }
   }
+
+  private boolean isTagVisible() throws PermissionBackendException {
+    if (projectControl.asForProject().test(ProjectPermission.READ)) {
+      // The user has READ on refs/* with no effective block permission. This is the broadest
+      // permission one can assign. There is no way to grant access to (specific) tags in Gerrit,
+      // so we have to assume that these users can see all tags because there could be tags that
+      // aren't reachable by any visible ref while the user can see all non-Gerrit refs. This
+      // matches Gerrit's historic behavior.
+      // This makes it so that these users could see commits that they can't see otherwise
+      // (e.g. a private change ref) if a tag was attached to it. Tags are meant to be used on
+      // the regular Git tree that users interact with, not on any of the Gerrit trees, so this
+      // is a negligible risk.
+      return true;
+    }
+
+    try (Repository repo =
+        repositoryManager.openRepository(projectControl.getProject().getNameKey())) {
+      // Tag visibility requires going through RefFilter because it entails loading all taggable
+      // refs and filtering them all by visibility.
+      Ref resolvedRef = repo.getRefDatabase().exactRef(refName);
+      if (resolvedRef == null) {
+        return false;
+      }
+      return visibleRefFilterFactory.create(projectControl.getProjectState(), repo)
+          .filter(ImmutableMap.of(resolvedRef.getName(), resolvedRef), true).values().stream()
+          .anyMatch(r -> refName.equals(r.getName()));
+    } catch (IOException e) {
+      throw new PermissionBackendException(e);
+    }
+  }
 }
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
index 1fc95c18bc..dff5af07a9 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
@@ -55,12 +55,15 @@ import com.google.gerrit.server.config.AllProjectsNameProvider;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.AllUsersNameProvider;
 import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.git.TransferConfig;
+import com.google.gerrit.server.git.VisibleRefFilter;
 import com.google.gerrit.server.index.SingleVersionModule.SingleVersionListener;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.permissions.RefPermission;
+import com.google.gerrit.server.permissions.RefVisibilityControl;
 import com.google.gerrit.server.schema.SchemaCreator;
 import com.google.gerrit.server.util.RequestContext;
 import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -118,11 +121,13 @@ public class RefControlTest {
   }
 
   private void assertCanRead(String ref, ProjectControl u) {
-    assertThat(u.controlForRef(ref).isVisible()).named("can read " + ref).isTrue();
+    assertThat(u.controlForRef(ref).hasReadPermissionOnRef(true)).named("can read " + ref).isTrue();
   }
 
   private void assertCannotRead(String ref, ProjectControl u) {
-    assertThat(u.controlForRef(ref).isVisible()).named("cannot read " + ref).isFalse();
+    assertThat(u.controlForRef(ref).hasReadPermissionOnRef(true))
+        .named("cannot read " + ref)
+        .isFalse();
   }
 
   private void assertCanSubmit(String ref, ProjectControl u) {
@@ -209,6 +214,9 @@ public class RefControlTest {
   @Inject private InMemoryDatabase schemaFactory;
   @Inject private ThreadLocalRequestContext requestContext;
   @Inject private TransferConfig transferConfig;
+  @Inject private RefVisibilityControl refVisibilityControl;
+  @Inject private GitRepositoryManager gitRepositoryManager;
+  @Inject private VisibleRefFilter.Factory visibleRefFilterFactory;
 
   @Before
   public void setUp() throws Exception {
@@ -886,6 +894,9 @@ public class RefControlTest {
         null, // commitsCollection
         changeControlFactory,
         permissionBackend,
+        refVisibilityControl,
+        gitRepositoryManager,
+        visibleRefFilterFactory,
         new MockUser(name, memberOf),
         newProjectState(local));
   }
diff --git a/tools/BUILD b/tools/BUILD
index 646414037a..094cb0149f 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -48,7 +48,7 @@ java_package_configuration(
         "-Xep:ElementsCountedInLoop:WARN",
         "-Xep:EqualsHashCode:WARN",
         "-Xep:EqualsIncompatibleType:WARN",
-        "-Xep:ExpectedExceptionChecker:ERROR",
+        "-Xep:ExpectedExceptionChecker:WARN",
         "-Xep:Finally:WARN",
         "-Xep:FloatingPointLiteralPrecision:WARN",
         "-Xep:FragmentInjection:WARN",