diff options
author | Luca Milanesio <luca.milanesio@gmail.com> | 2021-08-20 23:25:06 +0100 |
---|---|---|
committer | Luca Milanesio <luca.milanesio@gmail.com> | 2021-08-20 23:25:31 +0100 |
commit | c93ad6eef036826609ed400f62a8a64d59fc4bff (patch) | |
tree | b1d96891661ea035a363b9645060dadea96a6d4d | |
parent | 0c2ae5195a73a31ced75866c63b6cd4c9dfa19e2 (diff) | |
parent | be55580ebefaae317d8676d49dad494979b96558 (diff) |
Merge branch 'stable-3.3-2021-07' into stable-3.3
* stable-3.3-2021-07:
Set version to 3.3.7-SNAPSHOT
Set version to 3.3.6
Update Gitiles plugin
IncludedIn: filter out non-visible branches
Point gitiles submodule to gitiles security fix repo
Change-Id: I332bbb5657b53f6857b7cc2237797b29fc8ee04e
-rw-r--r-- | Documentation/rest-api-projects.txt | 3 | ||||
-rw-r--r-- | java/com/google/gerrit/server/change/IncludedIn.java | 53 | ||||
-rw-r--r-- | java/com/google/gerrit/server/change/IncludedInResolver.java | 17 | ||||
-rw-r--r-- | java/com/google/gerrit/server/restapi/change/ChangeIncludedIn.java | 4 | ||||
-rw-r--r-- | java/com/google/gerrit/server/restapi/project/CommitIncludedIn.java | 4 | ||||
-rw-r--r-- | java/com/google/gerrit/truth/NullAwareCorrespondence.java | 9 | ||||
-rw-r--r-- | javatests/com/google/gerrit/acceptance/api/project/CommitIT.java | 62 | ||||
-rw-r--r-- | javatests/com/google/gerrit/server/change/IncludedInResolverTest.java | 24 | ||||
-rw-r--r-- | tools/maven/gerrit-acceptance-framework_pom.xml | 2 | ||||
-rw-r--r-- | tools/maven/gerrit-extension-api_pom.xml | 2 | ||||
-rw-r--r-- | tools/maven/gerrit-plugin-api_pom.xml | 2 | ||||
-rw-r--r-- | tools/maven/gerrit-war_pom.xml | 2 | ||||
-rw-r--r-- | version.bzl | 2 |
13 files changed, 153 insertions, 33 deletions
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt index 92759b64de..d53bd9c256 100644 --- a/Documentation/rest-api-projects.txt +++ b/Documentation/rest-api-projects.txt @@ -2556,6 +2556,9 @@ is returned that describes the commit. Retrieves the branches and tags in which a change is included. As result an link:rest-api-changes.html#included-in-info[IncludedInInfo] entity is returned. +Branches that are not visible to the calling user according to the project's +read permissions are filtered out from the result. + .Request ---- GET /projects/work%2Fmy-project/commits/a8a477efffbbf3b44169bb9a1d3a334cbbd9aa96/in HTTP/1.0 diff --git a/java/com/google/gerrit/server/change/IncludedIn.java b/java/com/google/gerrit/server/change/IncludedIn.java index 3c66c2c8bb..c06ce82c05 100644 --- a/java/com/google/gerrit/server/change/IncludedIn.java +++ b/java/com/google/gerrit/server/change/IncludedIn.java @@ -14,6 +14,12 @@ package com.google.gerrit.server.change; +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet; +import static java.util.Comparator.naturalOrder; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.ListMultimap; import com.google.common.collect.MultimapBuilder; import com.google.gerrit.entities.Project; @@ -23,13 +29,18 @@ import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.plugincontext.PluginSetContext; import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; +import java.util.Collection; import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; @@ -37,17 +48,21 @@ import org.eclipse.jgit.revwalk.RevWalk; @Singleton public class IncludedIn { private final GitRepositoryManager repoManager; + private final PermissionBackend permissionBackend; private final PluginSetContext<ExternalIncludedIn> externalIncludedIn; @Inject IncludedIn( - GitRepositoryManager repoManager, PluginSetContext<ExternalIncludedIn> externalIncludedIn) { + GitRepositoryManager repoManager, + PermissionBackend permissionBackend, + PluginSetContext<ExternalIncludedIn> externalIncludedIn) { this.repoManager = repoManager; + this.permissionBackend = permissionBackend; this.externalIncludedIn = externalIncludedIn; } public IncludedInInfo apply(Project.NameKey project, String revisionId) - throws RestApiException, IOException { + throws RestApiException, IOException, PermissionBackendException { try (Repository r = repoManager.openRepository(project); RevWalk rw = new RevWalk(r)) { rw.setRetainBody(false); @@ -61,18 +76,48 @@ public class IncludedIn { } IncludedInResolver.Result d = IncludedInResolver.resolve(r, rw, rev); + + // Filter branches and tags according to their visbility by the user + ImmutableSortedSet<String> filteredBranches = + sortedShortNames(filterReadableRefs(project, d.branches())); + ImmutableSortedSet<String> filteredTags = + sortedShortNames(filterReadableRefs(project, d.tags())); + ListMultimap<String, String> external = MultimapBuilder.hashKeys().arrayListValues().build(); externalIncludedIn.runEach( ext -> { ListMultimap<String, String> extIncludedIns = - ext.getIncludedIn(project.get(), rev.name(), d.tags(), d.branches()); + ext.getIncludedIn(project.get(), rev.name(), filteredBranches, filteredTags); if (extIncludedIns != null) { external.putAll(extIncludedIns); } }); return new IncludedInInfo( - d.branches(), d.tags(), (!external.isEmpty() ? external.asMap() : null)); + filteredBranches, filteredTags, (!external.isEmpty() ? external.asMap() : null)); + } + } + + /** + * Filter readable branches or tags according to the caller's refs visibility. + * + * @param project specific Gerrit project. + * @param inputRefs a list of branches (in short name) as strings + */ + private Collection<String> filterReadableRefs( + Project.NameKey project, ImmutableList<Ref> inputRefs) + throws IOException, PermissionBackendException { + PermissionBackend.ForProject perm = permissionBackend.currentUser().project(project); + try (Repository repo = repoManager.openRepository(project)) { + return perm.filter(inputRefs, repo, RefFilterOptions.defaults()).stream() + .map(Ref::getName) + .collect(toImmutableList()); } } + + private ImmutableSortedSet<String> sortedShortNames(Collection<String> refs) { + return refs.stream() + .map(Repository::shortenRefName) + .collect(toImmutableSortedSet(naturalOrder())); + } } diff --git a/java/com/google/gerrit/server/change/IncludedInResolver.java b/java/com/google/gerrit/server/change/IncludedInResolver.java index 09ca258103..38917006e1 100644 --- a/java/com/google/gerrit/server/change/IncludedInResolver.java +++ b/java/com/google/gerrit/server/change/IncludedInResolver.java @@ -14,13 +14,11 @@ package com.google.gerrit.server.change; -import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet; import static java.util.Comparator.comparing; -import static java.util.Comparator.naturalOrder; import static java.util.stream.Collectors.toList; import com.google.auto.value.AutoValue; -import com.google.common.collect.ImmutableSortedSet; +import com.google.common.collect.ImmutableList; import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; @@ -171,13 +169,12 @@ public class IncludedInResolver { * Returns the short names of refs which are as well in the matchingRefs list as well as in the * allRef list. */ - private static ImmutableSortedSet<String> getMatchingRefNames( + private static ImmutableList<Ref> getMatchingRefNames( Set<String> matchingRefs, Collection<Ref> allRefs) { return allRefs.stream() - .map(Ref::getName) - .filter(matchingRefs::contains) - .map(Repository::shortenRefName) - .collect(toImmutableSortedSet(naturalOrder())); + .filter(r -> matchingRefs.contains(r.getName())) + .distinct() + .collect(ImmutableList.toImmutableList()); } /** Parse commit of ref and store the relation between ref and commit. */ @@ -211,8 +208,8 @@ public class IncludedInResolver { @AutoValue public abstract static class Result { - public abstract ImmutableSortedSet<String> branches(); + public abstract ImmutableList<Ref> branches(); - public abstract ImmutableSortedSet<String> tags(); + public abstract ImmutableList<Ref> tags(); } } diff --git a/java/com/google/gerrit/server/restapi/change/ChangeIncludedIn.java b/java/com/google/gerrit/server/restapi/change/ChangeIncludedIn.java index 67b5870ba7..517fbdf9a8 100644 --- a/java/com/google/gerrit/server/restapi/change/ChangeIncludedIn.java +++ b/java/com/google/gerrit/server/restapi/change/ChangeIncludedIn.java @@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.change.IncludedIn; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; @@ -38,7 +39,8 @@ public class ChangeIncludedIn implements RestReadView<ChangeResource> { } @Override - public Response<IncludedInInfo> apply(ChangeResource rsrc) throws RestApiException, IOException { + public Response<IncludedInInfo> apply(ChangeResource rsrc) + throws RestApiException, IOException, PermissionBackendException { PatchSet ps = psUtil.current(rsrc.getNotes()); return Response.ok(includedIn.apply(rsrc.getProject(), ps.commitId().name())); } diff --git a/java/com/google/gerrit/server/restapi/project/CommitIncludedIn.java b/java/com/google/gerrit/server/restapi/project/CommitIncludedIn.java index a4a82ce2d7..e566858782 100644 --- a/java/com/google/gerrit/server/restapi/project/CommitIncludedIn.java +++ b/java/com/google/gerrit/server/restapi/project/CommitIncludedIn.java @@ -20,6 +20,7 @@ import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.server.change.IncludedIn; +import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.project.CommitResource; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -36,7 +37,8 @@ public class CommitIncludedIn implements RestReadView<CommitResource> { } @Override - public Response<IncludedInInfo> apply(CommitResource rsrc) throws RestApiException, IOException { + public Response<IncludedInInfo> apply(CommitResource rsrc) + throws RestApiException, IOException, PermissionBackendException { RevCommit commit = rsrc.getCommit(); Project.NameKey project = rsrc.getProjectState().getNameKey(); return Response.ok(includedIn.apply(project, commit.getId().getName())); diff --git a/java/com/google/gerrit/truth/NullAwareCorrespondence.java b/java/com/google/gerrit/truth/NullAwareCorrespondence.java index 687ad94ac5..5b107a6929 100644 --- a/java/com/google/gerrit/truth/NullAwareCorrespondence.java +++ b/java/com/google/gerrit/truth/NullAwareCorrespondence.java @@ -7,15 +7,6 @@ // http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software -// 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 diff --git a/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java b/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java index 80e04c0d21..23563275b8 100644 --- a/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java +++ b/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java @@ -15,7 +15,10 @@ package com.google.gerrit.acceptance.api.project; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.acceptance.GitUtil.pushHead; import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow; +import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block; +import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; import static java.util.stream.Collectors.toList; import static org.eclipse.jgit.lib.Constants.R_TAGS; @@ -26,6 +29,7 @@ import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.entities.BranchNameKey; import com.google.gerrit.entities.Permission; +import com.google.gerrit.entities.RefNames; import com.google.gerrit.extensions.api.changes.CherryPickInput; import com.google.gerrit.extensions.api.changes.IncludedInInfo; import com.google.gerrit.extensions.api.changes.ReviewInput; @@ -96,6 +100,53 @@ public class CommitIT extends AbstractDaemonTest { } @Test + public void includedInMergedChange_filtersOutNonVisibleBranches() throws Exception { + Result baseChange = createAndSubmitChange("refs/for/master"); + + createBranch(BranchNameKey.create(project, "test-branch-1")); + createBranch(BranchNameKey.create(project, "test-branch-2")); + createAndSubmitChange("refs/for/test-branch-1"); + createAndSubmitChange("refs/for/test-branch-2"); + + assertThat(getIncludedIn(baseChange.getCommit().getId()).branches) + .containsExactly("master", "test-branch-1", "test-branch-2"); + + projectOperations + .project(project) + .forUpdate() + .add(block(Permission.READ).ref("refs/heads/test-branch-1").group(REGISTERED_USERS)) + .update(); + + assertThat(getIncludedIn(baseChange.getCommit().getId()).branches) + .containsExactly("master", "test-branch-2"); + } + + @Test + public void includedInMergedChange_filtersOutNonVisibleTags() throws Exception { + String tagBase = "tag_base"; + String tagBranch1 = "tag_1"; + + Result baseChange = createAndSubmitChange("refs/for/master"); + createLightWeightTag(tagBase); + assertThat(getIncludedIn(baseChange.getCommit().getId()).tags).containsExactly(tagBase); + + createBranch(BranchNameKey.create(project, "test-branch-1")); + createAndSubmitChange("refs/for/test-branch-1"); + createLightWeightTag(tagBranch1); + assertThat(getIncludedIn(baseChange.getCommit().getId()).tags) + .containsExactly(tagBase, tagBranch1); + + projectOperations + .project(project) + .forUpdate() + // Tag permissions are controlled by read permissions on branches. Blocking read permission + // on test-branch-1 so that tagBranch1 becomes non-visible + .add(block(Permission.READ).ref("refs/heads/test-branch-1").group(REGISTERED_USERS)) + .update(); + assertThat(getIncludedIn(baseChange.getCommit().getId()).tags).containsExactly(tagBase); + } + + @Test public void cherryPickWithoutMessage() throws Exception { String branch = "foo"; @@ -192,4 +243,15 @@ public class CommitIT extends AbstractDaemonTest { assertThat(actual.email).isEqualTo(expected.email()); assertThat(actual.name).isEqualTo(expected.fullName()); } + + private Result createAndSubmitChange(String branch) throws Exception { + Result r = createChange(branch); + approve(r.getChangeId()); + gApi.changes().id(r.getChangeId()).current().submit(); + return r; + } + + private void createLightWeightTag(String tagName) throws Exception { + pushHead(testRepo, RefNames.REFS_TAGS + tagName, false, false); + } } diff --git a/javatests/com/google/gerrit/server/change/IncludedInResolverTest.java b/javatests/com/google/gerrit/server/change/IncludedInResolverTest.java index 19c479d3b8..b69a894b14 100644 --- a/javatests/com/google/gerrit/server/change/IncludedInResolverTest.java +++ b/javatests/com/google/gerrit/server/change/IncludedInResolverTest.java @@ -17,9 +17,13 @@ package com.google.gerrit.server.change; import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.entities.RefNames.REFS_TAGS; +import com.google.common.truth.Correspondence; +import com.google.gerrit.truth.NullAwareCorrespondence; import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevObject; import org.eclipse.jgit.revwalk.RevTag; @@ -112,8 +116,12 @@ public class IncludedInResolverTest { IncludedInResolver.Result detail = resolve(commit_v2_5); // Check that only tags and branches which refer the tip are returned - assertThat(detail.tags()).containsExactly(TAG_2_5, TAG_2_5_ANNOTATED, TAG_2_5_ANNOTATED_TWICE); - assertThat(detail.branches()).containsExactly(BRANCH_2_5); + assertThat(detail.tags()) + .comparingElementsUsing(hasShortName()) + .containsExactly(TAG_2_5, TAG_2_5_ANNOTATED, TAG_2_5_ANNOTATED_TWICE); + assertThat(detail.branches()) + .comparingElementsUsing(hasShortName()) + .containsExactly(BRANCH_2_5); } @Test @@ -123,6 +131,7 @@ public class IncludedInResolverTest { // Check whether all tags and branches are returned assertThat(detail.tags()) + .comparingElementsUsing(hasShortName()) .containsExactly( TAG_1_0, TAG_1_0_1, @@ -133,6 +142,7 @@ public class IncludedInResolverTest { TAG_2_5_ANNOTATED, TAG_2_5_ANNOTATED_TWICE); assertThat(detail.branches()) + .comparingElementsUsing(hasShortName()) .containsExactly(BRANCH_MASTER, BRANCH_1_0, BRANCH_1_3, BRANCH_2_0, BRANCH_2_5); } @@ -143,8 +153,11 @@ public class IncludedInResolverTest { // Check whether all succeeding tags and branches are returned assertThat(detail.tags()) + .comparingElementsUsing(hasShortName()) .containsExactly(TAG_1_3, TAG_2_5, TAG_2_5_ANNOTATED, TAG_2_5_ANNOTATED_TWICE); - assertThat(detail.branches()).containsExactly(BRANCH_1_3, BRANCH_2_5); + assertThat(detail.branches()) + .comparingElementsUsing(hasShortName()) + .containsExactly(BRANCH_1_3, BRANCH_2_5); } private IncludedInResolver.Result resolve(RevCommit commit) throws Exception { @@ -154,4 +167,9 @@ public class IncludedInResolverTest { private RevTag tag(String name, RevObject dest) throws Exception { return tr.update(REFS_TAGS + name, tr.tag(name, dest)); } + + private static Correspondence<Ref, String> hasShortName() { + return NullAwareCorrespondence.transforming( + ref -> Repository.shortenRefName(ref.getName()), "has short name"); + } } diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml index 4e209483ea..cf83bfca39 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.3.6-SNAPSHOT</version> + <version>3.3.7-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 15cfc334e4..a8106e8525 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.3.6-SNAPSHOT</version> + <version>3.3.7-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 37857cc7e2..a31dbc8d1d 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.3.6-SNAPSHOT</version> + <version>3.3.7-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 eb3f657afe..f7a009707b 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.3.6-SNAPSHOT</version> + <version>3.3.7-SNAPSHOT</version> <packaging>war</packaging> <name>Gerrit Code Review - WAR</name> <description>Gerrit WAR</description> diff --git a/version.bzl b/version.bzl index ef493881c2..5c083b23b2 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.3.6-SNAPSHOT" +GERRIT_VERSION = "3.3.7-SNAPSHOT" |