Merge branch 'stable-3.5' into stable-3.6 * stable-3.5: Give execute permission to entrypoint.sh Remove unused env variables Return empty list when prefix is not matched in suggest Expose debug ports in Gerrit Do no update Gerrit rpm Remove /a from fetch url in replication.config.template Allow anonymous clones when pull-replication plugin is enabled. Also update Gerrit image version to 3.6.3. Change-Id: Icf3d9ea111a8f0ad4a78ab355cb50dff2a5b7835
diff --git a/example-setup/Dockerfile b/example-setup/Dockerfile index 368f26e..fea2ac9 100644 --- a/example-setup/Dockerfile +++ b/example-setup/Dockerfile
@@ -1,13 +1,10 @@ -FROM gerritcodereview/gerrit:3.5.4-almalinux8 +FROM gerritcodereview/gerrit:3.6.3-almalinux8 USER root -RUN yum update -y &&\ - yum install -y gettext +RUN yum install -y gettext ARG JAVA_OPTS='--add-opens java.base/java.net=ALL-UNNAMED --add-opens java.base/java.lang.invoke=ALL-UNNAMED' -ARG GERRIT_BRANCH=stable-3.5 -ARG LAST_BUILD=lastSuccessfulBuild/artifact/bazel-bin/plugins RUN java $JAVA_OPTS -jar /var/gerrit/bin/gerrit.war init --batch --install-all-plugins -d /var/gerrit && \ java $JAVA_OPTS -jar /var/gerrit/bin/gerrit.war reindex -d /var/gerrit @@ -17,8 +14,7 @@ COPY --chown=gerrit:gerrit pull-replication.jar /var/gerrit/plugins/pull-replication.jar COPY --chown=gerrit:gerrit pull-replication.jar /var/gerrit/lib/pull-replication.jar COPY --chown=gerrit:gerrit entrypoint.sh /tmp/ -RUN chmod +x /tmp/entrypoint.sh COPY --chown=gerrit:gerrit configs/replication.config.template /var/gerrit/etc/ COPY --chown=gerrit:gerrit configs/gerrit.config.template /var/gerrit/etc/ -ENTRYPOINT [ "/tmp/entrypoint.sh" ] \ No newline at end of file +ENTRYPOINT [ "/tmp/entrypoint.sh" ]
diff --git a/example-setup/configs/gerrit.config.template b/example-setup/configs/gerrit.config.template index 8bc8391..db97a37 100644 --- a/example-setup/configs/gerrit.config.template +++ b/example-setup/configs/gerrit.config.template
@@ -6,6 +6,7 @@ [container] javaOptions = "-Dflogger.backend_factory=com.google.common.flogger.backend.log4j.Log4jBackendFactory#getInstance" javaOptions = "-Dflogger.logging_context=com.google.gerrit.server.logging.LoggingContext#getInstance" + javaOptions = "-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:$DEBUG_PORT" replica = $REPLICA [index] type = LUCENE
diff --git a/example-setup/configs/replication.config.template b/example-setup/configs/replication.config.template index 2939297..7e81055 100644 --- a/example-setup/configs/replication.config.template +++ b/example-setup/configs/replication.config.template
@@ -10,7 +10,7 @@ syncRefs="ALL REFS ASYNC" maxApiPayloadSize=40000 [remote "$REMOTE"] - url = http://$REMOTE_URL:8080/a/#{name}#.git + url = http://$REMOTE_URL:8080/#{name}#.git apiUrl = http://$REMOTE_URL:8080 fetch = +refs/*:refs/* mirror = true
diff --git a/example-setup/docker-compose.yaml b/example-setup/docker-compose.yaml index b36f426..af90341 100644 --- a/example-setup/docker-compose.yaml +++ b/example-setup/docker-compose.yaml
@@ -7,9 +7,11 @@ - REPLICA=false - REMOTE=replica-1 - REMOTE_URL=gerrit2 + - DEBUG_PORT=5005 ports: - "8080:8080" - "29418:29418" + - "5005:5005" gerrit2: build: . environment: @@ -17,8 +19,10 @@ - REPLICA=true - REMOTE=primary - REMOTE_URL=gerrit1 + - DEBUG_PORT=5006 ports: - "8081:8080" - "29419:29418" + - "5006:5006" depends_on: - - gerrit1 \ No newline at end of file + - gerrit1
diff --git a/example-setup/entrypoint.sh b/example-setup/entrypoint.sh old mode 100644 new mode 100755
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/BearerAuthenticationFilter.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/BearerAuthenticationFilter.java index 8147149..be71946 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/BearerAuthenticationFilter.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/BearerAuthenticationFilter.java
@@ -44,6 +44,7 @@ public class BearerAuthenticationFilter extends AllRequestFilter { private static final String BEARER_TOKEN = "BearerToken"; + private static final String BEARER_TOKEN_PREFIX = "Bearer"; private final DynamicItem<WebSession> session; private final String pluginName; private final PullReplicationInternalUser pluginUser; @@ -79,13 +80,14 @@ HttpServletRequest httpRequest = (HttpServletRequest) servletRequest; HttpServletResponse httpResponse = (HttpServletResponse) servletResponse; String requestURI = httpRequest.getRequestURI(); + Optional<String> authorizationHeader = + Optional.ofNullable(httpRequest.getHeader("Authorization")); if (isBasicAuthenticationRequest(requestURI)) { filterChain.doFilter(servletRequest, servletResponse); - } else if (isPullReplicationApiRequest(requestURI) || isGitUploadPackRequest(httpRequest)) { - Optional<String> authorizationHeader = - Optional.ofNullable(httpRequest.getHeader("Authorization")); - + } else if (isPullReplicationApiRequest(requestURI) + || (isGitUploadPackRequest(httpRequest) + && isAuthenticationHeaderWithBearerToken(authorizationHeader))) { if (isBearerTokenAuthenticated(authorizationHeader, bearerToken)) try (ManualRequestContext ctx = new ManualRequestContext(pluginUser, threadLocalRequestContext.get())) { @@ -137,4 +139,8 @@ } return Optional.empty(); } + + private boolean isAuthenticationHeaderWithBearerToken(Optional<String> authorizationHeader) { + return authorizationHeader.map(h -> h.startsWith(BEARER_TOKEN_PREFIX)).orElse(false); + } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/auth/PullReplicationGroupBackend.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/auth/PullReplicationGroupBackend.java index 9521004..7bcf3d2 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/auth/PullReplicationGroupBackend.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/auth/PullReplicationGroupBackend.java
@@ -27,6 +27,8 @@ import com.google.inject.Singleton; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; +import java.util.List; /** Backend to expose the pull-replication internal user group membership. */ @Singleton @@ -81,10 +83,9 @@ @Override public Collection<GroupReference> suggest(String name, ProjectState project) { - return Arrays.asList( - NAME_PREFIX.contains(name.toLowerCase()) - ? GroupReference.create(INTERNAL_GROUP_UUID, INTERNAL_GROUP_NAME) - : GroupReference.create(name)); + return NAME_PREFIX.contains(name.toLowerCase()) + ? List.of(GroupReference.create(INTERNAL_GROUP_UUID, INTERNAL_GROUP_NAME)) + : Collections.emptyList(); } @Override
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/BearerAuthenticationFilterTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/BearerAuthenticationFilterTest.java index 824496a..ca69f06 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/BearerAuthenticationFilterTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/BearerAuthenticationFilterTest.java
@@ -16,7 +16,6 @@ import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; import static org.mockito.Mockito.atMost; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -108,17 +107,53 @@ } @Test - public void shouldAuthenticateWhenGitUploadPacket() throws ServletException, IOException { + public void shouldAuthenticateWhenGitUploadPack() throws ServletException, IOException { authenticateAndFilter("any-prefix/git-upload-pack", NO_QUERY_PARAMETERS); } @Test - public void shouldAuthenticateWhenGitUploadPacketInQueryParameter() + public void shouldAuthenticateWhenGitUploadPackInQueryParameter() throws ServletException, IOException { authenticateAndFilter("any-prefix", GIT_UPLOAD_PACK_QUERY_PARAMETER); } @Test + public void shouldGoNextInChainWhenGitUploadPackWithoutAuthenticationHeader() + throws ServletException, IOException { + when(httpServletRequest.getRequestURI()).thenReturn("any-prefix/git-upload-pack"); + + final BearerAuthenticationFilter filter = + new BearerAuthenticationFilter( + session, + pluginName, + pluginUser, + threadLocalRequestContextProvider, + "some-bearer-token"); + filter.doFilter(httpServletRequest, httpServletResponse, filterChain); + + verify(httpServletRequest).getHeader("Authorization"); + verify(filterChain).doFilter(httpServletRequest, httpServletResponse); + } + + @Test + public void shouldGoNextInChainWhenGitUploadPackWithAuthenticationHeaderDifferentThanBearer() + throws ServletException, IOException { + when(httpServletRequest.getRequestURI()).thenReturn("any-prefix/git-upload-pack"); + when(httpServletRequest.getHeader("Authorization")).thenReturn("some-authorization"); + final BearerAuthenticationFilter filter = + new BearerAuthenticationFilter( + session, + pluginName, + pluginUser, + threadLocalRequestContextProvider, + "some-bearer-token"); + filter.doFilter(httpServletRequest, httpServletResponse, filterChain); + + verify(httpServletRequest).getHeader("Authorization"); + verify(filterChain).doFilter(httpServletRequest, httpServletResponse); + } + + @Test public void shouldBe401WhenBearerTokenDoesNotMatch() throws ServletException, IOException { when(httpServletRequest.getRequestURI()).thenReturn("any-prefix/pull-replication~fetch"); when(httpServletRequest.getHeader("Authorization")) @@ -171,6 +206,7 @@ filter.doFilter(httpServletRequest, httpServletResponse, filterChain); verify(httpServletRequest).getRequestURI(); + verify(httpServletRequest).getHeader("Authorization"); verify(httpServletResponse).sendError(SC_UNAUTHORIZED); } @@ -187,7 +223,7 @@ "some-bearer-token"); filter.doFilter(httpServletRequest, httpServletResponse, filterChain); - verify(httpServletRequest, times(2)).getRequestURI(); + verify(httpServletRequest).getHeader("Authorization"); verify(filterChain).doFilter(httpServletRequest, httpServletResponse); } @@ -206,7 +242,7 @@ "some-bearer-token"); filter.doFilter(httpServletRequest, httpServletResponse, filterChain); - verify(httpServletRequest).getRequestURI(); + verify(httpServletRequest).getHeader("Authorization"); verify(filterChain).doFilter(httpServletRequest, httpServletResponse); } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/auth/PullReplicationGroupBackendIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/auth/PullReplicationGroupBackendIT.java index 87738e3..df7da98 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/auth/PullReplicationGroupBackendIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/auth/PullReplicationGroupBackendIT.java
@@ -58,6 +58,13 @@ } @Test + public void shouldSuggestEmptyListIfNameNotMatched() { + Collection<GroupReference> groups = groupBackend.suggest("nonMatchablePrefix", null); + + assertThat(groups).isEmpty(); + } + + @Test public void pullReplicationInternalUserShouldHaveMembershipOfInternalGroupAndAnonymousUsers() { assertMemberOfInternalAndAnonymousUsers( groupBackend.membershipsOf(getPullReplicationInternalUser()));