From 967cb8b2bfc1c57864419865f0f9fb2e1288d90d Mon Sep 17 00:00:00 2001 From: Ankit Tiwari Date: Thu, 13 Jun 2024 15:38:33 +0530 Subject: [PATCH 1/2] fix: session refresh loop if expired token is passed in headers --- CHANGELOG.md | 6 ++ app/build.gradle | 2 +- .../SuperTokensCustomHttpURLConnection.java | 29 +++++++++ .../session/SuperTokensHttpURLConnection.java | 25 +++++--- .../session/SuperTokensInterceptor.java | 43 +++++++++---- .../java/com/supertokens/session/Utils.java | 21 ++---- examples/with-thirdparty/README.md | 2 +- examples/with-thirdparty/app/build.gradle.kts | 2 +- .../SuperTokensHttpURLConnectionTest.java | 64 +++++++++++++++++-- .../example/SuperTokensOkHttpHeaderTests.java | 53 +++++++++++++-- .../SuperTokensRetrofitHeaderTests.java | 38 +++++++++-- 11 files changed, 233 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11251e3..4fcfe81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.5.1] - 2024-06-13 + +### Changes + +- Fixed session refresh loop caused by passing an expired access token in the Authorization header. + ## [0.5.0] - 2024-06-06 ### Changes diff --git a/app/build.gradle b/app/build.gradle index d8716e0..bf69c22 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -1,6 +1,6 @@ apply plugin: 'com.android.library' apply plugin: 'maven-publish' -def publishVersionID = "0.5.0" +def publishVersionID = "0.5.1" android { compileSdkVersion 32 diff --git a/app/src/main/java/com/supertokens/session/SuperTokensCustomHttpURLConnection.java b/app/src/main/java/com/supertokens/session/SuperTokensCustomHttpURLConnection.java index 0446c6c..7c91197 100644 --- a/app/src/main/java/com/supertokens/session/SuperTokensCustomHttpURLConnection.java +++ b/app/src/main/java/com/supertokens/session/SuperTokensCustomHttpURLConnection.java @@ -34,6 +34,7 @@ public class SuperTokensCustomHttpURLConnection extends HttpURLConnection { HttpURLConnection original; Context applicationContext; + private boolean wasAuthHeaderRemovedInitially = false; public SuperTokensCustomHttpURLConnection(HttpURLConnection original, Context applicationContext) { super(original.getURL()); @@ -41,6 +42,17 @@ public SuperTokensCustomHttpURLConnection(HttpURLConnection original, Context ap this.applicationContext = applicationContext; } + public SuperTokensCustomHttpURLConnection(HttpURLConnection original, Context applicationContext, boolean wasAuthHeaderRemovedInitially) { + super(original.getURL()); + this.original = original; + this.applicationContext = applicationContext; + this.wasAuthHeaderRemovedInitially = wasAuthHeaderRemovedInitially; + } + + public boolean getWasAuthHeaderRemovedInitially() { + return wasAuthHeaderRemovedInitially; + } + @Override public void disconnect() { original.disconnect(); @@ -249,9 +261,18 @@ public void setDefaultUseCaches(boolean defaultusecaches) { } private boolean shouldAllowSettingAuthHeader(String value) { + // This check ensures that if the authorization header was removed initially (because it matched the local access token), + // it remains removed in subsequent retries even after the session is refreshed. + // This prevents the use of an expired access token which would no longer match the updated local access token. + if (wasAuthHeaderRemovedInitially) { + return false; + } + + String accessToken = Utils.getTokenForHeaderAuth(Utils.TokenType.ACCESS, applicationContext); String refreshToken = Utils.getTokenForHeaderAuth(Utils.TokenType.REFRESH, applicationContext); if (accessToken != null && refreshToken != null && value.equals("Bearer " + accessToken)) { + wasAuthHeaderRemovedInitially = true; // We ignore the attempt to set the header because it matches the existing access token // which will get added by the SDK return false; @@ -275,6 +296,14 @@ public void setRequestProperty(String key, String value, boolean force) { original.setRequestProperty(key, value); } + // Sets the authorization header without performing the "shouldAllowSettingAuthHeader" check. + // This bypass is necessary because the "shouldAllowSettingAuthHeader" function tracks whether + // setting the auth header was disallowed, which is only intended for custom headers set by the user, + // not for headers set by our library code. + public void setRequestPropertyIgnoringOverride(String key, String value) { + original.setRequestProperty(key, value); + } + public void setRequestProperty(String key, String value) { setRequestProperty(key, value, false); } diff --git a/app/src/main/java/com/supertokens/session/SuperTokensHttpURLConnection.java b/app/src/main/java/com/supertokens/session/SuperTokensHttpURLConnection.java index 83aa543..8cdbf16 100644 --- a/app/src/main/java/com/supertokens/session/SuperTokensHttpURLConnection.java +++ b/app/src/main/java/com/supertokens/session/SuperTokensHttpURLConnection.java @@ -24,7 +24,6 @@ import java.net.HttpURLConnection; import java.net.URISyntaxException; import java.net.URL; -import java.net.URLConnection; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -37,16 +36,21 @@ public class SuperTokensHttpURLConnection { private static final ReentrantReadWriteLock refreshAPILock = new ReentrantReadWriteLock(); private static void setAuthorizationHeaderIfRequired(SuperTokensCustomHttpURLConnection connection, Context context) { - Map headersToSet = Utils.getAuthorizationHeaderIfRequired(context); - for (Map.Entry entry: headersToSet.entrySet()) { - connection.setRequestProperty(entry.getKey(), entry.getValue(), true); + String authHeader = Utils.getAuthorizationHeaderIfExists(false, context); + + // NOTE: We do not check for existing Auth headers here because they are added after this function runs. + // The `setRequestProperty` method in SuperTokensCustomHttpURLConnection is overridden to prevent users from adding + // an auth header that matches the locally stored access token. + if (authHeader != null) { + connection.setRequestPropertyIgnoringOverride("Authorization", authHeader); } } private static void setAuthorizationHeaderIfRequiredForRefresh(HttpURLConnection connection, Context context) { - Map headersToSet = Utils.getAuthorizationHeaderIfRequired(true, context); - for (Map.Entry entry: headersToSet.entrySet()) { - connection.setRequestProperty(entry.getKey(), entry.getValue()); + String authHeader = Utils.getAuthorizationHeaderIfExists(true, context); + // NOTE: Checking for an existing auth header is not necessary for a refresh API call. + if (authHeader != null) { + connection.setRequestProperty("Authorization", authHeader); } } @@ -130,16 +134,17 @@ public static HttpURLConnection newRequest(URL url, PreConnectCallback preConnec try { int sessionRefreshAttempts = 0; + HttpURLConnection connection; + SuperTokensCustomHttpURLConnection customConnection = null; while (true) { - HttpURLConnection connection; - SuperTokensCustomHttpURLConnection customConnection; Utils.LocalSessionState preRequestLocalSessionState; int responseCode; // TODO: write comment as to why we have this lock here. Do we also have this lock for iOS and website package? refreshAPILock.readLock().lock(); try { + boolean wasAuthHeaderRemovedInitially = customConnection != null && customConnection.getWasAuthHeaderRemovedInitially(); connection = (HttpURLConnection) url.openConnection(); - customConnection = new SuperTokensCustomHttpURLConnection(connection, applicationContext); + customConnection = new SuperTokensCustomHttpURLConnection(connection, applicationContext, wasAuthHeaderRemovedInitially); // Add antiCSRF token, if present in storage, to the request headers preRequestLocalSessionState = Utils.getLocalSessionState(applicationContext); diff --git a/app/src/main/java/com/supertokens/session/SuperTokensInterceptor.java b/app/src/main/java/com/supertokens/session/SuperTokensInterceptor.java index feef7cf..2a35483 100644 --- a/app/src/main/java/com/supertokens/session/SuperTokensInterceptor.java +++ b/app/src/main/java/com/supertokens/session/SuperTokensInterceptor.java @@ -34,7 +34,10 @@ public class SuperTokensInterceptor implements Interceptor { private static final Object refreshTokenLock = new Object(); private static final ReentrantReadWriteLock refreshAPILock = new ReentrantReadWriteLock(); - private Request removeAuthHeaderIfMatchesLocalToken(Request request, Request.Builder builder, Context context) { + + // Returns true authorization header in the provided request matches the current local access token. + // This is used to decide whether the authorization header should be removed before making the request. + private boolean shouldRemoveAuthHeader(Request request, Context context) { String originalHeader = request.header("Authorization"); if (originalHeader == null) { @@ -46,18 +49,20 @@ private Request removeAuthHeaderIfMatchesLocalToken(Request request, Request.Bui String refreshToken = Utils.getTokenForHeaderAuth(Utils.TokenType.REFRESH, context); if (accessToken != null && refreshToken != null && originalHeader.equals("Bearer " + accessToken)) { - builder.removeHeader("Authorization"); - builder.removeHeader("authorization"); + return true; } } - return builder.build(); + return false; } - private static Request setAuthorizationHeaderIfRequired(Request.Builder builder, Context context, boolean addRefreshToken) { - Map headersToSet = Utils.getAuthorizationHeaderIfRequired(addRefreshToken, context); - for (Map.Entry entry: headersToSet.entrySet()) { - builder.header(entry.getKey(), entry.getValue()); + + private static Request setAuthorizationHeaderIfRequired(Request request, Request.Builder builder, Context context, boolean addRefreshToken) { + String authHeader = Utils.getAuthorizationHeaderIfExists(addRefreshToken, context); + boolean hasExistingAuthHeader = request.header("Authorization") != null || request.header("authorization") != null; + + if (authHeader != null && !hasExistingAuthHeader) { + builder.header("Authorization", authHeader); } return builder.build(); @@ -98,6 +103,7 @@ public Response intercept(@NotNull Chain chain) throws IOException { } try { + boolean wasAuthHeaderRemovedInitially = false; int sessionRefreshAttempts = 0; while (true) { Request.Builder requestBuilder = chain.request().newBuilder(); @@ -120,8 +126,19 @@ public Response intercept(@NotNull Chain chain) throws IOException { request = request.newBuilder().header("rid", "anti-csrf").build(); } - request = removeAuthHeaderIfMatchesLocalToken(request, request.newBuilder(), applicationContext); - request = setAuthorizationHeaderIfRequired(request.newBuilder(), applicationContext, false); + // Check if the Authorization header should be removed + // This is necessary to ensure that if the auth header was removed initially, + // it remains removed in subsequent retries even if the token has changed. + if (wasAuthHeaderRemovedInitially || shouldRemoveAuthHeader(request, applicationContext)) { + Request.Builder builder = request.newBuilder(); + builder.removeHeader("Authorization"); + builder.removeHeader("authorization"); + request = builder.build(); + + wasAuthHeaderRemovedInitially = true; + } + + request = setAuthorizationHeaderIfRequired(request, request.newBuilder(), applicationContext, false); response = makeRequest(chain, request); Utils.saveTokenFromHeaders(response, applicationContext); @@ -207,6 +224,8 @@ private static Utils.Unauthorised onUnauthorisedResponse(Utils.LocalSessionState Request.Builder refreshRequestBuilder = new Request.Builder(); refreshRequestBuilder.url(SuperTokens.refreshTokenUrl); refreshRequestBuilder.method("POST", new FormBody.Builder().build()); + + Request refreshRequest = refreshRequestBuilder.build(); if (preRequestLocalSessionState.status == Utils.LocalSessionStateStatus.EXISTS) { String antiCSRFToken = AntiCSRF.getToken(applicationContext, preRequestLocalSessionState.lastAccessTokenUpdate); @@ -220,7 +239,7 @@ private static Utils.Unauthorised onUnauthorisedResponse(Utils.LocalSessionState refreshRequestBuilder.header("fdi-version", Utils.join(Version.supported_fdi, ",")); refreshRequestBuilder.header("st-auth-mode", SuperTokens.config.tokenTransferMethod); - refreshRequestBuilder = setAuthorizationHeaderIfRequired(refreshRequestBuilder, applicationContext, true).newBuilder(); + refreshRequestBuilder = setAuthorizationHeaderIfRequired(refreshRequest, refreshRequestBuilder, applicationContext, true).newBuilder(); Map customRefreshHeaders = SuperTokens.config.customHeaderMapper.getRequestHeaders(CustomHeaderProvider.RequestType.REFRESH); if (customRefreshHeaders != null) { @@ -229,7 +248,7 @@ private static Utils.Unauthorised onUnauthorisedResponse(Utils.LocalSessionState } } - Request refreshRequest = refreshRequestBuilder.build(); + refreshRequest = refreshRequestBuilder.build(); refreshResponse = makeRequest(chain, refreshRequest); Utils.saveTokenFromHeaders(refreshResponse, applicationContext); diff --git a/app/src/main/java/com/supertokens/session/Utils.java b/app/src/main/java/com/supertokens/session/Utils.java index 6d141b9..415b969 100644 --- a/app/src/main/java/com/supertokens/session/Utils.java +++ b/app/src/main/java/com/supertokens/session/Utils.java @@ -319,10 +319,6 @@ public static String getTokenForHeaderAuth(TokenType tokenType, Context context) return getFromStorage(name, context); } - public static Map getAuthorizationHeaderIfRequired(Context context) { - return getAuthorizationHeaderIfRequired(false, context); - } - // Checks if a key exists in a map regardless of case public static T getIgnoreCase(Map map, String key) { for (Map.Entry entry : map.entrySet()) { @@ -332,33 +328,28 @@ public static T getIgnoreCase(Map map, String key) { return null; } - public static Map getAuthorizationHeaderIfRequired(boolean addRefreshToken, Context context) { - // We set the Authorization header even if the tokenTransferMethod preference + public static String getAuthorizationHeaderIfExists(boolean addRefreshToken, Context context) { + // We return the Authorization header even if the tokenTransferMethod preference // set in the config is cookies // since the active session may be using cookies. By default, we want to allow // users to continue these sessions. // The new session preference should be applied at the start of the next // session, if the backend allows it. - Map headers = new HashMap<>(); String accessToken = getTokenForHeaderAuth(TokenType.ACCESS, context); String refreshToken = getTokenForHeaderAuth(TokenType.REFRESH, context); // We don't always need the refresh token because that's only required by the // refresh call - // Still, we only add the Authorization header if both are present, because we + // Still, we only return the Authorization header if both are present, because we // are planning to add an option to expose the // access token to the frontend while using cookie based auth - so that users // can get the access token to use if (accessToken != null && refreshToken != null) { - if (getIgnoreCase(headers, "Authorization") != null) { - // no-op - } else { - String tokenToAdd = addRefreshToken ? refreshToken : accessToken; - headers.put("Authorization", "Bearer " + tokenToAdd); - } + String tokenToAdd = addRefreshToken ? refreshToken : accessToken; + return "Bearer " + tokenToAdd; } - return headers; + return null; } public static void fireSessionUpdateEventsIfNecessary( diff --git a/examples/with-thirdparty/README.md b/examples/with-thirdparty/README.md index 107f360..0d09798 100644 --- a/examples/with-thirdparty/README.md +++ b/examples/with-thirdparty/README.md @@ -23,7 +23,7 @@ dependencyResolutionManagement { Add the following to your app level `build.gradle` ```gradle -implementation("com.github.supertokens:supertokens-android:0.5.0") +implementation("com.github.supertokens:supertokens-android:0.5.1") implementation ("com.google.android.gms:play-services-auth:20.7.0") implementation("com.squareup.retrofit2:retrofit:2.9.0") implementation("net.openid:appauth:0.11.1") diff --git a/examples/with-thirdparty/app/build.gradle.kts b/examples/with-thirdparty/app/build.gradle.kts index 67b85f4..42c3016 100644 --- a/examples/with-thirdparty/app/build.gradle.kts +++ b/examples/with-thirdparty/app/build.gradle.kts @@ -41,7 +41,7 @@ dependencies { implementation("androidx.appcompat:appcompat:1.6.1") implementation("com.google.android.material:material:1.8.0") implementation("androidx.constraintlayout:constraintlayout:2.1.4") - implementation("com.github.supertokens:supertokens-android:0.5.0") + implementation("com.github.supertokens:supertokens-android:0.5.1") implementation ("com.google.android.gms:play-services-auth:20.7.0") implementation("com.squareup.retrofit2:retrofit:2.9.0") implementation("net.openid:appauth:0.11.1") diff --git a/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensHttpURLConnectionTest.java b/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensHttpURLConnectionTest.java index 442afc5..22878c1 100644 --- a/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensHttpURLConnectionTest.java +++ b/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensHttpURLConnectionTest.java @@ -904,7 +904,7 @@ public void doAction(HttpURLConnection con) throws IOException { } @Test - public void httpUrlConnection_testThatAuthHeaderIsNotIgnoredEvenIfItMatchesTheStoredAccessToken() throws Exception { + public void httpUrlConnection_testThatAuthHeaderIsNotIgnoredIfItDoesntMatchTheStoredAccessToken() throws Exception { com.example.TestUtils.startST(); new SuperTokens.Builder(context, Constants.apiDomain).build(); @@ -931,9 +931,6 @@ public void doAction(HttpURLConnection con) throws IOException { loginRequestConnection.disconnect(); - Thread.sleep(5000); - Utils.setToken(Utils.TokenType.ACCESS, "myOwnHeHe", context); - HttpURLConnection connection = SuperTokensHttpURLConnection.newRequest(new URL(baseCustomAuthUrl), new SuperTokensHttpURLConnection.PreConnectCallback() { @Override public void doAction(HttpURLConnection con) throws IOException { @@ -1104,4 +1101,63 @@ public void doAction(HttpURLConnection con) throws IOException { throw new Exception("Expected session refresh endpoint to be called 0 times but it was called " + sessionRefreshCalledCount + " times"); } } + + @Test + public void httpUrlConnection_shouldNotEndUpInRefreshLoopIfExpiredAccessTokenWasPassedInHeaders() throws Exception{ + com.example.TestUtils.startST(1, true, 144000); + new SuperTokens.Builder(context, Constants.apiDomain).build(); + + //login request + HttpURLConnection loginRequestConnection = SuperTokensHttpURLConnection.newRequest(new URL(loginAPIURL), new SuperTokensHttpURLConnection.PreConnectCallback() { + @Override + public void doAction(HttpURLConnection con) throws IOException { + con.setDoOutput(true); + con.setRequestMethod("POST"); + con.setRequestProperty("Accept", "application/json"); + con.setRequestProperty("Content-Type", "application/json"); + + JsonObject bodyJson = new JsonObject(); + bodyJson.addProperty("userId", Constants.userId); + + OutputStream outputStream = con.getOutputStream(); + outputStream.write(bodyJson.toString().getBytes(StandardCharsets.UTF_8)); + outputStream.close(); + } + }); + + if (loginRequestConnection.getResponseCode() != 200) { + throw new Exception("Login request failed"); + } + + loginRequestConnection.disconnect(); + + String expiredAccessToken = Utils.getTokenForHeaderAuth(Utils.TokenType.ACCESS, context); + + // wait for access token expiry + Thread.sleep(2000); + + int sessionRefreshCalledCount = com.example.TestUtils.getRefreshTokenCounter(); + if (sessionRefreshCalledCount != 0) { + throw new Exception("Expected session refresh endpoint to be called 0 times but it was called " + sessionRefreshCalledCount + " times"); + } + + HttpURLConnection userInfoRequestConnection = SuperTokensHttpURLConnection.newRequest(new URL(userInfoAPIURL), new SuperTokensHttpURLConnection.PreConnectCallback() { + @Override + public void doAction(HttpURLConnection con) throws IOException { + con.setRequestMethod("GET"); + con.setRequestProperty("Authorization", "Bearer " + expiredAccessToken); + } + }); + + if (userInfoRequestConnection.getResponseCode() != 200) { + throw new Exception("userInfo api failed"); + } + + sessionRefreshCalledCount = com.example.TestUtils.getRefreshTokenCounter(); + if (sessionRefreshCalledCount != 1) { + throw new Exception("Expected session refresh endpoint to be called 1 time but it was called " + sessionRefreshCalledCount + " times"); + } + + userInfoRequestConnection.disconnect(); + } } diff --git a/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensOkHttpHeaderTests.java b/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensOkHttpHeaderTests.java index 8e7149d..ec5fecd 100644 --- a/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensOkHttpHeaderTests.java +++ b/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensOkHttpHeaderTests.java @@ -1072,7 +1072,7 @@ public void okhttpHeaders_testThatFrontTokenRemoveRemovesAccessAndRefreshAsWell( } @Test - public void okhttpHeaders_testThatAuthHeaderIsNotIgnoredEvenIfItMatchesTheStoredAccessToken() throws Exception { + public void okhttpHeaders_testThatAuthHeaderIsNotIgnoredIfItDoesntMatchTheStoredAccessToken() throws Exception { com.example.TestUtils.startST(); new SuperTokens.Builder(context, Constants.apiDomain) .build(); @@ -1092,9 +1092,6 @@ public void okhttpHeaders_testThatAuthHeaderIsNotIgnoredEvenIfItMatchesTheStored } loginResponse.close(); - Thread.sleep(5000); - Utils.setToken(Utils.TokenType.ACCESS, "myOwnHeHe", context); - Request request2 = new Request.Builder() .url(baseCustomAuthUrl) .addHeader("Accept", "application/json") @@ -1221,6 +1218,54 @@ public void okhttpHeaders_testShouldNotDoSessionRefreshIfMaxRetryAttemptsForSess } } + @Test + public void okHttpHeaders_shouldNotEndUpInRefreshLoopIfExpiredAccessTokenWasPassedInHeaders() throws Exception { + com.example.TestUtils.startST(1, true, 144000); + new SuperTokens.Builder(context, Constants.apiDomain) + .build(); + JsonObject bodyJson = new JsonObject(); + bodyJson.addProperty("userId", Constants.userId); + RequestBody body = RequestBody.create(MediaType.parse("application/json; charset=utf-8"), bodyJson.toString()); + Request request = new Request.Builder() + .url(loginAPIURL) + .method("POST", body) + .addHeader("Accept", "application/json") + .addHeader("Content-Type", "application/json") + .build(); + Response loginResponse = okHttpClient.newCall(request).execute(); + if (loginResponse.code() != 200) { + throw new Exception("Error making login request"); + } + loginResponse.close(); + + // wait for the token to expire + Thread.sleep(2000); + + int sessionRefreshCalledCount = com.example.TestUtils.getRefreshTokenCounter(); + if (sessionRefreshCalledCount != 0) { + throw new Exception("Expected session refresh endpoint to be called 0 times but it was called " + sessionRefreshCalledCount + " times"); + } + + String accessToken = Utils.getTokenForHeaderAuth(Utils.TokenType.ACCESS, context); + + Request userInfoRequest = new Request.Builder() + .url(userInfoAPIURL) + .addHeader("Authorization", "Bearer " + accessToken) + .build(); + + Response userInfoResponse = okHttpClient.newCall(userInfoRequest).execute(); + if (userInfoResponse.code() != 200) { + throw new Exception("User info API failed even after calling refresh"); + } + + userInfoResponse.close(); + + sessionRefreshCalledCount = com.example.TestUtils.getRefreshTokenCounter(); + if (sessionRefreshCalledCount != 1) { + throw new Exception("Expected session refresh endpoint to be called 1 time but it was called " + sessionRefreshCalledCount + " times"); + } + } + //custom interceptors class customInterceptors implements Interceptor { @NotNull diff --git a/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensRetrofitHeaderTests.java b/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensRetrofitHeaderTests.java index b0f4de8..77d6dc1 100644 --- a/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensRetrofitHeaderTests.java +++ b/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensRetrofitHeaderTests.java @@ -724,7 +724,7 @@ public void retrofitHeaders_testThatFrontTokenRemoveRemovesAccessAndRefreshAsWel } @Test - public void retrofitHeaders_testThatAuthHeaderIsNotIgnoredEvenIfItMatchesTheStoredAccessToken() throws Exception { + public void retrofitHeaders_testThatAuthHeaderIsNotIgnoredIfItDoesntMatchTheStoredAccessToken() throws Exception { com.example.TestUtils.startST(); new SuperTokens.Builder(context, Constants.apiDomain) .build(); @@ -736,9 +736,6 @@ public void retrofitHeaders_testThatAuthHeaderIsNotIgnoredEvenIfItMatchesTheStor throw new Exception("Error making login request"); } - Thread.sleep(5000); - Utils.setToken(Utils.TokenType.ACCESS, "myOwnHeHe", context); - Response response2 = retrofitTestAPIService.baseCustomAuth("Bearer myOwnHeHe").execute(); if (response2.code() != 200) { throw new Exception("Error making api request"); @@ -822,6 +819,39 @@ public void retrofitHeaders_testShouldNotDoSessionRefreshIfMaxRetryAttemptsForSe } } + @Test + public void retrofitHeaders_shouldNotEndUpInRefreshLoopIfExpiredAccessTokenWasPassedInHeaders() throws Exception { + com.example.TestUtils.startST(1, true, 144000); + new SuperTokens.Builder(context, Constants.apiDomain) + .build(); + + JsonObject body = new JsonObject(); + body.addProperty("userId", Constants.userId); + Response loginResponse = retrofitTestAPIService.login(body).execute(); + if (loginResponse.code() != 200) { + throw new Exception("Error making login request"); + } + + // wait for the token to expire + Thread.sleep(2000); + + int sessionRefreshCalledCount = com.example.TestUtils.getRefreshTokenCounter(); + if (sessionRefreshCalledCount != 0) { + throw new Exception("Expected session refresh endpoint to be called 0 times but it was called " + sessionRefreshCalledCount + " times"); + } + + Response userInfoResponse = retrofitTestAPIService.userInfo().execute(); + if (userInfoResponse.code() != 200) { + throw new Exception("User info API failed even after calling refresh"); + } + + sessionRefreshCalledCount = com.example.TestUtils.getRefreshTokenCounter(); + if (sessionRefreshCalledCount != 1) { + throw new Exception("Expected session refresh endpoint to be called 1 time but it was called " + sessionRefreshCalledCount + " times"); + } + + } + class customInterceptors implements Interceptor { @NotNull @Override From 1b71201138accdde6d14589c85c9425fd38c704f Mon Sep 17 00:00:00 2001 From: Ankit Tiwari Date: Mon, 17 Jun 2024 12:52:40 +0530 Subject: [PATCH 2/2] fix: PR changes --- .../SuperTokensCustomHttpURLConnection.java | 2 +- .../session/SuperTokensInterceptor.java | 2 +- .../SuperTokensHttpURLConnectionTest.java | 61 ++++++++++++++++++- 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/supertokens/session/SuperTokensCustomHttpURLConnection.java b/app/src/main/java/com/supertokens/session/SuperTokensCustomHttpURLConnection.java index 7c91197..4d70519 100644 --- a/app/src/main/java/com/supertokens/session/SuperTokensCustomHttpURLConnection.java +++ b/app/src/main/java/com/supertokens/session/SuperTokensCustomHttpURLConnection.java @@ -271,7 +271,7 @@ private boolean shouldAllowSettingAuthHeader(String value) { String accessToken = Utils.getTokenForHeaderAuth(Utils.TokenType.ACCESS, applicationContext); String refreshToken = Utils.getTokenForHeaderAuth(Utils.TokenType.REFRESH, applicationContext); - if (accessToken != null && refreshToken != null && value.equals("Bearer " + accessToken)) { + if (accessToken != null && refreshToken != null && (value.equals("Bearer " + accessToken) || value.equals("bearer " + accessToken))) { wasAuthHeaderRemovedInitially = true; // We ignore the attempt to set the header because it matches the existing access token // which will get added by the SDK diff --git a/app/src/main/java/com/supertokens/session/SuperTokensInterceptor.java b/app/src/main/java/com/supertokens/session/SuperTokensInterceptor.java index 2a35483..6bc3d15 100644 --- a/app/src/main/java/com/supertokens/session/SuperTokensInterceptor.java +++ b/app/src/main/java/com/supertokens/session/SuperTokensInterceptor.java @@ -48,7 +48,7 @@ private boolean shouldRemoveAuthHeader(Request request, Context context) { String accessToken = Utils.getTokenForHeaderAuth(Utils.TokenType.ACCESS, context); String refreshToken = Utils.getTokenForHeaderAuth(Utils.TokenType.REFRESH, context); - if (accessToken != null && refreshToken != null && originalHeader.equals("Bearer " + accessToken)) { + if (accessToken != null && refreshToken != null && (originalHeader.equals("Bearer " + accessToken) || originalHeader.equals("bearer " + accessToken))) { return true; } } diff --git a/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensHttpURLConnectionTest.java b/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensHttpURLConnectionTest.java index 22878c1..4a69205 100644 --- a/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensHttpURLConnectionTest.java +++ b/testHelpers/testapp/app/src/test/java/com/example/example/SuperTokensHttpURLConnectionTest.java @@ -1103,7 +1103,7 @@ public void doAction(HttpURLConnection con) throws IOException { } @Test - public void httpUrlConnection_shouldNotEndUpInRefreshLoopIfExpiredAccessTokenWasPassedInHeaders() throws Exception{ + public void httpUrlConnection_shouldNotRefreshLoopWhenExpiredTokenPassedInHeadersBeforeDoAction() throws Exception{ com.example.TestUtils.startST(1, true, 144000); new SuperTokens.Builder(context, Constants.apiDomain).build(); @@ -1131,8 +1131,65 @@ public void doAction(HttpURLConnection con) throws IOException { loginRequestConnection.disconnect(); + // wait for access token expiry + Thread.sleep(2000); + String expiredAccessToken = Utils.getTokenForHeaderAuth(Utils.TokenType.ACCESS, context); + int sessionRefreshCalledCount = com.example.TestUtils.getRefreshTokenCounter(); + if (sessionRefreshCalledCount != 0) { + throw new Exception("Expected session refresh endpoint to be called 0 times but it was called " + sessionRefreshCalledCount + " times"); + } + + HttpURLConnection userInfoRequestConnection = SuperTokensHttpURLConnection.newRequest(new URL(userInfoAPIURL), new SuperTokensHttpURLConnection.PreConnectCallback() { + @Override + public void doAction(HttpURLConnection con) throws IOException { + con.setRequestMethod("GET"); + con.setRequestProperty("Authorization", "Bearer " + expiredAccessToken); + } + }); + + if (userInfoRequestConnection.getResponseCode() != 200) { + throw new Exception("userInfo api failed"); + } + + sessionRefreshCalledCount = com.example.TestUtils.getRefreshTokenCounter(); + if (sessionRefreshCalledCount != 1) { + throw new Exception("Expected session refresh endpoint to be called 1 time but it was called " + sessionRefreshCalledCount + " times"); + } + + userInfoRequestConnection.disconnect(); + } + + @Test + public void httpUrlConnection_shouldNotRefreshLoopWhenExpiredTokenRetrievedInsideDoAction() throws Exception{ + com.example.TestUtils.startST(1, true, 144000); + new SuperTokens.Builder(context, Constants.apiDomain).build(); + + //login request + HttpURLConnection loginRequestConnection = SuperTokensHttpURLConnection.newRequest(new URL(loginAPIURL), new SuperTokensHttpURLConnection.PreConnectCallback() { + @Override + public void doAction(HttpURLConnection con) throws IOException { + con.setDoOutput(true); + con.setRequestMethod("POST"); + con.setRequestProperty("Accept", "application/json"); + con.setRequestProperty("Content-Type", "application/json"); + + JsonObject bodyJson = new JsonObject(); + bodyJson.addProperty("userId", Constants.userId); + + OutputStream outputStream = con.getOutputStream(); + outputStream.write(bodyJson.toString().getBytes(StandardCharsets.UTF_8)); + outputStream.close(); + } + }); + + if (loginRequestConnection.getResponseCode() != 200) { + throw new Exception("Login request failed"); + } + + loginRequestConnection.disconnect(); + // wait for access token expiry Thread.sleep(2000); @@ -1145,6 +1202,8 @@ public void doAction(HttpURLConnection con) throws IOException { @Override public void doAction(HttpURLConnection con) throws IOException { con.setRequestMethod("GET"); + + String expiredAccessToken = Utils.getTokenForHeaderAuth(Utils.TokenType.ACCESS, context); con.setRequestProperty("Authorization", "Bearer " + expiredAccessToken); } });