Merge branch 'stable-3.9' * stable-3.9: Fallback to public organization information Redirect to login on anonymous access to profile pages Detect the default branch during the repo import from GitHub Bump plugin and Gerrit to v3.9.0-rc5 Do not import changes refs from GitHub Bump to v3.9.0-rc4 and reuse FakeHttpServletRequest/Response Refresh GitHub groups upon Gerrit successful login Change-Id: I8242d270b9e5ff0ca8e221d0b426a337e7710c12
diff --git a/github-oauth/pom.xml b/github-oauth/pom.xml index 256abc8..d027b61 100644 --- a/github-oauth/pom.xml +++ b/github-oauth/pom.xml
@@ -21,7 +21,7 @@ <parent> <groupId>com.googlesource.gerrit.plugins.github</groupId> <artifactId>github-parent</artifactId> - <version>3.9.0-rc2</version> + <version>3.9.0-rc5</version> </parent> <artifactId>github-oauth</artifactId> <name>Gerrit Code Review - GitHub OAuth login</name>
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubLogin.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubLogin.java index adfe5e3..02889a1 100644 --- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubLogin.java +++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubLogin.java
@@ -38,6 +38,7 @@ import org.kohsuke.github.GHMyself; import org.kohsuke.github.GitHub; import org.kohsuke.github.GitHubBuilder; +import org.kohsuke.github.HttpException; import org.kohsuke.github.connector.GitHubConnector; import org.kohsuke.github.internal.GitHubConnectorHttpConnectorAdapter; import org.slf4j.Logger; @@ -75,9 +76,21 @@ return null; } - public Set<String> getMyOrganisationsLogins() throws IOException { + public Set<String> getMyOrganisationsLogins(String username) throws IOException { if (isLoggedIn()) { - return getHub().getMyOrganizations().keySet(); + try { + return getHub().getMyOrganizations().keySet(); + } catch (HttpException httpException) { + if (!httpException.getMessage().contains("You need at least")) { + throw httpException; + } + log.info( + "Cannot access organizations for user '{}': falling back to list of public" + + " organisations", + username); + + return getHub().getUserPublicOrganizations(username).keySet(); + } } return Collections.emptySet(); }
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthWebFilter.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthWebFilter.java index 0bca570..20b80bd 100644 --- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthWebFilter.java +++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthWebFilter.java
@@ -131,7 +131,7 @@ String user = myself.getLogin(); updateSecureConfigWithRetry( - ghLogin.getHub().getMyOrganizations().keySet(), user, ghLogin.getToken().accessToken); + ghLogin.getMyOrganisationsLogins(user), user, ghLogin.getToken().accessToken); } }
diff --git a/github-plugin/pom.xml b/github-plugin/pom.xml index 96d3290..1eec4f3 100644 --- a/github-plugin/pom.xml +++ b/github-plugin/pom.xml
@@ -20,7 +20,7 @@ <parent> <artifactId>github-parent</artifactId> <groupId>com.googlesource.gerrit.plugins.github</groupId> - <version>3.9.0-rc2</version> + <version>3.9.0-rc5</version> </parent> <artifactId>github-plugin</artifactId> @@ -152,6 +152,12 @@ <scope>provided</scope> </dependency> <dependency> + <groupId>com.google.gerrit</groupId> + <artifactId>gerrit-acceptance-framework</artifactId> + <version>${Gerrit-ApiVersion}</version> + <scope>test</scope> + </dependency> + <dependency> <groupId>${project.groupId}</groupId> <artifactId>github-oauth</artifactId> <version>${project.version}</version>
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GuiceHttpModule.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GuiceHttpModule.java index 661b0bf..ef24191 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GuiceHttpModule.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GuiceHttpModule.java
@@ -18,10 +18,13 @@ import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.webui.JavaScriptPlugin; import com.google.gerrit.extensions.webui.WebUiPlugin; +import com.google.gerrit.httpd.AllRequestFilter; +import com.google.inject.Scopes; import com.google.inject.TypeLiteral; import com.google.inject.assistedinject.FactoryModuleBuilder; import com.google.inject.name.Names; import com.google.inject.servlet.ServletModule; +import com.googlesource.gerrit.plugins.github.filters.GitHubGroupCacheRefreshFilter; import com.googlesource.gerrit.plugins.github.filters.GitHubOAuthFilter; import com.googlesource.gerrit.plugins.github.git.CreateProjectStep; import com.googlesource.gerrit.plugins.github.git.GitCloneStep; @@ -103,5 +106,9 @@ serve("/static/*").with(VelocityViewServlet.class); filterRegex("(?!/webhook).*").through(GitHubOAuthFilter.class); + + DynamicSet.bind(binder(), AllRequestFilter.class) + .to(GitHubGroupCacheRefreshFilter.class) + .in(Scopes.SINGLETON); } }
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/filters/GitHubGroupCacheRefreshFilter.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/filters/GitHubGroupCacheRefreshFilter.java new file mode 100644 index 0000000..eb34962 --- /dev/null +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/filters/GitHubGroupCacheRefreshFilter.java
@@ -0,0 +1,78 @@ +// Copyright (C) 2023 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.plugins.github.filters; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.httpd.AllRequestFilter; +import com.googlesource.gerrit.plugins.github.group.GitHubGroupsCache; +import java.io.IOException; +import java.util.Optional; +import javax.inject.Inject; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +public class GitHubGroupCacheRefreshFilter extends AllRequestFilter { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final String LOGIN_URL = "/login"; + private static final String LOGIN_QUERY_FINAL = "final=true"; + private static final String ACCOUNT_COOKIE = "GerritAccount"; + private static final String INVALIDATE_CACHED_GROUPS = "RefreshGroups"; + + private final GitHubGroupsCache ghGroupsCache; + + @Inject + @VisibleForTesting + public GitHubGroupCacheRefreshFilter(GitHubGroupsCache ghGroupsCache) { + this.ghGroupsCache = ghGroupsCache; + } + + @Override + public void init(FilterConfig filterConfig) throws ServletException {} + + @Override + public void doFilter( + ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) + throws IOException, ServletException { + filterChain.doFilter(servletRequest, servletResponse); + + HttpServletRequest req = (HttpServletRequest) servletRequest; + if (req.getRequestURI().endsWith(LOGIN_URL) && req.getQueryString().equals(LOGIN_QUERY_FINAL)) { + HttpServletResponse resp = (HttpServletResponse) servletResponse; + String cookieResponse = resp.getHeader("Set-Cookie"); + if (cookieResponse != null && cookieResponse.contains(ACCOUNT_COOKIE)) { + req.getSession().setAttribute(INVALIDATE_CACHED_GROUPS, Boolean.TRUE); + } + } else if (hasSessionFlagForInvalidatingCachedUserGroups(req)) { + ghGroupsCache.invalidateCurrentUserGroups(); + req.getSession().removeAttribute(INVALIDATE_CACHED_GROUPS); + } + } + + private static boolean hasSessionFlagForInvalidatingCachedUserGroups(HttpServletRequest req) { + return Optional.ofNullable(req.getSession(false)) + .flatMap(session -> Optional.ofNullable(session.getAttribute(INVALIDATE_CACHED_GROUPS))) + .filter(refresh -> (Boolean) refresh) + .isPresent(); + } + + @Override + public void destroy() {} +}
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/GitCloneStep.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/GitCloneStep.java index 25d953b..503712c 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/GitCloneStep.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/git/GitCloneStep.java
@@ -13,6 +13,8 @@ // limitations under the License. package com.googlesource.gerrit.plugins.github.git; +import static java.util.stream.Collectors.toList; + import com.google.gerrit.entities.Project; import com.google.gerrit.extensions.api.GerritApi; import com.google.gerrit.extensions.api.changes.NotifyHandling; @@ -30,6 +32,7 @@ import com.googlesource.gerrit.plugins.github.GitHubConfig; import java.io.File; import java.io.IOException; +import java.util.stream.Stream; import org.apache.commons.io.FileUtils; import org.eclipse.jgit.api.FetchCommand; import org.eclipse.jgit.api.Git; @@ -95,7 +98,9 @@ try (ManualRequestContext requestContext = context.openAs(config.importAccountId)) { ProjectInput pi = new ProjectInput(); pi.name = projectName; - pi.parent = config.getBaseProject(getRepository().isPrivate()); + GitHubRepository ghRepository = getRepository(); + pi.parent = config.getBaseProject(ghRepository.isPrivate()); + pi.branches = Stream.ofNullable(ghRepository.getDefaultBranch()).collect(toList()); gerritApi.projects().create(pi).get(); } catch (ResourceConflictException e) { throw new GitDestinationAlreadyExistsException(projectName); @@ -109,7 +114,8 @@ createNewProject(); String sourceUri = getSourceUri(); try (Git git = Git.open(destinationDirectory)) { - FetchCommand fetch = git.fetch().setRefSpecs("refs/*:refs/*").setRemote(sourceUri); + FetchCommand fetch = + git.fetch().setRefSpecs("^refs/changes/*", "refs/*:refs/*").setRemote(sourceUri); fetch.setCredentialsProvider(getRepository().getCredentialsProvider()); if (progress != null) { fetch.setProgressMonitor(progress);
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/CurrentUsernameProvider.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/CurrentUsernameProvider.java new file mode 100644 index 0000000..028aa6b --- /dev/null +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/CurrentUsernameProvider.java
@@ -0,0 +1,41 @@ +// Copyright (C) 2023 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.plugins.github.group; + +import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.IdentifiedUser; +import com.google.inject.Inject; +import com.google.inject.Provider; +import java.util.Optional; + +public class CurrentUsernameProvider implements Provider<String> { + public static final String CURRENT_USERNAME = "CurrentUsername"; + + private final Provider<CurrentUser> userProvider; + + @Inject + CurrentUsernameProvider(Provider<CurrentUser> userProvider) { + this.userProvider = userProvider; + } + + @Override + public String get() { + return Optional.ofNullable(userProvider.get()) + .filter(CurrentUser::isIdentifiedUser) + .map(CurrentUser::asIdentifiedUser) + .flatMap(IdentifiedUser::getUserName) + .orElse(null); + } +}
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupsCache.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupsCache.java index 3504f2c..8f9c776 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupsCache.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/group/GitHubGroupsCache.java
@@ -14,19 +14,21 @@ package com.googlesource.gerrit.plugins.github.group; +import static com.googlesource.gerrit.plugins.github.group.CurrentUsernameProvider.CURRENT_USERNAME; import static java.time.temporal.ChronoUnit.MINUTES; +import com.google.common.annotations.VisibleForTesting; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableSet; import com.google.gerrit.entities.AccountGroup.UUID; -import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.cache.CacheModule; import com.google.inject.Inject; import com.google.inject.Module; import com.google.inject.Provider; import com.google.inject.Singleton; import com.google.inject.name.Named; +import com.google.inject.name.Names; import com.googlesource.gerrit.plugins.github.groups.OrganizationStructure; import com.googlesource.gerrit.plugins.github.oauth.GitHubLogin; import com.googlesource.gerrit.plugins.github.oauth.UserScopedProvider; @@ -95,7 +97,7 @@ private void loadOrganisations( String username, OrganizationStructure orgsTeams, GitHubLogin ghLogin) throws IOException { logger.debug("Getting list of public organisations for user '{}'", username); - Set<String> organisations = ghLogin.getMyOrganisationsLogins(); + Set<String> organisations = ghLogin.getMyOrganisationsLogins(username); for (String org : organisations) { orgsTeams.put(org, EVERYONE_TEAM_NAME); } @@ -106,6 +108,9 @@ return new CacheModule() { @Override protected void configure() { + bind(String.class) + .annotatedWith(Names.named(CurrentUsernameProvider.CURRENT_USERNAME)) + .toProvider(CurrentUsernameProvider.class); persist(ORGS_CACHE_NAME, String.class, OrganizationStructure.class) .expireAfterWrite(Duration.of(GROUPS_CACHE_TTL_MINS, MINUTES)) .loader(OrganisationLoader.class); @@ -115,14 +120,15 @@ } private final LoadingCache<String, OrganizationStructure> orgTeamsByUsername; - private final Provider<IdentifiedUser> userProvider; + private final Provider<String> usernameProvider; @Inject - GitHubGroupsCache( + @VisibleForTesting + public GitHubGroupsCache( @Named(ORGS_CACHE_NAME) LoadingCache<String, OrganizationStructure> byUsername, - Provider<IdentifiedUser> userProvider) { + @Named(CURRENT_USERNAME) Provider<String> usernameProvider) { this.orgTeamsByUsername = byUsername; - this.userProvider = userProvider; + this.usernameProvider = usernameProvider; } Set<String> getOrganizationsForUser(String username) { @@ -135,7 +141,7 @@ } Set<String> getOrganizationsForCurrentUser() throws ExecutionException { - return orgTeamsByUsername.get(userProvider.get().getUserName().get()).keySet(); + return orgTeamsByUsername.get(usernameProvider.get()).keySet(); } Set<String> getTeamsForUser(String organizationName, String username) { @@ -156,7 +162,7 @@ } Set<String> getTeamsForCurrentUser(String organizationName) { - return getTeamsForUser(organizationName, userProvider.get().getUserName().get()); + return getTeamsForUser(organizationName, usernameProvider.get()); } public Set<UUID> getGroupsForUser(String username) { @@ -170,4 +176,8 @@ } return groupsBuilder.build(); } + + public void invalidateCurrentUserGroups() { + orgTeamsByUsername.invalidate(usernameProvider.get()); + } }
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityViewServlet.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityViewServlet.java index b7dc08f..5e4498f 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityViewServlet.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/velocity/VelocityViewServlet.java
@@ -13,8 +13,11 @@ // limitations under the License. package com.googlesource.gerrit.plugins.github.velocity; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.GITHUB_PLUGIN_OAUTH_SCOPE; + import com.google.common.base.MoreObjects; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.config.AuthConfig; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -53,6 +56,7 @@ private final GitHubConfig config; private final VirtualDomainConfig virtualDomainConfig; private final CanonicalWebUrls canonicalWebUrls; + private final AuthConfig authConfig; @Inject public VelocityViewServlet( @@ -62,7 +66,8 @@ Provider<CurrentUser> userProvider, GitHubConfig config, VirtualDomainConfig virutalDomainConfig, - CanonicalWebUrls canonicalWebUrls) { + CanonicalWebUrls canonicalWebUrls, + AuthConfig authConfig) { this.velocityRuntime = velocityRuntime; this.modelProvider = modelProvider; @@ -71,6 +76,7 @@ this.config = config; this.virtualDomainConfig = virutalDomainConfig; this.canonicalWebUrls = canonicalWebUrls; + this.authConfig = authConfig; } @Override @@ -79,6 +85,12 @@ HttpServletRequest req = (HttpServletRequest) request; HttpServletResponse resp = (HttpServletResponse) response; + if (!(req.getRequestURI().equals(GITHUB_PLUGIN_OAUTH_SCOPE) + || userProvider.get().isIdentifiedUser())) { + resp.sendRedirect(authConfig.getLoginUrl()); + return; + } + String pathInfo = STATIC_PREFIX + MoreObjects.firstNonNull((String) req.getAttribute("destUrl"), req.getPathInfo());
diff --git a/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/FakeHttpSession.java b/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/FakeHttpSession.java new file mode 100644 index 0000000..6a820e7 --- /dev/null +++ b/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/FakeHttpSession.java
@@ -0,0 +1,112 @@ +// Copyright (C) 2023 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.plugins.github; + +import java.util.Enumeration; +import java.util.HashMap; +import javax.servlet.ServletContext; +import javax.servlet.http.HttpSession; +import javax.servlet.http.HttpSessionContext; + +public class FakeHttpSession implements HttpSession { + private final HashMap<String, Object> attributes; + + public FakeHttpSession() { + this.attributes = new HashMap<>(); + } + + @Override + public long getCreationTime() { + return 0; + } + + @Override + public String getId() { + return null; + } + + @Override + public long getLastAccessedTime() { + return 0; + } + + @Override + public ServletContext getServletContext() { + return null; + } + + @Override + public void setMaxInactiveInterval(int i) {} + + @Override + public int getMaxInactiveInterval() { + return 0; + } + + @Override + public HttpSessionContext getSessionContext() { + return null; + } + + @Override + public Object getAttribute(String s) { + return attributes.get(s); + } + + @Override + public Object getValue(String s) { + return getAttribute(s); + } + + @Override + public Enumeration<String> getAttributeNames() { + return java.util.Collections.enumeration(attributes.keySet()); + } + + @Override + public String[] getValueNames() { + return attributes.keySet().toArray(new String[0]); + } + + @Override + public void setAttribute(String s, Object o) { + attributes.put(s, o); + } + + @Override + public void putValue(String s, Object o) { + setAttribute(s, o); + } + + @Override + public void removeAttribute(String s) { + attributes.remove(s); + } + + @Override + public void removeValue(String s) { + removeAttribute(s); + } + + @Override + public void invalidate() { + attributes.clear(); + } + + @Override + public boolean isNew() { + return false; + } +}
diff --git a/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/GitHubGroupCacheRefreshFilterTest.java b/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/GitHubGroupCacheRefreshFilterTest.java new file mode 100644 index 0000000..5c1707f --- /dev/null +++ b/github-plugin/src/test/java/com/googlesource/gerrit/plugins/github/GitHubGroupCacheRefreshFilterTest.java
@@ -0,0 +1,125 @@ +// Copyright (C) 2023 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.plugins.github; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; +import com.google.gerrit.util.http.testutil.FakeHttpServletRequest; +import com.google.gerrit.util.http.testutil.FakeHttpServletResponse; +import com.googlesource.gerrit.plugins.github.filters.GitHubGroupCacheRefreshFilter; +import com.googlesource.gerrit.plugins.github.group.GitHubGroupsCache; +import com.googlesource.gerrit.plugins.github.groups.OrganizationStructure; +import javax.servlet.FilterChain; +import javax.servlet.ServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; +import org.junit.Before; +import org.junit.Test; + +public class GitHubGroupCacheRefreshFilterTest { + private static final FilterChain NOOP_FILTER_CHAIN_TEST = (req, res) -> {}; + private static final String GITHUB_USERNAME_TEST = "somegithubuser"; + private static final OrganizationStructure GITHUB_USER_ORGANIZATION = new OrganizationStructure(); + private static final String TEST_SERVER = "test-server"; + private static final int TEST_PORT = 80; + + private LoadingCache<String, OrganizationStructure> groupsByUsernameCache; + private GitHubGroupCacheRefreshFilter filter; + private FakeGroupCacheLoader groupsCacheLoader; + private int initialLoadCount; + + private static class FakeGroupCacheLoader extends CacheLoader<String, OrganizationStructure> { + private final String username; + private final OrganizationStructure organizationStructure; + private int loadCount; + + FakeGroupCacheLoader(String username, OrganizationStructure organizationStructure) { + this.username = username; + this.organizationStructure = organizationStructure; + } + + @Override + public OrganizationStructure load(String u) throws Exception { + if (u.equals(username)) { + loadCount++; + return organizationStructure; + } else { + return null; + } + } + + public int getLoadCount() { + return loadCount; + } + } + + @Before + public void setUp() throws Exception { + groupsCacheLoader = new FakeGroupCacheLoader(GITHUB_USERNAME_TEST, GITHUB_USER_ORGANIZATION); + groupsByUsernameCache = CacheBuilder.newBuilder().build(groupsCacheLoader); + filter = + new GitHubGroupCacheRefreshFilter( + new GitHubGroupsCache(groupsByUsernameCache, () -> GITHUB_USERNAME_TEST)); + // Trigger the initial load of the groups cache + assertThat(groupsByUsernameCache.get(GITHUB_USERNAME_TEST)).isEqualTo(GITHUB_USER_ORGANIZATION); + initialLoadCount = groupsCacheLoader.getLoadCount(); + } + + @Test + public void shouldReloadGroupsUponSuccessfulLogin() throws Exception { + FakeHttpServletRequest finalLoginRequest = newFinalLoginRequest(); + filter.doFilter(finalLoginRequest, newFinalLoginRedirectWithCookie(), NOOP_FILTER_CHAIN_TEST); + filter.doFilter( + newHomepageRequest(finalLoginRequest.getSession()), + new FakeHttpServletResponse(), + NOOP_FILTER_CHAIN_TEST); + + assertThat(groupsByUsernameCache.get(GITHUB_USERNAME_TEST)).isEqualTo(GITHUB_USER_ORGANIZATION); + assertThat(groupsCacheLoader.getLoadCount()).isEqualTo(initialLoadCount + 1); + } + + @Test + public void shouldNotReloadGroupsOnRegularRequests() throws Exception { + FakeHttpServletRequest regularRequest = new FakeHttpServletRequest(); + filter.doFilter(regularRequest, new FakeHttpServletResponse(), NOOP_FILTER_CHAIN_TEST); + filter.doFilter( + newHomepageRequest(null), new FakeHttpServletResponse(), NOOP_FILTER_CHAIN_TEST); + + assertThat(groupsByUsernameCache.get(GITHUB_USERNAME_TEST)).isEqualTo(GITHUB_USER_ORGANIZATION); + assertThat(groupsCacheLoader.getLoadCount()).isEqualTo(initialLoadCount); + } + + private ServletRequest newHomepageRequest(HttpSession session) { + return new FakeHttpServletRequest(TEST_SERVER, TEST_PORT, "", "/", null, session); + } + + private static HttpServletResponse newFinalLoginRedirectWithCookie() { + HttpServletResponse res = new FakeHttpServletResponse(); + res.setHeader("Set-Cookie", "GerritAccount=foo"); + return res; + } + + private static FakeHttpServletRequest newFinalLoginRequest() { + FakeHttpServletRequest req = + new FakeHttpServletRequest( + TEST_SERVER, TEST_PORT, "", "", () -> new FakeHttpSession(), null); + req.setQueryString("final=true"); + req.setPathInfo("/login"); + return req; + } +}
diff --git a/pom.xml b/pom.xml index a0c4c21..d0f54d8 100644 --- a/pom.xml +++ b/pom.xml
@@ -18,7 +18,7 @@ <modelVersion>4.0.0</modelVersion> <groupId>com.googlesource.gerrit.plugins.github</groupId> <artifactId>github-parent</artifactId> - <version>3.9.0-rc2</version> + <version>3.9.0-rc5</version> <name>Gerrit Code Review - GitHub integration</name> <url>http://www.gerritforge.com</url> <packaging>pom</packaging>