Filter out user-specific refs in VisibleRefFilter

If we can extract an account ID from a ref, then that ref is visible
if and only if the extracted account ID matches the current user.

Change-Id: I6bde71ad4d26849a742d873808230b871c0820b1
This commit is contained in:
Dave Borowitz
2014-09-10 12:32:41 +02:00
parent 418936d725
commit abe0b927eb
3 changed files with 53 additions and 3 deletions

View File

@@ -27,9 +27,12 @@ import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.projects.BranchInput; import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.reviewdb.client.AccountGroup; 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.reviewdb.client.RefNames;
import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.config.AllProjectsName; 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.GitRepositoryManager;
import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.notedb.NotesMigration;
@@ -69,6 +72,9 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
@Inject @Inject
private GroupCache groupCache; private GroupCache groupCache;
@Inject
private ChangeEditModifier editModifier;
private AccountGroup.UUID admins; private AccountGroup.UUID admins;
@Before @Before
@@ -185,6 +191,31 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
"refs/tags/master-tag"); "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. * Assert that refs seen by a non-admin user match expected.
* *

View File

@@ -25,6 +25,7 @@ public class AccountTest {
assertRef(1, "refs/users/01/1"); assertRef(1, "refs/users/01/1");
assertRef(1, "refs/users/01/1-drafts"); assertRef(1, "refs/users/01/1-drafts");
assertRef(1, "refs/users/01/1-drafts/2"); assertRef(1, "refs/users/01/1-drafts/2");
assertRef(1, "refs/users/01/1/edit/2");
assertNotRef(null); assertNotRef(null);
assertNotRef(""); assertNotRef("");

View File

@@ -16,10 +16,12 @@ package com.google.gerrit.server.git;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps; 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.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectControl;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -76,14 +78,30 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
return r; return r;
} }
final Set<Change.Id> visibleChanges = visibleChanges(); Account.Id currAccountId = projectCtl.getCurrentUser().isIdentifiedUser()
final Map<String, Ref> result = new HashMap<>(); ? ((IdentifiedUser) projectCtl.getCurrentUser()).getAccountId()
final List<Ref> deferredTags = new ArrayList<>(); : null;
Set<Change.Id> visibleChanges = visibleChanges();
Map<String, Ref> result = new HashMap<>();
List<Ref> deferredTags = new ArrayList<>();
for (Ref ref : refs.values()) { for (Ref ref : refs.values()) {
Change.Id changeId; Change.Id changeId;
Account.Id accountId;
if (ref.getName().startsWith(RefNames.REFS_CACHE_AUTOMERGE)) { if (ref.getName().startsWith(RefNames.REFS_CACHE_AUTOMERGE)) {
continue; 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) { } else if ((changeId = Change.Id.fromRef(ref.getName())) != null) {
// Reference related to a change is visible if the change is visible. // Reference related to a change is visible if the change is visible.
// //