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 3 commits into
base: 4.22
Choose a base branch
from
+101 −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
3 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
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
36 changes: 14 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 @@ -40,9 +40,6 @@

public class GoogleOAuth2Provider extends AdapterBase implements UserOAuth2Authenticator {

protected String accessToken = null;
protected String refreshToken = null;

@Inject
OauthProviderDao _oauthProviderDao;

Expand Down Expand Up @@ -71,14 +68,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());
OauthProviderVO googleProvider = _oauthProviderDao.findByProvider(getName());
String clientId = googleProvider.getClientId();
String secret = googleProvider.getSecretKey();
String redirectURI = googleProvider.getRedirectUri();
String clientId = githubProvider.getClientId();
String secret = githubProvider.getSecretKey();
String redirectURI = githubProvider.getRedirectUri();
Expand All @@ -96,18 +95,16 @@ public String verifyCodeAndFetchEmail(String secretCode) {
httpTransport, jsonFactory, clientSecrets, scopes)
.build();

if (StringUtils.isAnyEmpty(accessToken, refreshToken)) {
GoogleTokenResponse tokenResponse = null;
try {
tokenResponse = flow.newTokenRequest(secretCode)
.setRedirectUri(redirectURI)
.execute();
} catch (IOException e) {
throw new RuntimeException(e);
}
accessToken = tokenResponse.getAccessToken();
refreshToken = tokenResponse.getRefreshToken();
GoogleTokenResponse tokenResponse = null;
try {
tokenResponse = flow.newTokenRequest(secretCode)
.setRedirectUri(redirectURI)
.execute();
} catch (IOException e) {
throw new CloudRuntimeException(String.format("Failed to exchange the OAuth2 authorization code for tokens: %s", e.getMessage()), e);
}
String accessToken = tokenResponse.getAccessToken();
String refreshToken = tokenResponse.getRefreshToken();

GoogleCredential credential = new GoogleCredential.Builder()
.setTransport(httpTransport)
Expand All @@ -122,16 +119,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
114 changes: 87 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,34 @@ 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 testVerifyCodeAndFetchEmailExchangesCodeOnEveryCall() 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();
}
}
}