From 031de81c649a54a35ace1abe2866ddbb794f7fa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Denis?= Date: Mon, 16 Apr 2018 17:57:25 +0200 Subject: [PATCH] Add retry logic in GoogleAuth.getTokenInfoRemote to handle 5xx errors --- .../spi/auth/EndpointsAuthenticator.java | 3 +- .../auth/GoogleAppEngineAuthenticator.java | 7 +-- .../api/server/spi/auth/GoogleAuth.java | 51 +++++++++++++++++-- .../spi/auth/GoogleOAuth2Authenticator.java | 5 +- .../spi/auth/EndpointsAuthenticatorTest.java | 9 ++-- .../GoogleAppEngineAuthenticatorTest.java | 21 ++++---- .../api/server/spi/auth/GoogleAuthTest.java | 23 ++++++++- .../auth/GoogleOAuth2AuthenticatorTest.java | 17 ++++--- 8 files changed, 101 insertions(+), 35 deletions(-) diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/auth/EndpointsAuthenticator.java b/endpoints-framework/src/main/java/com/google/api/server/spi/auth/EndpointsAuthenticator.java index bdb59c44..ecdb7f7e 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/auth/EndpointsAuthenticator.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/auth/EndpointsAuthenticator.java @@ -20,6 +20,7 @@ import com.google.api.server.spi.config.Authenticator; import com.google.api.server.spi.config.Singleton; import com.google.api.server.spi.request.Attribute; +import com.google.api.server.spi.response.ServiceUnavailableException; import com.google.common.annotations.VisibleForTesting; import javax.servlet.http.HttpServletRequest; @@ -51,7 +52,7 @@ public EndpointsAuthenticator(GoogleJwtAuthenticator jwtAuthenticator, } @Override - public User authenticate(HttpServletRequest request) { + public User authenticate(HttpServletRequest request) throws ServiceUnavailableException { Attribute attr = Attribute.from(request); User user = jwtAuthenticator.authenticate(request); if (user == null) { diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/auth/GoogleAppEngineAuthenticator.java b/endpoints-framework/src/main/java/com/google/api/server/spi/auth/GoogleAppEngineAuthenticator.java index 601134d7..57053d3e 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/auth/GoogleAppEngineAuthenticator.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/auth/GoogleAppEngineAuthenticator.java @@ -22,6 +22,7 @@ import com.google.api.server.spi.config.model.ApiMethodConfig; import com.google.api.server.spi.config.scope.AuthScopeExpression; import com.google.api.server.spi.request.Attribute; +import com.google.api.server.spi.response.ServiceUnavailableException; import com.google.appengine.api.oauth.OAuthRequestException; import com.google.appengine.api.oauth.OAuthService; import com.google.appengine.api.oauth.OAuthServiceFactory; @@ -56,7 +57,7 @@ public GoogleAppEngineAuthenticator(OAuthService oauthService, UserService userS } @VisibleForTesting - String getOAuth2ClientIdDev(String token) { + String getOAuth2ClientIdDev(String token) throws ServiceUnavailableException { GoogleAuth.TokenInfo tokenInfo = GoogleAuth.getTokenInfoRemote(token); return tokenInfo != null ? tokenInfo.clientId : null; } @@ -68,7 +69,7 @@ boolean shouldTryCookieAuth(ApiMethodConfig config) { @VisibleForTesting com.google.appengine.api.users.User getOAuth2User(HttpServletRequest request, - ApiMethodConfig config) { + ApiMethodConfig config) throws ServiceUnavailableException { String token = GoogleAuth.getAuthToken(request); if (!GoogleAuth.isOAuth2Token(token)) { return null; @@ -114,7 +115,7 @@ com.google.appengine.api.users.User getOAuth2User(HttpServletRequest request, } @Override - public User authenticate(HttpServletRequest request) { + public User authenticate(HttpServletRequest request) throws ServiceUnavailableException { Attribute attr = Attribute.from(request); if (!EnvUtil.isRunningOnAppEngine()) { return null; diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/auth/GoogleAuth.java b/endpoints-framework/src/main/java/com/google/api/server/spi/auth/GoogleAuth.java index fd38bf81..9db726b0 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/auth/GoogleAuth.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/auth/GoogleAuth.java @@ -16,12 +16,16 @@ package com.google.api.server.spi.auth; import com.google.api.client.http.GenericUrl; +import com.google.api.client.http.HttpIOExceptionHandler; import com.google.api.client.http.HttpRequest; +import com.google.api.client.http.HttpResponse; +import com.google.api.client.http.HttpUnsuccessfulResponseHandler; import com.google.api.client.util.Key; import com.google.api.server.spi.Client; import com.google.api.server.spi.Constant; import com.google.api.server.spi.Strings; import com.google.api.server.spi.request.Attribute; +import com.google.api.server.spi.response.ServiceUnavailableException; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; @@ -172,29 +176,66 @@ public static class TokenInfo { @Key("issued_to") public String clientId; @Key("scope") public String scopes; @Key("user_id") public String userId; + @Key("error_description") public String errorDescription; } /** * Get OAuth2 token info from remote token validation API. + * Retries IOExceptions and 5xx responses once. */ - static TokenInfo getTokenInfoRemote(String token) { + static TokenInfo getTokenInfoRemote(String token) throws ServiceUnavailableException { try { HttpRequest request = Client.getInstance().getJsonHttpRequestFactory() .buildGetRequest(new GenericUrl(TOKEN_INFO_ENDPOINT + token)); + configureErrorHandling(request); return parseTokenInfo(request); } catch (IOException e) { - logger.log(Level.WARNING, "Failed to retrieve tokeninfo", e); - return null; + throw new ServiceUnavailableException("Failed to perform access token validation", e); } } @VisibleForTesting - static TokenInfo parseTokenInfo(HttpRequest request) throws IOException { - TokenInfo info = request.execute().parseAs(TokenInfo.class); + static TokenInfo parseTokenInfo(HttpRequest request) + throws IOException, ServiceUnavailableException { + HttpResponse response = request.execute(); + int statusCode = response.getStatusCode(); + TokenInfo info = response.parseAs(TokenInfo.class); + if (statusCode != 200) { + String errorDescription = "Unknown error"; + if (info != null && info.errorDescription != null) { + errorDescription = info.errorDescription; + } + errorDescription += " (" + statusCode + ")"; + if (statusCode >= 500) { + logger.log(Level.SEVERE, "Error validating access token: " + errorDescription); + throw new ServiceUnavailableException("Failed to validate access token"); + } + logger.log(Level.INFO, "Invalid access token: " + errorDescription); + return null; + } if (info == null || Strings.isEmptyOrWhitespace(info.email)) { logger.log(Level.WARNING, "Access token does not contain email scope"); return null; } return info; } + + @VisibleForTesting + static void configureErrorHandling(HttpRequest request) { + request.setNumberOfRetries(1) + .setThrowExceptionOnExecuteError(false) + .setIOExceptionHandler(new HttpIOExceptionHandler() { + @Override + public boolean handleIOException(HttpRequest request, boolean supportsRetry) { + return true; // consider all IOException as transient + } + }) + .setUnsuccessfulResponseHandler(new HttpUnsuccessfulResponseHandler() { + @Override + public boolean handleResponse(HttpRequest request, HttpResponse response, + boolean supportsRetry) { + return response.getStatusCode() >= 500; // only retry Google's backend errors + } + }); + } } diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/auth/GoogleOAuth2Authenticator.java b/endpoints-framework/src/main/java/com/google/api/server/spi/auth/GoogleOAuth2Authenticator.java index 5524bb6e..3ce915e9 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/auth/GoogleOAuth2Authenticator.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/auth/GoogleOAuth2Authenticator.java @@ -22,6 +22,7 @@ import com.google.api.server.spi.config.Singleton; import com.google.api.server.spi.config.model.ApiMethodConfig; import com.google.api.server.spi.request.Attribute; +import com.google.api.server.spi.response.ServiceUnavailableException; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableSet; @@ -39,7 +40,7 @@ public class GoogleOAuth2Authenticator implements Authenticator { private static final Logger logger = Logger.getLogger(GoogleOAuth2Authenticator.class.getName()); @Override - public User authenticate(HttpServletRequest request) { + public User authenticate(HttpServletRequest request) throws ServiceUnavailableException { Attribute attr = Attribute.from(request); if (attr.isEnabled(Attribute.SKIP_TOKEN_AUTH)) { return null; @@ -89,7 +90,7 @@ public User authenticate(HttpServletRequest request) { } @VisibleForTesting - TokenInfo getTokenInfoRemote(String token) { + TokenInfo getTokenInfoRemote(String token) throws ServiceUnavailableException { return GoogleAuth.getTokenInfoRemote(token); } } diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/auth/EndpointsAuthenticatorTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/auth/EndpointsAuthenticatorTest.java index 1fc9b6a7..f9ce9db5 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/auth/EndpointsAuthenticatorTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/auth/EndpointsAuthenticatorTest.java @@ -21,6 +21,7 @@ import com.google.api.server.spi.EnvUtil; import com.google.api.server.spi.auth.common.User; import com.google.api.server.spi.request.Attribute; +import com.google.api.server.spi.response.ServiceUnavailableException; import org.junit.Before; import org.junit.Test; @@ -53,13 +54,13 @@ public void setUp() throws Exception { } @Test - public void testAuthenticate_jwt() { + public void testAuthenticate_jwt() throws ServiceUnavailableException { when(jwtAuthenticator.authenticate(request)).thenReturn(USER); assertEquals(USER, authenticator.authenticate(request)); } @Test - public void testAuthenticate_appEngine() { + public void testAuthenticate_appEngine() throws ServiceUnavailableException { when(jwtAuthenticator.authenticate(request)).thenReturn(null); when(appEngineAuthenticator.authenticate(request)).thenReturn(USER); @@ -70,7 +71,7 @@ public void testAuthenticate_appEngine() { } @Test - public void testAuthenticate_oauth2NonAppEngine() { + public void testAuthenticate_oauth2NonAppEngine() throws ServiceUnavailableException { when(jwtAuthenticator.authenticate(request)).thenReturn(null); when(oauth2Authenticator.authenticate(request)).thenReturn(USER); @@ -81,7 +82,7 @@ public void testAuthenticate_oauth2NonAppEngine() { } @Test - public void testAuthenticate_oAuth2NotRequireAppEngineUser() { + public void testAuthenticate_oAuth2NotRequireAppEngineUser() throws ServiceUnavailableException { when(jwtAuthenticator.authenticate(request)).thenReturn(null); when(oauth2Authenticator.authenticate(request)).thenReturn(USER); diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/auth/GoogleAppEngineAuthenticatorTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/auth/GoogleAppEngineAuthenticatorTest.java index 679c3670..3265bb83 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/auth/GoogleAppEngineAuthenticatorTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/auth/GoogleAppEngineAuthenticatorTest.java @@ -24,6 +24,7 @@ import com.google.api.server.spi.config.model.ApiMethodConfig; import com.google.api.server.spi.config.scope.AuthScopeExpressions; import com.google.api.server.spi.request.Attribute; +import com.google.api.server.spi.response.ServiceUnavailableException; import com.google.appengine.api.oauth.OAuthRequestException; import com.google.appengine.api.oauth.OAuthService; import com.google.appengine.api.users.UserService; @@ -87,7 +88,7 @@ private void initializeRequest(String bearerString) { } @Test - public void testGetOAuth2UserNonOAuth2() { + public void testGetOAuth2UserNonOAuth2() throws ServiceUnavailableException { initializeRequest("Bearer badToken"); assertNull(authenticator.getOAuth2User(request, config)); @@ -152,7 +153,7 @@ public void testGetOAuth2UserSkipClientIdCheck() throws Exception { } @Test - public void testGetOAuth2UserAppEngineDevClientIdNotAllowed() { + public void testGetOAuth2UserAppEngineDevClientIdNotAllowed() throws ServiceUnavailableException { System.setProperty(EnvUtil.ENV_APPENGINE_RUNTIME, "Developement"); when(config.getScopeExpression()).thenReturn(AuthScopeExpressions.interpret(SCOPES)); when(config.getClientIds()).thenReturn(ImmutableList.of("clientId2")); @@ -160,19 +161,19 @@ public void testGetOAuth2UserAppEngineDevClientIdNotAllowed() { } @Test - public void testAuthenticateNonAppEngine() { + public void testAuthenticateNonAppEngine() throws ServiceUnavailableException { System.clearProperty(EnvUtil.ENV_APPENGINE_RUNTIME); assertNull(authenticator.authenticate(request)); } @Test - public void testAuthenticateSkipTokenAuth() { + public void testAuthenticateSkipTokenAuth() throws ServiceUnavailableException { attr.set(Attribute.SKIP_TOKEN_AUTH, true); assertNull(authenticator.authenticate(request)); } @Test - public void testAuthenticateOAuth2Fail() { + public void testAuthenticateOAuth2Fail() throws ServiceUnavailableException { authenticator = new GoogleAppEngineAuthenticator(oauthService, userService) { @Override com.google.appengine.api.users.User getOAuth2User(HttpServletRequest request, @@ -189,7 +190,7 @@ boolean shouldTryCookieAuth(ApiMethodConfig config) { } @Test - public void testAuthenticateOAuth2() { + public void testAuthenticateOAuth2() throws ServiceUnavailableException { authenticator = new GoogleAppEngineAuthenticator(oauthService, userService) { @Override com.google.appengine.api.users.User getOAuth2User(HttpServletRequest request, @@ -202,7 +203,7 @@ com.google.appengine.api.users.User getOAuth2User(HttpServletRequest request, } @Test - public void testAuthenticateSkipTokenAuthCookieAuthFail() { + public void testAuthenticateSkipTokenAuthCookieAuthFail() throws ServiceUnavailableException { attr.set(Attribute.SKIP_TOKEN_AUTH, true); authenticator = new GoogleAppEngineAuthenticator(oauthService, userService) { @Override @@ -215,7 +216,7 @@ boolean shouldTryCookieAuth(ApiMethodConfig config) { } @Test - public void testAuthenticateSkipTokenAuthCookieAuth() { + public void testAuthenticateSkipTokenAuthCookieAuth() throws ServiceUnavailableException { attr.set(Attribute.SKIP_TOKEN_AUTH, true); authenticator = new GoogleAppEngineAuthenticator(oauthService, userService) { @Override @@ -229,7 +230,7 @@ boolean shouldTryCookieAuth(ApiMethodConfig config) { } @Test - public void testAuthenticateOAuth2CookieAuthBothFail() { + public void testAuthenticateOAuth2CookieAuthBothFail() throws ServiceUnavailableException { authenticator = new GoogleAppEngineAuthenticator(oauthService, userService) { @Override com.google.appengine.api.users.User getOAuth2User(HttpServletRequest request, @@ -247,7 +248,7 @@ boolean shouldTryCookieAuth(ApiMethodConfig config) { } @Test - public void testAuthenticateOAuth2FailCookieAuth() { + public void testAuthenticateOAuth2FailCookieAuth() throws ServiceUnavailableException { authenticator = new GoogleAppEngineAuthenticator(oauthService, userService) { @Override com.google.appengine.api.users.User getOAuth2User(HttpServletRequest request, diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/auth/GoogleAuthTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/auth/GoogleAuthTest.java index 61f05896..3b27c775 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/auth/GoogleAuthTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/auth/GoogleAuthTest.java @@ -31,6 +31,7 @@ import com.google.api.client.testing.http.MockLowLevelHttpRequest; import com.google.api.client.testing.http.MockLowLevelHttpResponse; import com.google.api.server.spi.auth.GoogleAuth.TokenInfo; +import com.google.api.server.spi.response.ServiceUnavailableException; import com.google.common.collect.ImmutableList; import org.junit.Test; @@ -166,7 +167,23 @@ public void testParseTokenInfo_withoutEmail() throws Exception { assertNull(GoogleAuth.parseTokenInfo(request)); } + @Test + public void testParseTokenInfo_with400() throws Exception { + HttpRequest request = constructHttpRequest("{\"error_description\": \"Invalid Value\"}", 400); + assertNull(GoogleAuth.parseTokenInfo(request)); + } + + @Test(expected = ServiceUnavailableException.class) + public void testParseTokenInfo_with500() throws Exception { + HttpRequest request = constructHttpRequest("{\"error_description\": \"Backend Error\"}", 500); + GoogleAuth.parseTokenInfo(request); + } + private HttpRequest constructHttpRequest(final String content) throws IOException { + return constructHttpRequest(content, 200); + } + + private HttpRequest constructHttpRequest(final String content, final int statusCode) throws IOException { HttpTransport transport = new MockHttpTransport() { @Override public LowLevelHttpRequest buildRequest(String method, String url) throws IOException { @@ -176,12 +193,14 @@ public LowLevelHttpResponse execute() throws IOException { MockLowLevelHttpResponse result = new MockLowLevelHttpResponse(); result.setContentType("application/json"); result.setContent(content); + result.setStatusCode(statusCode); return result; } }; } }; - return transport.createRequestFactory().buildGetRequest(new GenericUrl("https://google.com")) - .setParser(new JsonObjectParser(new JacksonFactory())); + HttpRequest httpRequest = transport.createRequestFactory().buildGetRequest(new GenericUrl("https://google.com")).setParser(new JsonObjectParser(new JacksonFactory())); + GoogleAuth.configureErrorHandling(httpRequest); + return httpRequest; } } diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/auth/GoogleOAuth2AuthenticatorTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/auth/GoogleOAuth2AuthenticatorTest.java index b86a0f9c..77320a5f 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/auth/GoogleOAuth2AuthenticatorTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/auth/GoogleOAuth2AuthenticatorTest.java @@ -24,6 +24,7 @@ import com.google.api.server.spi.config.model.ApiMethodConfig; import com.google.api.server.spi.config.scope.AuthScopeExpressions; import com.google.api.server.spi.request.Attribute; +import com.google.api.server.spi.response.ServiceUnavailableException; import com.google.common.collect.ImmutableList; import org.junit.Before; @@ -65,38 +66,38 @@ private void initializeRequest(String bearerString) { } @Test - public void testAuthenticate_skipTokenAuth() { + public void testAuthenticate_skipTokenAuth() throws ServiceUnavailableException { attr.set(Attribute.SKIP_TOKEN_AUTH, true); assertNull(authenticator.authenticate(request)); } @Test - public void testAuthenticate_notOAuth2() { + public void testAuthenticate_notOAuth2() throws ServiceUnavailableException { initializeRequest("Bearer badToken"); assertNull(authenticator.authenticate(request)); } @Test - public void testAuthenticate_nullTokenInfo() { + public void testAuthenticate_nullTokenInfo() throws ServiceUnavailableException { authenticator = createAuthenticator(null, null, null, null); assertNull(authenticator.authenticate(request)); } @Test - public void testAuthenticate_scopeNotAllowed() { + public void testAuthenticate_scopeNotAllowed() throws ServiceUnavailableException { when(config.getScopeExpression()).thenReturn(AuthScopeExpressions.interpret("scope3")); assertNull(authenticator.authenticate(request)); } @Test - public void testAuthenticate_clientIdNotAllowed() { + public void testAuthenticate_clientIdNotAllowed() throws ServiceUnavailableException { when(config.getScopeExpression()).thenReturn(AuthScopeExpressions.interpret("scope1")); when(config.getClientIds()).thenReturn(ImmutableList.of("clientId2")); assertNull(authenticator.authenticate(request)); } @Test - public void testAuthenticate_skipClientIdCheck() { + public void testAuthenticate_skipClientIdCheck() throws ServiceUnavailableException { request.removeAttribute(Attribute.ENABLE_CLIENT_ID_WHITELIST); when(config.getScopeExpression()).thenReturn(AuthScopeExpressions.interpret("scope1")); when(config.getClientIds()).thenReturn(ImmutableList.of("clientId2")); @@ -106,7 +107,7 @@ public void testAuthenticate_skipClientIdCheck() { } @Test - public void testAuthenticate() { + public void testAuthenticate() throws ServiceUnavailableException { when(config.getScopeExpression()).thenReturn(AuthScopeExpressions.interpret("scope1")); when(config.getClientIds()).thenReturn(ImmutableList.of(CLIENT_ID)); User user = authenticator.authenticate(request); @@ -115,7 +116,7 @@ public void testAuthenticate() { } @Test - public void testAuthenticate_appEngineUser() { + public void testAuthenticate_appEngineUser() throws ServiceUnavailableException { attr.set(Attribute.REQUIRE_APPENGINE_USER, true); when(config.getScopeExpression()).thenReturn(AuthScopeExpressions.interpret("scope1")); when(config.getClientIds()).thenReturn(ImmutableList.of(CLIENT_ID));