Merge branch 'stable-3.2' into stable-3.3
* stable-3.2:
Set version to 3.2.10
Set version to 3.1.15
Don't serve polygerrit assets for git requests
Fix PUT/POST/DELETE REST-API with cookie authentication
NoShellIT: Increase the timeout to avoid failures
Set version to 3.2.10-SNAPSHOT
Set version to 3.2.9
Set version to 3.1.14
download_bower: download to GERRIT_CACHE_HOME
Change-Id: Ib1803ca5f5164cd744a52209203277d3bf6797ca
diff --git a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
index 7fcd4f8..de989ac 100644
--- a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
+++ b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
@@ -52,6 +52,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpServletResponseWrapper;
+import org.eclipse.jgit.http.server.GitSmartHttpTools;
/**
* Authenticates the current user by HTTP basic authentication.
@@ -100,11 +101,21 @@
HttpServletRequest req = (HttpServletRequest) request;
Response rsp = new Response((HttpServletResponse) response);
- if (session.get().isSignedIn() || verify(req, rsp)) {
+ if (isSignedInGitRequest(req) || verify(req, rsp)) {
chain.doFilter(req, rsp);
}
}
+ private boolean isSignedInGitRequest(HttpServletRequest req) {
+ boolean isGitRequest = req.getRequestURI() != null && GitSmartHttpTools.isGitClient(req);
+ boolean isAlreadySignedIn = session.get().isSignedIn();
+ boolean res = isAlreadySignedIn && isGitRequest;
+ logger.atFine().log(
+ "HTTP:%s %s signedIn=%s (isAlreadySignedIn=%s, isGitRequest=%s)",
+ req.getMethod(), req.getRequestURI(), res, isAlreadySignedIn, isGitRequest);
+ return res;
+ }
+
private boolean verify(HttpServletRequest req, Response rsp) throws IOException {
final String hdr = req.getHeader(AUTHORIZATION);
if (hdr == null || !hdr.startsWith(LIT_BASIC)) {
@@ -145,6 +156,9 @@
if (gitBasicAuthPolicy == GitBasicAuthPolicy.HTTP
|| gitBasicAuthPolicy == GitBasicAuthPolicy.HTTP_LDAP) {
if (PasswordVerifier.checkPassword(who.externalIds(), username, password)) {
+ logger.atFine().log(
+ "HTTP:%s %s username/password authentication succeeded",
+ req.getMethod(), req.getRequestURI());
return succeedAuthentication(who, null);
}
}
@@ -159,6 +173,8 @@
try {
AuthResult whoAuthResult = accountManager.authenticate(whoAuth);
setUserIdentified(whoAuthResult.getAccountId(), whoAuthResult);
+ logger.atFine().log(
+ "HTTP:%s %s Realm authentication succeeded", req.getMethod(), req.getRequestURI());
return true;
} catch (NoSuchUserException e) {
if (PasswordVerifier.checkPassword(who.externalIds(), username, password)) {
diff --git a/java/com/google/gerrit/httpd/XsrfCookieFilter.java b/java/com/google/gerrit/httpd/XsrfCookieFilter.java
index d15ecac..079efa4 100644
--- a/java/com/google/gerrit/httpd/XsrfCookieFilter.java
+++ b/java/com/google/gerrit/httpd/XsrfCookieFilter.java
@@ -32,6 +32,7 @@
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jgit.http.server.GitSmartHttpTools;
@Singleton
public class XsrfCookieFilter implements Filter {
@@ -50,8 +51,11 @@
@Override
public void doFilter(ServletRequest req, ServletResponse rsp, FilterChain chain)
throws IOException, ServletException {
- WebSession s = user.get().isIdentifiedUser() ? session.get() : null;
- setXsrfTokenCookie((HttpServletRequest) req, (HttpServletResponse) rsp, s);
+ HttpServletRequest httpRequest = (HttpServletRequest) req;
+ if (!GitSmartHttpTools.isGitClient(httpRequest)) {
+ WebSession s = user.get().isIdentifiedUser() ? session.get() : null;
+ setXsrfTokenCookie(httpRequest, (HttpServletResponse) rsp, s);
+ }
chain.doFilter(req, rsp);
}
diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java
index 66e107b..13327ca 100644
--- a/java/com/google/gerrit/httpd/raw/StaticModule.java
+++ b/java/com/google/gerrit/httpd/raw/StaticModule.java
@@ -53,6 +53,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jgit.http.server.GitSmartHttpTools;
import org.eclipse.jgit.lib.Config;
public class StaticModule extends ServletModule {
@@ -372,16 +373,18 @@
HttpServletRequest req = (HttpServletRequest) request;
HttpServletResponse res = (HttpServletResponse) response;
- GuiceFilterRequestWrapper reqWrapper = new GuiceFilterRequestWrapper(req);
- String path = pathInfo(req);
+ if (!GitSmartHttpTools.isGitClient(req)) {
+ GuiceFilterRequestWrapper reqWrapper = new GuiceFilterRequestWrapper(req);
+ String path = pathInfo(req);
- if (isPolyGerritIndex(path)) {
- polyGerritIndex.service(reqWrapper, res);
- return;
- }
- if (isPolyGerritAsset(path)) {
- polygerritUI.service(reqWrapper, res);
- return;
+ if (isPolyGerritIndex(path)) {
+ polyGerritIndex.service(reqWrapper, res);
+ return;
+ }
+ if (isPolyGerritAsset(path)) {
+ polygerritUI.service(reqWrapper, res);
+ return;
+ }
}
chain.doFilter(req, res);
diff --git a/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java b/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java
index 735abbf..2f0fafa 100644
--- a/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java
+++ b/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java
@@ -37,10 +37,12 @@
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.util.http.testutil.FakeHttpServletRequest;
import com.google.gerrit.util.http.testutil.FakeHttpServletResponse;
+import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.Optional;
import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
import javax.servlet.http.HttpServletResponse;
import org.junit.Before;
import org.junit.Test;
@@ -94,7 +96,7 @@
@Before
public void setUp() throws Exception {
- req = new FakeHttpServletRequest();
+ req = new FakeHttpServletRequest("gerrit.example.com", 80, "", "");
res = new FakeHttpServletResponse();
authSuccessful =
@@ -102,9 +104,8 @@
doReturn(Optional.of(accountState)).when(accountCache).getByUsername(AUTH_USER);
doReturn(Optional.of(accountState)).when(accountCache).get(AUTH_ACCOUNT_ID);
doReturn(account).when(accountState).account();
- doReturn(ImmutableSet.builder().add(AUTH_USER_PASSWORD_EXTERNAL_ID).build())
- .when(accountState)
- .externalIds();
+ doReturn(true).when(account).isActive();
+ doReturn(authSuccessful).when(accountManager).authenticate(any());
doReturn(new WebSessionManager.Key(AUTH_COOKIE_VALUE)).when(webSessionManager).createKey(any());
WebSessionManager.Val webSessionValue =
@@ -114,23 +115,6 @@
.createVal(any(), any(), eq(false), any(), any(), any());
}
- private void initWebSessionWithCookie(String cookie) {
- req.addHeader("Cookie", cookie);
- initWebSessionWithoutCookie();
- }
-
- private void initWebSessionWithoutCookie() {
- webSession =
- new CacheBasedWebSession(
- req, res, webSessionManager, authConfig, null, userRequestFactory, accountCache) {};
- doReturn(webSession).when(webSessionItem).get();
- }
-
- private void initMockedWebSession() {
- webSession = mock(WebSession.class);
- doReturn(webSession).when(webSessionItem).get();
- }
-
@Test
public void shouldAllowAnonymousRequest() throws Exception {
initMockedWebSession();
@@ -168,7 +152,6 @@
res.setStatus(HttpServletResponse.SC_OK);
doReturn(true).when(account).isActive();
- doReturn(authSuccessful).when(accountManager).authenticate(any());
doReturn(GitBasicAuthPolicy.LDAP).when(authConfig).getGitBasicAuthPolicy();
ProjectBasicAuthFilter basicAuthFilter =
@@ -187,10 +170,9 @@
public void shouldValidateUserPasswordAndNotReturnCookie() throws Exception {
initWebSessionWithoutCookie();
requestBasicAuth(req);
- res.setStatus(HttpServletResponse.SC_OK);
-
- doReturn(true).when(account).isActive();
+ initMockedUsernamePasswordExternalId();
doReturn(GitBasicAuthPolicy.HTTP).when(authConfig).getGitBasicAuthPolicy();
+ res.setStatus(HttpServletResponse.SC_OK);
ProjectBasicAuthFilter basicAuthFilter =
new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
@@ -205,16 +187,11 @@
}
@Test
- public void shouldNotReauthenticateIfAlreadySignedIn() throws Exception {
- initMockedWebSession();
- doReturn(true).when(webSession).isSignedIn();
- requestBasicAuth(req);
- res.setStatus(HttpServletResponse.SC_OK);
-
- ProjectBasicAuthFilter basicAuthFilter =
- new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
-
- basicAuthFilter.doFilter(req, res, chain);
+ public void shouldNotReauthenticateForGitPostRequest() throws Exception {
+ req.setPathInfo("/a/project.git/git-upload-pack");
+ req.setMethod("POST");
+ req.addHeader("Content-Type", "application/x-git-upload-pack-request");
+ doFilterForRequestWhenAlreadySignedIn();
verify(accountManager, never()).authenticate(any());
verify(chain).doFilter(eq(req), any());
@@ -222,16 +199,28 @@
}
@Test
- public void shouldNotReauthenticateIfHasExistingCookie() throws Exception {
+ public void shouldReauthenticateForRegularRequestEvenIfAlreadySignedIn() throws Exception {
+ doReturn(GitBasicAuthPolicy.LDAP).when(authConfig).getGitBasicAuthPolicy();
+ doFilterForRequestWhenAlreadySignedIn();
+
+ verify(accountManager).authenticate(any());
+ verify(chain).doFilter(eq(req), any());
+ assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
+ }
+
+ @Test
+ public void shouldReauthenticateEvenIfHasExistingCookie() throws Exception {
initWebSessionWithCookie("GerritAccount=" + AUTH_COOKIE_VALUE);
res.setStatus(HttpServletResponse.SC_OK);
+ requestBasicAuth(req);
+ doReturn(GitBasicAuthPolicy.LDAP).when(authConfig).getGitBasicAuthPolicy();
ProjectBasicAuthFilter basicAuthFilter =
new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
basicAuthFilter.doFilter(req, res, chain);
- verify(accountManager, never()).authenticate(any());
+ verify(accountManager).authenticate(any());
verify(chain).doFilter(eq(req), any());
assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
}
@@ -256,6 +245,44 @@
assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
}
+ private void doFilterForRequestWhenAlreadySignedIn()
+ throws IOException, ServletException, AccountException {
+ initMockedWebSession();
+ doReturn(true).when(account).isActive();
+ doReturn(true).when(webSession).isSignedIn();
+ doReturn(authSuccessful).when(accountManager).authenticate(any());
+ requestBasicAuth(req);
+ res.setStatus(HttpServletResponse.SC_OK);
+
+ ProjectBasicAuthFilter basicAuthFilter =
+ new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
+
+ basicAuthFilter.doFilter(req, res, chain);
+ }
+
+ private void initWebSessionWithCookie(String cookie) {
+ req.addHeader("Cookie", cookie);
+ initWebSessionWithoutCookie();
+ }
+
+ private void initWebSessionWithoutCookie() {
+ webSession =
+ new CacheBasedWebSession(
+ req, res, webSessionManager, authConfig, null, userRequestFactory, accountCache) {};
+ doReturn(webSession).when(webSessionItem).get();
+ }
+
+ private void initMockedWebSession() {
+ webSession = mock(WebSession.class);
+ doReturn(webSession).when(webSessionItem).get();
+ }
+
+ private void initMockedUsernamePasswordExternalId() {
+ doReturn(ImmutableSet.builder().add(AUTH_USER_PASSWORD_EXTERNAL_ID).build())
+ .when(accountState)
+ .externalIds();
+ }
+
private void requestBasicAuth(FakeHttpServletRequest fakeReq) {
fakeReq.addHeader(
"Authorization",
diff --git a/javatests/com/google/gerrit/integration/ssh/NoShellIT.java b/javatests/com/google/gerrit/integration/ssh/NoShellIT.java
index ccaf085..2bbbf1a 100644
--- a/javatests/com/google/gerrit/integration/ssh/NoShellIT.java
+++ b/javatests/com/google/gerrit/integration/ssh/NoShellIT.java
@@ -41,7 +41,7 @@
private String identityPath;
- @Test(timeout = 30000)
+ @Test(timeout = 60000)
public void verifyCommandsIsClosed() throws Exception {
try (ServerContext ctx = startServer()) {
setUpTestHarness(ctx);
diff --git a/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java b/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java
index 2efa94b..0bb4de4 100644
--- a/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java
+++ b/javatests/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java
@@ -67,6 +67,7 @@
private String contextPath;
private String servletPath;
private String path;
+ private String method;
public FakeHttpServletRequest() {
this("gerrit.example.com", 80, "", SERVLET_PATH);
@@ -81,6 +82,7 @@
attributes = Maps.newConcurrentMap();
parameters = LinkedListMultimap.create();
headers = LinkedListMultimap.create();
+ method = "GET";
}
@Override
@@ -105,6 +107,11 @@
@Override
public String getContentType() {
+ List<String> contentType = headers.get("Content-Type");
+ if (contentType != null && !contentType.isEmpty()) {
+ return contentType.get(0);
+ }
+
return null;
}
@@ -297,7 +304,11 @@
@Override
public String getMethod() {
- return "GET";
+ return method;
+ }
+
+ public void setMethod(String method) {
+ this.method = method;
}
@Override
diff --git a/tools/js/download_bower.py b/tools/js/download_bower.py
index 1df4b82..d541b56 100755
--- a/tools/js/download_bower.py
+++ b/tools/js/download_bower.py
@@ -25,8 +25,12 @@
import bowerutil
-CACHE_DIR = os.path.expanduser(os.path.join(
- '~', '.gerritcodereview', 'bazel-cache', 'downloaded-artifacts'))
+CACHE_DIR = os.environ.get(
+ 'GERRIT_CACHE_HOME',
+ os.path.expanduser(os.path.join(
+ '~', '.gerritcodereview', 'bazel-cache', 'downloaded-artifacts'
+ ))
+)
def bower_cmd(bower, *args):
