diff --git a/java/com/google/gerrit/entities/RefNames.java b/java/com/google/gerrit/entities/RefNames.java index 400861c8a3..5595bc75e3 100644 --- a/java/com/google/gerrit/entities/RefNames.java +++ b/java/com/google/gerrit/entities/RefNames.java @@ -288,10 +288,16 @@ public class RefNames { * 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. + * + *

Any ref for which this method evaluates to true will be served to users who have the {@code + * ACCESS_DATABASE} capability. + * + *

CautionAny 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 @@ public class RefNames { || 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 0b4828b232..37c773ac31 100644 --- a/java/com/google/gerrit/server/permissions/ChangeControl.java +++ b/java/com/google/gerrit/server/permissions/ChangeControl.java @@ -61,11 +61,12 @@ class ChangeControl { } /** 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? */ @@ -201,17 +202,13 @@ class ChangeControl { private ForChangeImpl() {} - private ChangeData changeData() { - return changeData; - } - @Override public String resourcePath() { if (resourcePath == null) { resourcePath = String.format( "/projects/%s/+changes/%s", - getProjectControl().getProjectState().getName(), changeData().getId().get()); + getProjectControl().getProjectState().getName(), changeData.getId().get()); } return resourcePath; } @@ -256,7 +253,7 @@ class ChangeControl { 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 37de0d15aa..eca30b60c8 100644 --- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java +++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java @@ -16,10 +16,7 @@ package com.google.gerrit.server.permissions; 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.Iterables; 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.Counter0; 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 @@ class DefaultRefFilter { 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 @@ class DefaultRefFilter { 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 @@ class DefaultRefFilter { 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 resultRefs = new ArrayList<>(refs.size()); List 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 @@ class DefaultRefFilter { 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. @@ -519,10 +437,6 @@ class DefaultRefFilter { 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); @@ -543,17 +457,6 @@ class DefaultRefFilter { 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 724017dbf4..1ef4269c12 100644 --- a/java/com/google/gerrit/server/permissions/ProjectControl.java +++ b/java/com/google/gerrit/server/permissions/ProjectControl.java @@ -38,6 +38,7 @@ import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.account.GroupMembership; 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,6 +69,8 @@ class ProjectControl { private final Set uploadGroups; private final Set receiveGroups; private final PermissionBackend permissionBackend; + private final RefVisibilityControl refVisibilityControl; + private final GitRepositoryManager repositoryManager; private final CurrentUser user; private final ProjectState state; private final PermissionCollection.Factory permissionFilter; @@ -84,6 +87,8 @@ class ProjectControl { @GitReceivePackGroups Set receiveGroups, PermissionCollection.Factory permissionFilter, PermissionBackend permissionBackend, + RefVisibilityControl refVisibilityControl, + GitRepositoryManager repositoryManager, DefaultRefFilter.Factory refFilterFactory, ChangeData.Factory changeDataFactory, @Assisted CurrentUser who, @@ -92,6 +97,8 @@ class ProjectControl { this.receiveGroups = receiveGroups; this.permissionFilter = permissionFilter; this.permissionBackend = permissionBackend; + this.refVisibilityControl = refVisibilityControl; + this.repositoryManager = repositoryManager; this.refFilterFactory = refFilterFactory; this.changeDataFactory = changeDataFactory; user = who; @@ -117,7 +124,9 @@ class ProjectControl { RefControl ctl = refControls.get(refName); if (ctl == null) { PermissionCollection relevant = permissionFilter.filter(access(), refName, user); - ctl = new RefControl(changeDataFactory, this, refName, relevant); + ctl = + new RefControl( + changeDataFactory, refVisibilityControl, this, repositoryManager, refName, relevant); refControls.put(refName, ctl); } return ctl; @@ -442,7 +451,7 @@ class ProjectControl { 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 bc802cc5a3..fb90a047d5 100644 --- a/java/com/google/gerrit/server/permissions/RefControl.java +++ b/java/com/google/gerrit/server/permissions/RefControl.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.permissions; import static com.google.common.base.Preconditions.checkArgument; +import com.google.common.collect.ImmutableList; import com.google.common.flogger.FluentLogger; import com.google.gerrit.entities.Change; import com.google.gerrit.entities.Permission; @@ -28,23 +29,31 @@ import com.google.gerrit.exceptions.StorageException; 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 ChangeData.Factory changeDataFactory; + private final RefVisibilityControl refVisibilityControl; private final ProjectControl projectControl; + private final GitRepositoryManager repositoryManager; private final String refName; /** All permissions that apply to this reference. */ @@ -57,15 +66,19 @@ class RefControl { private Boolean owner; private Boolean canForgeAuthor; private Boolean canForgeCommitter; - private Boolean isVisible; + private Boolean hasReadPermissionOnRef; RefControl( ChangeData.Factory changeDataFactory, + RefVisibilityControl refVisibilityControl, ProjectControl projectControl, + GitRepositoryManager repositoryManager, String ref, PermissionCollection relevant) { this.changeDataFactory = changeDataFactory; + this.refVisibilityControl = refVisibilityControl; this.projectControl = projectControl; + this.repositoryManager = repositoryManager; this.refName = ref; this.relevant = relevant; this.callerFinder = @@ -98,12 +111,27 @@ class RefControl { 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 */ @@ -578,7 +606,10 @@ class RefControl { 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)); @@ -628,6 +659,38 @@ class RefControl { } 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 0000000000..474403732b --- /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).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).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 496a2b4406..18f1223b86 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -1061,6 +1061,11 @@ public class ChangeIT extends AbstractDaemonTest { 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 cd4b24de51..8dbec28415 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 @@ public class GroupsIT extends AbstractDaemonTest { @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 @@ public class GroupsIT extends AbstractDaemonTest { @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 191d5c5068..531357a7e4 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.GitUtil; import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.entities.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 @@ public abstract class AbstractPushTag extends AbstractDaemonTest { } } + @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; @@ -92,6 +101,20 @@ public abstract class AbstractPushTag extends AbstractDaemonTest { removePushFromRefsTags(); } + @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(); @@ -109,6 +132,15 @@ public abstract class AbstractPushTag extends AbstractDaemonTest { 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 @@ public abstract class AbstractPushTag extends AbstractDaemonTest { 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 0000000000..b4b1be0e66 --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/rest/project/GetBranchIT.java @@ -0,0 +1,594 @@ +// 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.change.ChangeOperations; +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.entities.AccountGroup; +import com.google.gerrit.entities.BranchNameKey; +import com.google.gerrit.entities.Change; +import com.google.gerrit.entities.PatchSet; +import com.google.gerrit.entities.Permission; +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 ChangeOperations changeOperations; + @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 = changeOperations.newChange().project(project).create(); + + // 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 = changeOperations.newChange().project(project).branch(branchName).create(); + + // 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 = changeOperations.newChange().project(project).create(); + + // 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 = changeOperations.newChange().project(project).branch(branchName).create(); + + // 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 = changeOperations.newChange().project(project).create(); + + // 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 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); + } + + @Test + public void cannotGetAutoMergeRef() throws Exception { + String file = "foo/a.txt"; + + // Create a base change. + Change.Id baseChange = + changeOperations + .newChange() + .project(project) + .branch("master") + .file(file) + .content("base content") + .create(); + approve(Integer.toString(baseChange.get())); + gApi.changes().id(baseChange.get()).current().submit(); + + // Create another branch + String branchName = "foo"; + createBranchWithRevision( + BranchNameKey.create(project, branchName), + projectOperations.project(project).getHead("master").name()); + + // Create a change in master that touches the file. + Change.Id changeInMaster = + changeOperations + .newChange() + .project(project) + .branch("master") + .file(file) + .content("master content") + .create(); + approve(Integer.toString(changeInMaster.get())); + gApi.changes().id(changeInMaster.get()).current().submit(); + + // Create a change in the other branch and that touches the file. + Change.Id changeInOtherBranch = + changeOperations + .newChange() + .project(project) + .branch(branchName) + .file(file) + .content("other content") + .create(); + approve(Integer.toString(changeInOtherBranch.get())); + gApi.changes().id(changeInOtherBranch.get()).current().submit(); + + // Create a merge change with a conflict resolution for the file. + Change.Id mergeChange = + changeOperations + .newChange() + .project(project) + .branch("master") + .mergeOfButBaseOnFirst() + .tipOfBranch("master") + .and() + .tipOfBranch(branchName) + .file(file) + .content("merged content") + .create(); + + String mergeRevision = + changeOperations.change(mergeChange).currentPatchset().get().commitId().name(); + assertBranchNotFound(project, RefNames.refsCacheAutomerge(mergeRevision)); + } + + 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 64f9392f97..445cc973ec 100644 --- a/javatests/com/google/gerrit/server/permissions/RefControlTest.java +++ b/javatests/com/google/gerrit/server/permissions/RefControlTest.java @@ -105,11 +105,21 @@ public class RefControlTest { } 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) {