Merge "Move VisibleRefFilter into PermissionBackend"

This commit is contained in:
Patrick Hiesel
2018-02-14 15:13:16 +00:00
committed by Gerrit Code Review
25 changed files with 293 additions and 168 deletions

View File

@@ -30,13 +30,14 @@ import com.google.gerrit.server.RemotePeer;
import com.google.gerrit.server.RequestCleanup;
import com.google.gerrit.server.config.GerritRequestModule;
import com.google.gerrit.server.config.RequestScopedReviewDbProvider;
import com.google.gerrit.server.git.DefaultAdvertiseRefsHook;
import com.google.gerrit.server.git.ReceivePackInitializer;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.UploadPackInitializer;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.git.receive.AsyncReceiveCommits;
import com.google.gerrit.server.git.validators.UploadValidators;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.ProjectCache;
@@ -208,7 +209,6 @@ class InProcessProtocol extends TestProtocol<Context> {
private static class Upload implements UploadPackFactory<Context> {
private final Provider<CurrentUser> userProvider;
private final VisibleRefFilter.Factory refFilterFactory;
private final TransferConfig transferConfig;
private final DynamicSet<UploadPackInitializer> uploadPackInitializers;
private final DynamicSet<PreUploadHook> preUploadHooks;
@@ -220,7 +220,6 @@ class InProcessProtocol extends TestProtocol<Context> {
@Inject
Upload(
Provider<CurrentUser> userProvider,
VisibleRefFilter.Factory refFilterFactory,
TransferConfig transferConfig,
DynamicSet<UploadPackInitializer> uploadPackInitializers,
DynamicSet<PreUploadHook> preUploadHooks,
@@ -229,7 +228,6 @@ class InProcessProtocol extends TestProtocol<Context> {
ProjectCache projectCache,
PermissionBackend permissionBackend) {
this.userProvider = userProvider;
this.refFilterFactory = refFilterFactory;
this.transferConfig = transferConfig;
this.uploadPackInitializers = uploadPackInitializers;
this.preUploadHooks = preUploadHooks;
@@ -248,11 +246,9 @@ class InProcessProtocol extends TestProtocol<Context> {
threadContext.setContext(req);
current.set(req);
PermissionBackend.ForProject perm = permissionBackend.user(userProvider).project(req.project);
try {
permissionBackend
.user(userProvider)
.project(req.project)
.check(ProjectPermission.RUN_UPLOAD_PACK);
perm.check(ProjectPermission.RUN_UPLOAD_PACK);
} catch (AuthException e) {
throw new ServiceNotAuthorizedException();
} catch (PermissionBackendException e) {
@@ -271,7 +267,7 @@ class InProcessProtocol extends TestProtocol<Context> {
UploadPack up = new UploadPack(repo);
up.setPackConfig(transferConfig.getPackConfig());
up.setTimeout(transferConfig.getTimeout());
up.setAdvertiseRefsHook(refFilterFactory.create(projectState, repo));
up.setAdvertiseRefsHook(new DefaultAdvertiseRefsHook(perm, RefFilterOptions.defaults()));
List<PreUploadHook> hooks = Lists.newArrayList(preUploadHooks);
hooks.add(uploadValidatorsFactory.create(projectState.getProject(), repo, "localhost-test"));
up.setPreUploadHook(PreUploadHookChain.newChain(hooks));

View File

@@ -25,13 +25,14 @@ import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.git.DefaultAdvertiseRefsHook;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.UploadPackInitializer;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.git.receive.AsyncReceiveCommits;
import com.google.gerrit.server.git.validators.UploadValidators;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.ProjectCache;
@@ -238,18 +239,15 @@ public class GitOverHttpServlet extends GitServlet {
}
static class UploadFilter implements Filter {
private final VisibleRefFilter.Factory refFilterFactory;
private final UploadValidators.Factory uploadValidatorsFactory;
private final PermissionBackend permissionBackend;
private final Provider<CurrentUser> userProvider;
@Inject
UploadFilter(
VisibleRefFilter.Factory refFilterFactory,
UploadValidators.Factory uploadValidatorsFactory,
PermissionBackend permissionBackend,
Provider<CurrentUser> userProvider) {
this.refFilterFactory = refFilterFactory;
this.uploadValidatorsFactory = uploadValidatorsFactory;
this.permissionBackend = permissionBackend;
this.userProvider = userProvider;
@@ -262,12 +260,10 @@ public class GitOverHttpServlet extends GitServlet {
Repository repo = ServletUtils.getRepository(request);
ProjectState state = (ProjectState) request.getAttribute(ATT_STATE);
UploadPack up = (UploadPack) request.getAttribute(ServletUtils.ATTRIBUTE_HANDLER);
PermissionBackend.ForProject perm =
permissionBackend.user(userProvider).project(state.getNameKey());
try {
permissionBackend
.user(userProvider)
.project(state.getNameKey())
.check(ProjectPermission.RUN_UPLOAD_PACK);
perm.check(ProjectPermission.RUN_UPLOAD_PACK);
} catch (AuthException e) {
GitSmartHttpTools.sendError(
(HttpServletRequest) request,
@@ -284,8 +280,7 @@ public class GitOverHttpServlet extends GitServlet {
uploadValidatorsFactory.create(state.getProject(), repo, request.getRemoteHost());
up.setPreUploadHook(
PreUploadHookChain.newChain(Lists.newArrayList(up.getPreUploadHook(), uploadValidators)));
up.setAdvertiseRefsHook(refFilterFactory.create(state, repo));
up.setAdvertiseRefsHook(new DefaultAdvertiseRefsHook(perm, RefFilterOptions.defaults()));
next.doFilter(request, response);
}

View File

@@ -54,7 +54,6 @@ import com.google.gerrit.server.config.GitUploadPackGroups;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.git.receive.ReceiveCommitsExecutorModule;
import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
import com.google.gerrit.server.notedb.NoteDbModule;
@@ -134,7 +133,6 @@ public class BatchProgramModule extends FactoryModule {
factory(MergeUtil.Factory.class);
factory(PatchSetInserter.Factory.class);
factory(RebaseChangeOp.Factory.class);
factory(VisibleRefFilter.Factory.class);
// As Reindex is a batch program, don't assume the index is available for
// the change cache.

View File

@@ -22,6 +22,7 @@ import com.google.gerrit.extensions.api.projects.TagInput;
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.TagResource;
import com.google.gerrit.server.restapi.project.CreateTag;
@@ -88,7 +89,7 @@ public class TagApiImpl implements TagApi {
}
}
private TagResource resource() throws RestApiException, IOException {
private TagResource resource() throws RestApiException, IOException, PermissionBackendException {
return tags.parse(project, IdString.fromDecoded(ref));
}
}

View File

@@ -121,7 +121,6 @@ import com.google.gerrit.server.git.NotesBranchUtil;
import com.google.gerrit.server.git.ReceivePackInitializer;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.git.receive.ReceiveCommitsModule;
import com.google.gerrit.server.git.strategy.SubmitStrategy;
import com.google.gerrit.server.git.validators.CommitValidationListener;
@@ -266,7 +265,6 @@ public class GerritGlobalModule extends FactoryModule {
factory(RegisterNewEmailSender.Factory.class);
factory(ReplacePatchSetSender.Factory.class);
factory(SetAssigneeSender.Factory.class);
factory(VisibleRefFilter.Factory.class);
bind(PermissionCollection.Factory.class);
bind(AccountVisibility.class).toProvider(AccountVisibilityProvider.class).in(SINGLETON);
factory(ProjectOwnerGroupsProvider.Factory.class);

View File

@@ -0,0 +1,52 @@
// Copyright (C) 2018 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.git;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import java.util.Map;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.AbstractAdvertiseRefsHook;
import org.eclipse.jgit.transport.ServiceMayNotContinueException;
/**
* Wrapper around {@link PermissionBackend.ForProject} that implements {@link
* org.eclipse.jgit.transport.AdvertiseRefsHook}.
*/
public class DefaultAdvertiseRefsHook extends AbstractAdvertiseRefsHook {
private final PermissionBackend.ForProject perm;
private final PermissionBackend.RefFilterOptions opts;
public DefaultAdvertiseRefsHook(
PermissionBackend.ForProject perm, PermissionBackend.RefFilterOptions opts) {
this.perm = perm;
this.opts = opts;
}
@Override
protected Map<String, Ref> getAdvertisedRefs(Repository repo, RevWalk revWalk)
throws ServiceMayNotContinueException {
try {
return perm.filter(repo.getAllRefs(), repo, opts);
} catch (PermissionBackendException e) {
ServiceMayNotContinueException ex = new ServiceMayNotContinueException();
ex.initCause(e);
throw ex;
}
}
}

View File

@@ -82,7 +82,7 @@ public class TagCache {
}
}
TagSetHolder get(Project.NameKey name) {
public TagSetHolder get(Project.NameKey name) {
EntryVal val = cache.getIfPresent(name.get());
if (val == null) {
synchronized (createLock) {

View File

@@ -23,7 +23,7 @@ import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
class TagMatcher {
public class TagMatcher {
final BitSet mask = new BitSet();
final List<Ref> newRefs = new ArrayList<>();
final List<LostRef> lostRefs = new ArrayList<>();
@@ -50,7 +50,7 @@ class TagMatcher {
this.updated = updated;
}
boolean isReachable(Ref tagRef) {
public boolean isReachable(Ref tagRef) {
tagRef = db.peel(tagRef);
ObjectId tagObj = tagRef.getPeeledObjectId();

View File

@@ -21,7 +21,7 @@ import java.util.Collection;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
class TagSetHolder {
public class TagSetHolder {
private final Object buildLock = new Object();
private final Project.NameKey projectName;
private volatile TagSet tags;
@@ -42,7 +42,7 @@ class TagSetHolder {
this.tags = tags;
}
TagMatcher matcher(TagCache cache, Repository db, Collection<Ref> include) {
public TagMatcher matcher(TagCache cache, Repository db, Collection<Ref> include) {
include = include.stream().filter(r -> !TagSet.skip(r)).collect(toList());
TagSet tags = this.tags;

View File

@@ -23,13 +23,14 @@ import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.DefaultAdvertiseRefsHook;
import com.google.gerrit.server.git.HackPushNegotiateHook;
import com.google.gerrit.server.git.MultiProgressMonitor;
import com.google.gerrit.server.git.ProjectRunnable;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.ContributorAgreementsChecker;
@@ -183,7 +184,6 @@ public class AsyncReceiveCommits implements PreReceiveHook {
AsyncReceiveCommits(
ReceiveCommits.Factory factory,
PermissionBackend permissionBackend,
VisibleRefFilter.Factory refFilterFactory,
Provider<InternalChangeQuery> queryProvider,
@ReceiveCommitsExecutor ExecutorService executor,
RequestScopePropagator scopePropagator,
@@ -236,7 +236,8 @@ public class AsyncReceiveCommits implements PreReceiveHook {
List<AdvertiseRefsHook> advHooks = new ArrayList<>(4);
allRefsWatcher = new AllRefsWatcher();
advHooks.add(allRefsWatcher);
advHooks.add(refFilterFactory.create(projectState, repo).setShowMetadata(false));
advHooks.add(
new DefaultAdvertiseRefsHook(perm, RefFilterOptions.builder().setFilterMeta(true).build()));
advHooks.add(new ReceiveCommitsAdvertiseRefsHook(queryProvider, projectName));
advHooks.add(new HackPushNegotiateHook());
rp.setAdvertiseRefsHook(AdvertiseRefsHookChain.newChain(advHooks));

View File

@@ -28,8 +28,9 @@ public class DefaultPermissionBackendModule extends AbstractModule {
public static class LegacyControlsModule extends FactoryModule {
@Override
protected void configure() {
// TODO(sop) Hide ProjectControl, RefControl, ChangeControl related bindings.
// TODO(hiesel) Hide ProjectControl, RefControl, ChangeControl related bindings.
factory(ProjectControl.Factory.class);
factory(DefaultRefFilter.Factory.class);
bind(ChangeControl.Factory.class);
}
}

View File

@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.git;
package com.google.gerrit.server.permissions;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CACHE_AUTOMERGE;
@@ -33,15 +33,13 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.TagMatcher;
import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeNotes.Factory.ChangeNotesResult;
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.ProjectPermission;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
@@ -58,67 +56,54 @@ import java.util.Objects;
import java.util.stream.Stream;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.SymbolicRef;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.AbstractAdvertiseRefsHook;
import org.eclipse.jgit.transport.ServiceMayNotContinueException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
private static final Logger log = LoggerFactory.getLogger(VisibleRefFilter.class);
class DefaultRefFilter {
private static final Logger log = LoggerFactory.getLogger(DefaultRefFilter.class);
public interface Factory {
VisibleRefFilter create(ProjectState projectState, Repository git);
interface Factory {
DefaultRefFilter create(CurrentUser who, ProjectState projectState);
}
private final TagCache tagCache;
private final ChangeNotes.Factory changeNotesFactory;
@Nullable private final SearchingChangeCacheImpl changeCache;
private final Provider<ReviewDb> db;
private final Provider<CurrentUser> user;
private final GroupCache groupCache;
private final PermissionBackend permissionBackend;
private final PermissionBackend.ForProject perm;
private final PermissionBackend.ForProject permissionBackendForProject;
private final CurrentUser user;
private final ProjectState projectState;
private final Repository git;
private boolean showMetadata = true;
private String userEditPrefix;
private Map<Change.Id, Branch.NameKey> visibleChanges;
@Inject
VisibleRefFilter(
DefaultRefFilter(
TagCache tagCache,
ChangeNotes.Factory changeNotesFactory,
@Nullable SearchingChangeCacheImpl changeCache,
Provider<ReviewDb> db,
Provider<CurrentUser> user,
GroupCache groupCache,
PermissionBackend permissionBackend,
@Assisted ProjectState projectState,
@Assisted Repository git) {
@Assisted CurrentUser user,
@Assisted ProjectState projectState) {
this.tagCache = tagCache;
this.changeNotesFactory = changeNotesFactory;
this.changeCache = changeCache;
this.db = db;
this.user = user;
this.groupCache = groupCache;
this.permissionBackend = permissionBackend;
this.perm =
permissionBackend.user(user).database(db).project(projectState.getProject().getNameKey());
this.permissionBackendForProject =
permissionBackend.user(user).database(db).project(projectState.getNameKey());
this.user = user;
this.projectState = projectState;
this.git = git;
}
/** Show change references. Default is {@code true}. */
public VisibleRefFilter setShowMetadata(boolean show) {
showMetadata = show;
return this;
}
public Map<String, Ref> filter(Map<String, Ref> refs, boolean filterTagsSeparately) {
Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException {
if (projectState.isAllUsers()) {
refs = addUsersSelfSymref(refs);
}
@@ -137,12 +122,11 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
boolean isAdmin;
Account.Id userId;
IdentifiedUser identifiedUser;
if (user.get().isIdentifiedUser()) {
if (user.isIdentifiedUser()) {
viewMetadata = withUser.testOrFalse(GlobalPermission.ACCESS_DATABASE);
isAdmin = withUser.testOrFalse(GlobalPermission.ADMINISTRATE_SERVER);
identifiedUser = user.get().asIdentifiedUser();
identifiedUser = user.asIdentifiedUser();
userId = identifiedUser.getAccountId();
userEditPrefix = RefNames.refsEditPrefix(userId);
} else {
viewMetadata = false;
isAdmin = false;
@@ -158,16 +142,16 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
Change.Id changeId;
Account.Id accountId;
AccountGroup.UUID accountGroupUuid;
if (name.startsWith(REFS_CACHE_AUTOMERGE) || (!showMetadata && isMetadata(name))) {
if (name.startsWith(REFS_CACHE_AUTOMERGE) || (opts.filterMeta() && isMetadata(name))) {
continue;
} else if (RefNames.isRefsEdit(name)) {
// Edits are visible only to the owning user, if change is visible.
if (viewMetadata || visibleEdit(name)) {
if (viewMetadata || visibleEdit(repo, 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)) {
if (viewMetadata || visible(repo, changeId)) {
result.put(name, ref);
}
} else if ((accountId = Account.Id.fromRef(name)) != null) {
@@ -218,14 +202,20 @@ 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() && (!result.isEmpty() || opts.filterTagsSeparately())) {
TagMatcher tags =
tagCache
.get(projectState.getNameKey())
.matcher(
tagCache,
git,
filterTagsSeparately ? filter(git.getAllRefs()).values() : result.values());
repo,
opts.filterTagsSeparately()
? filter(
repo.getAllRefs(),
repo,
opts.toBuilder().setFilterTagsSeparately(false).build())
.values()
: result.values());
for (Ref tag : deferredTags) {
if (tags.isReachable(tag)) {
result.put(tag.getName(), tag);
@@ -246,8 +236,8 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
}
private Map<String, Ref> addUsersSelfSymref(Map<String, Ref> refs) {
if (user.get().isIdentifiedUser()) {
Ref r = refs.get(RefNames.refsUsers(user.get().getAccountId()));
if (user.isIdentifiedUser()) {
Ref r = refs.get(RefNames.refsUsers(user.getAccountId()));
if (r != null) {
SymbolicRef s = new SymbolicRef(REFS_USERS_SELF, r);
refs = new HashMap<>(refs);
@@ -257,28 +247,10 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
return refs;
}
@Override
protected Map<String, Ref> getAdvertisedRefs(Repository repository, RevWalk revWalk)
throws ServiceMayNotContinueException {
try {
return filter(repository.getRefDatabase().getRefs(RefDatabase.ALL));
} catch (ServiceMayNotContinueException e) {
throw e;
} catch (IOException e) {
ServiceMayNotContinueException ex = new ServiceMayNotContinueException();
ex.initCause(e);
throw ex;
}
}
private Map<String, Ref> filter(Map<String, Ref> refs) {
return filter(refs, false);
}
private boolean visible(Change.Id changeId) {
private boolean visible(Repository repo, Change.Id changeId) {
if (visibleChanges == null) {
if (changeCache == null) {
visibleChanges = visibleChangesByScan();
visibleChanges = visibleChangesByScan(repo);
} else {
visibleChanges = visibleChangesBySearch();
}
@@ -286,22 +258,26 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
return visibleChanges.containsKey(changeId);
}
private boolean visibleEdit(String name) {
private boolean visibleEdit(Repository repo, String name) {
Change.Id id = Change.Id.fromEditRefPart(name);
// Initialize if it wasn't yet
if (visibleChanges == null) {
visible(id);
visible(repo, id);
}
if (id == null) {
return false;
}
if (userEditPrefix != null && name.startsWith(userEditPrefix) && visible(id)) {
if (user.isIdentifiedUser()
&& name.startsWith(RefNames.refsEditPrefix(user.asIdentifiedUser().getAccountId()))
&& visible(repo, id)) {
return true;
}
if (visibleChanges.containsKey(id)) {
try {
// Default to READ_PRIVATE_CHANGES as there is no special permission for reading edits.
perm.ref(visibleChanges.get(id).get()).check(RefPermission.READ_PRIVATE_CHANGES);
permissionBackendForProject
.ref(visibleChanges.get(id).get())
.check(RefPermission.READ_PRIVATE_CHANGES);
return true;
} catch (PermissionBackendException | AuthException e) {
return false;
@@ -317,7 +293,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
for (ChangeData cd : changeCache.getChangeData(db.get(), project)) {
ChangeNotes notes = changeNotesFactory.createFromIndexedChange(cd.change());
if (projectState.statePermitsRead()
&& perm.indexedChange(cd, notes).test(ChangePermission.READ)) {
&& permissionBackendForProject.indexedChange(cd, notes).test(ChangePermission.READ)) {
visibleChanges.put(cd.getId(), cd.change().getDest());
}
}
@@ -329,32 +305,34 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
}
}
private Map<Change.Id, Branch.NameKey> visibleChangesByScan() {
private Map<Change.Id, Branch.NameKey> visibleChangesByScan(Repository repo) {
Project.NameKey p = projectState.getNameKey();
Stream<ChangeNotesResult> s;
try {
s = changeNotesFactory.scan(git, db.get(), p);
s = changeNotesFactory.scan(repo, db.get(), p);
} catch (IOException e) {
log.error("Cannot load changes for project " + p + ", assuming no changes are visible", e);
return Collections.emptyMap();
}
return s.map(r -> toNotes(p, r))
return s.map(r -> toNotes(r))
.filter(Objects::nonNull)
.collect(toMap(n -> n.getChangeId(), n -> n.getChange().getDest()));
}
@Nullable
private ChangeNotes toNotes(Project.NameKey p, ChangeNotesResult r) {
private ChangeNotes toNotes(ChangeNotesResult r) {
if (r.error().isPresent()) {
log.warn("Failed to load change " + r.id() + " in " + p, r.error().get());
log.warn(
"Failed to load change " + r.id() + " in " + projectState.getName(), r.error().get());
return null;
}
try {
if (projectState.statePermitsRead() && perm.change(r.notes()).test(ChangePermission.READ)) {
if (projectState.statePermitsRead()
&& permissionBackendForProject.change(r.notes()).test(ChangePermission.READ)) {
return r.notes();
}
} catch (PermissionBackendException e) {
log.warn("Failed to check permission for " + r.id() + " in " + p, e);
log.warn("Failed to check permission for " + r.id() + " in " + projectState.getName(), e);
}
return null;
}
@@ -373,7 +351,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
private boolean canReadRef(String ref) {
try {
perm.ref(ref).check(RefPermission.READ);
permissionBackendForProject.ref(ref).check(RefPermission.READ);
} catch (AuthException e) {
return false;
} catch (PermissionBackendException e) {
@@ -392,8 +370,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
} catch (PermissionBackendException e) {
log.error(
String.format(
"Can't check permission for user %s on project %s",
user.get(), projectState.getName()),
"Can't check permission for user %s on project %s", user, projectState.getName()),
e);
return false;
}

View File

@@ -20,10 +20,14 @@ import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
import com.google.gerrit.server.permissions.PermissionBackend.ForProject;
import com.google.gerrit.server.permissions.PermissionBackend.ForRef;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Provider;
import java.util.Collection;
import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
/**
* Helpers for {@link PermissionBackend} that must fail.
@@ -103,6 +107,12 @@ public class FailedPermissionBackend {
throws PermissionBackendException {
throw new PermissionBackendException(message, cause);
}
@Override
public Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException {
throw new PermissionBackendException(message, cause);
}
}
private static class FailedRef extends ForRef {

View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server.permissions;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.util.stream.Collectors.toSet;
import com.google.auto.value.AutoValue;
import com.google.common.collect.Sets;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
@@ -36,7 +37,10 @@ import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -305,6 +309,45 @@ public abstract class PermissionBackend {
public BooleanCondition testCond(ProjectPermission perm) {
return new PermissionBackendCondition.ForProject(this, perm);
}
/**
* @return a partition of the provided refs that are visible to the user that this instance is
* scoped to.
*/
public abstract Map<String, Ref> filter(
Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException;
}
/** Options for filtering refs using {@link ForProject}. */
@AutoValue
public abstract static class RefFilterOptions {
/** Remove all NoteDb refs (refs/changes/*, refs/users/*, edit refs) from the result. */
public abstract boolean filterMeta();
/** Separately add reachable tags. */
public abstract boolean filterTagsSeparately();
public abstract Builder toBuilder();
public static Builder builder() {
return new AutoValue_PermissionBackend_RefFilterOptions.Builder()
.setFilterMeta(false)
.setFilterTagsSeparately(false);
}
@AutoValue.Builder
public abstract static class Builder {
public abstract Builder setFilterMeta(boolean val);
public abstract Builder setFilterTagsSeparately(boolean val);
public abstract RefFilterOptions build();
}
public static RefFilterOptions defaults() {
return builder().build();
}
}
/** PermissionBackend scoped to a user, project and reference. */

View File

@@ -36,6 +36,7 @@ import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
import com.google.gerrit.server.permissions.PermissionBackend.ForProject;
import com.google.gerrit.server.permissions.PermissionBackend.ForRef;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SectionMatcher;
import com.google.gerrit.server.query.change.ChangeData;
@@ -50,6 +51,8 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
/** Access control management for a user accessing a project's data. */
class ProjectControl {
@@ -64,6 +67,7 @@ class ProjectControl {
private final ProjectState state;
private final ChangeControl.Factory changeControlFactory;
private final PermissionCollection.Factory permissionFilter;
private final DefaultRefFilter.Factory refFilterFactory;
private List<SectionMatcher> allSections;
private Map<String, RefControl> refControls;
@@ -76,6 +80,7 @@ class ProjectControl {
PermissionCollection.Factory permissionFilter,
ChangeControl.Factory changeControlFactory,
PermissionBackend permissionBackend,
DefaultRefFilter.Factory refFilterFactory,
@Assisted CurrentUser who,
@Assisted ProjectState ps) {
this.changeControlFactory = changeControlFactory;
@@ -83,6 +88,7 @@ class ProjectControl {
this.receiveGroups = receiveGroups;
this.permissionFilter = permissionFilter;
this.permissionBackend = permissionBackend;
this.refFilterFactory = refFilterFactory;
user = who;
state = ps;
}
@@ -95,6 +101,7 @@ class ProjectControl {
permissionFilter,
changeControlFactory,
permissionBackend,
refFilterFactory,
who,
state);
// Not per-user, and reusing saves lookup time.
@@ -381,6 +388,12 @@ class ProjectControl {
return ok;
}
@Override
public Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
throws PermissionBackendException {
return refFilterFactory.create(getUser(), getProjectState()).filter(refs, repo, opts);
}
private boolean can(ProjectPermission perm) throws PermissionBackendException {
switch (perm) {
case ACCESS:

View File

@@ -16,9 +16,14 @@ package com.google.gerrit.server.project;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.IncludedInResolver;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collection;
import java.util.Map;
@@ -35,22 +40,31 @@ import org.slf4j.LoggerFactory;
* Report whether a commit is reachable from a set of commits. This is used for checking if a user
* has read permissions on a commit.
*/
@Singleton
public class Reachable {
private final VisibleRefFilter.Factory refFilter;
private static final Logger log = LoggerFactory.getLogger(Reachable.class);
private final PermissionBackend permissionBackend;
private final Provider<CurrentUser> user;
@Inject
Reachable(VisibleRefFilter.Factory refFilter) {
this.refFilter = refFilter;
Reachable(PermissionBackend permissionBackend, Provider<CurrentUser> user) {
this.permissionBackend = permissionBackend;
this.user = user;
}
/** @return true if a commit is reachable from a given set of refs. */
public boolean fromRefs(
ProjectState state, Repository repo, RevCommit commit, Map<String, Ref> refs) {
try (RevWalk rw = new RevWalk(repo)) {
Map<String, Ref> filtered = refFilter.create(state, repo).filter(refs, true);
// TODO(hiesel) Convert interface to Project.NameKey
Map<String, Ref> filtered =
permissionBackend
.user(user)
.project(state.getNameKey())
.filter(refs, repo, RefFilterOptions.builder().setFilterTagsSeparately(true).build());
return IncludedInResolver.includedInAny(repo, rw, commit, filtered.values());
} catch (IOException e) {
} catch (IOException | PermissionBackendException e) {
log.error(
String.format(
"Cannot verify permissions to commit object %s in repository %s",

View File

@@ -27,8 +27,9 @@ import com.google.gerrit.server.CommonConverters;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.ProjectState;
@@ -58,7 +59,6 @@ public class ListTags implements RestReadView<ProjectResource> {
private final GitRepositoryManager repoManager;
private final PermissionBackend permissionBackend;
private final Provider<CurrentUser> user;
private final VisibleRefFilter.Factory refFilterFactory;
private final WebLinks links;
@Option(
@@ -111,12 +111,10 @@ public class ListTags implements RestReadView<ProjectResource> {
GitRepositoryManager repoManager,
PermissionBackend permissionBackend,
Provider<CurrentUser> user,
VisibleRefFilter.Factory refFilterFactory,
WebLinks webLinks) {
this.repoManager = repoManager;
this.permissionBackend = permissionBackend;
this.user = user;
this.refFilterFactory = refFilterFactory;
this.links = webLinks;
}
@@ -130,7 +128,7 @@ public class ListTags implements RestReadView<ProjectResource> {
@Override
public List<TagInfo> apply(ProjectResource resource)
throws IOException, ResourceNotFoundException, RestApiException {
throws IOException, ResourceNotFoundException, RestApiException, PermissionBackendException {
resource.getProjectState().checkStatePermitsRead();
List<TagInfo> tags = new ArrayList<>();
@@ -139,8 +137,7 @@ public class ListTags implements RestReadView<ProjectResource> {
try (Repository repo = getRepository(resource.getNameKey());
RevWalk rw = new RevWalk(repo)) {
Map<String, Ref> all =
visibleTags(
resource.getProjectState(), repo, repo.getRefDatabase().getRefs(Constants.R_TAGS));
visibleTags(resource.getNameKey(), repo, repo.getRefDatabase().getRefs(Constants.R_TAGS));
for (Ref ref : all.values()) {
tags.add(
createTagInfo(perm.ref(ref.getName()), ref, rw, resource.getProjectState(), links));
@@ -165,7 +162,7 @@ public class ListTags implements RestReadView<ProjectResource> {
}
public TagInfo get(ProjectResource resource, IdString id)
throws ResourceNotFoundException, IOException {
throws ResourceNotFoundException, IOException, PermissionBackendException {
try (Repository repo = getRepository(resource.getNameKey());
RevWalk rw = new RevWalk(repo)) {
String tagName = id.get();
@@ -174,7 +171,7 @@ public class ListTags implements RestReadView<ProjectResource> {
}
Ref ref = repo.getRefDatabase().exactRef(tagName);
if (ref != null
&& !visibleTags(resource.getProjectState(), repo, ImmutableMap.of(ref.getName(), ref))
&& !visibleTags(resource.getNameKey(), repo, ImmutableMap.of(ref.getName(), ref))
.isEmpty()) {
return createTagInfo(
permissionBackend
@@ -235,7 +232,15 @@ public class ListTags implements RestReadView<ProjectResource> {
}
}
private Map<String, Ref> visibleTags(ProjectState state, Repository repo, Map<String, Ref> tags) {
return refFilterFactory.create(state, repo).setShowMetadata(false).filter(tags, true);
private Map<String, Ref> visibleTags(
Project.NameKey project, Repository repo, Map<String, Ref> tags)
throws PermissionBackendException {
return permissionBackend
.user(user)
.project(project)
.filter(
tags,
repo,
RefFilterOptions.builder().setFilterMeta(true).setFilterTagsSeparately(true).build());
}
}

View File

@@ -21,6 +21,7 @@ import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.TagResource;
import com.google.inject.Inject;
@@ -52,7 +53,7 @@ public class TagsCollection
@Override
public TagResource parse(ProjectResource parent, IdString id)
throws RestApiException, IOException {
throws RestApiException, IOException, PermissionBackendException {
parent.getProjectState().checkStatePermitsRead();
return new TagResource(parent.getProjectState(), parent.getUser(), list.get().get(parent, id));
}

View File

@@ -24,7 +24,9 @@ import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
@@ -49,7 +51,7 @@ import org.kohsuke.args4j.Option;
public class LsUserRefs extends SshCommand {
@Inject private AccountResolver accountResolver;
@Inject private OneOffRequestContext requestContext;
@Inject private VisibleRefFilter.Factory refFilterFactory;
@Inject private PermissionBackend permissionBackend;
@Inject private GitRepositoryManager repoManager;
@Option(
@@ -92,16 +94,17 @@ public class LsUserRefs extends SshCommand {
ManualRequestContext ctx = requestContext.openAs(userAccount.getId())) {
try {
Map<String, Ref> refsMap =
refFilterFactory
.create(projectState, repo)
.filter(repo.getRefDatabase().getRefs(ALL), false);
permissionBackend
.user(user)
.project(projectName)
.filter(repo.getRefDatabase().getRefs(ALL), repo, RefFilterOptions.defaults());
for (String ref : refsMap.keySet()) {
if (!onlyRefsHeads || ref.startsWith(RefNames.REFS_HEADS)) {
stdout.println(ref);
}
}
} catch (IOException e) {
} catch (IOException | PermissionBackendException e) {
throw new Failure(1, "fatal: Error reading refs: '" + projectName, e);
}
} catch (RepositoryNotFoundException e) {

View File

@@ -20,7 +20,7 @@ import com.google.gerrit.common.data.Capable;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.git.DefaultAdvertiseRefsHook;
import com.google.gerrit.server.git.receive.AsyncReceiveCommits;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -134,14 +134,14 @@ final class Receive extends AbstractGitCommand {
msg.append(" AdvertiseRefsHook: ").append(rp.getAdvertiseRefsHook());
if (rp.getAdvertiseRefsHook() == AdvertiseRefsHook.DEFAULT) {
msg.append("DEFAULT");
} else if (rp.getAdvertiseRefsHook() instanceof VisibleRefFilter) {
msg.append("VisibleRefFilter");
} else if (rp.getAdvertiseRefsHook() instanceof DefaultAdvertiseRefsHook) {
msg.append("DefaultAdvertiseRefsHook");
} else {
msg.append(rp.getAdvertiseRefsHook().getClass());
}
msg.append("\n");
if (rp.getAdvertiseRefsHook() instanceof VisibleRefFilter) {
if (rp.getAdvertiseRefsHook() instanceof DefaultAdvertiseRefsHook) {
Map<String, Ref> adv = rp.getAdvertisedRefs();
msg.append(" Visible references (").append(adv.size()).append("):\n");
for (Ref ref : adv.values()) {

View File

@@ -17,12 +17,13 @@ package com.google.gerrit.sshd.commands;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.git.DefaultAdvertiseRefsHook;
import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.UploadPackInitializer;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.git.validators.UploadValidationException;
import com.google.gerrit.server.git.validators.UploadValidators;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.sshd.AbstractGitCommand;
@@ -39,7 +40,6 @@ import org.eclipse.jgit.transport.UploadPack;
/** Publishes Git repositories over SSH using the Git upload-pack protocol. */
final class Upload extends AbstractGitCommand {
@Inject private TransferConfig config;
@Inject private VisibleRefFilter.Factory refFilterFactory;
@Inject private DynamicSet<PreUploadHook> preUploadHooks;
@Inject private DynamicSet<PostUploadHook> postUploadHooks;
@Inject private DynamicSet<UploadPackInitializer> uploadPackInitializers;
@@ -49,11 +49,11 @@ final class Upload extends AbstractGitCommand {
@Override
protected void runImpl() throws IOException, Failure {
PermissionBackend.ForProject perm =
permissionBackend.user(user).project(projectState.getNameKey());
try {
permissionBackend
.user(user)
.project(projectState.getNameKey())
.check(ProjectPermission.RUN_UPLOAD_PACK);
perm.check(ProjectPermission.RUN_UPLOAD_PACK);
} catch (AuthException e) {
throw new Failure(1, "fatal: upload-pack not permitted on this server");
} catch (PermissionBackendException e) {
@@ -61,7 +61,7 @@ final class Upload extends AbstractGitCommand {
}
final UploadPack up = new UploadPack(repo);
up.setAdvertiseRefsHook(refFilterFactory.create(projectState, repo));
up.setAdvertiseRefsHook(new DefaultAdvertiseRefsHook(perm, RefFilterOptions.defaults()));
up.setPackConfig(config.getPackConfig());
up.setTimeout(config.getTimeout());
up.setPostUploadHook(PostUploadHookChain.newChain(Lists.newArrayList(postUploadHooks)));