From f16b3d704b10ffa3c695de918e78c66e24d26233 Mon Sep 17 00:00:00 2001 From: Hope | Revan - Hugo CHASSAING Date: Tue, 7 Jul 2026 18:24:11 +0200 Subject: [PATCH 1/3] plugin/oauth2: fix thread-safety of GoogleOAuth2Provider token handling GoogleOAuth2Provider is a Spring singleton but stored the OAuth2 access and refresh tokens in shared instance fields. Under concurrent logins, the StringUtils.isAnyEmpty() guard made a second login reuse the first login's tokens instead of exchanging its own authorization code, so Google returned the first user's email and verification failed with "Unable to verify the email address with the provided secret". Additionally, clearAccessAndRefreshTokens() was only invoked on the success path of verifyUser(), so any failure left stale tokens in the singleton and every subsequent login failed until a management server restart. Fix by removing the instance fields entirely: always exchange the authorization code in verifyCodeAndFetchEmail() and keep the tokens in local variables. Also surface token exchange failures as a CloudRuntimeException with a meaningful message instead of a bare RuntimeException. Co-Authored-By: Claude Fable 5 --- .../oauth2/google/GoogleOAuth2Provider.java | 29 ++--- .../google/GoogleOAuth2ProviderTest.java | 114 +++++++++++++----- 2 files changed, 96 insertions(+), 47 deletions(-) diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java index 42ed1451ccd5..9501a0ddb9a5 100644 --- a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java +++ b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java @@ -40,9 +40,6 @@ public class GoogleOAuth2Provider extends AdapterBase implements UserOAuth2Authenticator { - protected String accessToken = null; - protected String refreshToken = null; - @Inject OauthProviderDao _oauthProviderDao; @@ -71,7 +68,6 @@ 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; } @@ -96,18 +92,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) @@ -127,11 +121,6 @@ public String verifyCodeAndFetchEmail(String secretCode) { return userinfo.getEmail(); } - protected void clearAccessAndRefreshTokens() { - accessToken = null; - refreshToken = null; - } - @Override public String getUserEmailAddress() throws CloudRuntimeException { return null; diff --git a/plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2ProviderTest.java b/plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2ProviderTest.java index fa8a5a7c03cf..5ab28d4bc76c 100644 --- a/plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2ProviderTest.java +++ b/plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2ProviderTest.java @@ -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; @@ -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 { @@ -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"); @@ -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 ignored = Mockito.mockConstruction(Oauth2.Builder.class, + try (MockedConstruction ignoredFlow = Mockito.mockConstruction(GoogleAuthorizationCodeFlow.Builder.class, + (mock, context) -> when(mock.build()).thenReturn(flow)); + MockedConstruction 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); @@ -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 ignored = Mockito.mockConstruction(Oauth2.Builder.class, + try (MockedConstruction ignoredFlow = Mockito.mockConstruction(GoogleAuthorizationCodeFlow.Builder.class, + (mock, context) -> when(mock.build()).thenReturn(flow)); + MockedConstruction 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); @@ -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 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 ignored = Mockito.mockConstruction(Oauth2.Builder.class, + try (MockedConstruction ignoredFlow = Mockito.mockConstruction(GoogleAuthorizationCodeFlow.Builder.class, + (mock, context) -> when(mock.build()).thenReturn(flow)); + MockedConstruction 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); @@ -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 ignoredFlow = Mockito.mockConstruction(GoogleAuthorizationCodeFlow.Builder.class, + (mock, context) -> when(mock.build()).thenReturn(flow)); + MockedConstruction 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(); } } } From c83e076ac94900638d772709691182b34cce6c15 Mon Sep 17 00:00:00 2001 From: Hope | Revan - Hugo CHASSAING Date: Wed, 8 Jul 2026 13:05:14 +0200 Subject: [PATCH 2/3] Update plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java Co-authored-by: prrssshhhh --- .../cloudstack/oauth2/google/GoogleOAuth2Provider.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java index 9501a0ddb9a5..69a7f39f51ef 100644 --- a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java +++ b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java @@ -74,7 +74,10 @@ public boolean verifyUser(String email, String secretCode) { @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(); From 2237bfc479b81a817b0ba303d06e64916304acbd Mon Sep 17 00:00:00 2001 From: Hope | Revan - Hugo CHASSAING Date: Wed, 8 Jul 2026 13:05:21 +0200 Subject: [PATCH 3/3] Update plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java Co-authored-by: prrssshhhh --- .../apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java index 69a7f39f51ef..c0e402d0a922 100644 --- a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java +++ b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java @@ -119,7 +119,7 @@ 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(); }