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..7d4927b9cf27 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 @@ -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; @@ -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; @@ -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> tokensByCode = CacheBuilder.newBuilder() + .maximumSize(1000) + .expireAfterWrite(10, TimeUnit.MINUTES) + .build(); @Inject OauthProviderDao _oauthProviderDao; @@ -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) @@ -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 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) @@ -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; 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..72e52fbe2863 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,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 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(); + } + } + + @Test + public void testVerifyCodeAndFetchEmailReusesTokensForSameCode() 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"); + + // 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(); } } }