summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLuca Milanesio <luca.milanesio@gmail.com>2021-05-05 22:39:03 +0100
committerLuca Milanesio <luca.milanesio@gmail.com>2021-05-07 14:56:32 +0100
commit736795b48443727ef3bfe9ed2ce8d7db49226a47 (patch)
tree56eae406ae1f2031c21d208a5f5679706b27d2ec
parentdfbc9d03bc2c98729af947a294858f72a51046d3 (diff)
Avoid multiple auth requests for Git/HTTP access
When authenticating incoming Git calls over HTTP the BasicAuth filter was called 3 times per call triggering multiple authentications against the backend. Example: Git protocol v1 triggering 2x HTTP calls, one for refs-advertisement and another for upload-pack was generating 6x authentication requests. When the backend is Gerrit's HTTP password authentication the operation is quite fast making the impact of the extra authentications negligible. However, when authenticating against a slower backend (e.g. corporate LDAP with groups resolution) the extra authentication calls were introducing unneeded latency and generating extra workload to the LDAP server. NOTE: It is still not possible to have one single authenticated session for multiple HTTP calls, because of the lack of support for GerritAccount cookie from the ProjectBasicAuthFilter. The next follow-up change is focused in solving that problem specifically, bringing the number of authentication requests to one. Bug: Issue 14497 Change-Id: Ibe41df0357b6be10bcdf0bd1f5a1b6160c34d4a4
-rw-r--r--java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java2
-rw-r--r--javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java22
2 files changed, 22 insertions, 2 deletions
diff --git a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
index e75d8feada..55ffef797b 100644
--- a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
+++ b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
@@ -99,7 +99,7 @@ class ProjectBasicAuthFilter implements Filter {
HttpServletRequest req = (HttpServletRequest) request;
Response rsp = new Response((HttpServletResponse) response);
- if (verify(req, rsp)) {
+ if (session.get().isSignedIn() || verify(req, rsp)) {
chain.doFilter(req, rsp);
}
}
diff --git a/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java b/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java
index 1f06472f59..f7792ed2d3 100644
--- a/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java
+++ b/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java
@@ -142,6 +142,27 @@ public class ProjectBasicAuthFilterTest {
}
@Test
+ public void shouldNotReauthenticateIfAlreadySignedIn() throws Exception {
+ req.addHeader(
+ "Authorization",
+ "Basic "
+ + B64_ENC.encodeToString(
+ (AUTH_USER + ":" + AUTH_PASSWORD).getBytes(StandardCharsets.UTF_8)));
+ res.setStatus(HttpServletResponse.SC_OK);
+
+ doReturn(true).when(webSession).isSignedIn();
+
+ ProjectBasicAuthFilter basicAuthFilter =
+ new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
+
+ basicAuthFilter.doFilter(req, res, chain);
+
+ verify(accountManager, never()).authenticate(any());
+ verify(chain).doFilter(eq(req), any());
+ assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
+ }
+
+ @Test
public void shouldFailedAuthenticationAgainstRealm() throws Exception {
req.addHeader(
"Authorization",
@@ -157,7 +178,6 @@ public class ProjectBasicAuthFilterTest {
new ProjectBasicAuthFilter(webSessionItem, accountCache, accountManager, authConfig);
basicAuthFilter.doFilter(req, res, chain);
- basicAuthFilter.destroy();
verify(accountManager).authenticate(any());