From 98e4eb708ff5ea359595105ee06232f2df1673bf Mon Sep 17 00:00:00 2001 From: Gareth Smith Date: Wed, 4 Aug 2021 03:06:39 +0100 Subject: [PATCH] [BUG]Java] Fix a race condition in RetryingOAuth.mustache (#10087) If there were multiple concurrent requests at a time at which the OAuth token had expired, only a single request would be retried. The other requests would fail because of the expired token, but not be retried and so the failures would be propagated to the caller. --- .../okhttp-gson/auth/RetryingOAuth.mustache | 4 +- .../Java/libraries/okhttp-gson/pom.mustache | 6 + .../okhttp-gson-dynamicOperations/pom.xml | 6 + .../client/auth/RetryingOAuth.java | 4 +- .../java/okhttp-gson-parcelableModel/pom.xml | 6 + .../client/auth/RetryingOAuth.java | 4 +- .../okhttp-gson/.openapi-generator-ignore | 1 + .../client/petstore/java/okhttp-gson/pom.xml | 6 + .../client/auth/RetryingOAuth.java | 4 +- .../client/auth/RetryingOAuthTest.java | 112 ++++++++++++++++++ 10 files changed, 141 insertions(+), 12 deletions(-) create mode 100644 samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/auth/RetryingOAuthTest.java diff --git a/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/auth/RetryingOAuth.mustache b/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/auth/RetryingOAuth.mustache index 3ddacedb70e..6e6562448da 100644 --- a/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/auth/RetryingOAuth.mustache +++ b/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/auth/RetryingOAuth.mustache @@ -156,14 +156,12 @@ public class RetryingOAuth extends OAuth implements Interceptor { oAuthClient.accessToken(tokenRequestBuilder.buildBodyMessage()); if (accessTokenResponse != null && accessTokenResponse.getAccessToken() != null) { setAccessToken(accessTokenResponse.getAccessToken()); - return !getAccessToken().equals(requestAccessToken); } } catch (OAuthSystemException | OAuthProblemException e) { throw new IOException(e); } } - - return false; + return getAccessToken() == null || !getAccessToken().equals(requestAccessToken); } public TokenRequestBuilder getTokenRequestBuilder() { diff --git a/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pom.mustache b/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pom.mustache index 38596f5369e..39f6f2b5c4b 100644 --- a/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pom.mustache +++ b/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pom.mustache @@ -332,6 +332,12 @@ ${junit-version} test + + org.mockito + mockito-core + 3.11.2 + test + {{#java8}}1.8{{/java8}}{{^java8}}1.7{{/java8}} diff --git a/samples/client/petstore/java/okhttp-gson-dynamicOperations/pom.xml b/samples/client/petstore/java/okhttp-gson-dynamicOperations/pom.xml index 656360eff39..c4c2c923a10 100644 --- a/samples/client/petstore/java/okhttp-gson-dynamicOperations/pom.xml +++ b/samples/client/petstore/java/okhttp-gson-dynamicOperations/pom.xml @@ -279,6 +279,12 @@ ${junit-version} test + + org.mockito + mockito-core + 3.11.2 + test + 1.7 diff --git a/samples/client/petstore/java/okhttp-gson-dynamicOperations/src/main/java/org/openapitools/client/auth/RetryingOAuth.java b/samples/client/petstore/java/okhttp-gson-dynamicOperations/src/main/java/org/openapitools/client/auth/RetryingOAuth.java index 9cab81a7176..cb79b34ca87 100644 --- a/samples/client/petstore/java/okhttp-gson-dynamicOperations/src/main/java/org/openapitools/client/auth/RetryingOAuth.java +++ b/samples/client/petstore/java/okhttp-gson-dynamicOperations/src/main/java/org/openapitools/client/auth/RetryingOAuth.java @@ -155,14 +155,12 @@ public class RetryingOAuth extends OAuth implements Interceptor { oAuthClient.accessToken(tokenRequestBuilder.buildBodyMessage()); if (accessTokenResponse != null && accessTokenResponse.getAccessToken() != null) { setAccessToken(accessTokenResponse.getAccessToken()); - return !getAccessToken().equals(requestAccessToken); } } catch (OAuthSystemException | OAuthProblemException e) { throw new IOException(e); } } - - return false; + return getAccessToken() == null || !getAccessToken().equals(requestAccessToken); } public TokenRequestBuilder getTokenRequestBuilder() { diff --git a/samples/client/petstore/java/okhttp-gson-parcelableModel/pom.xml b/samples/client/petstore/java/okhttp-gson-parcelableModel/pom.xml index 6761db7d59d..5b92eb11c69 100644 --- a/samples/client/petstore/java/okhttp-gson-parcelableModel/pom.xml +++ b/samples/client/petstore/java/okhttp-gson-parcelableModel/pom.xml @@ -281,6 +281,12 @@ ${junit-version} test + + org.mockito + mockito-core + 3.11.2 + test + 1.7 diff --git a/samples/client/petstore/java/okhttp-gson-parcelableModel/src/main/java/org/openapitools/client/auth/RetryingOAuth.java b/samples/client/petstore/java/okhttp-gson-parcelableModel/src/main/java/org/openapitools/client/auth/RetryingOAuth.java index 9cab81a7176..cb79b34ca87 100644 --- a/samples/client/petstore/java/okhttp-gson-parcelableModel/src/main/java/org/openapitools/client/auth/RetryingOAuth.java +++ b/samples/client/petstore/java/okhttp-gson-parcelableModel/src/main/java/org/openapitools/client/auth/RetryingOAuth.java @@ -155,14 +155,12 @@ public class RetryingOAuth extends OAuth implements Interceptor { oAuthClient.accessToken(tokenRequestBuilder.buildBodyMessage()); if (accessTokenResponse != null && accessTokenResponse.getAccessToken() != null) { setAccessToken(accessTokenResponse.getAccessToken()); - return !getAccessToken().equals(requestAccessToken); } } catch (OAuthSystemException | OAuthProblemException e) { throw new IOException(e); } } - - return false; + return getAccessToken() == null || !getAccessToken().equals(requestAccessToken); } public TokenRequestBuilder getTokenRequestBuilder() { diff --git a/samples/client/petstore/java/okhttp-gson/.openapi-generator-ignore b/samples/client/petstore/java/okhttp-gson/.openapi-generator-ignore index fa2ddf08fcc..6ba435b5655 100644 --- a/samples/client/petstore/java/okhttp-gson/.openapi-generator-ignore +++ b/samples/client/petstore/java/okhttp-gson/.openapi-generator-ignore @@ -5,6 +5,7 @@ src/test/java/org/openapitools/client/ApiClientTest.java src/test/java/org/openapitools/client/ConfigurationTest.java src/test/java/org/openapitools/client/auth/ApiKeyAuthTest.java src/test/java/org/openapitools/client/auth/HttpBasicAuthTest.java +src/test/java/org/openapitools/client/auth/RetryingOAuthTest.java src/test/java/org/openapitools/client/model/EnumValueTest.java src/test/java/org/openapitools/client/model/PetTest.java src/test/java/org/openapitools/client/model/ArrayOfArrayOfNumberOnlyTest.java diff --git a/samples/client/petstore/java/okhttp-gson/pom.xml b/samples/client/petstore/java/okhttp-gson/pom.xml index 54685657f87..302f2cf0248 100644 --- a/samples/client/petstore/java/okhttp-gson/pom.xml +++ b/samples/client/petstore/java/okhttp-gson/pom.xml @@ -274,6 +274,12 @@ ${junit-version} test + + org.mockito + mockito-core + 3.11.2 + test + 1.7 diff --git a/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/auth/RetryingOAuth.java b/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/auth/RetryingOAuth.java index 9cab81a7176..cb79b34ca87 100644 --- a/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/auth/RetryingOAuth.java +++ b/samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/auth/RetryingOAuth.java @@ -155,14 +155,12 @@ public class RetryingOAuth extends OAuth implements Interceptor { oAuthClient.accessToken(tokenRequestBuilder.buildBodyMessage()); if (accessTokenResponse != null && accessTokenResponse.getAccessToken() != null) { setAccessToken(accessTokenResponse.getAccessToken()); - return !getAccessToken().equals(requestAccessToken); } } catch (OAuthSystemException | OAuthProblemException e) { throw new IOException(e); } } - - return false; + return getAccessToken() == null || !getAccessToken().equals(requestAccessToken); } public TokenRequestBuilder getTokenRequestBuilder() { diff --git a/samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/auth/RetryingOAuthTest.java b/samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/auth/RetryingOAuthTest.java new file mode 100644 index 00000000000..dca9b7c69ea --- /dev/null +++ b/samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/auth/RetryingOAuthTest.java @@ -0,0 +1,112 @@ +package org.openapitools.client.auth; + +import okhttp3.Interceptor.Chain; +import okhttp3.*; +import okhttp3.Response.Builder; +import org.apache.commons.lang3.reflect.FieldUtils; +import org.apache.oltu.oauth2.client.OAuthClient; +import org.apache.oltu.oauth2.client.request.OAuthClientRequest; +import org.apache.oltu.oauth2.client.response.OAuthJSONAccessTokenResponse; +import org.apache.oltu.oauth2.common.exception.OAuthProblemException; +import org.apache.oltu.oauth2.common.exception.OAuthSystemException; +import org.junit.Before; +import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import java.io.IOException; +import java.net.HttpURLConnection; +import java.util.Collections; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + +import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class RetryingOAuthTest { + + private RetryingOAuth oauth; + + @Before + public void setUp() throws Exception { + oauth = new RetryingOAuth("_clientId", "_clientSecret", OAuthFlow.accessCode, + "https://token.example.com", Collections.emptyMap()); + oauth.setAccessToken("expired-access-token"); + FieldUtils.writeField(oauth, "oAuthClient", mockOAuthClient(), true); + } + + @Test + public void testSingleRequestUnauthorized() throws Exception { + Response response = oauth.intercept(mockChain()); + assertEquals(HttpURLConnection.HTTP_OK, response.code()); + } + + @Test + public void testTwoConcurrentRequestsUnauthorized() throws Exception { + + Callable callable = new Callable() { + @Override + public Response call() throws Exception { + return oauth.intercept(mockChain()); + } + }; + ExecutorService executor = Executors.newFixedThreadPool(2); + try { + Future response1 = executor.submit(callable); + Future response2 = executor.submit(callable); + + assertEquals(HttpURLConnection.HTTP_OK, response1.get().code()); + assertEquals(HttpURLConnection.HTTP_OK, response2.get().code()); + } finally { + executor.shutdown(); + } + } + + private OAuthClient mockOAuthClient() throws OAuthProblemException, OAuthSystemException { + OAuthJSONAccessTokenResponse response = mock(OAuthJSONAccessTokenResponse.class); + when(response.getAccessToken()).thenAnswer(new Answer() { + @Override + public String answer(InvocationOnMock invocation) throws Throwable { + // sleep ensures that the bug is triggered. + Thread.sleep(1000); + return "new-access-token"; + } + }); + + OAuthClient client = mock(OAuthClient.class); + when(client.accessToken(any(OAuthClientRequest.class))).thenReturn(response); + return client; + } + + private Chain mockChain() throws IOException { + Chain chain = mock(Chain.class); + + final Request request = new Request.Builder() + .url("https://api.example.com") + .build(); + when(chain.request()).thenReturn(request); + + when(chain.proceed(any(Request.class))).thenAnswer(new Answer() { + @Override + public Response answer(InvocationOnMock inv) { + Request r = inv.getArgument(0); + int responseCode = "Bearer new-access-token".equals(r.header("Authorization")) + ? HttpURLConnection.HTTP_OK + : HttpURLConnection.HTTP_UNAUTHORIZED; + return new Builder() + .protocol(Protocol.HTTP_1_0) + .message("sup") + .request(request) + .body(ResponseBody.create(new byte[0], MediaType.get("application/test"))) + .code(responseCode) + .build(); + } + }); + + return chain; + } +}