You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by sh...@apache.org on 2026/08/18 12:49:13 UTC
(cloudstack) 14/20: oauth2: improve google provider flow
This is an automated email from the ASF dual-hosted git repository.
shwstppr pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit b17edb538e0f54c864e4c029b138ccf1fa9d984f
Author: Abhishek Kumar <ab...@gmail.com>
AuthorDate: Thu Jun 18 15:51:56 2026 +0530
oauth2: improve google provider flow
Improves Google OAuth provider.
Signed-off-by: Abhishek Kumar <ab...@gmail.com>
---
.../oauth2/google/GoogleOAuth2Provider.java | 121 +++++++++-----
.../oauth2/google/GoogleOAuth2ProviderTest.java | 177 ++++++++++++---------
2 files changed, 177 insertions(+), 121 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 42ed1451ccd..e383b41999e 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
@@ -16,36 +16,66 @@
//under the License.
package org.apache.cloudstack.oauth2.google;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.auth.UserOAuth2Authenticator;
+import org.apache.cloudstack.oauth2.dao.OauthProviderDao;
+import org.apache.cloudstack.oauth2.vo.OauthProviderVO;
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang3.StringUtils;
+
import com.cloud.exception.CloudAuthenticationException;
import com.cloud.utils.component.AdapterBase;
import com.cloud.utils.exception.CloudRuntimeException;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.google.api.client.auth.oauth2.BearerToken;
+import com.google.api.client.auth.oauth2.ClientParametersAuthentication;
+import com.google.api.client.auth.oauth2.Credential;
import com.google.api.client.googleapis.auth.oauth2.GoogleAuthorizationCodeFlow;
import com.google.api.client.googleapis.auth.oauth2.GoogleClientSecrets;
-import com.google.api.client.googleapis.auth.oauth2.GoogleCredential;
import com.google.api.client.googleapis.auth.oauth2.GoogleTokenResponse;
import com.google.api.client.http.javanet.NetHttpTransport;
import com.google.api.client.json.JsonFactory;
import com.google.api.client.json.jackson2.JacksonFactory;
import com.google.api.services.oauth2.Oauth2;
import com.google.api.services.oauth2.model.Userinfo;
-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;
public class GoogleOAuth2Provider extends AdapterBase implements UserOAuth2Authenticator {
- protected String accessToken = null;
- protected String refreshToken = null;
-
@Inject
OauthProviderDao _oauthProviderDao;
+ private final Cache<String, String> validatedEmailCache =
+ Caffeine.newBuilder()
+ .expireAfterWrite(60, TimeUnit.SECONDS)
+ .maximumSize(1024)
+ .build();
+
+ private String getCacheKey(final String secretCode) {
+ return DigestUtils.sha256Hex(secretCode);
+ }
+
+ private void addValidatedEmailToCache(final String secretCode, final String email) {
+ validatedEmailCache.put(getCacheKey(secretCode), email);
+ }
+
+ private String consumeValidatedEmailFromCache(final String secretCode) {
+ final String key = getCacheKey(secretCode);
+ final String email = validatedEmailCache.getIfPresent(key);
+
+ if (email != null) {
+ validatedEmailCache.invalidate(key);
+ }
+
+ return email;
+ }
+
@Override
public String getName() {
return "google";
@@ -67,21 +97,24 @@ public class GoogleOAuth2Provider extends AdapterBase implements UserOAuth2Authe
throw new CloudAuthenticationException("Google provider is not registered, so user cannot be verified");
}
- String verifiedEmail = verifyCodeAndFetchEmail(secretCode);
- if (verifiedEmail == null || !email.equals(verifiedEmail)) {
+ String verifiedEmail = consumeValidatedEmailFromCache(secretCode);
+ if (StringUtils.isEmpty(verifiedEmail)) {
+ verifiedEmail = verifyCodeAndFetchEmailInternal(secretCode, providerVO);
+ }
+ if (!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();
+ protected String verifyCodeAndFetchEmailInternal(String secretCode, OauthProviderVO googleProvider) {
+ if (googleProvider == null) {
+ 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)
@@ -90,46 +123,48 @@ public class GoogleOAuth2Provider extends AdapterBase implements UserOAuth2Authe
NetHttpTransport httpTransport = new NetHttpTransport();
JsonFactory jsonFactory = new JacksonFactory();
List<String> scopes = Arrays.asList(
- "https://www.googleapis.com/auth/userinfo.profile",
- "https://www.googleapis.com/auth/userinfo.email");
+ "https://www.googleapis.com/auth/userinfo.profile",
+ "https://www.googleapis.com/auth/userinfo.email");
GoogleAuthorizationCodeFlow flow = new GoogleAuthorizationCodeFlow.Builder(
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;
+ try {
+ tokenResponse = flow.newTokenRequest(secretCode)
+ .setRedirectUri(redirectURI)
+ .execute();
+ } catch (IOException e) {
+ throw new CloudRuntimeException("Failed to verify secret code", e);
}
- GoogleCredential credential = new GoogleCredential.Builder()
+ String accessToken = tokenResponse.getAccessToken();
+ String refreshToken = tokenResponse.getRefreshToken();
+
+ Credential credential = new Credential.Builder(BearerToken.authorizationHeaderAccessMethod())
.setTransport(httpTransport)
.setJsonFactory(jsonFactory)
- .setClientSecrets(clientSecrets)
+ .setTokenServerEncodedUrl("https://oauth2.googleapis.com/token")
+ .setClientAuthentication(new ClientParametersAuthentication(clientId, secret))
.build()
.setAccessToken(accessToken)
.setRefreshToken(refreshToken);
Oauth2 oauth2 = new Oauth2.Builder(httpTransport, jsonFactory, credential).build();
- Userinfo userinfo = null;
+ Userinfo userinfo;
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()));
}
- return userinfo.getEmail();
+ String verifiedEmail = userinfo.getEmail();
+ addValidatedEmailToCache(secretCode, verifiedEmail);
+ return verifiedEmail;
}
- protected void clearAccessAndRefreshTokens() {
- accessToken = null;
- refreshToken = null;
+ @Override
+ public String verifyCodeAndFetchEmail(String secretCode) {
+ return verifyCodeAndFetchEmailInternal(secretCode, null);
}
@Override
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 fa8a5a7c03c..a6c628182b4 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
@@ -17,10 +17,15 @@
package org.apache.cloudstack.oauth2.google;
-import com.cloud.exception.CloudAuthenticationException;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.google.api.services.oauth2.Oauth2;
-import com.google.api.services.oauth2.model.Userinfo;
+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 org.apache.cloudstack.oauth2.dao.OauthProviderDao;
import org.apache.cloudstack.oauth2.vo.OauthProviderVO;
import org.junit.After;
@@ -28,18 +33,11 @@ import org.junit.Before;
import org.junit.Test;
import org.mockito.InjectMocks;
import org.mockito.Mock;
-import org.mockito.MockedConstruction;
-import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.mockito.Spy;
-import java.io.IOException;
-
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import com.cloud.exception.CloudAuthenticationException;
+import com.cloud.utils.exception.CloudRuntimeException;
public class GoogleOAuth2ProviderTest {
@@ -52,9 +50,15 @@ public class GoogleOAuth2ProviderTest {
private AutoCloseable closeable;
+ @Mock
+ private OauthProviderVO mockProvider;
+
@Before
public void setUp() {
closeable = MockitoAnnotations.openMocks(this);
+ when(mockProvider.getClientId()).thenReturn("test_client_id");
+ when(mockProvider.getSecretKey()).thenReturn("test_secret_key");
+ when(mockProvider.getRedirectUri()).thenReturn("http://localhost/redirect");
}
@After
@@ -79,78 +83,95 @@ public class GoogleOAuth2ProviderTest {
}
@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";
- Oauth2 oauth2 = mock(Oauth2.class);
- try (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");
- }
+ public void testVerifyUserWithInvalidSecretCode() {
+ when(_oauthProviderDao.findByProvider(anyString())).thenReturn(mockProvider);
+ doReturn(null).when(_googleOAuth2Provider).verifyCodeAndFetchEmailInternal(
+ "secretCode", mockProvider);
+
+ _googleOAuth2Provider.verifyUser("email@example.com", "secretCode");
}
@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";
- Oauth2 oauth2 = mock(Oauth2.class);
- try (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");
+ 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);
+ }
+
+ @Test
+ public void testCacheInitializationAndCleanupResources() {
+ // Verifies that cache cleanup executor is properly initialized
+ GoogleOAuth2Provider provider = new GoogleOAuth2Provider();
+ assertNotNull("GoogleOAuth2Provider should initialize", provider);
+ }
+
+ @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"));
+ }
+ }
+
+ @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());
}
}
@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";
- Oauth2 oauth2 = mock(Oauth2.class);
- try (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);
- assertNull(_googleOAuth2Provider.accessToken);
- assertNull(_googleOAuth2Provider.refreshToken);
+ 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"));
+ }
}
}
}