Damans227 commented on code in PR #13564:
URL: https://github.com/apache/cloudstack/pull/13564#discussion_r4273992301
##########
plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java:
##########
@@ -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();
Review Comment:
does this build? the merge with 4.22 seems to have left the old token lines
here, and verifyUser still calls the method that was removed. the last package
run failed too
##########
plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/google/GoogleOAuth2Provider.java:
##########
@@ -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()
Review Comment:
do we still need this token cache? 4.22 already keeps the checked email for
each code for a minute, which covers the same code being used twice
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
