[BugFix] /login not enriched when GerritOAuth expires
When GerritOAuth token expires but the JSESSIONID is already
active, the login to GitHub was not performed but the HTTP
request not enriched. The consequence was the inability to login
and the error displayed was the usual Gerrit page on the missing HTTP header.
Change-Id: I9ca91387b040dd35ca1d61671002df0a48b2a9dd
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 eb19484..4eca364 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
@@ -38,7 +38,6 @@
import org.kohsuke.github.GHMyself;
import org.slf4j.LoggerFactory;
-import com.google.common.base.Strings;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -50,7 +49,6 @@
public static final String GERRIT_COOKIE_NAME = "GerritAccount";
private final GitHubOAuthConfig config;
- private final OAuthCookieProvider cookieProvider;
private final Random retryRandom = new Random(System.currentTimeMillis());
private SitePaths sites;
private ScopedProvider<GitHubLogin> loginProvider;
@@ -63,7 +61,6 @@
this.config = config;
this.sites = sites;
this.loginProvider = loginProvider;
- this.cookieProvider = new OAuthCookieProvider(TokenCipher.get(), config);
}
@Override
@@ -83,16 +80,17 @@
try {
GitHubLogin ghLogin = loginProvider.get(httpRequest);
- OAuthCookie authCookie =
- getOAuthCookie(httpRequest, (HttpServletResponse) response);
-
if (OAuthProtocol.isOAuthLogout(httpRequest)) {
logout(request, response, chain, httpRequest);
} else if (OAuthProtocol.isOAuthRequest(httpRequest)
&& !ghLogin.isLoggedIn()) {
login(request, httpRequest, httpResponse, ghLogin);
} else {
- httpRequest = enrichAuthenticatedRequest(httpRequest, authCookie);
+ if (ghLogin != null && ghLogin.isLoggedIn()) {
+ httpRequest =
+ new AuthenticatedHttpRequest(httpRequest, config.httpHeader,
+ ghLogin.getMyself().getLogin());
+ }
if (OAuthProtocol.isOAuthFinalForOthers(httpRequest)) {
httpResponse.sendRedirect(OAuthProtocol
@@ -117,14 +115,6 @@
}
}
- private HttpServletRequest enrichAuthenticatedRequest(
- HttpServletRequest httpRequest, OAuthCookie authCookie) {
- httpRequest =
- authCookie == null ? httpRequest : new AuthenticatedHttpRequest(
- httpRequest, config.httpHeader, authCookie.user);
- return httpRequest;
- }
-
private void login(ServletRequest request, HttpServletRequest httpRequest,
HttpServletResponse httpResponse, GitHubLogin ghLogin) throws IOException {
if (ghLogin.login(httpRequest, httpResponse)) {
@@ -246,28 +236,6 @@
return cookies == null ? new Cookie[0] : cookies;
}
- private OAuthCookie getOAuthCookie(HttpServletRequest request,
- HttpServletResponse response) {
- for (Cookie cookie : getCookies(request)) {
- if (cookie.getName().equalsIgnoreCase(OAuthCookie.OAUTH_COOKIE_NAME)
- && !Strings.isNullOrEmpty(cookie.getValue())) {
- try {
- return cookieProvider.getFromCookie(cookie);
- } catch (OAuthTokenException e) {
- log.warn(
- "Invalid cookie detected: cleaning up and sending a reset back to the browser",
- e);
- cookie.setValue("");
- cookie.setPath("/");
- cookie.setMaxAge(0);
- response.addCookie(cookie);
- return null;
- }
- }
- }
- return null;
- }
-
@Override
public void destroy() {
log.info("Init");
