diff options
| -rw-r--r-- | java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java | 33 | ||||
| -rw-r--r-- | javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java | 40 |
2 files changed, 61 insertions, 12 deletions
diff --git a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java index b0c7615ada..7cb23571d1 100644 --- a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java +++ b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java @@ -148,19 +148,20 @@ class ProjectBasicAuthFilter implements Filter { username = username.toLowerCase(Locale.US); } - Optional<AccountState> accountState = - accountCache.getByUsername(username).filter(a -> a.account().isActive()); - if (!accountState.isPresent()) { - logger.atWarning().log( - "Authentication failed for %s: account inactive or not provisioned in Gerrit", username); - rsp.sendError(SC_UNAUTHORIZED); - return false; - } - - AccountState who = accountState.get(); GitBasicAuthPolicy gitBasicAuthPolicy = authConfig.getGitBasicAuthPolicy(); if (gitBasicAuthPolicy == GitBasicAuthPolicy.HTTP || gitBasicAuthPolicy == GitBasicAuthPolicy.HTTP_LDAP) { + Optional<AccountState> accountState = + accountCache.getByUsername(username).filter(a -> a.account().isActive()); + if (!accountState.isPresent()) { + logger.atWarning().log( + "Authentication failed for %s: account inactive or not provisioned in Gerrit", + username); + rsp.sendError(SC_UNAUTHORIZED); + return false; + } + + AccountState who = accountState.get(); if (passwordVerifier.checkPassword(who.externalIds(), username, password)) { logger.atFine().log( "HTTP:%s %s username/password authentication succeeded", @@ -183,8 +184,16 @@ class ProjectBasicAuthFilter implements Filter { "HTTP:%s %s Realm authentication succeeded", req.getMethod(), req.getRequestURI()); return true; } catch (NoSuchUserException e) { - if (passwordVerifier.checkPassword(who.externalIds(), username, password)) { - return succeedAuthentication(who, null); + // We are here in a condition where the basic authentication is set to LDAP and + // the user does not exist on the LDAP server. In theory this should trigger an + // authentication error; however, Edwin has introduced a fallback with I030c88108 + // where the account is checked *also* for its HTTP password as a fallback, because + // of the need to still be able to authenticate service users. + Optional<AccountState> who = + accountCache.getByUsername(username).filter(a -> a.account().isActive()); + if (who.isPresent() + && passwordVerifier.checkPassword(who.get().externalIds(), username, password)) { + return succeedAuthentication(who.get(), null); } logger.atWarning().withCause(e).log("%s", authenticationFailedMsg(username, req)); rsp.sendError(SC_UNAUTHORIZED); diff --git a/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java b/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java index 0389c4f8a8..d67b6bd146 100644 --- a/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java +++ b/javatests/com/google/gerrit/httpd/ProjectBasicAuthFilterTest.java @@ -162,6 +162,46 @@ public class ProjectBasicAuthFilterTest { } @Test + public void shouldAuthenticateWithRealmEvenIfGerritUserDoesNotExist() throws Exception { + initWebSessionWithoutCookie(); + requestBasicAuth(req); + doReturn(authSuccessful).when(accountManager).authenticate(any()); + doReturn(GitBasicAuthPolicy.LDAP).when(authConfig).getGitBasicAuthPolicy(); + + ProjectBasicAuthFilter basicAuthFilter = + new ProjectBasicAuthFilter( + webSessionItem, + accountCache, + accountManager, + authConfig, + authRequestFactory, + pwdVerifier); + + basicAuthFilter.doFilter(req, res, chain); + assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_OK); + } + + @Test + public void shouldFailAuthenticationIfGerritUserDoesNotExist() throws Exception { + doReturn(Optional.empty()).when(accountCache).getByUsername(AUTH_USER); + initWebSessionWithoutCookie(); + requestBasicAuth(req); + doReturn(GitBasicAuthPolicy.HTTP).when(authConfig).getGitBasicAuthPolicy(); + + ProjectBasicAuthFilter basicAuthFilter = + new ProjectBasicAuthFilter( + webSessionItem, + accountCache, + accountManager, + authConfig, + authRequestFactory, + pwdVerifier); + + basicAuthFilter.doFilter(req, res, chain); + assertThat(res.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED); + } + + @Test public void shouldAuthenticateSucessfullyAgainstRealmAndReturnCookie() throws Exception { initAccount(); initWebSessionWithoutCookie(); |