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 10 commits into
base: 4.22
Choose a base branch
from
+189 −100
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
10 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
54a5cdd
Merge branch '4.22' into fix/oauth2-google-thread-safety-4.22
rhenar0 Sep 11, 2026
8b8d750
Merge branch '4.22' into fix/oauth2-google-thread-safety-4.22
rhenar0 Sep 18, 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
52 changes: 36 additions & 16 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 @@ -30,6 +30,7 @@
import org.apache.commons.lang3.StringUtils;

import com.cloud.exception.CloudAuthenticationException;
import com.cloud.utils.Pair;
import com.cloud.utils.component.AdapterBase;
import com.cloud.utils.exception.CloudRuntimeException;
import com.github.benmanes.caffeine.cache.Cache;
Expand All @@ -45,9 +46,28 @@
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;
import org.apache.commons.lang3.StringUtils;

import javax.inject.Inject;
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 final Cache<String, Pair<String, String>> tokensByCode = CacheBuilder.newBuilder()
.maximumSize(1000)
.expireAfterWrite(10, TimeUnit.MINUTES)
.build();

@Inject
OauthProviderDao _oauthProviderDao;

Expand Down Expand Up @@ -108,10 +128,9 @@ public boolean verifyUser(String email, String secretCode) {
return true;
}

protected String verifyCodeAndFetchEmailInternal(String secretCode, OauthProviderVO googleProvider) {
if (googleProvider == null) {
googleProvider = _oauthProviderDao.findByProvider(getName());
}
@Override
public String verifyCodeAndFetchEmail(String secretCode) {
OauthProviderVO googleProvider = _oauthProviderDao.findByProvider(getName());
String clientId = googleProvider.getClientId();
String secret = googleProvider.getSecretKey();
String redirectURI = googleProvider.getRedirectUri();
Expand All @@ -129,14 +148,20 @@ protected String verifyCodeAndFetchEmailInternal(String secretCode, OauthProvide
httpTransport, jsonFactory, clientSecrets, scopes)
.build();

GoogleTokenResponse tokenResponse;
Pair<String, String> tokens;
try {
tokenResponse = flow.newTokenRequest(secretCode)
.setRedirectUri(redirectURI)
.execute();
} catch (IOException e) {
throw new CloudRuntimeException("Failed to verify secret code", e);
tokens = tokensByCode.get(secretCode, () -> {
GoogleTokenResponse tokenResponse = flow.newTokenRequest(secretCode)
.setRedirectUri(redirectURI)
.execute();
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();

String accessToken = tokenResponse.getAccessToken();
String refreshToken = tokenResponse.getRefreshToken();
Expand All @@ -155,18 +180,13 @@ protected String verifyCodeAndFetchEmailInternal(String secretCode, OauthProvide
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);
}
String verifiedEmail = userinfo.getEmail();
addValidatedEmailToCache(secretCode, verifiedEmail);
return verifiedEmail;
}

@Override
public String verifyCodeAndFetchEmail(String secretCode) {
return verifyCodeAndFetchEmailInternal(secretCode, null);
}

@Override
public String getUserEmailAddress() throws CloudRuntimeException {
return null;
Expand Down
237 changes: 153 additions & 84 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 @@ -17,15 +17,13 @@

package org.apache.cloudstack.oauth2.google;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.when;

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;
import org.apache.cloudstack.oauth2.vo.OauthProviderVO;
import org.junit.After;
Expand All @@ -36,8 +34,16 @@
import org.mockito.MockitoAnnotations;
import org.mockito.Spy;

import com.cloud.exception.CloudAuthenticationException;
import com.cloud.utils.exception.CloudRuntimeException;
import java.io.IOException;

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 Down Expand Up @@ -66,6 +72,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 @@ -83,95 +109,138 @@ public void testVerifyUserWithUnregisteredProvider() {
}

@Test(expected = CloudRuntimeException.class)
public void testVerifyUserWithInvalidSecretCode() {
when(_oauthProviderDao.findByProvider(anyString())).thenReturn(mockProvider);
doReturn(null).when(_googleOAuth2Provider).verifyCodeAndFetchEmailInternal(
"secretCode", mockProvider);

_googleOAuth2Provider.verifyUser("email@example.com", "secretCode");
public void testVerifyUserWithInvalidSecretCode() 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(null);

_googleOAuth2Provider.verifyUser("email@example.com", "secretCode");
}
}

@Test(expected = CloudRuntimeException.class)
public void testVerifyUserWithMismatchedEmail() {
when(_oauthProviderDao.findByProvider(anyString())).thenReturn(mockProvider);
doReturn("otheremail@example.com").when(_googleOAuth2Provider).verifyCodeAndFetchEmailInternal(
"secretCode", mockProvider);

_googleOAuth2Provider.verifyUser("email@example.com", "secretCode");
}

@Test
public void testVerifyUserEmail() {
when(_oauthProviderDao.findByProvider(anyString())).thenReturn(mockProvider);
doReturn("email@example.com").when(_googleOAuth2Provider).verifyCodeAndFetchEmailInternal(
"secretCode", mockProvider);

boolean result = _googleOAuth2Provider.verifyUser("email@example.com", "secretCode");

assertTrue(result);
public void testVerifyUserWithMismatchedEmail() 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("otheremail@example.com");

_googleOAuth2Provider.verifyUser("email@example.com", "secretCode");
}
}

@Test
public void testCacheInitializationAndCleanupResources() {
// Verifies that cache cleanup executor is properly initialized
GoogleOAuth2Provider provider = new GoogleOAuth2Provider();
assertNotNull("GoogleOAuth2Provider should initialize", provider);
@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 testNoSensitiveDataInErrorMessages() {
// Verifies that error messages don't expose sensitive information
when(_oauthProviderDao.findByProvider(anyString())).thenReturn(mockProvider);
String testSecret = "secret_key";
String testEmail = "email@example.com";

try {
_googleOAuth2Provider.verifyUser(testEmail, testSecret);
fail("Expected exception");
} catch (Exception e) {
String errorMsg = e.getMessage();
// Verify sensitive terms are not in error messages
assertFalse("Error should not contain secret", errorMsg.toLowerCase().contains(testSecret));
assertFalse("Error should not contain email", errorMsg.toLowerCase().contains(testEmail));
assertFalse("Error should not contain token", errorMsg.toLowerCase().contains("access_token"));
public void testVerifyUserEmail() 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");

boolean result = _googleOAuth2Provider.verifyUser("email@example.com", "secretCode");

assertTrue(result);
verify(tokenRequest, times(1)).execute();
}
}

@Test
public void testVerifyUserErrorHandlingAndCleanup() {
// Tests that any authentication error properly cleans up
when(_oauthProviderDao.findByProvider(anyString())).thenReturn(mockProvider);

doReturn("error@example.com").when(_googleOAuth2Provider).verifyCodeAndFetchEmailInternal(
"bad_code", mockProvider);

try {
_googleOAuth2Provider.verifyUser("expected@example.com", "bad_code");
fail("Should throw exception for email mismatch");
} catch (CloudRuntimeException e) {
assertEquals("Should have proper error message",
"Unable to verify the email address with the provided secret",
e.getMessage());
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 testMultipleFailedVerificationAttempts() {
// Tests that multiple failures are handled gracefully without cache pollution
when(_oauthProviderDao.findByProvider(anyString())).thenReturn(mockProvider);

// Multiple failed attempts
for (int i = 0; i < 5; i++) {
try {
doReturn("wrong@example.com").when(_googleOAuth2Provider)
.verifyCodeAndFetchEmailInternal("code_" + i, mockProvider);
_googleOAuth2Provider.verifyUser("correct@example.com", "code_" + i);
fail("Should fail on attempt " + i);
} catch (CloudRuntimeException e) {
// Expected - cache should be cleaned for each failure
assertTrue("Should report email verification failure",
e.getMessage().contains("email"));
}
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();
}
}
}