Skip to content

Commit

Permalink
Merge branch 'stable-3.2' into stable-3.3
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@lucamilanesio
lucamilanesio committed May 17, 2021
2 parents b813834 + 94e76ed commit 3a1db95
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 52 deletions.
18 changes: 17 additions & 1 deletion java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -100,11 +101,21 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
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)) {
Expand Down Expand Up @@ -145,6 +156,9 @@ private boolean verify(HttpServletRequest req, Response rsp) throws IOException
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);
}
}
Expand All @@ -159,6 +173,8 @@ private boolean verify(HttpServletRequest req, Response rsp) throws IOException
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)) {
Expand Down
8 changes: 6 additions & 2 deletions java/com/google/gerrit/httpd/XsrfCookieFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
}

Expand Down
21 changes: 12 additions & 9 deletions java/com/google/gerrit/httpd/raw/StaticModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -372,16 +373,18 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
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);
Expand Down
99 changes: 63 additions & 36 deletions javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -94,17 +96,16 @@ public class ProjectBasicAuthFilterTest {

@Before
public void setUp() throws Exception {
req = new FakeHttpServletRequest();
req = new FakeHttpServletRequest("gerrit.example.com", 80, "", "");
res = new FakeHttpServletResponse();

authSuccessful =
new AuthResult(AUTH_ACCOUNT_ID, ExternalId.Key.create("username", AUTH_USER), false);
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 =
Expand All @@ -114,23 +115,6 @@ public void setUp() throws Exception {
.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();
Expand Down Expand Up @@ -168,7 +152,6 @@ public void shouldAuthenticateSucessfullyAgainstRealmAndReturnCookie() throws Ex
res.setStatus(HttpServletResponse.SC_OK);

doReturn(true).when(account).isActive();
doReturn(authSuccessful).when(accountManager).authenticate(any());
doReturn(GitBasicAuthPolicy.LDAP).when(authConfig).getGitBasicAuthPolicy();

ProjectBasicAuthFilter basicAuthFilter =
Expand All @@ -187,10 +170,9 @@ public void shouldAuthenticateSucessfullyAgainstRealmAndReturnCookie() throws Ex
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);
Expand All @@ -205,33 +187,40 @@ public void shouldValidateUserPasswordAndNotReturnCookie() throws Exception {
}

@Test
public void shouldNotReauthenticateIfAlreadySignedIn() throws Exception {
initMockedWebSession();
doReturn(true).when(webSession).isSignedIn();
requestBasicAuth(req);
res.setStatus(HttpServletResponse.SC_OK);
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();

ProjectBasicAuthFilter basicAuthFilter =
new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
verify(accountManager, never()).authenticate(any());
verify(chain).doFilter(eq(req), any());
assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
}

basicAuthFilter.doFilter(req, res, chain);
@Test
public void shouldReauthenticateForRegularRequestEvenIfAlreadySignedIn() throws Exception {
doReturn(GitBasicAuthPolicy.LDAP).when(authConfig).getGitBasicAuthPolicy();
doFilterForRequestWhenAlreadySignedIn();

verify(accountManager, never()).authenticate(any());
verify(accountManager).authenticate(any());
verify(chain).doFilter(eq(req), any());
assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
}

@Test
public void shouldNotReauthenticateIfHasExistingCookie() throws Exception {
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);
}
Expand All @@ -256,6 +245,44 @@ public void shouldFailedAuthenticationAgainstRealm() throws Exception {
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",
Expand Down
2 changes: 1 addition & 1 deletion javatests/com/google/gerrit/integration/ssh/NoShellIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 12 additions & 1 deletion javatests/com/google/gerrit/util/http/testutil/FakeHttpServletRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -81,6 +82,7 @@ public FakeHttpServletRequest(String hostName, int port, String contextPath, Str
attributes = Maps.newConcurrentMap();
parameters = LinkedListMultimap.create();
headers = LinkedListMultimap.create();
method = "GET";
}

@Override
Expand All @@ -105,6 +107,11 @@ public int getContentLength() {

@Override
public String getContentType() {
List<String> contentType = headers.get("Content-Type");
if (contentType != null && !contentType.isEmpty()) {
return contentType.get(0);
}

return null;
}

Expand Down Expand Up @@ -297,7 +304,11 @@ public int getIntHeader(String name) {

@Override
public String getMethod() {
return "GET";
return method;
}

public void setMethod(String method) {
this.method = method;
}

@Override
Expand Down
8 changes: 6 additions & 2 deletions tools/js/download_bower.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit 3a1db95

Please sign in to comment.