Allow users with accessDatabase to view metadata refs
Administrators may want to inspect the full ref state of a repository without having direct git access. Reuse the accessDatabase capability within VisibleRefFilter for this purpose. Users with this capability can access basically all data on the server, so we are not giving them significantly more visibility. Change-Id: I592557528915ab216acce5fa7e057df8f2fc1640
This commit is contained in:
parent
9c1393279f
commit
d9b8b39088
@ -1145,7 +1145,8 @@ Below you find a list of capabilities available:
|
||||
[[capability_accessDatabase]]
|
||||
=== Access Database
|
||||
|
||||
Allow users to access the database using the `gsql` command.
|
||||
Allow users to access the database using the `gsql` command, and view code
|
||||
review metadata refs in repositories.
|
||||
|
||||
|
||||
[[capability_administrateServer]]
|
||||
|
@ -33,6 +33,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.IdentifiedUser;
|
||||
import com.google.gerrit.server.OutputFormat;
|
||||
import com.google.gerrit.server.change.ChangeJson;
|
||||
import com.google.gerrit.server.config.AllProjectsName;
|
||||
import com.google.gerrit.server.git.MetaDataUpdate;
|
||||
import com.google.gerrit.server.git.ProjectConfig;
|
||||
import com.google.gerrit.server.project.ProjectCache;
|
||||
@ -65,6 +66,9 @@ public abstract class AbstractDaemonTest {
|
||||
@ConfigSuite.Parameter
|
||||
public Config baseConfig;
|
||||
|
||||
@Inject
|
||||
protected AllProjectsName allProjects;
|
||||
|
||||
@Inject
|
||||
protected AccountCreator accounts;
|
||||
|
||||
@ -238,6 +242,13 @@ public abstract class AbstractDaemonTest {
|
||||
saveProjectConfig(project, cfg);
|
||||
}
|
||||
|
||||
protected void allowGlobalCapability(String capabilityName,
|
||||
AccountGroup.UUID id) throws Exception {
|
||||
ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig();
|
||||
Util.allow(cfg, capabilityName, id);
|
||||
saveProjectConfig(allProjects, cfg);
|
||||
}
|
||||
|
||||
protected void deny(String permission, AccountGroup.UUID id, String ref)
|
||||
throws Exception {
|
||||
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
|
||||
|
@ -24,6 +24,7 @@ import com.google.common.collect.Ordering;
|
||||
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
||||
import com.google.gerrit.acceptance.PushOneCommit;
|
||||
import com.google.gerrit.common.data.AccessSection;
|
||||
import com.google.gerrit.common.data.GlobalCapability;
|
||||
import com.google.gerrit.common.data.Permission;
|
||||
import com.google.gerrit.extensions.api.projects.BranchInput;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
@ -31,7 +32,6 @@ 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;
|
||||
@ -60,9 +60,6 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
|
||||
return cfg;
|
||||
}
|
||||
|
||||
@Inject
|
||||
private AllProjectsName allProjects;
|
||||
|
||||
@Inject
|
||||
private NotesMigration notesMigration;
|
||||
|
||||
@ -216,6 +213,35 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
|
||||
"refs/users/01/1000001/edit-1");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void subsetOfRefsVisibleWithAccessDatabase() throws Exception {
|
||||
deny(Permission.READ, REGISTERED_USERS, "refs/heads/master");
|
||||
allow(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
|
||||
allowGlobalCapability(GlobalCapability.ACCESS_DATABASE, REGISTERED_USERS);
|
||||
|
||||
Change c1 = db.changes().get(new Change.Id(1));
|
||||
PatchSet ps1 = db.patchSets().get(new PatchSet.Id(c1.getId(), 1));
|
||||
setApiUser(admin);
|
||||
editModifier.createEdit(c1, ps1);
|
||||
setApiUser(user);
|
||||
editModifier.createEdit(c1, ps1);
|
||||
|
||||
assertRefs(
|
||||
// Change 1 is visible due to accessDatabase capability, even though
|
||||
// refs/heads/master is not.
|
||||
"refs/changes/01/1/1",
|
||||
"refs/changes/01/1/meta",
|
||||
"refs/changes/02/2/1",
|
||||
"refs/changes/02/2/meta",
|
||||
"refs/heads/branch",
|
||||
"refs/tags/branch-tag",
|
||||
// See comment in subsetOfBranchesVisibleNotIncludingHead.
|
||||
"refs/tags/master-tag",
|
||||
// All edits are visible due to accessDatabase capability.
|
||||
"refs/users/00/1000000/edit-1",
|
||||
"refs/users/01/1000001/edit-1");
|
||||
}
|
||||
|
||||
/**
|
||||
* Assert that refs seen by a non-admin user match expected.
|
||||
*
|
||||
|
@ -54,19 +54,18 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
private final Project.NameKey projectName;
|
||||
private final ProjectControl projectCtl;
|
||||
private final ReviewDb reviewDb;
|
||||
private final boolean showChanges;
|
||||
private final boolean showMetadata;
|
||||
|
||||
public VisibleRefFilter(final TagCache tagCache, final ChangeCache changeCache,
|
||||
final Repository db,
|
||||
final ProjectControl projectControl, final ReviewDb reviewDb,
|
||||
final boolean showChanges) {
|
||||
public VisibleRefFilter(TagCache tagCache, ChangeCache changeCache,
|
||||
Repository db, ProjectControl projectControl, ReviewDb reviewDb,
|
||||
boolean showMetadata) {
|
||||
this.tagCache = tagCache;
|
||||
this.changeCache = changeCache;
|
||||
this.db = db;
|
||||
this.projectName = projectControl.getProject().getNameKey();
|
||||
this.projectCtl = projectControl;
|
||||
this.reviewDb = reviewDb;
|
||||
this.showChanges = showChanges;
|
||||
this.showMetadata = showMetadata;
|
||||
}
|
||||
|
||||
public Map<String, Ref> filter(Map<String, Ref> refs, boolean filterTagsSeperately) {
|
||||
@ -78,9 +77,16 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
return r;
|
||||
}
|
||||
|
||||
Account.Id currAccountId = projectCtl.getCurrentUser().isIdentifiedUser()
|
||||
? ((IdentifiedUser) projectCtl.getCurrentUser()).getAccountId()
|
||||
: null;
|
||||
Account.Id currAccountId;
|
||||
boolean canViewMetadata;
|
||||
if (projectCtl.getCurrentUser().isIdentifiedUser()) {
|
||||
IdentifiedUser user = ((IdentifiedUser) projectCtl.getCurrentUser());
|
||||
currAccountId = user.getAccountId();
|
||||
canViewMetadata = user.getCapabilities().canAccessDatabase();
|
||||
} else {
|
||||
currAccountId = null;
|
||||
canViewMetadata = false;
|
||||
}
|
||||
|
||||
Set<Change.Id> visibleChanges = visibleChanges();
|
||||
Map<String, Ref> result = new HashMap<>();
|
||||
@ -98,14 +104,16 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
// 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)) {
|
||||
if (showMetadata
|
||||
&& (canViewMetadata || 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.
|
||||
//
|
||||
if (showChanges && visibleChanges.contains(changeId)) {
|
||||
if (showMetadata
|
||||
&& (canViewMetadata || visibleChanges.contains(changeId))) {
|
||||
result.put(ref.getName(), ref);
|
||||
}
|
||||
|
||||
@ -162,7 +170,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
}
|
||||
|
||||
private Set<Change.Id> visibleChanges() {
|
||||
if (!showChanges) {
|
||||
if (!showMetadata) {
|
||||
return Collections.emptySet();
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user