Skip to content

Commit 2ee36f3

Browse files
committed
plugin/oauth2: cache OAuth2 tokens per authorization code
The CloudStack OAuth2 login flow exchanges the same one-time authorization code twice: once in verifyOauthCodeAndGetUser to fetch the user's email, and once in the subsequent oauthlogin call that authenticates the user. Unconditionally exchanging the code broke the login flow, as Google rejects the second exchange of a one-time code with invalid_grant. Cache the exchanged tokens keyed by the authorization code instead (bounded size, short TTL): the second phase of a login reuses the tokens obtained for its own code, while distinct logins never share state, keeping the provider thread-safe. Failed exchanges are not cached, so a failure cannot poison subsequent logins. Tested on a 4.22.0.0 environment: concurrent Google logins succeed and a failed login no longer breaks subsequent ones.
1 parent eace76f commit 2ee36f3

2 files changed

Lines changed: 53 additions & 9 deletions

File tree

plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.apache.cloudstack.oauth2.google;
1818

1919
import com.cloud.exception.CloudAuthenticationException;
20+
import com.cloud.utils.Pair;
2021
import com.cloud.utils.component.AdapterBase;
2122
import com.cloud.utils.exception.CloudRuntimeException;
2223
import com.google.api.client.googleapis.auth.oauth2.GoogleAuthorizationCodeFlow;
@@ -28,6 +29,9 @@
2829
import com.google.api.client.json.jackson2.JacksonFactory;
2930
import com.google.api.services.oauth2.Oauth2;
3031
import com.google.api.services.oauth2.model.Userinfo;
32+
import com.google.common.cache.Cache;
33+
import com.google.common.cache.CacheBuilder;
34+
import com.google.common.util.concurrent.UncheckedExecutionException;
3135
import org.apache.cloudstack.auth.UserOAuth2Authenticator;
3236
import org.apache.cloudstack.oauth2.dao.OauthProviderDao;
3337
import org.apache.cloudstack.oauth2.vo.OauthProviderVO;
@@ -37,9 +41,19 @@
3741
import java.io.IOException;
3842
import java.util.Arrays;
3943
import java.util.List;
44+
import java.util.concurrent.ExecutionException;
45+
import java.util.concurrent.TimeUnit;
4046

4147
public class GoogleOAuth2Provider extends AdapterBase implements UserOAuth2Authenticator {
4248

49+
// The login flow exchanges the same one-time authorization code twice (once in
50+
// verifyOauthCodeAndGetUser, once in the subsequent oauthlogin), so the tokens are
51+
// cached per code; entries are short-lived as codes are only valid for a few minutes.
52+
protected final Cache<String, Pair<String, String>> tokensByCode = CacheBuilder.newBuilder()
53+
.maximumSize(1000)
54+
.expireAfterWrite(10, TimeUnit.MINUTES)
55+
.build();
56+
4357
@Inject
4458
OauthProviderDao _oauthProviderDao;
4559

@@ -92,16 +106,20 @@ public String verifyCodeAndFetchEmail(String secretCode) {
92106
httpTransport, jsonFactory, clientSecrets, scopes)
93107
.build();
94108

95-
GoogleTokenResponse tokenResponse = null;
109+
Pair<String, String> tokens;
96110
try {
97-
tokenResponse = flow.newTokenRequest(secretCode)
98-
.setRedirectUri(redirectURI)
99-
.execute();
100-
} catch (IOException e) {
101-
throw new CloudRuntimeException(String.format("Failed to exchange the OAuth2 authorization code for tokens: %s", e.getMessage()), e);
111+
tokens = tokensByCode.get(secretCode, () -> {
112+
GoogleTokenResponse tokenResponse = flow.newTokenRequest(secretCode)
113+
.setRedirectUri(redirectURI)
114+
.execute();
115+
return new Pair<>(tokenResponse.getAccessToken(), tokenResponse.getRefreshToken());
116+
});
117+
} catch (ExecutionException | UncheckedExecutionException e) {
118+
Throwable cause = e.getCause() != null ? e.getCause() : e;
119+
throw new CloudRuntimeException(String.format("Failed to exchange the OAuth2 authorization code for tokens: %s", cause.getMessage()), cause);
102120
}
103-
String accessToken = tokenResponse.getAccessToken();
104-
String refreshToken = tokenResponse.getRefreshToken();
121+
String accessToken = tokens.first();
122+
String refreshToken = tokens.second();
105123

106124
GoogleCredential credential = new GoogleCredential.Builder()
107125
.setTransport(httpTransport)

plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2ProviderTest.java

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ public void testVerifyUserEmail() throws IOException {
188188
}
189189

190190
@Test
191-
public void testVerifyCodeAndFetchEmailExchangesCodeOnEveryCall() throws IOException {
191+
public void testVerifyCodeAndFetchEmailExchangesEachCodeIndependently() throws IOException {
192192
mockRegisteredProvider();
193193
GoogleAuthorizationCodeTokenRequest tokenRequest = mock(GoogleAuthorizationCodeTokenRequest.class);
194194
GoogleAuthorizationCodeFlow flow = mockTokenExchangeFlow(tokenRequest);
@@ -213,4 +213,30 @@ public void testVerifyCodeAndFetchEmailExchangesCodeOnEveryCall() throws IOExcep
213213
verify(tokenRequest, times(2)).execute();
214214
}
215215
}
216+
217+
@Test
218+
public void testVerifyCodeAndFetchEmailReusesTokensForSameCode() throws IOException {
219+
mockRegisteredProvider();
220+
GoogleAuthorizationCodeTokenRequest tokenRequest = mock(GoogleAuthorizationCodeTokenRequest.class);
221+
GoogleAuthorizationCodeFlow flow = mockTokenExchangeFlow(tokenRequest);
222+
Oauth2 oauth2 = mock(Oauth2.class);
223+
try (MockedConstruction<GoogleAuthorizationCodeFlow.Builder> ignoredFlow = Mockito.mockConstruction(GoogleAuthorizationCodeFlow.Builder.class,
224+
(mock, context) -> when(mock.build()).thenReturn(flow));
225+
MockedConstruction<Oauth2.Builder> ignored = Mockito.mockConstruction(Oauth2.Builder.class,
226+
(mock, context) -> when(mock.build()).thenReturn(oauth2))) {
227+
Userinfo userinfo = mock(Userinfo.class);
228+
Oauth2.Userinfo userinfo1 = mock(Oauth2.Userinfo.class);
229+
when(oauth2.userinfo()).thenReturn(userinfo1);
230+
Oauth2.Userinfo.Get userinfoGet = mock(Oauth2.Userinfo.Get.class);
231+
when(userinfo1.get()).thenReturn(userinfoGet);
232+
when(userinfoGet.execute()).thenReturn(userinfo);
233+
when(userinfo.getEmail()).thenReturn("email@example.com");
234+
235+
// the login flow uses the same one-time code twice: verifyOauthCodeAndGetUser then oauthlogin
236+
assertEquals("email@example.com", _googleOAuth2Provider.verifyCodeAndFetchEmail("secretCode"));
237+
assertTrue(_googleOAuth2Provider.verifyUser("email@example.com", "secretCode"));
238+
239+
verify(tokenRequest, times(1)).execute();
240+
}
241+
}
216242
}

0 commit comments

Comments
 (0)