Hide RefControl#isVisible and migrate callers to PermissionBackend

Change-Id: Iddbb691ecbad1cbf2449095bf4efdcf7188d6c18
This commit is contained in:
Patrick Hiesel 2017-08-09 13:27:58 +02:00
parent 57945a0f41
commit 4d1a89add5
16 changed files with 121 additions and 46 deletions

View File

@ -33,9 +33,9 @@ import com.google.gerrit.server.permissions.ChangePermission;
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.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@ -93,7 +93,8 @@ public class EventBroker implements EventDispatcher {
}
@Override
public void postEvent(Branch.NameKey branchName, RefEvent event) {
public void postEvent(Branch.NameKey branchName, RefEvent event)
throws PermissionBackendException {
fireEvent(branchName, event);
}
@ -132,7 +133,8 @@ public class EventBroker implements EventDispatcher {
fireEventForUnrestrictedListeners(event);
}
protected void fireEvent(Branch.NameKey branchName, RefEvent event) {
protected void fireEvent(Branch.NameKey branchName, RefEvent event)
throws PermissionBackendException {
for (UserScopedEventListener listener : listeners) {
if (isVisibleTo(branchName, listener.getUser())) {
listener.onEvent(event);
@ -176,13 +178,13 @@ public class EventBroker implements EventDispatcher {
.test(ChangePermission.READ);
}
protected boolean isVisibleTo(Branch.NameKey branchName, CurrentUser user) {
protected boolean isVisibleTo(Branch.NameKey branchName, CurrentUser user)
throws PermissionBackendException {
ProjectState pe = projectCache.get(branchName.getParentKey());
if (pe == null) {
return false;
}
ProjectControl pc = pe.controlFor(user);
return pc.controlForRef(branchName).isVisible();
return permissionBackend.user(user).ref(branchName).test(RefPermission.READ);
}
protected boolean isVisibleTo(Event event, CurrentUser user)

View File

@ -41,8 +41,9 @@ public interface EventDispatcher {
*
* @param branchName The branch that the event is related to
* @param event The event to post
* @throws PermissionBackendException on failure of permission checks
*/
void postEvent(Branch.NameKey branchName, RefEvent event);
void postEvent(Branch.NameKey branchName, RefEvent event) throws PermissionBackendException;
/**
* Post a stream event that is related to a project.

View File

@ -384,7 +384,11 @@ public class StreamEventsApiListener
refName);
}
});
dispatcher.get().postEvent(refName, event);
try {
dispatcher.get().postEvent(refName, event);
} catch (PermissionBackendException e) {
log.error("error while posting event", e);
}
}
@Override

View File

@ -21,6 +21,7 @@ import static com.google.gerrit.reviewdb.client.RefNames.REFS_USERS_SELF;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
@ -34,6 +35,7 @@ import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
@ -147,8 +149,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
}
} else if ((accountId = Account.Id.fromRef(name)) != null) {
// Account ref is visible only to corresponding account.
if (viewMetadata
|| (accountId.equals(userId) && projectCtl.controlForRef(name).isVisible())) {
if (viewMetadata || (accountId.equals(userId) && canReadRef(name))) {
result.put(name, ref);
}
} else if (isTag(ref)) {
@ -166,7 +167,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
if (viewMetadata) {
result.put(name, ref);
}
} else if (projectCtl.controlForRef(ref.getLeaf().getName()).isVisible()) {
} 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.
@ -196,7 +197,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
}
private Map<String, Ref> fastHideRefsMetaConfig(Map<String, Ref> refs) {
if (refs.containsKey(REFS_CONFIG) && !projectCtl.controlForRef(REFS_CONFIG).isVisible()) {
if (refs.containsKey(REFS_CONFIG) && !canReadRef(REFS_CONFIG)) {
Map<String, Ref> r = new HashMap<>(refs);
r.remove(REFS_CONFIG);
return r;
@ -309,4 +310,20 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
private static boolean isTag(Ref ref) {
return ref.getLeaf().getName().startsWith(Constants.R_TAGS);
}
private boolean canReadRef(String ref) {
try {
permissionBackend
.user(user)
.project(projectCtl.getProject().getNameKey())
.ref(ref)
.check(RefPermission.READ);
} catch (AuthException e) {
return false;
} catch (PermissionBackendException e) {
log.error("unable to check permissions", e);
return false;
}
return true;
}
}

View File

@ -23,6 +23,7 @@ import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AcceptsCreate;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@ -34,6 +35,9 @@ import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.UrlEncoded;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.gson.annotations.SerializedName;
import com.google.inject.Inject;
import com.google.inject.Provider;
@ -57,17 +61,20 @@ class DashboardsCollection
private final DynamicMap<RestView<DashboardResource>> views;
private final Provider<ListDashboards> list;
private final Provider<SetDefaultDashboard.CreateDefault> createDefault;
private final PermissionBackend permissionBackend;
@Inject
DashboardsCollection(
GitRepositoryManager gitManager,
DynamicMap<RestView<DashboardResource>> views,
Provider<ListDashboards> list,
Provider<SetDefaultDashboard.CreateDefault> createDefault) {
Provider<SetDefaultDashboard.CreateDefault> createDefault,
PermissionBackend permissionBackend) {
this.gitManager = gitManager;
this.views = views;
this.list = list;
this.createDefault = createDefault;
this.permissionBackend = permissionBackend;
}
@Override
@ -87,7 +94,8 @@ class DashboardsCollection
@Override
public DashboardResource parse(ProjectResource parent, IdString id)
throws ResourceNotFoundException, IOException, ConfigInvalidException {
throws ResourceNotFoundException, IOException, ConfigInvalidException,
PermissionBackendException {
ProjectControl myCtl = parent.getControl();
if (id.toString().equals("default")) {
return DashboardResource.projectDefault(myCtl);
@ -115,12 +123,22 @@ class DashboardsCollection
private DashboardResource parse(ProjectControl ctl, String ref, String path, ProjectControl myCtl)
throws ResourceNotFoundException, IOException, AmbiguousObjectException,
IncorrectObjectTypeException, ConfigInvalidException {
IncorrectObjectTypeException, ConfigInvalidException, PermissionBackendException {
String id = ref + ":" + path;
if (!ref.startsWith(REFS_DASHBOARDS)) {
ref = REFS_DASHBOARDS + ref;
}
if (!Repository.isValidRefName(ref) || !ctl.controlForRef(ref).isVisible()) {
try {
permissionBackend
.user(ctl.getUser())
.project(ctl.getProject().getNameKey())
.ref(ref)
.check(RefPermission.READ);
} catch (AuthException e) {
// Don't leak the project's existence
throw new ResourceNotFoundException(id);
}
if (!Repository.isValidRefName(ref)) {
throw new ResourceNotFoundException(id);
}

View File

@ -21,6 +21,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.DashboardsCollection.DashboardInfo;
import com.google.inject.Inject;
import com.google.inject.Provider;
@ -39,7 +40,8 @@ class DeleteDashboard implements RestModifyView<DashboardResource, SetDashboard.
@Override
public Response<DashboardInfo> apply(DashboardResource resource, SetDashboard.Input input)
throws AuthException, BadRequestException, ResourceConflictException,
ResourceNotFoundException, MethodNotAllowedException, IOException {
ResourceNotFoundException, MethodNotAllowedException, IOException,
PermissionBackendException {
if (resource.isProjectDefault()) {
SetDashboard.Input in = new SetDashboard.Input();
in.commitMessage = input != null ? input.commitMessage : null;

View File

@ -17,6 +17,7 @@ package com.google.gerrit.server.project;
import com.google.gerrit.extensions.api.projects.BranchInfo;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@ -32,7 +33,8 @@ public class GetBranch implements RestReadView<BranchResource> {
}
@Override
public BranchInfo apply(BranchResource rsrc) throws ResourceNotFoundException, IOException {
public BranchInfo apply(BranchResource rsrc)
throws ResourceNotFoundException, IOException, PermissionBackendException {
return list.get().toBranchInfo(rsrc);
}
}

View File

@ -24,6 +24,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.DashboardsCollection.DashboardInfo;
import com.google.inject.Inject;
import java.io.IOException;
@ -44,7 +45,8 @@ class GetDashboard implements RestReadView<DashboardResource> {
@Override
public DashboardInfo apply(DashboardResource resource)
throws ResourceNotFoundException, ResourceConflictException, IOException {
throws ResourceNotFoundException, ResourceConflictException, IOException,
PermissionBackendException {
if (inherited && !resource.isProjectDefault()) {
// inherited flag can only be used with default.
throw new ResourceNotFoundException("inherited");
@ -70,7 +72,8 @@ class GetDashboard implements RestReadView<DashboardResource> {
}
private DashboardResource defaultOf(ProjectControl ctl)
throws ResourceNotFoundException, IOException, ConfigInvalidException {
throws ResourceNotFoundException, IOException, ConfigInvalidException,
PermissionBackendException {
String id = ctl.getProject().getLocalDefaultDashboard();
if (Strings.isNullOrEmpty(id)) {
id = ctl.getProject().getDefaultDashboard();
@ -96,7 +99,8 @@ class GetDashboard implements RestReadView<DashboardResource> {
}
private DashboardResource parse(ProjectControl ctl, String id)
throws ResourceNotFoundException, IOException, ConfigInvalidException {
throws ResourceNotFoundException, IOException, ConfigInvalidException,
PermissionBackendException {
List<String> p = Lists.newArrayList(Splitter.on(':').limit(2).split(id));
String ref = Url.encode(p.get(0));
String path = Url.encode(p.get(1));

View File

@ -18,6 +18,9 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@ -34,26 +37,33 @@ import org.eclipse.jgit.revwalk.RevWalk;
public class GetHead implements RestReadView<ProjectResource> {
private final GitRepositoryManager repoManager;
private final CommitsCollection commits;
private final PermissionBackend permissionBackend;
@Inject
GetHead(GitRepositoryManager repoManager, CommitsCollection commits) {
GetHead(
GitRepositoryManager repoManager,
CommitsCollection commits,
PermissionBackend permissionBackend) {
this.repoManager = repoManager;
this.commits = commits;
this.permissionBackend = permissionBackend;
}
@Override
public String apply(ProjectResource rsrc)
throws AuthException, ResourceNotFoundException, IOException {
throws AuthException, ResourceNotFoundException, IOException, PermissionBackendException {
try (Repository repo = repoManager.openRepository(rsrc.getNameKey())) {
Ref head = repo.getRefDatabase().exactRef(Constants.HEAD);
if (head == null) {
throw new ResourceNotFoundException(Constants.HEAD);
} else if (head.isSymbolic()) {
String n = head.getTarget().getName();
if (rsrc.getControl().controlForRef(n).isVisible()) {
return n;
}
throw new AuthException("not allowed to see HEAD");
permissionBackend
.user(rsrc.getControl().getUser())
.project(rsrc.getNameKey())
.ref(n)
.check(RefPermission.READ);
return n;
} else if (head.getObjectId() != null) {
try (RevWalk rw = new RevWalk(repo)) {
RevCommit commit = rw.parseCommit(head.getObjectId());

View File

@ -32,6 +32,7 @@ import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.extensions.webui.UiActions;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.inject.Inject;
import com.google.inject.Provider;
@ -120,7 +121,8 @@ public class ListBranches implements RestReadView<ProjectResource> {
@Override
public List<BranchInfo> apply(ProjectResource rsrc)
throws ResourceNotFoundException, IOException, BadRequestException {
throws ResourceNotFoundException, IOException, BadRequestException,
PermissionBackendException {
return new RefFilter<BranchInfo>(Constants.R_HEADS)
.subString(matchSubstring)
.regex(matchRegex)
@ -129,7 +131,8 @@ public class ListBranches implements RestReadView<ProjectResource> {
.filter(allBranches(rsrc));
}
BranchInfo toBranchInfo(BranchResource rsrc) throws IOException, ResourceNotFoundException {
BranchInfo toBranchInfo(BranchResource rsrc)
throws IOException, ResourceNotFoundException, PermissionBackendException {
try (Repository db = repoManager.openRepository(rsrc.getNameKey())) {
Ref r = db.exactRef(rsrc.getRef());
if (r == null) {
@ -142,7 +145,7 @@ public class ListBranches implements RestReadView<ProjectResource> {
}
private List<BranchInfo> allBranches(ProjectResource rsrc)
throws IOException, ResourceNotFoundException {
throws IOException, ResourceNotFoundException, PermissionBackendException {
List<Ref> refs;
try (Repository db = repoManager.openRepository(rsrc.getNameKey())) {
Collection<Ref> heads = db.getRefDatabase().getRefs(Constants.R_HEADS).values();
@ -158,7 +161,8 @@ public class ListBranches implements RestReadView<ProjectResource> {
return toBranchInfo(rsrc, refs);
}
private List<BranchInfo> toBranchInfo(ProjectResource rsrc, List<Ref> refs) {
private List<BranchInfo> toBranchInfo(ProjectResource rsrc, List<Ref> refs)
throws PermissionBackendException {
Set<String> targets = Sets.newHashSetWithExpectedSize(1);
for (Ref ref : refs) {
if (ref.isSymbolic()) {
@ -175,8 +179,7 @@ public class ListBranches implements RestReadView<ProjectResource> {
// showing the resolved value, show the name it references.
//
String target = ref.getTarget().getName();
RefControl targetRefControl = pctl.controlForRef(target);
if (!targetRefControl.isVisible()) {
if (!perm.ref(target).test(RefPermission.READ)) {
continue;
}
if (target.startsWith(Constants.R_HEADS)) {
@ -194,7 +197,7 @@ public class ListBranches implements RestReadView<ProjectResource> {
continue;
}
if (pctl.controlForRef(ref.getName()).isVisible()) {
if (perm.ref(ref.getName()).test(RefPermission.READ)) {
branches.add(createBranchInfo(perm.ref(ref.getName()), ref, pctl, targets));
}
}

View File

@ -24,6 +24,7 @@ import com.google.gerrit.server.git.GitRepositoryManager;
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.project.DashboardsCollection.DashboardInfo;
import com.google.inject.Inject;
import com.google.inject.Provider;
@ -100,13 +101,15 @@ class ListDashboards implements RestReadView<ProjectResource> {
}
private List<DashboardInfo> scan(ProjectControl ctl, String project, boolean setDefault)
throws ResourceNotFoundException, IOException {
throws ResourceNotFoundException, IOException, PermissionBackendException {
Project.NameKey projectName = ctl.getProject().getNameKey();
PermissionBackend.ForProject perm =
permissionBackend.user(user).project(ctl.getProject().getNameKey());
try (Repository git = gitManager.openRepository(projectName);
RevWalk rw = new RevWalk(git)) {
List<DashboardInfo> all = new ArrayList<>();
for (Ref ref : git.getRefDatabase().getRefs(REFS_DASHBOARDS).values()) {
if (ctl.controlForRef(ref.getName()).isVisible()) {
if (perm.ref(ref.getName()).test(RefPermission.READ)) {
all.addAll(scanDashboards(ctl.getProject(), git, rw, ref, project, setDefault));
}
}

View File

@ -46,6 +46,7 @@ import com.google.gerrit.server.group.GroupsCollection;
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.util.RegexListSearcher;
import com.google.gerrit.server.util.TreeFormatter;
import com.google.gson.reflect.TypeToken;
@ -594,16 +595,21 @@ public class ListProjects implements RestReadView<TopLevelResource> {
private List<Ref> getBranchRefs(Project.NameKey projectName, ProjectControl projectControl) {
Ref[] result = new Ref[showBranch.size()];
try (Repository git = repoManager.openRepository(projectName)) {
PermissionBackend.ForProject perm = permissionBackend.user(currentUser).project(projectName);
for (int i = 0; i < showBranch.size(); i++) {
Ref ref = git.findRef(showBranch.get(i));
if (ref != null
&& ref.getObjectId() != null
&& (projectControl.controlForRef(ref.getLeaf().getName()).isVisible())
|| (all && projectControl.isOwner())) {
if (all && projectControl.isOwner()) {
result[i] = ref;
} else if (ref != null && ref.getObjectId() != null) {
try {
perm.ref(ref.getLeaf().getName()).check(RefPermission.READ);
result[i] = ref;
} catch (AuthException e) {
continue;
}
}
}
} catch (IOException ioe) {
} catch (IOException | PermissionBackendException e) {
// Fall through and return what is available.
}
return Arrays.asList(result);

View File

@ -113,7 +113,7 @@ public class RefControl {
}
/** Can this user see this reference exists? */
public boolean isVisible() {
boolean isVisible() {
if (isVisible == null) {
isVisible =
(getUser().isInternalUser() || canPerform(Permission.READ))

View File

@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.DashboardsCollection.DashboardInfo;
import com.google.gerrit.server.project.SetDashboard.Input;
import com.google.inject.Inject;
@ -46,7 +47,8 @@ class SetDashboard implements RestModifyView<DashboardResource, Input> {
@Override
public Response<DashboardInfo> apply(DashboardResource resource, Input input)
throws AuthException, BadRequestException, ResourceConflictException,
MethodNotAllowedException, ResourceNotFoundException, IOException {
MethodNotAllowedException, ResourceNotFoundException, IOException,
PermissionBackendException {
if (resource.isProjectDefault()) {
return defaultSetter.get().apply(resource, input);
}

View File

@ -26,6 +26,7 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.DashboardsCollection.DashboardInfo;
import com.google.gerrit.server.project.SetDashboard.Input;
import com.google.inject.Inject;
@ -59,7 +60,7 @@ class SetDefaultDashboard implements RestModifyView<DashboardResource, Input> {
@Override
public Response<DashboardInfo> apply(DashboardResource resource, Input input)
throws AuthException, BadRequestException, ResourceConflictException,
ResourceNotFoundException, IOException {
ResourceNotFoundException, IOException, PermissionBackendException {
if (input == null) {
input = new Input(); // Delete would set input to null.
}
@ -132,7 +133,7 @@ class SetDefaultDashboard implements RestModifyView<DashboardResource, Input> {
@Override
public Response<DashboardInfo> apply(ProjectResource resource, Input input)
throws AuthException, BadRequestException, ResourceConflictException,
ResourceNotFoundException, IOException {
ResourceNotFoundException, IOException, PermissionBackendException {
SetDefaultDashboard set = setDefault.get();
set.inherited = inherited;
return set.apply(DashboardResource.projectDefault(resource.getControl()), input);

@ -1 +1 @@
Subproject commit 0721b208ad863ff2f2c7fe1c89950dc2b921abaa
Subproject commit 084c09c1f82f2879c73bbfd1746a5659ac481a3c