diff options
author | Luca Milanesio <luca.milanesio@gmail.com> | 2021-05-05 22:39:03 +0100 |
---|---|---|
committer | Luca Milanesio <luca.milanesio@gmail.com> | 2021-05-07 14:56:32 +0100 |
commit | 736795b48443727ef3bfe9ed2ce8d7db49226a47 (patch) | |
tree | 56eae406ae1f2031c21d208a5f5679706b27d2ec | |
parent | dfbc9d03bc2c98729af947a294858f72a51046d3 (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.java | 2 | ||||
-rw-r--r-- | javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java | 22 |
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()); |