Merge "OAuthTokenCache: Use account id as key" into stable-2.13

gerrit-oauth/src/main/java/com/google/gerrit/httpd/auth/oauth/OAuthSession.java

@@ -62,6 +62,7 @@ class OAuthSession {
   private final OAuthTokenCache tokenCache;
   private OAuthServiceProvider serviceProvider;
   private OAuthUserInfo user;
+  private Account.Id accountId;
   private String redirectToken;
   private boolean linkMode;
 
@@ -80,7 +81,7 @@ class OAuthSession {
   }
 
   boolean isLoggedIn() {
-    return tokenCache.has(user);
+    return user != null;
   }
 
   boolean isOAuthFinal(HttpServletRequest request) {
@@ -101,13 +102,10 @@ class OAuthSession {
       OAuthToken token = oauth.getAccessToken(
           new OAuthVerifier(request.getParameter("code")));
       user = oauth.getUserInfo(token);
-      if (user != null && token != null) {
-        tokenCache.put(user, token);
-      }
 
       if (isLoggedIn()) {
         log.debug("Login-SUCCESS " + this);
-        authenticateAndRedirect(request, response);
+        authenticateAndRedirect(request, response, token);
         return true;
       }
       response.sendError(SC_UNAUTHORIZED);
@@ -128,7 +126,7 @@ class OAuthSession {
   }
 
   private void authenticateAndRedirect(HttpServletRequest req,
-      HttpServletResponse rsp) throws IOException {
+      HttpServletResponse rsp, OAuthToken token) throws IOException {
     AuthRequest areq = new AuthRequest(user.getExternalId());
     AuthResult arsp;
     try {
@@ -147,6 +145,9 @@ class OAuthSession {
       areq.setEmailAddress(user.getEmailAddress());
       areq.setDisplayName(user.getDisplayName());
       arsp = accountManager.authenticate(areq);
+
+      accountId = arsp.getAccountId();
+      tokenCache.put(accountId, token);
     } catch (AccountException e) {
       log.error("Unable to authenticate user \"" + user + "\"", e);
       rsp.sendError(HttpServletResponse.SC_FORBIDDEN);
@@ -215,7 +216,10 @@ class OAuthSession {
   }
 
   void logout() {
-    tokenCache.remove(user);
+    if (accountId != null) {
+      tokenCache.remove(accountId);
+      accountId = null;
+    }
     user = null;
     redirectToken = null;
     serviceProvider = null;
@@ -247,8 +251,8 @@ class OAuthSession {
 
   @Override
   public String toString() {
-    return "OAuthSession [token=" + tokenCache.get(user) + ", user=" + user
+    return "OAuthSession [token=" + tokenCache.get(accountId) + ", user="
-        + "]";
+        + user + "]";
   }
 
   public void setServiceProvider(OAuthServiceProvider provider) {

gerrit-server/src/main/java/com/google/gerrit/server/account/GetOAuthToken.java

@@ -18,6 +18,7 @@ import com.google.gerrit.extensions.auth.oauth.OAuthToken;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.auth.oauth.OAuthTokenCache;
 import com.google.gerrit.server.config.CanonicalWebUrl;
@@ -52,16 +53,13 @@ class GetOAuthToken implements RestReadView<AccountResource>{
     if (self.get() != rsrc.getUser()) {
       throw new AuthException("not allowed to get access token");
     }
-    String username = rsrc.getUser().getAccount().getUserName();
+    Account a = rsrc.getUser().getAccount();
-    if (username == null) {
+    OAuthToken accessToken = tokenCache.get(a.getId());
-      throw new ResourceNotFoundException();
-    }
-    OAuthToken accessToken = tokenCache.get(username);
     if (accessToken == null) {
       throw new ResourceNotFoundException();
     }
     OAuthTokenInfo accessTokenInfo = new OAuthTokenInfo();
-    accessTokenInfo.username = username;
+    accessTokenInfo.username = a.getUserName();
     accessTokenInfo.resourceHost = hostName;
     accessTokenInfo.accessToken = accessToken.getToken();
     accessTokenInfo.providerId = accessToken.getProviderId();

gerrit-server/src/main/java/com/google/gerrit/server/auth/oauth/OAuthTokenCache.java

@@ -19,8 +19,8 @@ import static com.google.common.base.Preconditions.checkNotNull;
 import com.google.common.cache.Cache;
 import com.google.gerrit.extensions.auth.oauth.OAuthToken;
 import com.google.gerrit.extensions.auth.oauth.OAuthTokenEncrypter;
-import com.google.gerrit.extensions.auth.oauth.OAuthUserInfo;
 import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.cache.CacheModule;
 import com.google.inject.Inject;
 import com.google.inject.Module;
@@ -37,54 +37,39 @@ public class OAuthTokenCache {
     return new CacheModule() {
       @Override
       protected void configure() {
-        persist(OAUTH_TOKENS, String.class, OAuthToken.class);
+        persist(OAUTH_TOKENS, Account.Id.class, OAuthToken.class);
       }
     };
   }
 
-  private final Cache<String, OAuthToken> cache;
+  private final Cache<Account.Id, OAuthToken> cache;
 
   @Inject
-  OAuthTokenCache(@Named(OAUTH_TOKENS) Cache<String, OAuthToken> cache,
+  OAuthTokenCache(@Named(OAUTH_TOKENS) Cache<Account.Id, OAuthToken> cache,
       DynamicItem<OAuthTokenEncrypter> encrypter) {
     this.cache = cache;
     this.encrypter = encrypter;
   }
 
-  public boolean has(OAuthUserInfo user) {
+  public OAuthToken get(Account.Id id) {
-    return user != null
+    OAuthToken accessToken = cache.getIfPresent(id);
-      ? cache.getIfPresent(user.getUserName()) != null
-      : false;
-  }
-
-  public OAuthToken get(OAuthUserInfo user) {
-    return user != null
-      ? get(user.getUserName())
-      : null;
-  }
-
-  public OAuthToken get(String userName) {
-    OAuthToken accessToken = cache.getIfPresent(userName);
     if (accessToken == null) {
       return null;
     }
     accessToken = decrypt(accessToken);
     if (accessToken.isExpired()) {
-      cache.invalidate(userName);
+      cache.invalidate(id);
       return null;
     }
     return accessToken;
   }
 
-  public void put(OAuthUserInfo user, OAuthToken accessToken) {
+  public void put(Account.Id id, OAuthToken accessToken) {
-    cache.put(checkNotNull(user.getUserName()),
+    cache.put(id, encrypt(checkNotNull(accessToken)));
-        encrypt(checkNotNull(accessToken)));
   }
 
-  public void remove(OAuthUserInfo user) {
+  public void remove(Account.Id id) {
-    if (user != null) {
+    cache.invalidate(id);
-      cache.invalidate(user.getUserName());
-    }
   }
 
   private OAuthToken encrypt(OAuthToken token) {