diff options
author | Luca Milanesio <luca.milanesio@gmail.com> | 2020-06-05 23:55:08 +0100 |
---|---|---|
committer | Luca Milanesio <luca.milanesio@gmail.com> | 2020-06-05 23:27:06 +0000 |
commit | 2a30db520976c94762a33c9cae2534295065a822 (patch) | |
tree | 9c47b94a4a7c1db1338ccc557933f89f9e66742f | |
parent | 938db887d803889e34a1adfd762c259eadcdd6a3 (diff) | |
parent | 06a3a5bfea30ef1907463c66ee1bf447eeb958e9 (diff) |
Merge branch 'stable-2.14' into stable-2.15
* stable-2.14:
Deny access over HTTP for disabled accounts
Bazel: Consistently use bazelisk during publishing of artifacts
Change-Id: I16c6c00e61ef39cc27f40da1ac1822e419eb2f2c
3 files changed, 115 insertions, 4 deletions
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java index ac71e54734..ea13599dca 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java @@ -56,6 +56,7 @@ import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.common.data.Permission; +import com.google.gerrit.extensions.api.accounts.AccountApi; import com.google.gerrit.extensions.api.accounts.AccountInput; import com.google.gerrit.extensions.api.accounts.EmailInput; import com.google.gerrit.extensions.api.changes.AddReviewerInput; @@ -81,6 +82,7 @@ import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.gpg.Fingerprint; import com.google.gerrit.gpg.PublicKeyStore; import com.google.gerrit.gpg.testutil.TestKey; +import com.google.gerrit.httpd.CacheBasedWebSession; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Change; @@ -123,6 +125,14 @@ import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; +import javax.servlet.http.HttpServletResponse; +import org.apache.http.HttpResponse; +import org.apache.http.client.ClientProtocolException; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.BasicCookieStore; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; import org.bouncycastle.bcpg.ArmoredOutputStream; import org.bouncycastle.openpgp.PGPPublicKey; import org.bouncycastle.openpgp.PGPPublicKeyRing; @@ -181,6 +191,8 @@ public class AccountIT extends AbstractDaemonTest { private RegistrationHandle refUpdateCounterHandle; private ExternalIdsUpdate externalIdsUpdate; private List<ExternalId> savedExternalIds; + private BasicCookieStore httpCookieStore; + private CloseableHttpClient httpclient; @Before public void addAccountIndexEventCounter() { @@ -217,6 +229,16 @@ public class AccountIT extends AbstractDaemonTest { savedExternalIds.addAll(externalIds.byAccount(user.id)); } + @Before + public void createHttpClient() { + httpCookieStore = new BasicCookieStore(); + httpclient = + HttpClientBuilder.create() + .disableRedirectHandling() + .setDefaultCookieStore(httpCookieStore) + .build(); + } + @After public void restoreExternalIds() throws Exception { if (savedExternalIds != null) { @@ -412,6 +434,43 @@ public class AccountIT extends AbstractDaemonTest { } @Test + @GerritConfig(name = "auth.type", value = "DEVELOPMENT_BECOME_ANY_ACCOUNT") + public void activeUserGetSessionCookieOnLogin() throws Exception { + Integer accountId = accountIdApi().get()._accountId; + assertThat(accountIdApi().getActive()).isTrue(); + + webLogin(accountId); + assertThat(getCookiesNames()).contains(CacheBasedWebSession.ACCOUNT_COOKIE); + } + + @Test + @GerritConfig(name = "auth.type", value = "DEVELOPMENT_BECOME_ANY_ACCOUNT") + public void inactiveUserDoesNotGetCookieOnLogin() throws Exception { + Integer accountId = accountIdApi().get()._accountId; + accountIdApi().setActive(false); + assertThat(accountIdApi().getActive()).isFalse(); + + webLogin(accountId); + assertThat(getCookiesNames()).isEmpty(); + } + + @Test + @GerritConfig(name = "auth.type", value = "DEVELOPMENT_BECOME_ANY_ACCOUNT") + public void userDeactivatedAfterLoginDoesNotGetCookie() throws Exception { + Integer accountId = accountIdApi().get()._accountId; + assertThat(accountIdApi().getActive()).isTrue(); + + webLogin(accountId); + assertThat(getCookiesNames()).contains(CacheBasedWebSession.ACCOUNT_COOKIE); + httpGetAndAssertStatus("accounts/self/detail", HttpServletResponse.SC_OK); + + accountIdApi().setActive(false); + assertThat(accountIdApi().getActive()).isFalse(); + + httpGetAndAssertStatus("accounts/self/detail", HttpServletResponse.SC_FORBIDDEN); + } + + @Test public void deactivateSelf() throws Exception { exception.expect(ResourceConflictException.class); exception.expectMessage("cannot deactivate own account"); @@ -2371,4 +2430,28 @@ public class AccountIT extends AbstractDaemonTest { clear(); } } + + private AccountApi accountIdApi() throws RestApiException { + return gApi.accounts().id("user"); + } + + private Set<String> getCookiesNames() { + Set<String> cookieNames = + httpCookieStore.getCookies().stream() + .map(cookie -> cookie.getName()) + .collect(Collectors.toSet()); + return cookieNames; + } + + private void webLogin(Integer accountId) throws IOException, ClientProtocolException { + httpGetAndAssertStatus( + "login?account_id=" + accountId, HttpServletResponse.SC_MOVED_TEMPORARILY); + } + + private void httpGetAndAssertStatus(String urlPath, int expectedHttpStatus) + throws ClientProtocolException, IOException { + HttpGet httpGet = new HttpGet(canonicalWebUrl.get() + urlPath); + HttpResponse loginResponse = httpclient.execute(httpGet); + assertThat(loginResponse.getStatusLine().getStatusCode()).isEqualTo(expectedHttpStatus); + } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java index 6a19be7455..6239e3b5e7 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java @@ -16,6 +16,7 @@ package com.google.gerrit.httpd; import static java.util.concurrent.TimeUnit.HOURS; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.HostPageData; @@ -28,6 +29,8 @@ import com.google.gerrit.server.AccessPath; import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.account.AccountCache; +import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.AuthResult; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.config.AuthConfig; @@ -41,7 +44,7 @@ import org.eclipse.jgit.http.server.GitSmartHttpTools; @RequestScoped public abstract class CacheBasedWebSession implements WebSession { - private static final String ACCOUNT_COOKIE = "GerritAccount"; + @VisibleForTesting public static final String ACCOUNT_COOKIE = "GerritAccount"; protected static final long MAX_AGE_MINUTES = HOURS.toMinutes(12); private final HttpServletRequest request; @@ -51,6 +54,7 @@ public abstract class CacheBasedWebSession implements WebSession { private final Provider<AnonymousUser> anonymousProvider; private final IdentifiedUser.RequestFactory identified; private final EnumSet<AccessPath> okPaths = EnumSet.of(AccessPath.UNKNOWN); + private final AccountCache byIdCache; private Cookie outCookie; private Key key; @@ -63,13 +67,15 @@ public abstract class CacheBasedWebSession implements WebSession { WebSessionManager manager, AuthConfig authConfig, Provider<AnonymousUser> anonymousProvider, - IdentifiedUser.RequestFactory identified) { + IdentifiedUser.RequestFactory identified, + final AccountCache byIdCache) { this.request = request; this.response = response; this.manager = manager; this.authConfig = authConfig; this.anonymousProvider = anonymousProvider; this.identified = identified; + this.byIdCache = byIdCache; if (request.getRequestURI() == null || !GitSmartHttpTools.isGitClient(request)) { String cookie = readCookie(request); @@ -86,6 +92,10 @@ public abstract class CacheBasedWebSession implements WebSession { authFromQueryParameter(token); } } + if (val != null && !checkAccountStatus(val.getAccountId())) { + val = null; + okPaths.clear(); + } if (val != null && val.needsCookieRefresh()) { // Session is more than half old; update cache entry with new expiration date. val = manager.createVal(key, val); @@ -178,6 +188,11 @@ public abstract class CacheBasedWebSession implements WebSession { manager.destroy(key); } + if (!checkAccountStatus(id)) { + val = null; + return; + } + key = manager.createKey(id); val = manager.createVal(key, id, rememberMe, identity, null, null); saveCookie(); @@ -208,6 +223,11 @@ public abstract class CacheBasedWebSession implements WebSession { return val != null ? val.getSessionId() : null; } + private boolean checkAccountStatus(Account.Id id) { + AccountState accountState = byIdCache.get(id); + return accountState != null && accountState.getAccount().isActive(); + } + private void saveCookie() { if (response == null) { return; diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/H2CacheBasedWebSession.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/H2CacheBasedWebSession.java index c466290b28..c77f76f7d8 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/H2CacheBasedWebSession.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/H2CacheBasedWebSession.java @@ -22,6 +22,7 @@ import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.httpd.WebSessionManager.Val; import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.IdentifiedUser.RequestFactory; +import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.cache.CacheModule; import com.google.gerrit.server.config.AuthConfig; import com.google.inject.Inject; @@ -62,8 +63,15 @@ public class H2CacheBasedWebSession extends CacheBasedWebSession { @Named(WebSessionManager.CACHE_NAME) Cache<String, Val> cache, AuthConfig authConfig, Provider<AnonymousUser> anonymousProvider, - RequestFactory identified) { + RequestFactory identified, + AccountCache byIdCache) { super( - request, response, managerFactory.create(cache), authConfig, anonymousProvider, identified); + request, + response, + managerFactory.create(cache), + authConfig, + anonymousProvider, + identified, + byIdCache); } } |