Skip to content

plugin/oauth2: fix thread-safety of GoogleOAuth2Provider token handling #13564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rhenar0 wants to merge 8 commits into
base: 4.22
Choose a base branch
from
+139 −49
Open

plugin/oauth2: fix thread-safety of GoogleOAuth2Provider token handling #13564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f16b3d7
plugin/oauth2: fix thread-safety of GoogleOAuth2Provider token handling
rhenar0 Jul 7, 2026
c83e076
Update plugins/user-authenticators/oauth2/src/main/java/org/apache/cl…
rhenar0 Jul 8, 2026
2237bfc
Update plugins/user-authenticators/oauth2/src/main/java/org/apache/cl…
rhenar0 Jul 8, 2026
eace76f
plugin/oauth2: fix misapplied review suggestion
rhenar0 Jul 9, 2026
2ee36f3
plugin/oauth2: cache OAuth2 tokens per authorization code
rhenar0 Jul 9, 2026
f258420
Remove redundant comments from GoogleOAuth2Provider
rhenar0 Jul 9, 2026
18750ba
Merge branch '4.22' into fix/oauth2-google-thread-safety-4.22
rhenar0 Jul 21, 2026
a1769cb
Merge branch '4.22' into fix/oauth2-google-thread-safety-4.22
rhenar0 Jul 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 26 additions & 22 deletions ...cators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.cloudstack.oauth2.google;

import com.cloud.exception.CloudAuthenticationException;
import com.cloud.utils.Pair;
import com.cloud.utils.component.AdapterBase;
import com.cloud.utils.exception.CloudRuntimeException;
import com.google.api.client.googleapis.auth.oauth2.GoogleAuthorizationCodeFlow;
Expand All @@ -28,6 +29,9 @@
import com.google.api.client.json.jackson2.JacksonFactory;
import com.google.api.services.oauth2.Oauth2;
import com.google.api.services.oauth2.model.Userinfo;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.util.concurrent.UncheckedExecutionException;
import org.apache.cloudstack.auth.UserOAuth2Authenticator;
import org.apache.cloudstack.oauth2.dao.OauthProviderDao;
import org.apache.cloudstack.oauth2.vo.OauthProviderVO;
Expand All @@ -37,11 +41,15 @@
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;

public class GoogleOAuth2Provider extends AdapterBase implements UserOAuth2Authenticator {

protected String accessToken = null;
protected String refreshToken = null;
protected final Cache<String, Pair<String, String>> tokensByCode = CacheBuilder.newBuilder()
.maximumSize(1000)
.expireAfterWrite(10, TimeUnit.MINUTES)
.build();

@Inject
OauthProviderDao _oauthProviderDao;
Expand Down Expand Up @@ -71,17 +79,16 @@ public boolean verifyUser(String email, String secretCode) {
if (verifiedEmail == null || !email.equals(verifiedEmail)) {
throw new CloudRuntimeException("Unable to verify the email address with the provided secret");
}
clearAccessAndRefreshTokens();

return true;
}

@Override
public String verifyCodeAndFetchEmail(String secretCode) {
OauthProviderVO githubProvider = _oauthProviderDao.findByProvider(getName());
String clientId = githubProvider.getClientId();
String secret = githubProvider.getSecretKey();
String redirectURI = githubProvider.getRedirectUri();
OauthProviderVO googleProvider = _oauthProviderDao.findByProvider(getName());
String clientId = googleProvider.getClientId();
String secret = googleProvider.getSecretKey();
String redirectURI = googleProvider.getRedirectUri();
GoogleClientSecrets clientSecrets = new GoogleClientSecrets()
.setWeb(new GoogleClientSecrets.Details()
.setClientId(clientId)
Expand All @@ -96,18 +103,20 @@ public String verifyCodeAndFetchEmail(String secretCode) {
httpTransport, jsonFactory, clientSecrets, scopes)
.build();

if (StringUtils.isAnyEmpty(accessToken, refreshToken)) {
GoogleTokenResponse tokenResponse = null;
try {
tokenResponse = flow.newTokenRequest(secretCode)
Pair<String, String> tokens;
try {
tokens = tokensByCode.get(secretCode, () -> {
GoogleTokenResponse tokenResponse = flow.newTokenRequest(secretCode)
.setRedirectUri(redirectURI)
.execute();
} catch (IOException e) {
throw new RuntimeException(e);
}
accessToken = tokenResponse.getAccessToken();
refreshToken = tokenResponse.getRefreshToken();
return new Pair<>(tokenResponse.getAccessToken(), tokenResponse.getRefreshToken());
});
} catch (ExecutionException | UncheckedExecutionException e) {
Throwable cause = e.getCause() != null ? e.getCause() : e;
throw new CloudRuntimeException(String.format("Failed to exchange the OAuth2 authorization code for tokens: %s", cause.getMessage()), cause);
}
String accessToken = tokens.first();
String refreshToken = tokens.second();

GoogleCredential credential = new GoogleCredential.Builder()
.setTransport(httpTransport)
Expand All @@ -122,16 +131,11 @@ public String verifyCodeAndFetchEmail(String secretCode) {
try {
userinfo = oauth2.userinfo().get().execute();
} catch (IOException e) {
throw new CloudRuntimeException(String.format("Failed to fetch the email address with the provided secret: %s" + e.getMessage()));
throw new CloudRuntimeException(String.format("Failed to fetch the email address with the provided secret: %s", e.getMessage()), e);
}
return userinfo.getEmail();
}

protected void clearAccessAndRefreshTokens() {
accessToken = null;
refreshToken = null;
}

@Override
public String getUserEmailAddress() throws CloudRuntimeException {
return null;
Expand Down
140 changes: 113 additions & 27 deletions ...rs/oauth2/src/test/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2ProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

import com.cloud.exception.CloudAuthenticationException;
import com.cloud.utils.exception.CloudRuntimeException;
import com.google.api.client.googleapis.auth.oauth2.GoogleAuthorizationCodeFlow;
import com.google.api.client.googleapis.auth.oauth2.GoogleAuthorizationCodeTokenRequest;
import com.google.api.client.googleapis.auth.oauth2.GoogleTokenResponse;
import com.google.api.services.oauth2.Oauth2;
import com.google.api.services.oauth2.model.Userinfo;
import org.apache.cloudstack.oauth2.dao.OauthProviderDao;
Expand All @@ -35,10 +38,13 @@

import java.io.IOException;

import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

public class GoogleOAuth2ProviderTest {
Expand All @@ -62,6 +68,26 @@ public void tearDown() throws Exception {
closeable.close();
}

private OauthProviderVO mockRegisteredProvider() {
OauthProviderVO providerVO = mock(OauthProviderVO.class);
when(_oauthProviderDao.findByProvider(anyString())).thenReturn(providerVO);
when(providerVO.getProvider()).thenReturn("testProvider");
when(providerVO.getSecretKey()).thenReturn("testSecret");
when(providerVO.getClientId()).thenReturn("testClientid");
return providerVO;
}

private GoogleAuthorizationCodeFlow mockTokenExchangeFlow(GoogleAuthorizationCodeTokenRequest tokenRequest) throws IOException {
GoogleAuthorizationCodeFlow flow = mock(GoogleAuthorizationCodeFlow.class);
GoogleTokenResponse tokenResponse = mock(GoogleTokenResponse.class);
when(flow.newTokenRequest(anyString())).thenReturn(tokenRequest);
when(tokenRequest.setRedirectUri(any())).thenReturn(tokenRequest);
when(tokenRequest.execute()).thenReturn(tokenResponse);
when(tokenResponse.getAccessToken()).thenReturn("testAccessToken");
when(tokenResponse.getRefreshToken()).thenReturn("testRefreshToken");
return flow;
}

@Test(expected = CloudAuthenticationException.class)
public void testVerifyUserWithNullEmail() {
_googleOAuth2Provider.verifyUser(null, "secretCode");
Expand All @@ -80,15 +106,13 @@ public void testVerifyUserWithUnregisteredProvider() {

@Test(expected = CloudRuntimeException.class)
public void testVerifyUserWithInvalidSecretCode() throws IOException {
OauthProviderVO providerVO = mock(OauthProviderVO.class);
when(_oauthProviderDao.findByProvider(anyString())).thenReturn(providerVO);
when(providerVO.getProvider()).thenReturn("testProvider");
when(providerVO.getSecretKey()).thenReturn("testSecret");
when(providerVO.getClientId()).thenReturn("testClientid");
_googleOAuth2Provider.accessToken = "testAccessToken";
_googleOAuth2Provider.refreshToken = "testRefreshToken";
mockRegisteredProvider();
GoogleAuthorizationCodeTokenRequest tokenRequest = mock(GoogleAuthorizationCodeTokenRequest.class);
GoogleAuthorizationCodeFlow flow = mockTokenExchangeFlow(tokenRequest);
Oauth2 oauth2 = mock(Oauth2.class);
try (MockedConstruction<Oauth2.Builder> ignored = Mockito.mockConstruction(Oauth2.Builder.class,
try (MockedConstruction<GoogleAuthorizationCodeFlow.Builder> ignoredFlow = Mockito.mockConstruction(GoogleAuthorizationCodeFlow.Builder.class,
(mock, context) -> when(mock.build()).thenReturn(flow));
MockedConstruction<Oauth2.Builder> ignored = Mockito.mockConstruction(Oauth2.Builder.class,
(mock, context) -> when(mock.build()).thenReturn(oauth2))) {
Userinfo userinfo = mock(Userinfo.class);
Oauth2.Userinfo userinfo1 = mock(Oauth2.Userinfo.class);
Expand All @@ -104,15 +128,13 @@ public void testVerifyUserWithInvalidSecretCode() throws IOException {

@Test(expected = CloudRuntimeException.class)
public void testVerifyUserWithMismatchedEmail() throws IOException {
OauthProviderVO providerVO = mock(OauthProviderVO.class);
when(_oauthProviderDao.findByProvider(anyString())).thenReturn(providerVO);
when(providerVO.getProvider()).thenReturn("testProvider");
when(providerVO.getSecretKey()).thenReturn("testSecret");
when(providerVO.getClientId()).thenReturn("testClientid");
_googleOAuth2Provider.accessToken = "testAccessToken";
_googleOAuth2Provider.refreshToken = "testRefreshToken";
mockRegisteredProvider();
GoogleAuthorizationCodeTokenRequest tokenRequest = mock(GoogleAuthorizationCodeTokenRequest.class);
GoogleAuthorizationCodeFlow flow = mockTokenExchangeFlow(tokenRequest);
Oauth2 oauth2 = mock(Oauth2.class);
try (MockedConstruction<Oauth2.Builder> ignored = Mockito.mockConstruction(Oauth2.Builder.class,
try (MockedConstruction<GoogleAuthorizationCodeFlow.Builder> ignoredFlow = Mockito.mockConstruction(GoogleAuthorizationCodeFlow.Builder.class,
(mock, context) -> when(mock.build()).thenReturn(flow));
MockedConstruction<Oauth2.Builder> ignored = Mockito.mockConstruction(Oauth2.Builder.class,
(mock, context) -> when(mock.build()).thenReturn(oauth2))) {
Userinfo userinfo = mock(Userinfo.class);
Oauth2.Userinfo userinfo1 = mock(Oauth2.Userinfo.class);
Expand All @@ -126,17 +148,29 @@ public void testVerifyUserWithMismatchedEmail() throws IOException {
}
}

@Test(expected = CloudRuntimeException.class)
public void testVerifyUserWithFailedTokenExchange() throws IOException {
mockRegisteredProvider();
GoogleAuthorizationCodeFlow flow = mock(GoogleAuthorizationCodeFlow.class);
GoogleAuthorizationCodeTokenRequest tokenRequest = mock(GoogleAuthorizationCodeTokenRequest.class);
when(flow.newTokenRequest(anyString())).thenReturn(tokenRequest);
when(tokenRequest.setRedirectUri(any())).thenReturn(tokenRequest);
when(tokenRequest.execute()).thenThrow(new IOException("invalid_grant"));
try (MockedConstruction<GoogleAuthorizationCodeFlow.Builder> ignoredFlow = Mockito.mockConstruction(GoogleAuthorizationCodeFlow.Builder.class,
(mock, context) -> when(mock.build()).thenReturn(flow))) {
_googleOAuth2Provider.verifyUser("email@example.com", "secretCode");
}
}

@Test
public void testVerifyUserEmail() throws IOException {
OauthProviderVO providerVO = mock(OauthProviderVO.class);
when(_oauthProviderDao.findByProvider(anyString())).thenReturn(providerVO);
when(providerVO.getProvider()).thenReturn("testProvider");
when(providerVO.getSecretKey()).thenReturn("testSecret");
when(providerVO.getClientId()).thenReturn("testClientid");
_googleOAuth2Provider.accessToken = "testAccessToken";
_googleOAuth2Provider.refreshToken = "testRefreshToken";
mockRegisteredProvider();
GoogleAuthorizationCodeTokenRequest tokenRequest = mock(GoogleAuthorizationCodeTokenRequest.class);
GoogleAuthorizationCodeFlow flow = mockTokenExchangeFlow(tokenRequest);
Oauth2 oauth2 = mock(Oauth2.class);
try (MockedConstruction<Oauth2.Builder> ignored = Mockito.mockConstruction(Oauth2.Builder.class,
try (MockedConstruction<GoogleAuthorizationCodeFlow.Builder> ignoredFlow = Mockito.mockConstruction(GoogleAuthorizationCodeFlow.Builder.class,
(mock, context) -> when(mock.build()).thenReturn(flow));
MockedConstruction<Oauth2.Builder> ignored = Mockito.mockConstruction(Oauth2.Builder.class,
(mock, context) -> when(mock.build()).thenReturn(oauth2))) {
Userinfo userinfo = mock(Userinfo.class);
Oauth2.Userinfo userinfo1 = mock(Oauth2.Userinfo.class);
Expand All @@ -149,8 +183,60 @@ public void testVerifyUserEmail() throws IOException {
boolean result = _googleOAuth2Provider.verifyUser("email@example.com", "secretCode");

assertTrue(result);
assertNull(_googleOAuth2Provider.accessToken);
assertNull(_googleOAuth2Provider.refreshToken);
verify(tokenRequest, times(1)).execute();
}
}

@Test
public void testVerifyCodeAndFetchEmailExchangesEachCodeIndependently() throws IOException {
mockRegisteredProvider();
GoogleAuthorizationCodeTokenRequest tokenRequest = mock(GoogleAuthorizationCodeTokenRequest.class);
GoogleAuthorizationCodeFlow flow = mockTokenExchangeFlow(tokenRequest);
Oauth2 oauth2 = mock(Oauth2.class);
try (MockedConstruction<GoogleAuthorizationCodeFlow.Builder> ignoredFlow = Mockito.mockConstruction(GoogleAuthorizationCodeFlow.Builder.class,
(mock, context) -> when(mock.build()).thenReturn(flow));
MockedConstruction<Oauth2.Builder> ignored = Mockito.mockConstruction(Oauth2.Builder.class,
(mock, context) -> when(mock.build()).thenReturn(oauth2))) {
Userinfo userinfo = mock(Userinfo.class);
Oauth2.Userinfo userinfo1 = mock(Oauth2.Userinfo.class);
when(oauth2.userinfo()).thenReturn(userinfo1);
Oauth2.Userinfo.Get userinfoGet = mock(Oauth2.Userinfo.Get.class);
when(userinfo1.get()).thenReturn(userinfoGet);
when(userinfoGet.execute()).thenReturn(userinfo);
when(userinfo.getEmail()).thenReturn("email@example.com");

assertEquals("email@example.com", _googleOAuth2Provider.verifyCodeAndFetchEmail("secretCode1"));
assertEquals("email@example.com", _googleOAuth2Provider.verifyCodeAndFetchEmail("secretCode2"));

verify(flow, times(1)).newTokenRequest("secretCode1");
verify(flow, times(1)).newTokenRequest("secretCode2");
verify(tokenRequest, times(2)).execute();
}
}

@Test
public void testVerifyCodeAndFetchEmailReusesTokensForSameCode() throws IOException {
mockRegisteredProvider();
GoogleAuthorizationCodeTokenRequest tokenRequest = mock(GoogleAuthorizationCodeTokenRequest.class);
GoogleAuthorizationCodeFlow flow = mockTokenExchangeFlow(tokenRequest);
Oauth2 oauth2 = mock(Oauth2.class);
try (MockedConstruction<GoogleAuthorizationCodeFlow.Builder> ignoredFlow = Mockito.mockConstruction(GoogleAuthorizationCodeFlow.Builder.class,
(mock, context) -> when(mock.build()).thenReturn(flow));
MockedConstruction<Oauth2.Builder> ignored = Mockito.mockConstruction(Oauth2.Builder.class,
(mock, context) -> when(mock.build()).thenReturn(oauth2))) {
Userinfo userinfo = mock(Userinfo.class);
Oauth2.Userinfo userinfo1 = mock(Oauth2.Userinfo.class);
when(oauth2.userinfo()).thenReturn(userinfo1);
Oauth2.Userinfo.Get userinfoGet = mock(Oauth2.Userinfo.Get.class);
when(userinfo1.get()).thenReturn(userinfoGet);
when(userinfoGet.execute()).thenReturn(userinfo);
when(userinfo.getEmail()).thenReturn("email@example.com");

// the login flow uses the same one-time code twice: verifyOauthCodeAndGetUser then oauthlogin
assertEquals("email@example.com", _googleOAuth2Provider.verifyCodeAndFetchEmail("secretCode"));
assertTrue(_googleOAuth2Provider.verifyUser("email@example.com", "secretCode"));

verify(tokenRequest, times(1)).execute();
}
}
}
Loading