Resilience against GerritOAuth and JSESSIONID cookies lost
GitHub plugin uses two main cookies to cache OAuth
information:
- GerritOAuth to enrich HTTP requests with auth headers used by
Gerrit HTTP authentication
- JSESSIONID to get session objects with GitHub client objects
When GerritOAuth or JSESSIONID expire, a transparent GitHub OAuth
renegotiation is needed to recreate them through a new authentication
challenge. OAuth scope is now stored in the GerritOAuth cookie in order
to re-create the same scope-level previously own.
NOTE: GerritOAuth is typically short lived to avoid
security leak of the OAuth information. JSESSIONID could be lost
because of workload balancing or failover.
Change-Id: I874c4ae75f2b238dccb0ec8c164628c28f1b5f01diff --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 459b9da..da86a02 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
@@ -15,9 +15,16 @@
package com.googlesource.gerrit.plugins.github.oauth;
import java.io.IOException;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
+import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -27,6 +34,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.base.Objects;
import com.google.common.base.Strings;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -42,6 +50,13 @@
public Provider(com.google.inject.Provider<GitHubLogin> provider) {
super(provider);
}
+
+ @Override
+ public GitHubLogin get(HttpServletRequest req) {
+ GitHubLogin login = super.get(req);
+ login.initOAuthCookie(req);
+ return login;
+ }
}
public AccessToken token;
@@ -50,7 +65,10 @@
private transient OAuthProtocol oauth;
private GHMyself myself;
+ private SortedSet<Scope> loginScopes;
private final OAuthCookieProvider cookieProvider;
+ private final GitHubOAuthConfig config;
+ private OAuthCookie oAuthCookie;
public GHMyself getMyself() {
if (isLoggedIn()) {
@@ -61,9 +79,10 @@
}
@Inject
- public GitHubLogin(OAuthProtocol oauth) {
+ public GitHubLogin(final OAuthProtocol oauth, final GitHubOAuthConfig config) {
this.oauth = oauth;
this.cookieProvider = new OAuthCookieProvider(TokenCipher.get());
+ this.config = config;
}
public boolean isLoggedIn() {
@@ -80,6 +99,25 @@
return loggedIn;
}
+ private void initOAuthCookie(HttpServletRequest request) {
+ for (Cookie cookie : getCookies(request)) {
+ if (cookie.getName().equalsIgnoreCase(OAuthCookie.OAUTH_COOKIE_NAME)
+ && !Strings.isNullOrEmpty(cookie.getValue())) {
+ try {
+ oAuthCookie = cookieProvider.getFromCookie(cookie);
+ loginScopes = oAuthCookie.scopes;
+ } catch (OAuthTokenException e) {
+ LOG.warn("Invalid cookie detected", e);
+ }
+ }
+ }
+ }
+
+ private Cookie[] getCookies(HttpServletRequest httpRequest) {
+ Cookie[] cookies = httpRequest.getCookies();
+ return cookies == null ? new Cookie[0] : cookies;
+ }
+
public boolean login(ServletRequest request, ServletResponse response,
Scope... scopes) throws IOException {
return login((HttpServletRequest) request, (HttpServletResponse) response,
@@ -106,7 +144,7 @@
.getName();
OAuthCookie userCookie =
- cookieProvider.getFromUser(user, email, fullName);
+ cookieProvider.getFromUser(user, email, fullName, loginScopes);
response.addCookie(userCookie);
response.sendRedirect(OAuthProtocol.getTargetUrl(request));
@@ -116,8 +154,9 @@
return false;
}
} else {
+ this.loginScopes = getScopes(getScopesKey(request), scopes);
LOG.debug("Login-PHASE1 " + this);
- oauth.loginPhase1(request, response, scopes);
+ oauth.loginPhase1(request, response, loginScopes);
return false;
}
}
@@ -139,7 +178,23 @@
@Override
public String toString() {
- return "GitHubLogin [token=" + token + ", myself=" + myself + "]";
+ return "GitHubLogin [token=" + token + ", myself=" + myself + ", scopes="
+ + loginScopes + "]";
+ }
+
+ private String getScopesKey(HttpServletRequest request) {
+ String scopeRequested = request.getParameter("scope");
+ return Objects.firstNonNull(scopeRequested, "scopes");
+ }
+
+ private SortedSet<Scope> getScopes(String baseScopeKey, Scope... scopes) {
+ HashSet<Scope> fullScopes =
+ oAuthCookie == null ? new HashSet<Scope>(
+ config.scopes.get(baseScopeKey)) : new HashSet<Scope>(
+ oAuthCookie.scopes);
+ fullScopes.addAll(Arrays.asList(scopes));
+
+ return new TreeSet<Scope>(fullScopes);
}
}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthCookie.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthCookie.java
index d886b82..54fee4f 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthCookie.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthCookie.java
@@ -13,8 +13,13 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.github.oauth;
+import java.util.SortedSet;
+import java.util.TreeSet;
+
import javax.servlet.http.Cookie;
+import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope;
+
public class OAuthCookie extends Cookie {
private static final long serialVersionUID = 2771690299147135167L;
public static final String OAUTH_COOKIE_NAME = "GerritOAuth";
@@ -22,16 +27,34 @@
public final String user;
public final String email;
public final String fullName;
+ public final SortedSet<Scope> scopes;
public OAuthCookie(TokenCipher cipher, final String user, final String email,
- final String fullName) throws OAuthTokenException {
- super(OAUTH_COOKIE_NAME, cipher.encode(String.format("%s\n%s\n%s", user,
- email, fullName)));
+ final String fullName, final SortedSet<Scope> scopes) throws OAuthTokenException {
+ super(OAUTH_COOKIE_NAME, cipher.encode(getClearTextCookie(user, email, fullName, scopes)));
this.user = user;
this.email = email;
this.fullName = fullName;
+ this.scopes = scopes;
setMaxAge((int) (TokenCipher.COOKIE_TIMEOUT/1000L));
setHttpOnly(true);
+ setPath("/");
+ }
+
+ private static String getClearTextCookie(final String user,
+ final String email, final String fullName, final SortedSet<Scope> scopes) {
+ StringBuilder clearTextCookie = new StringBuilder();
+ clearTextCookie.append(user);
+ clearTextCookie.append("\n");
+ clearTextCookie.append(email);
+ clearTextCookie.append("\n");
+ clearTextCookie.append(fullName);
+
+ for (Scope scope : scopes) {
+ clearTextCookie.append("\n");
+ clearTextCookie.append(scope);
+ }
+ return clearTextCookie.toString();
}
public OAuthCookie(TokenCipher cipher, Cookie cookie)
@@ -42,5 +65,11 @@
user = clearText.length > 0 ? clearText[0]:null;
email = clearText.length > 1 ? clearText[1]:null;
fullName = clearText.length > 2 ? clearText[2]:null;
+
+ this.scopes = new TreeSet<Scope>();
+ for (int i = 3; i < clearText.length; i++) {
+ Scope scope = Enum.valueOf(Scope.class, clearText[i]);
+ this.scopes.add(scope);
+ }
}
}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthCookieProvider.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthCookieProvider.java
index 65cd93e..185002c 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthCookieProvider.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthCookieProvider.java
@@ -13,17 +13,11 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.github.oauth;
-import java.net.URLEncoder;
-import java.security.NoSuchAlgorithmException;
-import java.security.SecureRandom;
+import java.util.SortedSet;
-import javax.crypto.Cipher;
-import javax.crypto.KeyGenerator;
-import javax.crypto.SecretKey;
-import javax.crypto.spec.IvParameterSpec;
import javax.servlet.http.Cookie;
-import org.slf4j.Logger;
+import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope;
public class OAuthCookieProvider {
@@ -33,9 +27,9 @@
this.cipher = cipher;
}
- public OAuthCookie getFromUser(String username, String email, String fullName) {
+ public OAuthCookie getFromUser(String username, String email, String fullName, SortedSet<Scope> scopes) {
try {
- return new OAuthCookie(cipher, username, email, fullName);
+ return new OAuthCookie(cipher, username, email, fullName, scopes);
} catch (OAuthTokenException e) {
return null;
}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthFilter.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthFilter.java
index ba057ff..fa604db 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthFilter.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthFilter.java
@@ -89,27 +89,16 @@
OAuthCookie authCookie =
getOAuthCookie(httpRequest, (HttpServletResponse) response);
- if(OAuthProtocol.isOAuthLogout((HttpServletRequest) request)) {
+ if (OAuthProtocol.isOAuthLogout((HttpServletRequest) request)) {
logout(request, response, chain, httpRequest);
- } else if ((OAuthProtocol.isOAuthLogin(httpRequest) || OAuthProtocol.isOAuthFinal(httpRequest)) && !ghLogin.isLoggedIn()) {
+ } else if (OAuthProtocol.isOAuthRequest(httpRequest)) {
login(request, httpRequest, httpResponse, ghLogin);
} else {
- if (gerritCookie != null && !OAuthProtocol.isOAuthLogin(httpRequest)) {
- if (authCookie != null) {
- authCookie.setMaxAge(0);
- authCookie.setValue("");
- httpResponse.addCookie(authCookie);
- }
- } else if (authCookie != null) {
- httpRequest =
- new AuthenticatedHttpRequest(httpRequest, config.httpHeader,
- authCookie.user, config.httpDisplaynameHeader,
- authCookie.fullName, config.httpEmailHeader, authCookie.email);
- }
+ httpRequest = enrichAuthenticatedRequest(httpRequest, authCookie);
if (OAuthProtocol.isOAuthFinalForOthers(httpRequest)) {
- httpResponse.sendRedirect(OAuthProtocol.getTargetOAuthFinal(httpRequest));
- return;
+ httpResponse.sendRedirect(OAuthProtocol
+ .getTargetOAuthFinal(httpRequest));
} else {
chain.doFilter(httpRequest, response);
}
@@ -130,16 +119,24 @@
}
}
- private void login(ServletRequest request,
- HttpServletRequest httpRequest, HttpServletResponse httpResponse,
- GitHubLogin ghLogin) throws IOException {
- if(ghLogin.login(httpRequest, httpResponse)) {
- GHMyself myself =
- ghLogin.getMyself();
+ private HttpServletRequest enrichAuthenticatedRequest(
+ HttpServletRequest httpRequest, OAuthCookie authCookie) {
+ httpRequest =
+ authCookie == null ? httpRequest : new AuthenticatedHttpRequest(
+ httpRequest, config.httpHeader, authCookie.user,
+ config.httpDisplaynameHeader, authCookie.fullName,
+ config.httpEmailHeader, authCookie.email);
+ return httpRequest;
+ }
+
+ private void login(ServletRequest request, HttpServletRequest httpRequest,
+ HttpServletResponse httpResponse, GitHubLogin ghLogin) throws IOException {
+ if (ghLogin.login(httpRequest, httpResponse)) {
+ GHMyself myself = ghLogin.getMyself();
String user = myself.getLogin();
- updateSecureConfigWithRetry(ghLogin.hub.getMyOrganizations().keySet(), user,
- ghLogin.token.access_token);
+ updateSecureConfigWithRetry(ghLogin.hub.getMyOrganizations().keySet(),
+ user, ghLogin.token.access_token);
}
}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthProtocol.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthProtocol.java
index 5fd96b5..d0d705f 100644
--- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthProtocol.java
+++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthProtocol.java
@@ -9,6 +9,7 @@
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import javax.servlet.ServletRequest;
import javax.servlet.http.HttpServletRequest;
@@ -56,8 +57,7 @@
}
}
private static final String ME_SEPARATOR = ",";
-
- private static final Logger log = LoggerFactory
+ private static final Logger LOG = LoggerFactory
.getLogger(OAuthProtocol.class);
private final GitHubOAuthConfig config;
@@ -91,31 +91,24 @@
}
public void loginPhase1(HttpServletRequest request,
- HttpServletResponse response, Scope... scopes) throws IOException {
- String scopeRequested = request.getParameter("scope");
- String baseScopeKey = Objects.firstNonNull(scopeRequested, "scopes");
+ HttpServletResponse response, Set<Scope> scopes) throws IOException {
+
+ String scopesString = getScope(scopes);
+ LOG.debug("Initiating GitHub Login for ClientId=" + config.gitHubClientId + " Scopes=" + scopesString);
response.sendRedirect(String.format(
"%s?client_id=%s%s&redirect_uri=%s&state=%s%s", config.gitHubOAuthUrl,
- config.gitHubClientId, getScope(baseScopeKey, scopes),
+ config.gitHubClientId, scopesString,
getURLEncoded(config.oAuthFinalRedirectUrl),
me(), getURLEncoded(request.getRequestURI().toString())));
}
- private String getScope(String baseScopeKey, Scope[] scopes) {
- List<Scope> baseScopes = config.scopes.get(baseScopeKey);
- if(baseScopes == null) {
- throw new IllegalArgumentException("Requested OAuth base scope id " + baseScopeKey + " is not configured in gerrit.config");
- }
-
- HashSet<Scope> fullScopes = new HashSet<OAuthProtocol.Scope>(baseScopes);
- fullScopes.addAll(Arrays.asList(scopes));
-
- if(fullScopes.size() <= 0) {
+ private String getScope(Set<Scope> scopes) {
+ if(scopes.size() <= 0) {
return "";
}
StringBuilder out = new StringBuilder();
- for (Scope scope : fullScopes) {
+ for (Scope scope : scopes) {
if(out.length() > 0) {
out.append(",");
}
@@ -149,6 +142,10 @@
return request.getRequestURI().indexOf(GitHubOAuthConfig.OAUTH_LOGOUT) >= 0;
}
+ public static boolean isOAuthRequest(HttpServletRequest httpRequest) {
+ return OAuthProtocol.isOAuthLogin(httpRequest) || OAuthProtocol.isOAuthFinal(httpRequest);
+ }
+
public AccessToken loginPhase2(HttpServletRequest request,
HttpServletResponse response) throws IOException {
@@ -169,7 +166,7 @@
try {
HttpResponse postResponse = http.execute(post);
if (postResponse.getStatusLine().getStatusCode() != HttpURLConnection.HTTP_OK) {
- log.error("POST " + config.gitHubOAuthAccessTokenUrl
+ LOG.error("POST " + config.gitHubOAuthAccessTokenUrl
+ " request for access token failed with status "
+ postResponse.getStatusLine());
response.sendError(HttpURLConnection.HTTP_UNAUTHORIZED,
@@ -183,7 +180,7 @@
.getContent(), "UTF-8"), AccessToken.class);
return token;
} catch (IOException e) {
- log.error("POST " + config.gitHubOAuthAccessTokenUrl
+ LOG.error("POST " + config.gitHubOAuthAccessTokenUrl
+ " request for access token failed", e);
response.sendError(HttpURLConnection.HTTP_UNAUTHORIZED,
"Request for access token not authorised");
