Refactor OAuth handshake: remove usage of GerritOAuth cookie The GerritOAuth cookie is now obsolete as we do rely on a shared GHLogin object in the HTTP Session. Whenever the session expires or is lost, the situation is detected and automatically recovered by triggering a new OAuth handshake with GitHub. The session GHLogin object is used as well to remember the user scopes and username for HTTP Header enrichment. Change-Id: Ibede5c16995f6659c8401c6d5083d44f1185e0ba
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 69f94da..6fb6fbe 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
@@ -14,6 +14,8 @@ package com.googlesource.gerrit.plugins.github.oauth; +import static java.util.concurrent.TimeUnit.DAYS; + import java.io.IOException; import java.util.Arrays; import java.util.HashSet; @@ -34,26 +36,24 @@ 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; import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.AccessToken; import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope; -import static java.util.concurrent.TimeUnit.*; public class GitHubLogin { private static final Logger LOG = LoggerFactory.getLogger(GitHubLogin.class); - private static final List<Scope> DEFAULT_SCOPES = Arrays.asList(Scope.PUBLIC_REPO, Scope.USER_EMAIL); + private static final List<Scope> DEFAULT_SCOPES = Arrays.asList( + Scope.PUBLIC_REPO, Scope.USER_EMAIL); private static final int YEARS = 365; - private static final long SCOPE_COOKIE_NEVER_EXPIRES = DAYS.toSeconds(50 * YEARS); + private static final long SCOPE_COOKIE_NEVER_EXPIRES = DAYS + .toSeconds(50 * YEARS); @Singleton public static class Provider extends HttpSessionProvider<GitHubLogin> { @Override public GitHubLogin get(HttpServletRequest request) { - GitHubLogin login = super.get(request); - login.initOAuthCookie(request); - return login; + return super.get(request); } } @@ -64,9 +64,7 @@ private GHMyself myself; private SortedSet<Scope> loginScopes; - private final OAuthCookieProvider cookieProvider; private final GitHubOAuthConfig config; - private OAuthCookie oAuthCookie; public GHMyself getMyself() { if (isLoggedIn()) { @@ -79,7 +77,6 @@ @Inject public GitHubLogin(final OAuthProtocol oauth, final GitHubOAuthConfig config) { this.oauth = oauth; - this.cookieProvider = new OAuthCookieProvider(TokenCipher.get(), config); this.config = config; } @@ -97,25 +94,6 @@ 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, @@ -135,16 +113,6 @@ login(oauth.loginPhase2(request, response)); if (isLoggedIn()) { LOG.debug("Login-SUCCESS " + this); - String user = myself.getLogin(); - String email = myself.getEmail(); - String fullName = - Strings.emptyToNull(myself.getName()) == null ? user : myself - .getName(); - - OAuthCookie userCookie = - cookieProvider.getFromUser(user, email, fullName, loginScopes); - response.addCookie(userCookie); - response.sendRedirect(OAuthProtocol.getTargetUrl(request)); return true; } else { @@ -181,13 +149,14 @@ + loginScopes + "]"; } - private String getScopesKey(HttpServletRequest request, HttpServletResponse response) { + private String getScopesKey(HttpServletRequest request, + HttpServletResponse response) { String scopeRequested = request.getParameter("scope"); - if(scopeRequested == null) { + if (scopeRequested == null) { scopeRequested = getScopesKeyFromCookie(request); } - if(scopeRequested != null) { + if (scopeRequested != null) { Cookie scopeCookie = new Cookie("scope", scopeRequested); scopeCookie.setPath("/"); scopeCookie.setMaxAge((int) SCOPE_COOKIE_NEVER_EXPIRES); @@ -199,12 +168,12 @@ private String getScopesKeyFromCookie(HttpServletRequest request) { Cookie[] cookies = request.getCookies(); - if(cookies == null) { + if (cookies == null) { return null; } - for(Cookie cookie : cookies) { - if(cookie.getName().equalsIgnoreCase("scope")) { + for (Cookie cookie : cookies) { + if (cookie.getName().equalsIgnoreCase("scope")) { return cookie.getValue(); } } @@ -212,16 +181,14 @@ } private SortedSet<Scope> getScopes(String baseScopeKey, Scope... scopes) { - HashSet<Scope> fullScopes = - oAuthCookie == null ? new HashSet<Scope>( - scopesForKey(baseScopeKey)) : new HashSet<Scope>( - oAuthCookie.scopes); + HashSet<Scope> fullScopes = new HashSet<Scope>(scopesForKey(baseScopeKey)); fullScopes.addAll(Arrays.asList(scopes)); return new TreeSet<Scope>(fullScopes); } private List<Scope> scopesForKey(String baseScopeKey) { - return Objects.firstNonNull(config.scopes.get(baseScopeKey), DEFAULT_SCOPES); + return Objects + .firstNonNull(config.scopes.get(baseScopeKey), DEFAULT_SCOPES); } }
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 deleted file mode 100644 index 9df3190..0000000 --- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthCookie.java +++ /dev/null
@@ -1,84 +0,0 @@ -// Copyright (C) 2013 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.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"; - public static final OAuthCookie ANONYMOUS = new OAuthCookie(); - - 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, final SortedSet<Scope> scopes, final long maxAgeSeconds) 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) maxAgeSeconds); - 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(); - } - - private OAuthCookie() { - super(OAUTH_COOKIE_NAME, ""); - this.user = ""; - this.scopes = null; - this.fullName = ""; - this.email = ""; - } - - public OAuthCookie(TokenCipher cipher, Cookie cookie) - throws OAuthTokenException { - super(OAUTH_COOKIE_NAME, cookie.getValue()); - String clearTextValue = cipher.decode(cookie.getValue()); - String[] clearText = clearTextValue.split("\n"); - 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 deleted file mode 100644 index 293abef..0000000 --- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthCookieProvider.java +++ /dev/null
@@ -1,53 +0,0 @@ -// Copyright (C) 2013 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.oauth; - -import static java.util.concurrent.TimeUnit.SECONDS; - -import java.util.SortedSet; - -import javax.servlet.http.Cookie; - -import com.google.gerrit.server.config.ConfigUtil; -import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope; - -public class OAuthCookieProvider { - private static final String CACHE_NAME = "web_sessions"; - - private TokenCipher cipher; - private GitHubOAuthConfig config; - - - public OAuthCookieProvider(TokenCipher cipher, GitHubOAuthConfig config) { - this.cipher = cipher; - this.config = config; - } - - public OAuthCookie getFromUser(String username, String email, String fullName, SortedSet<Scope> scopes) { - try { - return new OAuthCookie(cipher, username, email, fullName, scopes, getGerritSessionMaxAgeMillis()); - } catch (OAuthTokenException e) { - return null; - } - } - - public OAuthCookie getFromCookie(Cookie cookie) throws OAuthTokenException { - return new OAuthCookie(cipher, cookie); - } - - private long getGerritSessionMaxAgeMillis() { - return ConfigUtil.getTimeUnit(config.gerritConfig, "cache", CACHE_NAME, - "maxAge", TokenCipher.MAX_COOKIE_TIMEOUT_SECS, SECONDS); - } -}
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthGitFilter.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthGitFilter.java index 26793e2..326ad23 100644 --- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthGitFilter.java +++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/OAuthGitFilter.java
@@ -24,7 +24,6 @@ import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; -import java.util.TreeSet; import java.util.concurrent.ExecutionException; import javax.servlet.Filter; @@ -55,7 +54,6 @@ import com.google.inject.Inject; import com.google.inject.Singleton; import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.AccessToken; -import com.googlesource.gerrit.plugins.github.oauth.OAuthProtocol.Scope; @Singleton public class OAuthGitFilter implements Filter { @@ -71,8 +69,8 @@ private final AccountCache accountCache; private final GitHubHttpProvider httpClientProvider; private final GitHubOAuthConfig config; - private final OAuthCookieProvider cookieProvider; private final XGerritAuth xGerritAuth; + private ScopedProvider<GitHubLogin> ghLoginProvider; public static class BasicAuthHttpRequest extends HttpServletRequestWrapper { private HashMap<String, String> headers = new HashMap<String, String>(); @@ -122,12 +120,12 @@ this.accountCache = accountCache; this.httpClientProvider = httpClientProvider; this.config = config; - this.cookieProvider = new OAuthCookieProvider(TokenCipher.get(), config); this.xGerritAuth = xGerritAuth; } @Override public void init(FilterConfig filterConfig) throws ServletException { + this.ghLoginProvider = new GitHubLogin.Provider(); } @Override @@ -137,32 +135,29 @@ HttpServletRequest httpRequest = (HttpServletRequest) request; HttpServletResponse httpResponse = new OAuthGitWrappedResponse((HttpServletResponse) response); - log.debug("OAuthGitFilter(" + httpRequest.getRequestURL() + ") code=" - + request.getParameter("code")); + GitHubLogin ghLogin = ghLoginProvider.get(httpRequest); + log.debug("OAuthGitFilter(" + httpRequest.getRequestURL() + ") ghLogin=" + + ghLogin); - OAuthCookie oAuthCookie = - getAuthenticationCookieFromGitRequestUsingOAuthToken(httpRequest, + String username = + getAuthenticatedUserFromGitRequestUsingOAuthToken(httpRequest, httpResponse); - if (oAuthCookie == null) { + if (username == null) { return; } String gerritPassword = - oAuthCookie == OAuthCookie.ANONYMOUS ? null : accountCache - .getByUsername(oAuthCookie.user).getPassword(oAuthCookie.user); + accountCache.getByUsername(username).getPassword(username); - if (gerritPassword == null && oAuthCookie != OAuthCookie.ANONYMOUS) { + if (gerritPassword == null) { gerritPassword = - generateRandomGerritPassword(oAuthCookie, httpRequest, httpResponse, + generateRandomGerritPassword(username, httpRequest, httpResponse, chain); httpResponse.sendRedirect(getRequestPathWithQueryString(httpRequest)); return; } - if (oAuthCookie != OAuthCookie.ANONYMOUS) { - httpRequest = - new BasicAuthHttpRequest(httpRequest, oAuthCookie.user, - gerritPassword); - } + httpRequest = + new BasicAuthHttpRequest(httpRequest, username, gerritPassword); chain.doFilter(httpRequest, httpResponse); } @@ -175,21 +170,20 @@ return requestPathWithQueryString; } - private String generateRandomGerritPassword(OAuthCookie oAuthCookie, + private String generateRandomGerritPassword(String username, HttpServletRequest httpRequest, HttpServletResponse httpResponse, FilterChain chain) throws IOException, ServletException { - log.warn("User " + oAuthCookie.user + " has not a Gerrit HTTP password: " + log.warn("User " + username + " has not a Gerrit HTTP password: " + "generating a random one in order to be able to use Git over HTTP"); Cookie gerritCookie = - getGerritLoginCookie(oAuthCookie.user, httpRequest, httpResponse, chain); + getGerritLoginCookie(username, httpRequest, httpResponse, chain); String xGerritAuthValue = xGerritAuth.getAuthValue(gerritCookie); HttpPut putRequest = new HttpPut(getRequestUrlWithAlternatePath(httpRequest, "/accounts/self/password.http")); putRequest.setHeader("Cookie", - gerritCookie.getName() + "=" + gerritCookie.getValue() + "; " - + oAuthCookie.getName() + "=" + oAuthCookie.getValue()); + gerritCookie.getName() + "=" + gerritCookie.getValue()); putRequest.setHeader(XGerritAuth.X_GERRIT_AUTH, xGerritAuthValue); putRequest.setEntity(new StringEntity("{\"generate\":true}", @@ -197,12 +191,10 @@ HttpResponse putResponse = httpClientProvider.get().execute(putRequest); if (putResponse.getStatusLine().getStatusCode() != HttpStatus.SC_OK) { throw new ServletException( - "Cannot generate HTTP password for authenticating user " - + oAuthCookie.user); + "Cannot generate HTTP password for authenticating user " + username); } - return accountCache.getByUsername(oAuthCookie.user).getPassword( - oAuthCookie.user); + return accountCache.getByUsername(username).getPassword(username); } private URI getRequestUrlWithAlternatePath(HttpServletRequest httpRequest, @@ -235,11 +227,11 @@ return loginResponse.getGerritCookie(); } - private OAuthCookie getAuthenticationCookieFromGitRequestUsingOAuthToken( + private String getAuthenticatedUserFromGitRequestUsingOAuthToken( HttpServletRequest req, HttpServletResponse rsp) throws IOException { final String httpBasicAuth = getHttpBasicAuthenticationHeader(req); if (httpBasicAuth == null) { - return OAuthCookie.ANONYMOUS; + return null; } if (isInvalidHttpAuthenticationHeader(httpBasicAuth)) { @@ -256,7 +248,7 @@ } if (!oauthKeyword.equalsIgnoreCase(GITHUB_X_OAUTH_BASIC)) { - return OAuthCookie.ANONYMOUS; + return null; } boolean loginSuccessful = false; @@ -275,7 +267,7 @@ return null; } - return cookieProvider.getFromUser(oauthLogin, "", "", new TreeSet<Scope>()); + return oauthLogin; }
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/TokenCipher.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/TokenCipher.java deleted file mode 100644 index 042a301..0000000 --- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/TokenCipher.java +++ /dev/null
@@ -1,127 +0,0 @@ -// Copyright (C) 2013 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.oauth; - -import static java.util.concurrent.TimeUnit.HOURS; - -import java.net.URLDecoder; -import java.net.URLEncoder; -import java.security.NoSuchAlgorithmException; -import java.security.SecureRandom; - -import javax.crypto.Cipher; -import javax.crypto.KeyGenerator; -import javax.crypto.SecretKey; -import javax.crypto.spec.IvParameterSpec; - -import org.slf4j.Logger; - -import com.google.inject.Singleton; - -@Singleton -public class TokenCipher { - private static final String UTF8 = "UTF-8"; - private static final String ENC_ALGO_PADDING = "AES/CBC/PKCS5Padding"; - private static final String JCE_PROVIDER = "SunJCE"; - private static final String ENC_ALGO = "AES"; - private static final Logger log = org.slf4j.LoggerFactory - .getLogger(OAuthCookieProvider.class); - public static final Long MAX_COOKIE_TIMEOUT_SECS = HOURS.toSeconds(12); - - private SecretKey aesKey; - private byte[] IV; - private SecureRandom sessionRnd = new SecureRandom(); - - private static TokenCipher singleton = new TokenCipher(); - - private TokenCipher() { - KeyGenerator kgen; - try { - kgen = KeyGenerator.getInstance(ENC_ALGO); - kgen.init(128); - SecureRandom sr = new SecureRandom(); - sr.setSeed(System.currentTimeMillis()); - byte[] key = new byte[16]; - IV = new byte[16]; - sr.nextBytes(key); - sr.nextBytes(IV); - aesKey = kgen.generateKey(); - sessionRnd.setSeed(System.currentTimeMillis()); - } catch (NoSuchAlgorithmException e) { - log.error("Cannot find encryption algorithm " + ENC_ALGO); - throw new IllegalArgumentException(e); - } - } - - public static TokenCipher get() { - return singleton; - } - - public String encode(String clearText) throws OAuthTokenException { - try { - long sessionId = sessionRnd.nextLong(); - long ts = System.currentTimeMillis(); - String userSession = - String.format("%d/%d/%s", sessionId, ts, - URLEncoder.encode(clearText, UTF8)); - byte[] plainText = - (userSession + "/" + userSession.hashCode()).getBytes(UTF8); - - Cipher cipher = Cipher.getInstance(ENC_ALGO_PADDING, JCE_PROVIDER); - cipher.init(Cipher.ENCRYPT_MODE, aesKey, new IvParameterSpec(IV)); - byte[] enc = cipher.doFinal(plainText); - return org.eclipse.jgit.util.Base64.encodeBytes(enc).trim(); - } catch (Exception e) { - log.error("Encryption failed", e); - throw new OAuthTokenException("Cannot generate token for " - + clearText, e); - } - } - - public String decode(String sessionToken) throws OAuthTokenException { - try { - byte[] enc = - org.eclipse.jgit.util.Base64.decode(sessionToken.trim().getBytes(), - 0, sessionToken.length()); - Cipher cipher = Cipher.getInstance(ENC_ALGO_PADDING, JCE_PROVIDER); - cipher.init(Cipher.DECRYPT_MODE, aesKey, new IvParameterSpec(IV)); - - String[] clearTextParts = - new String(cipher.doFinal(enc), UTF8).split("/"); - - isValid(sessionToken, clearTextParts); - - return URLDecoder.decode(clearTextParts[2], UTF8); - } catch (Exception e) { - log.error("Decryption failed", e); - throw new OAuthTokenException("Invalid session token " + sessionToken, e); - } - } - - private void isValid(String sessionToken, String[] clearTextParts) - throws OAuthTokenException { - int hashCode = Integer.parseInt(clearTextParts[3]); - if (hashCode != (clearTextParts[0] + "/" + clearTextParts[1] + "/" + clearTextParts[2]) - .hashCode()) { - throw new OAuthTokenException("Invalid or forged token " + sessionToken); - } - - long ts = Long.parseLong(clearTextParts[1]); - if (((System.currentTimeMillis() - ts) / 1000) > MAX_COOKIE_TIMEOUT_SECS) { - throw new OAuthTokenException("Session token " + sessionToken - + " has expired"); - } - } -}