opendev
/
gerrit
Code Issues Proposed changes

Merge branch 'stable-3.1' into stable-3.2 

* stable-3.1:
  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.1.14
  download_bower: download to GERRIT_CACHE_HOME

Change-Id: I036ccd8618372407b5c693fce599fb8b80db254d
This commit is contained in:
Luca Milanesio 2021-05-17 21:39:01 +01:00
parent 9f56f39995 c66dfb9e4d
commit 894638f24f
7 changed files with 118 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
View File

@ -52,6 +52,7 @@ import javax.servlet.ServletResponse;
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 @@ class ProjectBasicAuthFilter implements Filter {
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 @@ class ProjectBasicAuthFilter implements Filter {
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 @@ class ProjectBasicAuthFilter implements Filter {
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)) {

8
java/com/google/gerrit/httpd/XsrfCookieFilter.java
View File

@ -32,6 +32,7 @@ import javax.servlet.ServletResponse;
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 @@ public class XsrfCookieFilter implements Filter {
@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);
}

21
java/com/google/gerrit/httpd/raw/StaticModule.java
View File

@ -53,6 +53,7 @@ import javax.servlet.http.HttpServlet;
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 {
@ -371,16 +372,18 @@ public class StaticModule extends ServletModule {
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);

101
javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java
View File

@ -37,10 +37,12 @@ import com.google.gerrit.server.account.externalids.ExternalId;
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 @@ public class ProjectBasicAuthFilterTest {
@Before
public void setUp() throws Exception {
req = new FakeHttpServletRequest();
req = new FakeHttpServletRequest("gerrit.example.com", 80, "", "");
res = new FakeHttpServletResponse();
authSuccessful =
@ -102,9 +104,8 @@ public class ProjectBasicAuthFilterTest {
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 @@ public class ProjectBasicAuthFilterTest {
.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 @@ public class ProjectBasicAuthFilterTest {
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 class ProjectBasicAuthFilterTest {
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 @@ public class ProjectBasicAuthFilterTest {
}
@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 @@ public class ProjectBasicAuthFilterTest {
}
@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 @@ public class ProjectBasicAuthFilterTest {
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",

2
javatests/com/google/gerrit/integration/ssh/NoShellIT.java
View File

@ -41,7 +41,7 @@ public class NoShellIT extends StandaloneSiteTest {
private String identityPath;
@Test(timeout = 30000)
@Test(timeout = 60000)
public void verifyCommandsIsClosed() throws Exception {
try (ServerContext ctx = startServer()) {
setUpTestHarness(ctx);

13
javatests/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java
View File

@ -67,6 +67,7 @@ public class FakeHttpServletRequest implements HttpServletRequest {
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 @@ public class FakeHttpServletRequest implements HttpServletRequest {
attributes = Maps.newConcurrentMap();
parameters = LinkedListMultimap.create();
headers = LinkedListMultimap.create();
method = "GET";
}
@Override
@ -105,6 +107,11 @@ public class FakeHttpServletRequest implements HttpServletRequest {
@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 @@ public class FakeHttpServletRequest implements HttpServletRequest {
@Override
public String getMethod() {
return "GET";
return method;
}
public void setMethod(String method) {
this.method = method;
}
@Override

8
tools/js/download_bower.py
View File

@ -25,8 +25,12 @@ import sys
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):