Merge "Merge branch 'stable-3.1' into stable-3.2" into stable-3.2
diff --git a/java/com/google/gerrit/entities/RefNames.java b/java/com/google/gerrit/entities/RefNames.java
index 400861c..5595bc7 100644
--- a/java/com/google/gerrit/entities/RefNames.java
+++ b/java/com/google/gerrit/entities/RefNames.java
@@ -288,10 +288,16 @@
* Whether the ref is managed by Gerrit. Covers all Gerrit-internal refs like refs/cache-automerge
* and refs/meta as well as refs/changes. Does not cover user-created refs like branches or custom
* ref namespaces like refs/my-company.
+ *
+ * <p>Any ref for which this method evaluates to true will be served to users who have the {@code
+ * ACCESS_DATABASE} capability.
+ *
+ * <p><b>Caution</b>Any ref not in this list will be served if the user was granted a READ
+ * permission on it using Gerrit's permission model.
*/
public static boolean isGerritRef(String ref) {
return ref.startsWith(REFS_CHANGES)
- || ref.startsWith(REFS_META)
+ || ref.startsWith(REFS_EXTERNAL_IDS)
|| ref.startsWith(REFS_CACHE_AUTOMERGE)
|| ref.startsWith(REFS_DRAFT_COMMENTS)
|| ref.startsWith(REFS_DELETED_GROUPS)
@@ -299,7 +305,8 @@
|| ref.startsWith(REFS_GROUPS)
|| ref.startsWith(REFS_GROUPNAMES)
|| ref.startsWith(REFS_USERS)
- || ref.startsWith(REFS_STARRED_CHANGES);
+ || ref.startsWith(REFS_STARRED_CHANGES)
+ || ref.startsWith(REFS_REJECT_COMMITS);
}
static Integer parseShardedRefPart(String name) {
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index 143547b..2c1894e 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -65,6 +65,8 @@
private final RefControl refControl;
private final ChangeNotes notes;
+ private ChangeData cd;
+
private ChangeControl(
ChangeData.Factory changeDataFactory, RefControl refControl, ChangeNotes notes) {
this.changeDataFactory = changeDataFactory;
@@ -73,7 +75,10 @@
}
ForChange asForChange(@Nullable ChangeData cd) {
- return new ForChangeImpl(cd);
+ if (cd != null) {
+ this.cd = cd;
+ }
+ return new ForChangeImpl();
}
private CurrentUser getUser() {
@@ -88,12 +93,20 @@
return notes.getChange();
}
+ private ChangeData changeData() {
+ if (cd == null) {
+ cd = changeDataFactory.create(notes);
+ }
+ return cd;
+ }
+
/** Can this user see this change? */
- private boolean isVisible(ChangeData cd) {
- if (getChange().isPrivate() && !isPrivateVisible(cd)) {
+ boolean isVisible() {
+ if (getChange().isPrivate() && !isPrivateVisible(changeData())) {
return false;
}
- return refControl.isVisible();
+ // Does the user have READ permission on the destination?
+ return refControl.asForRef().testOrFalse(RefPermission.READ);
}
/** Can this user abandon this change? */
@@ -224,20 +237,11 @@
}
private class ForChangeImpl extends ForChange {
- private ChangeData cd;
+
private Map<String, PermissionRange> labels;
private String resourcePath;
- ForChangeImpl(@Nullable ChangeData cd) {
- this.cd = cd;
- }
-
- private ChangeData changeData() {
- if (cd == null) {
- cd = changeDataFactory.create(notes);
- }
- return cd;
- }
+ private ForChangeImpl() {}
@Override
public String resourcePath() {
@@ -290,7 +294,7 @@
try {
switch (perm) {
case READ:
- return isVisible(changeData());
+ return isVisible();
case ABANDON:
return canAbandon();
case DELETE:
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index e92ada1..c7b1060 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -16,10 +16,7 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
-import static com.google.gerrit.entities.RefNames.REFS_CACHE_AUTOMERGE;
import static com.google.gerrit.entities.RefNames.REFS_CONFIG;
-import static com.google.gerrit.entities.RefNames.REFS_USERS_SELF;
-import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toCollection;
import com.google.auto.value.AutoValue;
@@ -29,8 +26,6 @@
import com.google.common.collect.Maps;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
-import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
@@ -41,13 +36,10 @@
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.config.GerritServerConfig;
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.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -79,8 +71,8 @@
private final TagCache tagCache;
private final ChangeNotes.Factory changeNotesFactory;
@Nullable private final SearchingChangeCacheImpl changeCache;
- private final GroupCache groupCache;
private final PermissionBackend permissionBackend;
+ private final RefVisibilityControl refVisibilityControl;
private final ProjectControl projectControl;
private final CurrentUser user;
private final ProjectState projectState;
@@ -96,16 +88,16 @@
TagCache tagCache,
ChangeNotes.Factory changeNotesFactory,
@Nullable SearchingChangeCacheImpl changeCache,
- GroupCache groupCache,
PermissionBackend permissionBackend,
+ RefVisibilityControl refVisibilityControl,
@GerritServerConfig Config config,
MetricMaker metricMaker,
@Assisted ProjectControl projectControl) {
this.tagCache = tagCache;
this.changeNotesFactory = changeNotesFactory;
this.changeCache = changeCache;
- this.groupCache = groupCache;
this.permissionBackend = permissionBackend;
+ this.refVisibilityControl = refVisibilityControl;
this.skipFullRefEvaluationIfAllRefsAreVisible =
config.getBoolean("auth", "skipFullRefEvaluationIfAllRefsAreVisible", true);
this.projectControl = projectControl;
@@ -226,131 +218,56 @@
logger.atFinest().log("Doing full ref filtering");
fullFilterCount.increment();
- boolean viewMetadata;
- boolean isAdmin;
- Account.Id userId;
- IdentifiedUser identifiedUser;
- PermissionBackend.WithUser withUser = permissionBackend.user(user);
- if (user.isIdentifiedUser()) {
- viewMetadata = withUser.testOrFalse(GlobalPermission.ACCESS_DATABASE);
- isAdmin = withUser.testOrFalse(GlobalPermission.ADMINISTRATE_SERVER);
- identifiedUser = user.asIdentifiedUser();
- userId = identifiedUser.getAccountId();
- logger.atFinest().log(
- "Account = %d; can view metadata = %s; is admin = %s",
- userId.get(), viewMetadata, isAdmin);
- } else {
- logger.atFinest().log("User is anonymous");
- viewMetadata = false;
- isAdmin = false;
- userId = null;
- identifiedUser = null;
- }
-
+ boolean hasAccessDatabase =
+ permissionBackend
+ .user(projectControl.getUser())
+ .testOrFalse(GlobalPermission.ACCESS_DATABASE);
List<Ref> resultRefs = new ArrayList<>(refs.size());
List<Ref> deferredTags = new ArrayList<>();
for (Ref ref : refs) {
- String name = ref.getName();
+ String refName = ref.getName();
Change.Id changeId;
- Account.Id accountId;
- AccountGroup.UUID accountGroupUuid;
- if (name.startsWith(REFS_CACHE_AUTOMERGE)) {
- continue;
- } else if (opts.filterMeta() && isMetadata(name)) {
- logger.atFinest().log("Filter out metadata ref %s", name);
- continue;
- } else if (RefNames.isRefsEdit(name)) {
- // Edits are visible only to the owning user, if change is visible.
- if (viewMetadata || visibleEdit(repo, name)) {
- logger.atFinest().log("Include edit ref %s", name);
- resultRefs.add(ref);
- } else {
- logger.atFinest().log("Filter out edit ref %s", name);
- }
- } else if ((changeId = Change.Id.fromRef(name)) != null) {
- // Change ref is visible only if the change is visible.
- if (viewMetadata || visible(repo, changeId)) {
- logger.atFinest().log("Include change ref %s", name);
- resultRefs.add(ref);
- } else {
- logger.atFinest().log("Filter out change ref %s", name);
- }
- } else if ((accountId = Account.Id.fromRef(name)) != null) {
- // Account ref is visible only to the corresponding account.
- if (viewMetadata || (accountId.equals(userId) && canReadRef(name))) {
- logger.atFinest().log("Include user ref %s", name);
- resultRefs.add(ref);
- } else {
- logger.atFinest().log("Filter out user ref %s", name);
- }
- } else if ((accountGroupUuid = AccountGroup.UUID.fromRef(name)) != null) {
- // Group ref is visible only to the corresponding owner group.
- InternalGroup group = groupCache.get(accountGroupUuid).orElse(null);
- if (viewMetadata
- || (group != null
- && isGroupOwner(group, identifiedUser, isAdmin)
- && canReadRef(name))) {
- logger.atFinest().log("Include group ref %s", name);
- resultRefs.add(ref);
- } else {
- logger.atFinest().log("Filter out group ref %s", name);
- }
+ if (opts.filterMeta() && isMetadata(refName)) {
+ logger.atFinest().log("Filter out metadata ref %s", refName);
} else if (isTag(ref)) {
if (hasReadOnRefsStar) {
- // The user has READ on refs/*. This is the broadest permission one can assign. There is
- // no way to grant access to (specific) tags in Gerrit, so we have to assume that these
- // users can see all tags because there could be tags that aren't reachable by any visible
- // ref while the user can see all non-Gerrit refs. This matches Gerrit's historic
- // behavior.
+ // The user has READ on refs/* with no effective block permission. This is the broadest
+ // permission one can assign. There is no way to grant access to (specific) tags in
+ // Gerrit,
+ // so we have to assume that these users can see all tags because there could be tags that
+ // aren't reachable by any visible ref while the user can see all non-Gerrit refs. This
+ // matches Gerrit's historic behavior.
// This makes it so that these users could see commits that they can't see otherwise
// (e.g. a private change ref) if a tag was attached to it. Tags are meant to be used on
// the regular Git tree that users interact with, not on any of the Gerrit trees, so this
// is a negligible risk.
- logger.atFinest().log("Include tag ref %s because user has read on refs/*", name);
+ logger.atFinest().log("Include tag ref %s because user has read on refs/*", refName);
resultRefs.add(ref);
} else {
// If its a tag, consider it later.
if (ref.getObjectId() != null) {
- logger.atFinest().log("Defer tag ref %s", name);
+ logger.atFinest().log("Defer tag ref %s", refName);
deferredTags.add(ref);
} else {
- logger.atFinest().log("Filter out tag ref %s that is not a tag", name);
+ logger.atFinest().log("Filter out tag ref %s that is not a tag", refName);
}
}
- } else if (name.startsWith(RefNames.REFS_SEQUENCES)) {
- // Sequences are internal database implementation details.
- if (viewMetadata) {
- logger.atFinest().log("Include sequence ref %s", name);
+ } else if ((changeId = Change.Id.fromRef(refName)) != null) {
+ // This is a mere performance optimization. RefVisibilityControl could determine the
+ // visibility of these refs just fine. But instead, we use highly-optimized logic that
+ // looks only on the last 10k most recent changes using the change index and a cache.
+ if (hasAccessDatabase) {
resultRefs.add(ref);
+ } else if (!visible(repo, changeId)) {
+ logger.atFinest().log("Filter out invisible change ref %s", refName);
+ } else if (RefNames.isRefsEdit(refName) && !visibleEdit(repo, refName)) {
+ logger.atFinest().log("Filter out invisible change edit ref %s", refName);
} else {
- logger.atFinest().log("Filter out sequence ref %s", name);
- }
- } else if (projectState.isAllUsers()
- && (name.equals(RefNames.REFS_EXTERNAL_IDS) || name.equals(RefNames.REFS_GROUPNAMES))) {
- // The notes branches with the external IDs / group names must not be exposed to normal
- // users.
- if (viewMetadata) {
- logger.atFinest().log("Include external IDs branch %s", name);
+ // Change is visible
resultRefs.add(ref);
- } else {
- logger.atFinest().log("Filter out external IDs branch %s", name);
}
- } 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.
- logger.atFinest().log(
- "Include ref %s because its leaf %s is readable", name, ref.getLeaf().getName());
+ } else if (refVisibilityControl.isVisible(projectControl, ref.getLeaf().getName())) {
resultRefs.add(ref);
- } else if (isRefsUsersSelf(ref)) {
- // viewMetadata allows to see all account refs, hence refs/users/self should be included as
- // well
- if (viewMetadata) {
- logger.atFinest().log("Include ref %s", REFS_USERS_SELF);
- resultRefs.add(ref);
- }
- } else {
- logger.atFinest().log("Filter out ref %s", name);
}
}
Result result = new AutoValue_DefaultRefFilter_Result(resultRefs, deferredTags);
@@ -373,7 +290,8 @@
r ->
!RefNames.isGerritRef(r.getName())
&& !r.getName().startsWith(RefNames.REFS_TAGS)
- && !r.isSymbolic())
+ && !r.isSymbolic()
+ && !r.getName().equals(RefNames.REFS_CONFIG))
// Don't use the default Java Collections.toList() as that is not size-aware and would
// expand an array list as new elements are added. Instead, provide a list that has the
// right size. This spares incremental list expansion which is quadratic in complexity.
@@ -520,10 +438,6 @@
return ref.getLeaf().getName().startsWith(Constants.R_TAGS);
}
- private static boolean isRefsUsersSelf(Ref ref) {
- return ref.getName().startsWith(REFS_USERS_SELF);
- }
-
private boolean canReadRef(String ref) throws PermissionBackendException {
try {
permissionBackendForProject.ref(ref).check(RefPermission.READ);
@@ -544,17 +458,6 @@
return true;
}
- private boolean isGroupOwner(
- InternalGroup group, @Nullable IdentifiedUser user, boolean isAdmin) {
- requireNonNull(group);
-
- // Keep this logic in sync with GroupControl#isOwner().
- boolean isGroupOwner =
- isAdmin || (user != null && user.getEffectiveGroups().contains(group.getOwnerGroupUUID()));
- logger.atFinest().log("User is owner of group %s = %s", group.getGroupUUID(), isGroupOwner);
- return isGroupOwner;
- }
-
/**
* Returns true if the user can see the provided change ref. Uses NoteDb for evaluation, hence
* does not suffer from the limitations documented in {@link SearchingChangeCacheImpl}.
diff --git a/java/com/google/gerrit/server/permissions/ProjectControl.java b/java/com/google/gerrit/server/permissions/ProjectControl.java
index 145e0b6..edffcc6 100644
--- a/java/com/google/gerrit/server/permissions/ProjectControl.java
+++ b/java/com/google/gerrit/server/permissions/ProjectControl.java
@@ -36,8 +36,10 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.GroupMembership;
+import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GitReceivePackGroups;
import com.google.gerrit.server.config.GitUploadPackGroups;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
@@ -68,11 +70,14 @@
private final Set<AccountGroup.UUID> uploadGroups;
private final Set<AccountGroup.UUID> receiveGroups;
private final PermissionBackend permissionBackend;
+ private final RefVisibilityControl refVisibilityControl;
+ private final GitRepositoryManager repositoryManager;
private final CurrentUser user;
private final ProjectState state;
private final ChangeControl.Factory changeControlFactory;
private final PermissionCollection.Factory permissionFilter;
private final DefaultRefFilter.Factory refFilterFactory;
+ private final AllUsersName allUsersName;
private List<SectionMatcher> allSections;
private Map<String, RefControl> refControls;
@@ -85,7 +90,10 @@
PermissionCollection.Factory permissionFilter,
ChangeControl.Factory changeControlFactory,
PermissionBackend permissionBackend,
+ RefVisibilityControl refVisibilityControl,
+ GitRepositoryManager repositoryManager,
DefaultRefFilter.Factory refFilterFactory,
+ AllUsersName allUsersName,
@Assisted CurrentUser who,
@Assisted ProjectState ps) {
this.changeControlFactory = changeControlFactory;
@@ -93,7 +101,10 @@
this.receiveGroups = receiveGroups;
this.permissionFilter = permissionFilter;
this.permissionBackend = permissionBackend;
+ this.refVisibilityControl = refVisibilityControl;
+ this.repositoryManager = repositoryManager;
this.refFilterFactory = refFilterFactory;
+ this.allUsersName = allUsersName;
user = who;
state = ps;
}
@@ -122,7 +133,7 @@
RefControl ctl = refControls.get(refName);
if (ctl == null) {
PermissionCollection relevant = permissionFilter.filter(access(), refName, user);
- ctl = new RefControl(this, refName, relevant);
+ ctl = new RefControl(refVisibilityControl, this, repositoryManager, refName, relevant);
refControls.put(refName, ctl);
}
return ctl;
@@ -169,7 +180,9 @@
}
boolean allRefsAreVisible(Set<String> ignore) {
- return user.isInternalUser() || canPerformOnAllRefs(Permission.READ, ignore);
+ return user.isInternalUser()
+ || (!getProject().getNameKey().equals(allUsersName)
+ && canPerformOnAllRefs(Permission.READ, ignore));
}
/** Can the user run upload pack? */
@@ -447,7 +460,7 @@
return canPushToAtLeastOneRef();
case READ_CONFIG:
- return controlForRef(RefNames.REFS_CONFIG).isVisible();
+ return controlForRef(RefNames.REFS_CONFIG).hasReadPermissionOnRef(false);
case BAN_COMMIT:
case READ_REFLOG:
diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java
index 7c5d6bd..9e78b7e 100644
--- a/java/com/google/gerrit/server/permissions/RefControl.java
+++ b/java/com/google/gerrit/server/permissions/RefControl.java
@@ -16,6 +16,7 @@
import static com.google.common.base.Preconditions.checkArgument;
+import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRange;
@@ -28,22 +29,30 @@
import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.logging.CallerFinder;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
import com.google.gerrit.server.permissions.PermissionBackend.ForRef;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.util.MagicBranch;
+import java.io.IOException;
import java.util.Collection;
import java.util.EnumSet;
import java.util.List;
import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
/** Manages access control for Git references (aka branches, tags). */
class RefControl {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final RefVisibilityControl refVisibilityControl;
private final ProjectControl projectControl;
+ private final GitRepositoryManager repositoryManager;
private final String refName;
/** All permissions that apply to this reference. */
@@ -56,10 +65,17 @@
private Boolean owner;
private Boolean canForgeAuthor;
private Boolean canForgeCommitter;
- private Boolean isVisible;
+ private Boolean hasReadPermissionOnRef;
- RefControl(ProjectControl projectControl, String ref, PermissionCollection relevant) {
+ RefControl(
+ RefVisibilityControl refVisibilityControl,
+ ProjectControl projectControl,
+ GitRepositoryManager repositoryManager,
+ String ref,
+ PermissionCollection relevant) {
+ this.refVisibilityControl = refVisibilityControl;
this.projectControl = projectControl;
+ this.repositoryManager = repositoryManager;
this.refName = ref;
this.relevant = relevant;
this.callerFinder =
@@ -92,12 +108,27 @@
return owner;
}
- /** Can this user see this reference exists? */
- boolean isVisible() {
- if (isVisible == null) {
- isVisible = getUser().isInternalUser() || canPerform(Permission.READ);
+ /**
+ * Returns {@code true} if the user has permission to read the ref. This method evaluates {@link
+ * RefPermission#READ} only. Hence, it is not authoritative. For example, it does not tell if the
+ * user can see NoteDb refs such as {@code refs/meta/external-ids} which requires {@link
+ * GlobalPermission#ACCESS_DATABASE} and deny access in this case.
+ */
+ boolean hasReadPermissionOnRef(boolean allowNoteDbRefs) {
+ // Don't allow checking for NoteDb refs unless instructed otherwise.
+ if (!allowNoteDbRefs
+ && (refName.startsWith(Constants.R_TAGS) || RefNames.isGerritRef(refName))) {
+ logger.atWarning().atMostEvery(30, TimeUnit.SECONDS).log(
+ "%s: Can't determine visibility of %s in RefControl. Denying access. "
+ + "This case should have been handled before.",
+ projectControl.getProject().getName(), refName);
+ return false;
}
- return isVisible;
+
+ if (hasReadPermissionOnRef == null) {
+ hasReadPermissionOnRef = getUser().isInternalUser() || canPerform(Permission.READ);
+ }
+ return hasReadPermissionOnRef;
}
/** @return true if this user can add a new patch set to this ref */
@@ -575,7 +606,10 @@
private boolean can(RefPermission perm) throws PermissionBackendException {
switch (perm) {
case READ:
- return isVisible();
+ if (refName.startsWith(Constants.R_TAGS)) {
+ return isTagVisible();
+ }
+ return refVisibilityControl.isVisible(projectControl, refName);
case CREATE:
// TODO This isn't an accurate test.
return canPerform(refPermissionName(perm));
@@ -625,6 +659,38 @@
}
throw new PermissionBackendException(perm + " unsupported");
}
+
+ private boolean isTagVisible() throws PermissionBackendException {
+ if (projectControl.asForProject().test(ProjectPermission.READ)) {
+ // The user has READ on refs/* with no effective block permission. This is the broadest
+ // permission one can assign. There is no way to grant access to (specific) tags in Gerrit,
+ // so we have to assume that these users can see all tags because there could be tags that
+ // aren't reachable by any visible ref while the user can see all non-Gerrit refs. This
+ // matches Gerrit's historic behavior.
+ // This makes it so that these users could see commits that they can't see otherwise
+ // (e.g. a private change ref) if a tag was attached to it. Tags are meant to be used on
+ // the regular Git tree that users interact with, not on any of the Gerrit trees, so this
+ // is a negligible risk.
+ return true;
+ }
+
+ try (Repository repo =
+ repositoryManager.openRepository(projectControl.getProject().getNameKey())) {
+ // Tag visibility requires going through RefFilter because it entails loading all taggable
+ // refs and filtering them all by visibility.
+ Ref resolvedRef = repo.getRefDatabase().exactRef(refName);
+ if (resolvedRef == null) {
+ return false;
+ }
+ return projectControl.asForProject()
+ .filter(
+ ImmutableList.of(resolvedRef), repo, PermissionBackend.RefFilterOptions.defaults())
+ .stream()
+ .anyMatch(r -> refName.equals(r.getName()));
+ } catch (IOException e) {
+ throw new PermissionBackendException(e);
+ }
+ }
}
private static String refPermissionName(RefPermission refPermission) {
diff --git a/java/com/google/gerrit/server/permissions/RefVisibilityControl.java b/java/com/google/gerrit/server/permissions/RefVisibilityControl.java
new file mode 100644
index 0000000..c8a1ebe
--- /dev/null
+++ b/java/com/google/gerrit/server/permissions/RefVisibilityControl.java
@@ -0,0 +1,181 @@
+// Copyright (C) 2020 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.permissions;
+
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.entities.RefNames.REFS_CACHE_AUTOMERGE;
+
+import com.google.common.base.Throwables;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.exceptions.NoSuchGroupException;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.GroupControl;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.query.change.ChangeData;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+import org.eclipse.jgit.lib.Constants;
+
+/**
+ * This class is a component that is internal to {@link DefaultPermissionBackend}. It can
+ * authoritatively tell if a ref is accessible by a user.
+ */
+@Singleton
+class RefVisibilityControl {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final PermissionBackend permissionBackend;
+ private final GroupControl.GenericFactory groupControlFactory;
+ private final ChangeData.Factory changeDataFactory;
+
+ @Inject
+ RefVisibilityControl(
+ PermissionBackend permissionBackend,
+ GroupControl.GenericFactory groupControlFactory,
+ ChangeData.Factory changeDataFactory) {
+ this.permissionBackend = permissionBackend;
+ this.groupControlFactory = groupControlFactory;
+ this.changeDataFactory = changeDataFactory;
+ }
+
+ /**
+ * Returns an authoritative answer if the ref is visible to the user. Does not have support for
+ * tags and will throw a {@link PermissionBackendException} if asked for tags visibility.
+ */
+ boolean isVisible(ProjectControl projectControl, String refName)
+ throws PermissionBackendException {
+ if (refName.startsWith(Constants.R_TAGS)) {
+ throw new PermissionBackendException(
+ "can't check tags through RefVisibilityControl. Use PermissionBackend#filter instead.");
+ }
+ if (!RefNames.isGerritRef(refName)) {
+ // This is not a special Gerrit ref and not a NoteDb ref. Likely, it's just a ref under
+ // refs/heads or another ref the user created. Apply the regular permissions with inheritance.
+ return projectControl.controlForRef(refName).hasReadPermissionOnRef(false);
+ }
+
+ if (refName.startsWith(REFS_CACHE_AUTOMERGE)) {
+ // Internal cache state that is accessible to no one.
+ return false;
+ }
+
+ boolean hasAccessDatabase =
+ permissionBackend
+ .user(projectControl.getUser())
+ .testOrFalse(GlobalPermission.ACCESS_DATABASE);
+ if (hasAccessDatabase) {
+ return true;
+ }
+
+ // Change and change edit visibility
+ Change.Id changeId;
+ if ((changeId = Change.Id.fromRef(refName)) != null) {
+ // Change ref is visible only if the change is visible.
+ ChangeData cd;
+ try {
+ cd = changeDataFactory.create(projectControl.getProject().getNameKey(), changeId);
+ checkState(cd.change().getId().equals(changeId));
+ } catch (StorageException e) {
+ if (Throwables.getCausalChain(e).stream()
+ .anyMatch(e2 -> e2 instanceof NoSuchChangeException)) {
+ // The change was deleted or is otherwise not accessible anymore.
+ // If the caller can see all refs and is allowed to see private changes on refs/, allow
+ // access. This is an escape hatch for receivers of "ref deleted" events.
+ PermissionBackend.ForProject forProject = projectControl.asForProject();
+ return forProject.test(ProjectPermission.READ)
+ && forProject.ref("refs/").test(RefPermission.READ_PRIVATE_CHANGES);
+ }
+ throw new PermissionBackendException(e);
+ }
+ if (RefNames.isRefsEdit(refName)) {
+ // Edits are visible only to the owning user, if change is visible.
+ return visibleEdit(refName, projectControl, cd);
+ }
+ return projectControl.controlFor(cd.notes()).isVisible();
+ }
+
+ // Account visibility
+ CurrentUser user = projectControl.getUser();
+ Account.Id currentUserAccountId = user.isIdentifiedUser() ? user.getAccountId() : null;
+ Account.Id accountId;
+ if ((accountId = Account.Id.fromRef(refName)) != null) {
+ // Account ref is visible only to the corresponding account.
+ if (accountId.equals(currentUserAccountId)
+ && projectControl.controlForRef(refName).hasReadPermissionOnRef(true)) {
+ return true;
+ }
+ return false;
+ }
+
+ // Group visibility
+ AccountGroup.UUID accountGroupUuid;
+ if ((accountGroupUuid = AccountGroup.UUID.fromRef(refName)) != null) {
+ // Group ref is visible only to the corresponding owner group.
+ try {
+ return projectControl.controlForRef(refName).hasReadPermissionOnRef(true)
+ && groupControlFactory.controlFor(user, accountGroupUuid).isOwner();
+ } catch (NoSuchGroupException e) {
+ // The group is broken, but the ref is still around. Pretend the ref is not visible.
+ logger.atWarning().withCause(e).log("Found group ref %s but group isn't parsable", refName);
+ return false;
+ }
+ }
+
+ // We are done checking all cases where we would allow access to Gerrit-managed refs. Deny
+ // access in case we got this far.
+ logger.atFine().log(
+ "Denying access to %s because user doesn't have access to this Gerrit ref", refName);
+ return false;
+ }
+
+ private boolean visibleEdit(String refName, ProjectControl projectControl, ChangeData cd)
+ throws PermissionBackendException {
+ Change.Id id = Change.Id.fromEditRefPart(refName);
+ if (id == null) {
+ throw new IllegalStateException("unable to parse change id from edit ref " + refName);
+ }
+
+ if (!projectControl.controlFor(cd.notes()).isVisible()) {
+ // The user can't see the change so they can't see any edits.
+ return false;
+ }
+
+ if (projectControl.getUser().isIdentifiedUser()
+ && refName.startsWith(
+ RefNames.refsEditPrefix(projectControl.getUser().asIdentifiedUser().getAccountId()))) {
+ logger.atFinest().log("Own change edit ref is visible: %s", refName);
+ return true;
+ }
+
+ try {
+ // Default to READ_PRIVATE_CHANGES as there is no special permission for reading edits.
+ projectControl
+ .asForProject()
+ .ref(cd.change().getDest().branch())
+ .check(RefPermission.READ_PRIVATE_CHANGES);
+ logger.atFinest().log("Foreign change edit ref is visible: %s", refName);
+ return true;
+ } catch (AuthException e) {
+ logger.atFinest().log("Foreign change edit ref is not visible: %s", refName);
+ return false;
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 5c786a5..bf77b91 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1069,6 +1069,11 @@
com.google.gerrit.acceptance.TestAccount deleteAs)
throws Exception {
try {
+ projectOperations
+ .project(projectName)
+ .forUpdate()
+ .add(allow(Permission.VIEW_PRIVATE_CHANGES).ref("refs/*").group(ANONYMOUS_USERS))
+ .update();
requestScopeOperations.setApiUser(owner.id());
ChangeInput in = new ChangeInput();
in.project = projectName.get();
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index dcf2afd..10bd03b 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -1160,6 +1160,11 @@
@Test
public void pushToDeletedGroupBranchIsRejectedForAllUsersRepo() throws Exception {
+ // refs/deleted-groups is only visible with ACCESS_DATABASE
+ projectOperations
+ .allProjectsForUpdate()
+ .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .update();
String groupRef =
RefNames.refsDeletedGroups(AccountGroup.uuid(gApi.groups().create(name("foo")).get().id));
createBranch(allUsers, groupRef);
@@ -1352,6 +1357,11 @@
@Test
public void cannotDeleteDeletedGroupBranch() throws Exception {
+ // refs/deleted-groups is only visible with ACCESS_DATABASE
+ projectOperations
+ .allProjectsForUpdate()
+ .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .update();
String groupRef = RefNames.refsDeletedGroups(AccountGroup.uuid(name("foo")));
createBranch(allUsers, groupRef);
testCannotDeleteGroupBranch(RefNames.REFS_DELETED_GROUPS + "*", groupRef);
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java b/javatests/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java
index d70d120..9276b9a 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AbstractPushTag.java
@@ -31,7 +31,9 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.testing.ConfigSuite;
import com.google.inject.Inject;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.PushResult;
@@ -52,6 +54,13 @@
}
}
+ @ConfigSuite.Config
+ public static Config skipFalse() {
+ Config config = new Config();
+ config.setBoolean("auth", null, "skipFullRefEvaluationIfAllRefsAreVisible", false);
+ return config;
+ }
+
@Inject private ProjectOperations projectOperations;
private RevCommit initialHead;
@@ -93,6 +102,20 @@
}
@Test
+ public void createTagForExistingCommit_withoutGlobalReadPermissions() throws Exception {
+ removeReadAccessOnRefsStar();
+ grantReadAccessOnRefsHeadsStar();
+ createTagForExistingCommit();
+ }
+
+ @Test
+ public void createTagForNewCommit_withoutGlobalReadPermissions() throws Exception {
+ removeReadAccessOnRefsStar();
+ grantReadAccessOnRefsHeadsStar();
+ createTagForNewCommit();
+ }
+
+ @Test
public void fastForward() throws Exception {
allowTagCreation();
String tagName = pushTagForExistingCommit(Status.OK);
@@ -109,6 +132,15 @@
fastForwardTagToExistingCommit(tagName, expectedStatus);
fastForwardTagToNewCommit(tagName, expectedStatus);
+ // Above we just fast-forwarded the tag to a new commit which is not part of any branch. By
+ // default this tag is not visible, as users can only see tags that point to commits that are
+ // part of visible branches, which is not the case for this tag. It's odd that we allow the user
+ // to create such a tag that is then not visible to the creator. Below we want to fast-forward
+ // this tag, but this is only possible if the tag is visible. To make it visible we must allow
+ // the user to read all tags, regardless of whether it points to a commit that is part of a
+ // visible branch.
+ allowReadingAllTag();
+
allowForcePushOnRefsTags();
fastForwardTagToExistingCommit(tagName, Status.OK);
fastForwardTagToNewCommit(tagName, Status.OK);
@@ -234,6 +266,49 @@
assertWithMessage(tagType.name()).that(refUpdate.getStatus()).isEqualTo(expectedStatus);
}
+ private void removeReadAccessOnRefsStar() {
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .remove(permissionKey(Permission.READ).ref("refs/*"))
+ .update();
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .remove(permissionKey(Permission.READ).ref("refs/*"))
+ .update();
+ }
+
+ private void grantReadAccessOnRefsHeadsStar() {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/heads/*").group(REGISTERED_USERS))
+ .update();
+ }
+
+ private void allowReadingAllTag() throws Exception {
+ // Tags are only visible if the commits to which they point are part of a visible branch.
+ // To make all tags visible, including tags that point to commits that are not part of a visible
+ // branch, either auth.skipFullRefEvaluationIfAllRefsAreVisible in gerrit.config needs to be
+ // true, or the user must have READ access for all refs in the repository.
+
+ if (cfg.getBoolean("auth", "skipFullRefEvaluationIfAllRefsAreVisible", true)) {
+ return;
+ }
+
+ // By default READ access in the All-Projects project is granted to registered users on refs/*,
+ // which makes all refs, except refs/meta/config, visible to them. refs/meta/config is not
+ // visible since by default READ access to it is exclusively granted to the project owners only.
+ // This means to make all refs, and thus all tags, visible, we must allow registered users to
+ // see the refs/meta/config branch.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/meta/config").group(REGISTERED_USERS))
+ .update();
+ }
+
private void allowTagCreation() throws Exception {
projectOperations
.project(project)
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/GetBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/GetBranchIT.java
new file mode 100644
index 0000000..964cff7
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/project/GetBranchIT.java
@@ -0,0 +1,526 @@
+// Copyright (C) 2020 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.acceptance.rest.project;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.capabilityKey;
+import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.api.changes.DraftInput;
+import com.google.gerrit.extensions.api.projects.BranchInfo;
+import com.google.gerrit.extensions.api.projects.TagInfo;
+import com.google.gerrit.extensions.api.projects.TagInput;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.notedb.Sequences;
+import com.google.gerrit.testing.ConfigSuite;
+import com.google.inject.Inject;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Test;
+
+public class GetBranchIT extends AbstractDaemonTest {
+ @Inject private GroupOperations groupOperations;
+ @Inject private ProjectOperations projectOperations;
+ @Inject private RequestScopeOperations requestScopeOperations;
+
+ @ConfigSuite.Config
+ public static Config skipFalse() {
+ Config config = new Config();
+ config.setBoolean("auth", null, "skipFullRefEvaluationIfAllRefsAreVisible", false);
+ return config;
+ }
+
+ @Test
+ public void cannotGetNonExistingBranch() {
+ assertBranchNotFound(project, RefNames.fullName("non-existing"));
+ }
+
+ @Test
+ public void getBranch() throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchFound(project, RefNames.fullName("master"));
+ }
+
+ @Test
+ public void getBranchByShortName() throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchFound(project, "master");
+ }
+
+ @Test
+ public void cannotGetNonVisibleBranch() {
+ String branchName = "master";
+
+ // block read access to the branch
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref(RefNames.fullName(branchName)).group(ANONYMOUS_USERS))
+ .update();
+
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(project, RefNames.fullName(branchName));
+ }
+
+ @Test
+ public void cannotGetNonVisibleBranchByShortName() {
+ String branchName = "master";
+
+ // block read access to the branch
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref(RefNames.fullName(branchName)).group(ANONYMOUS_USERS))
+ .update();
+
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(project, branchName);
+ }
+
+ @Test
+ public void getChangeRef() throws Exception {
+ // create a change
+ Change.Id changeId = createChange("refs/for/master").getPatchSetId().changeId();
+
+ // a user without the 'Access Database' capability can see the change ref
+ requestScopeOperations.setApiUser(user.id());
+ String changeRef = RefNames.patchSetRef(PatchSet.id(changeId, 1));
+ assertBranchFound(project, changeRef);
+ }
+
+ @Test
+ public void getChangeRefOfNonVisibleChange() throws Exception {
+ // create a change
+ String branchName = "master";
+ Change.Id changeId = createChange("refs/for/" + branchName).getPatchSetId().changeId();
+
+ // block read access to the branch
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref(RefNames.fullName(branchName)).group(ANONYMOUS_USERS))
+ .update();
+
+ // a user without the 'Access Database' capability cannot see the change ref
+ requestScopeOperations.setApiUser(user.id());
+ String changeRef = RefNames.patchSetRef(PatchSet.id(changeId, 1));
+ assertBranchNotFound(project, changeRef);
+
+ // a user with the 'Access Database' capability can see the change ref
+ testGetRefWithAccessDatabase(project, changeRef);
+ }
+
+ @Test
+ public void getChangeEditRef() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ // create a change
+ Change.Id changeId = createChange("refs/for/master").getPatchSetId().changeId();
+
+ // create a change edit by 'user'
+ requestScopeOperations.setApiUser(user.id());
+ gApi.changes().id(changeId.get()).edit().create();
+
+ // every user can see their own change edit refs
+ String changeEditRef = RefNames.refsEdit(user.id(), changeId, PatchSet.id(changeId, 1));
+ assertBranchFound(project, changeEditRef);
+
+ // a user without the 'Access Database' capability cannot see the change edit ref of another
+ // user
+ requestScopeOperations.setApiUser(user2.id());
+ assertBranchNotFound(project, changeEditRef);
+
+ // a user with the 'Access Database' capability can see the change edit ref of another user
+ testGetRefWithAccessDatabase(project, changeEditRef);
+ }
+
+ @Test
+ public void cannotGetChangeEditRefOfNonVisibleChange() throws Exception {
+ // create a change
+ String branchName = "master";
+ Change.Id changeId = createChange("refs/for/" + branchName).getPatchSetId().changeId();
+
+ // create a change edit by 'user'
+ requestScopeOperations.setApiUser(user.id());
+ gApi.changes().id(changeId.get()).edit().create();
+
+ // make the change non-visible by blocking read access on the destination
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref(RefNames.fullName(branchName)).group(ANONYMOUS_USERS))
+ .update();
+
+ // user cannot see their own change edit refs if the change is no longer visible
+ String changeEditRef = RefNames.refsEdit(user.id(), changeId, PatchSet.id(changeId, 1));
+ assertBranchNotFound(project, changeEditRef);
+
+ // a user with the 'Access Database' capability can see the change edit ref
+ testGetRefWithAccessDatabase(project, changeEditRef);
+ }
+
+ @Test
+ public void getChangeMetaRef() throws Exception {
+ // create a change
+ Change.Id changeId = createChange("refs/for/master").getPatchSetId().changeId();
+
+ // A user without the 'Access Database' capability can see the change meta ref.
+ // This may be surprising, as 'Access Database' guards access to meta refs and the change meta
+ // ref is a meta ref, however change meta refs have been always visible to all users that can
+ // see the change and some tools rely on seeing these refs, so we have to keep the current
+ // behaviour.
+ requestScopeOperations.setApiUser(user.id());
+ String changeMetaRef = RefNames.changeMetaRef(changeId);
+ assertBranchFound(project, changeMetaRef);
+ }
+
+ @Test
+ public void getRefsMetaConfig() throws Exception {
+ // a non-project owner cannot get the refs/meta/config branch
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(project, RefNames.REFS_CONFIG);
+
+ // a non-project owner cannot get the refs/meta/config branch even with the 'Access Database'
+ // capability
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .update();
+ try {
+ assertBranchNotFound(project, RefNames.REFS_CONFIG);
+ } finally {
+ projectOperations
+ .allProjectsForUpdate()
+ .remove(
+ capabilityKey(GlobalCapability.ACCESS_DATABASE)
+ .group(SystemGroupBackend.REGISTERED_USERS))
+ .update();
+ }
+
+ requestScopeOperations.setApiUser(user.id());
+
+ // a project owner can get the refs/meta/config branch
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.OWNER).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+ assertBranchFound(project, RefNames.REFS_CONFIG);
+ }
+
+ @Test
+ public void getUserRefOfOtherUser() throws Exception {
+ String userRef = RefNames.refsUsers(admin.id());
+
+ // a user without the 'Access Database' capability cannot see the user ref of another user
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(allUsers, userRef);
+
+ // a user with the 'Access Database' capability can see the user ref of another user
+ testGetRefWithAccessDatabase(allUsers, userRef);
+ }
+
+ @Test
+ public void getOwnUserRef() throws Exception {
+ // every user can see the own user ref
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchFound(allUsers, RefNames.refsUsers(user.id()));
+
+ // TODO: every user can see the own user ref via the magic ref/users/self ref
+ // requestScopeOperations.setApiUser(user.id());
+ // assertBranchFound(allUsers, RefNames.REFS_USERS_SELF);
+ }
+
+ @Test
+ public void getExternalIdsRefs() throws Exception {
+ // a user without the 'Access Database' capability cannot see the refs/meta/external-ids ref
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(allUsers, RefNames.REFS_EXTERNAL_IDS);
+
+ // a user with the 'Access Database' capability can see the refs/meta/external-ids ref
+ testGetRefWithAccessDatabase(allUsers, RefNames.REFS_EXTERNAL_IDS);
+ }
+
+ @Test
+ public void getGroupRef() throws Exception {
+ // create a group
+ AccountGroup.UUID ownerGroupUuid =
+ groupOperations.newGroup().name("owner-group").addMember(admin.id()).create();
+ AccountGroup.UUID testGroupUuid =
+ groupOperations.newGroup().name("test-group").ownerGroupUuid(ownerGroupUuid).create();
+
+ // a non-group owner without the 'Access Database' capability cannot see the group ref
+ requestScopeOperations.setApiUser(user.id());
+ String groupRef = RefNames.refsGroups(testGroupUuid);
+ assertBranchNotFound(allUsers, groupRef);
+
+ // a non-group owner with the 'Access Database' capability can see the group ref
+ testGetRefWithAccessDatabase(allUsers, groupRef);
+
+ // a group owner can see the group ref if the group ref is visible
+ groupOperations.group(ownerGroupUuid).forUpdate().addMember(user.id()).update();
+ assertBranchFound(allUsers, groupRef);
+
+ // A group owner cannot see the group ref if the group ref is not visible.
+ // The READ access for refs/groups/* must be blocked on All-Projects rather than All-Users.
+ // This is because READ access for refs/groups/* on All-Users is by default granted to
+ // REGISTERED_USERS, and if an ALLOW rule and a BLOCK rule are on the same project and ref,
+ // the ALLOW rule takes precedence.
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/groups/*").group(ANONYMOUS_USERS))
+ .update();
+ assertBranchNotFound(allUsers, groupRef);
+ }
+
+ @Test
+ public void getGroupNamesRef() throws Exception {
+ // a user without the 'Access Database' capability cannot see the refs/meta/group-names ref
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(allUsers, RefNames.REFS_GROUPNAMES);
+
+ // a user with the 'Access Database' capability can see the refs/meta/group-names ref
+ testGetRefWithAccessDatabase(allUsers, RefNames.REFS_GROUPNAMES);
+ }
+
+ @Test
+ public void getDeletedGroupRef() throws Exception {
+ // Create a deleted group ref. We must create a directly in the repo, since group deletion is
+ // not supported yet.
+ String deletedGroupRef = RefNames.refsDeletedGroups(AccountGroup.uuid("deleted-group"));
+ try (TestRepository<Repository> testRepo =
+ new TestRepository<>(repoManager.openRepository(allUsers))) {
+ testRepo
+ .branch(deletedGroupRef)
+ .commit()
+ .message("Some Message")
+ .add("group.config", "content")
+ .create();
+ }
+
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(allUsers, deletedGroupRef);
+
+ // a user with the 'Access Database' capability can see the deleted group ref
+ testGetRefWithAccessDatabase(allUsers, deletedGroupRef);
+ }
+
+ @Test
+ public void getDraftCommentsRef() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ // create a change
+ String fileName = "a.txt";
+ Change change = createChange("A Change", fileName, "content").getChange().change();
+
+ // create a draft comment by the by 'user'
+ requestScopeOperations.setApiUser(user.id());
+ DraftInput draftInput = new DraftInput();
+ draftInput.path = fileName;
+ draftInput.line = 0;
+ draftInput.message = "Some Comment";
+ gApi.changes().id(change.getChangeId()).current().createDraft(draftInput);
+
+ // every user can see their own draft comments refs
+ // TODO: is this a bug?
+ String draftCommentsRef = RefNames.refsDraftComments(change.getId(), user.id());
+ assertBranchFound(allUsers, draftCommentsRef);
+
+ // a user without the 'Access Database' capability cannot see the draft comments ref of another
+ // user
+ requestScopeOperations.setApiUser(user2.id());
+ assertBranchNotFound(allUsers, draftCommentsRef);
+
+ // a user with the 'Access Database' capability can see the draft comments ref of another user
+ testGetRefWithAccessDatabase(allUsers, draftCommentsRef);
+ }
+
+ @Test
+ public void getStarredChangesRef() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ // create a change
+ Change change = createChange().getChange().change();
+
+ // let user star the change
+ requestScopeOperations.setApiUser(user.id());
+ gApi.accounts().self().starChange(Integer.toString(change.getChangeId()));
+
+ // every user can see their own starred changes refs
+ // TODO: is this a bug?
+ String starredChangesRef = RefNames.refsStarredChanges(change.getId(), user.id());
+ assertBranchFound(allUsers, starredChangesRef);
+
+ // a user without the 'Access Database' capability cannot see the starred changes ref of another
+ // user
+ requestScopeOperations.setApiUser(user2.id());
+ assertBranchNotFound(allUsers, starredChangesRef);
+
+ // a user with the 'Access Database' capability can see the starred changes ref of another user
+ testGetRefWithAccessDatabase(allUsers, starredChangesRef);
+ }
+
+ @Test
+ public void getTagRef() throws Exception {
+ // create a tag
+ TagInput input = new TagInput();
+ input.message = "My Tag";
+ input.revision = projectOperations.project(project).getHead("master").name();
+ TagInfo tagInfo = gApi.projects().name(project.get()).tag("my-tag").create(input).get();
+
+ // any user who can see the project, can see the tag
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchFound(project, tagInfo.ref);
+ }
+
+ @Test
+ public void cannotGetTagRefThatPointsToNonVisibleBranch() throws Exception {
+ // create a tag
+ TagInput input = new TagInput();
+ input.message = "My Tag";
+ input.revision = projectOperations.project(project).getHead("master").name();
+ TagInfo tagInfo = gApi.projects().name(project.get()).tag("my-tag").create(input).get();
+
+ // block read access to the branch
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref(RefNames.fullName("master")).group(ANONYMOUS_USERS))
+ .update();
+
+ // if the user cannot see the project, the tag is not visible
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(project, tagInfo.ref);
+ }
+
+ @Test
+ public void getSymbolicRef() throws Exception {
+ // 'HEAD' is visible since it points to 'master' that is visible
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchFound(project, "HEAD");
+ }
+
+ @Test
+ public void cannotGetSymbolicRefThatPointsToNonVisibleBranch() {
+ // block read access to the branch to which HEAD points by default
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref(RefNames.fullName("master")).group(ANONYMOUS_USERS))
+ .update();
+
+ // since 'master' is not visible, 'HEAD' which points to 'master' is also not visible
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchNotFound(project, "HEAD");
+ }
+
+ @Test
+ public void getAccountSequenceRef() throws Exception {
+ // a user without the 'Access Database' capability cannot see the refs/sequences/accounts ref
+ requestScopeOperations.setApiUser(user.id());
+ String accountSequenceRef = RefNames.REFS_SEQUENCES + Sequences.NAME_ACCOUNTS;
+ assertBranchNotFound(allUsers, accountSequenceRef);
+
+ // a user with the 'Access Database' capability can see the refs/sequences/accounts ref
+ testGetRefWithAccessDatabase(allUsers, accountSequenceRef);
+ }
+
+ @Test
+ public void getChangeSequenceRef() throws Exception {
+ // a user without the 'Access Database' capability cannot see the refs/sequences/changes ref
+ requestScopeOperations.setApiUser(user.id());
+ String changeSequenceRef = RefNames.REFS_SEQUENCES + Sequences.NAME_CHANGES;
+ assertBranchNotFound(allProjects, changeSequenceRef);
+
+ // a user with the 'Access Database' capability can see the refs/sequences/changes ref
+ testGetRefWithAccessDatabase(allProjects, changeSequenceRef);
+ }
+
+ @Test
+ public void getGroupSequenceRef() throws Exception {
+ // a user without the 'Access Database' capability cannot see the refs/sequences/groups ref
+ requestScopeOperations.setApiUser(user.id());
+ String groupSequenceRef = RefNames.REFS_SEQUENCES + Sequences.NAME_GROUPS;
+ assertBranchNotFound(allUsers, groupSequenceRef);
+
+ // a user with the 'Access Database' capability can see the refs/sequences/groups ref
+ testGetRefWithAccessDatabase(allUsers, groupSequenceRef);
+ }
+
+ @Test
+ public void getVersionMetaRef() throws Exception {
+ // TODO: a user without the 'Access Database' capability cannot see the refs/meta/version ref
+ // requestScopeOperations.setApiUser(user.id());
+ // assertBranchNotFound(allProjects, RefNames.REFS_VERSION);
+
+ // a user with the 'Access Database' capability can see the refs/meta/vaersion ref
+ testGetRefWithAccessDatabase(allProjects, RefNames.REFS_VERSION);
+ }
+
+ private void testGetRefWithAccessDatabase(Project.NameKey project, String ref)
+ throws RestApiException {
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
+ .update();
+ try {
+ requestScopeOperations.setApiUser(user.id());
+ assertBranchFound(project, ref);
+ } finally {
+ projectOperations
+ .allProjectsForUpdate()
+ .remove(
+ capabilityKey(GlobalCapability.ACCESS_DATABASE)
+ .group(SystemGroupBackend.REGISTERED_USERS))
+ .update();
+ }
+ }
+
+ private void assertBranchNotFound(Project.NameKey project, String ref) {
+ ResourceNotFoundException exception =
+ assertThrows(
+ ResourceNotFoundException.class,
+ () -> gApi.projects().name(project.get()).branch(ref).get());
+ assertThat(exception).hasMessageThat().isEqualTo("Not found: " + ref);
+ }
+
+ private void assertBranchFound(Project.NameKey project, String ref) throws RestApiException {
+ BranchInfo branchInfo = gApi.projects().name(project.get()).branch(ref).get();
+ assertThat(branchInfo.ref).isEqualTo(RefNames.fullName(ref));
+ }
+}
diff --git a/javatests/com/google/gerrit/server/permissions/RefControlTest.java b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
index 33446e4..336b6fb 100644
--- a/javatests/com/google/gerrit/server/permissions/RefControlTest.java
+++ b/javatests/com/google/gerrit/server/permissions/RefControlTest.java
@@ -48,6 +48,7 @@
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.account.ListGroupMembership;
import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.index.SingleVersionModule.SingleVersionListener;
import com.google.gerrit.server.project.ProjectCache;
@@ -63,6 +64,7 @@
import com.google.inject.Inject;
import com.google.inject.Injector;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.Optional;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Repository;
@@ -90,6 +92,18 @@
assertWithMessage("not owner").that(u.isOwner()).isFalse();
}
+ private void assertAllRefsAreVisible(ProjectControl u) {
+ assertWithMessage("all refs visible")
+ .that(u.allRefsAreVisible(Collections.emptySet()))
+ .isTrue();
+ }
+
+ private void assertAllRefsAreNotVisible(ProjectControl u) {
+ assertWithMessage("all refs NOT visible")
+ .that(u.allRefsAreVisible(Collections.emptySet()))
+ .isFalse();
+ }
+
private void assertNotOwner(String ref, ProjectControl u) {
assertWithMessage("NOT OWN " + ref).that(u.controlForRef(ref).isOwner()).isFalse();
}
@@ -105,11 +119,21 @@
}
private void assertCanRead(String ref, ProjectControl u) {
- assertWithMessage("can read " + ref).that(u.controlForRef(ref).isVisible()).isTrue();
+ assertWithMessage("can read " + ref)
+ .that(
+ u.controlForRef(ref)
+ .hasReadPermissionOnRef(
+ true)) // This should be false but the test relies on inheritance into refs/tags
+ .isTrue();
}
private void assertCannotRead(String ref, ProjectControl u) {
- assertWithMessage("cannot read " + ref).that(u.controlForRef(ref).isVisible()).isFalse();
+ assertWithMessage("cannot read " + ref)
+ .that(
+ u.controlForRef(ref)
+ .hasReadPermissionOnRef(
+ true)) // This should be false but the test relies on inheritance into refs/tags
+ .isFalse();
}
private void assertCanSubmit(String ref, ProjectControl u) {
@@ -171,6 +195,7 @@
private final Project.NameKey parentKey = Project.nameKey("parent");
@Inject private AllProjectsName allProjectsName;
+ @Inject private AllUsersName allUsersName;
@Inject private InMemoryRepositoryManager repoManager;
@Inject private MetaDataUpdate.Server metaDataUpdateFactory;
@Inject private ProjectCache projectCache;
@@ -262,6 +287,32 @@
}
@Test
+ public void allRefsAreVisibleForRegularProject() throws Exception {
+ projectOperations
+ .project(localKey)
+ .forUpdate()
+ .add(allow(READ).ref("refs/*").group(DEVS))
+ .add(allow(READ).ref("refs/groups/*").group(DEVS))
+ .add(allow(READ).ref("refs/users/default").group(DEVS))
+ .update();
+
+ assertAllRefsAreVisible(user(localKey, DEVS));
+ }
+
+ @Test
+ public void allRefsAreNotVisibleForAllUsers() throws Exception {
+ projectOperations
+ .project(allUsersName)
+ .forUpdate()
+ .add(allow(READ).ref("refs/*").group(DEVS))
+ .add(allow(READ).ref("refs/groups/*").group(DEVS))
+ .add(allow(READ).ref("refs/users/default").group(DEVS))
+ .update();
+
+ assertAllRefsAreNotVisible(user(allUsersName, DEVS));
+ }
+
+ @Test
public void branchDelegation1() throws Exception {
projectOperations
.project(localKey)
diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml
index e9362e6..0cc5802 100644
--- a/tools/maven/gerrit-acceptance-framework_pom.xml
+++ b/tools/maven/gerrit-acceptance-framework_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-acceptance-framework</artifactId>
- <version>3.2.4-SNAPSHOT</version>
+ <version>3.2.6-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Acceptance Test Framework</name>
<description>Framework for Gerrit's acceptance tests</description>
diff --git a/tools/maven/gerrit-extension-api_pom.xml b/tools/maven/gerrit-extension-api_pom.xml
index 3566043..8447331 100644
--- a/tools/maven/gerrit-extension-api_pom.xml
+++ b/tools/maven/gerrit-extension-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-extension-api</artifactId>
- <version>3.2.4-SNAPSHOT</version>
+ <version>3.2.6-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Extension API</name>
<description>API for Gerrit Extensions</description>
diff --git a/tools/maven/gerrit-plugin-api_pom.xml b/tools/maven/gerrit-plugin-api_pom.xml
index 009f627e..d9a9326 100644
--- a/tools/maven/gerrit-plugin-api_pom.xml
+++ b/tools/maven/gerrit-plugin-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-plugin-api</artifactId>
- <version>3.2.4-SNAPSHOT</version>
+ <version>3.2.6-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Plugin API</name>
<description>API for Gerrit Plugins</description>
diff --git a/tools/maven/gerrit-war_pom.xml b/tools/maven/gerrit-war_pom.xml
index e67b4a6..ec6d363 100644
--- a/tools/maven/gerrit-war_pom.xml
+++ b/tools/maven/gerrit-war_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-war</artifactId>
- <version>3.2.4-SNAPSHOT</version>
+ <version>3.2.6-SNAPSHOT</version>
<packaging>war</packaging>
<name>Gerrit Code Review - WAR</name>
<description>Gerrit WAR</description>
diff --git a/version.bzl b/version.bzl
index e41f0aa..30337fa 100644
--- a/version.bzl
+++ b/version.bzl
@@ -2,4 +2,4 @@
# Used by :api_install and :api_deploy targets
# when talking to the destination repository.
#
-GERRIT_VERSION = "3.2.4-SNAPSHOT"
+GERRIT_VERSION = "3.2.6-SNAPSHOT"
