diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java index c06b0ff918..821f968795 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java @@ -27,9 +27,12 @@ import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.Permission; import com.google.gerrit.extensions.api.projects.BranchInput; import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.config.AllProjectsName; +import com.google.gerrit.server.edit.ChangeEditModifier; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.notedb.NotesMigration; @@ -69,6 +72,9 @@ public class VisibleRefFilterIT extends AbstractDaemonTest { @Inject private GroupCache groupCache; + @Inject + private ChangeEditModifier editModifier; + private AccountGroup.UUID admins; @Before @@ -185,6 +191,31 @@ public class VisibleRefFilterIT extends AbstractDaemonTest { "refs/tags/master-tag"); } + @Test + public void subsetOfBranchesVisibleWithEdit() throws Exception { + allow(Permission.READ, REGISTERED_USERS, "refs/heads/master"); + deny(Permission.READ, REGISTERED_USERS, "refs/heads/branch"); + + Change c1 = db.changes().get(new Change.Id(1)); + PatchSet ps1 = db.patchSets().get(new PatchSet.Id(c1.getId(), 1)); + + // Admin's edit is not visible. + setApiUser(admin); + editModifier.createEdit(c1, ps1); + + // User's edit is visible. + setApiUser(user); + editModifier.createEdit(c1, ps1); + + assertRefs( + "HEAD", + "refs/changes/01/1/1", + "refs/changes/01/1/meta", + "refs/heads/master", + "refs/tags/master-tag", + "refs/users/01/1000001/edit-1"); + } + /** * Assert that refs seen by a non-admin user match expected. * diff --git a/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/AccountTest.java b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/AccountTest.java index eaf17437f9..e8dc5e01e7 100644 --- a/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/AccountTest.java +++ b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/AccountTest.java @@ -25,6 +25,7 @@ public class AccountTest { assertRef(1, "refs/users/01/1"); assertRef(1, "refs/users/01/1-drafts"); assertRef(1, "refs/users/01/1-drafts/2"); + assertRef(1, "refs/users/01/1/edit/2"); assertNotRef(null); assertNotRef(""); 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 7bc457acdd..e43b39f3dd 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 @@ -16,10 +16,12 @@ package com.google.gerrit.server.git; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; +import com.google.gerrit.reviewdb.client.Account; 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.reviewdb.server.ReviewDb; +import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.project.ProjectControl; import com.google.gwtorm.server.OrmException; @@ -76,14 +78,30 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook { return r; } - final Set visibleChanges = visibleChanges(); - final Map result = new HashMap<>(); - final List deferredTags = new ArrayList<>(); + Account.Id currAccountId = projectCtl.getCurrentUser().isIdentifiedUser() + ? ((IdentifiedUser) projectCtl.getCurrentUser()).getAccountId() + : null; + + Set visibleChanges = visibleChanges(); + Map result = new HashMap<>(); + List deferredTags = new ArrayList<>(); for (Ref ref : refs.values()) { Change.Id changeId; + Account.Id accountId; if (ref.getName().startsWith(RefNames.REFS_CACHE_AUTOMERGE)) { continue; + } else if ((accountId = Account.Id.fromRef(ref.getName())) != null) { + // Reference related to an account is visible only for the current + // account. + // + // TODO(dborowitz): If a ref matches an account and a change, verify + // both (to exclude e.g. edits on changes that the user has lost access + // to). + if (accountId.equals(currAccountId)) { + result.put(ref.getName(), ref); + } + } else if ((changeId = Change.Id.fromRef(ref.getName())) != null) { // Reference related to a change is visible if the change is visible. //