merge 9.0

CHANGELOG.md

@@ -9,6 +9,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 - Add a new core API for fetching all the core properties
 
+## [9.0.1] - 2024-03-20
+
+- Fixes verify TOTP and verify device APIs to treat any code as invalid
+- Fixes the computation of the number of failed attempts when return `INVALID_TOTP_ERROR`
+
 ## [9.0.0] - 2024-03-13
 
 ### Added

README.md

@@ -232,6 +232,10 @@ Melvyn Hills</b></sub></a></td>
 <td align="center"><a href="https://github.com/daniil-borovoy"><img src="https://avatars.githubusercontent.com/u/74528634?v=4" width="100px;" alt=""/><br /><sub><b>Daniil Borovoy</b></sub></a></td>
 <td align="center"><a href="https://github.com/kriskw1999"><img src="https://avatars.githubusercontent.com/u/71312948?v=4" width="100px;" alt=""/><br /><sub><b>Krzysztof Witkowski</b></sub></a></td>
   </tr>
+  <tr>
+<td align="center"><a href="https://github.com/Lehoczky"><img src="https://avatars.githubusercontent.com/u/31937175?v=4" width="100px;" alt=""/><br /><sub><b>Lehoczky Zoltán</b></sub></a></td>
+Melvyn Hills</b></sub></a></td>
+  </tr>
 </table>
 
 ## 👩‍💻 Contributing

build.gradle

@@ -19,7 +19,7 @@ compileTestJava { options.encoding = "UTF-8" }
 //    }
 //}
 
-version = "9.0.0"
+version = "9.0.1"
 
 
 repositories {

src/main/java/io/supertokens/totp/Totp.java

@@ -203,17 +203,28 @@ private static void checkAndStoreCode(TenantIdentifier tenantIdentifier, Storage
 
                     // N represents # of invalid attempts that will trigger rate limiting:
                     int N = Config.getConfig(tenantIdentifier, main).getTotpMaxAttempts(); // (Default 5)
-                    // Count # of contiguous invalids in latest N attempts (stop at first valid):
-                    long invalidOutOfN = Arrays.stream(usedCodes).limit(N).takeWhile(usedCode -> !usedCode.isValid)
-                            .count();
+
+                    // filter only invalid codes, stop at first valid code
+                    TOTPUsedCode[] invalidUsedCodes = Arrays.stream(usedCodes).limit(N).takeWhile(
+                            usedCode -> !usedCode.isValid).toArray(TOTPUsedCode[]::new);
+
                     int rateLimitResetTimeInMs = Config.getConfig(tenantIdentifier, main)
                             .getTotpRateLimitCooldownTimeSec() *
                             1000; // (Default 15 mins)
 
+                    // Count how many of the latest invalid codes fall under the rate limiting compared
+                    // to the latest invalid attempt
+                    long invalidOutOfN = 0;
+                    if (invalidUsedCodes.length > 0) {
+                        invalidOutOfN = Arrays.stream(invalidUsedCodes).limit(N).takeWhile(
+                                usedCode -> usedCode.createdTime > invalidUsedCodes[0].createdTime - rateLimitResetTimeInMs
+                            ).count();
+                    }
+
                     // Check if the user has been rate limited:
                     if (invalidOutOfN == N) {
                         // All of the latest N attempts were invalid:
-                        long latestInvalidCodeCreatedTime = usedCodes[0].createdTime;
+                        long latestInvalidCodeCreatedTime = invalidUsedCodes[0].createdTime;
                         long now = System.currentTimeMillis();
 
                         if (now - latestInvalidCodeCreatedTime < rateLimitResetTimeInMs) {
@@ -225,6 +236,9 @@ private static void checkAndStoreCode(TenantIdentifier tenantIdentifier, Storage
                             // If we insert the used code here, then it will further delay the user from
                             // being able to login. So not inserting it here.
                         }
+
+                        // since we are past the cool down period, the user can retry all the attempts
+                        invalidOutOfN = 0;
                     }
 
                     // Check if the code is valid for any device:

src/main/java/io/supertokens/webserver/api/session/SessionRemoveAPI.java

@@ -113,19 +113,14 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I
                 String[] sessionHandlesRevoked;
 
                 if (revokeAcrossAllTenants) {
-                    // when revokeAcrossAllTenants is true, and given that the backend SDK might pass tenant id
-                    // we do not want to enforce public tenant here but behave as if this is an app specific API
-                    // So instead of calling enforcePublicTenantAndGetAllStoragesForApp, we simply do all the logic
-                    // here to fetch the storages and find the storage where `userId` exists. If user id does not
-                    // exist, we use the storage for the tenantId passed in the request.
                     AppIdentifier appIdentifier = getAppIdentifier(req);
-                    Storage[] storages = StorageLayer.getStoragesForApp(main, appIdentifier);
                     try {
-                        StorageAndUserIdMapping storageAndUserIdMapping = StorageLayer.findStorageAndUserIdMappingForUser(
-                                appIdentifier, storages, userId, UserIdType.ANY);
+                        StorageAndUserIdMapping storageAndUserIdMapping =
+                                enforcePublicTenantAndGetStorageAndUserIdMappingForAppSpecificApi(
+                                        req, userId, UserIdType.ANY, false);
                         storage = storageAndUserIdMapping.storage;
                     } catch (UnknownUserIdException e) {
-                        storage = getTenantStorage(req);
+                        throw new IllegalStateException("should never happen");
                     }
                     sessionHandlesRevoked = Session.revokeAllSessionsForUser(
                             main, appIdentifier, storage, userId, revokeSessionsForLinkedAccounts);
@@ -159,7 +154,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I
                 }
                 result.add("sessionHandlesRevoked", sessionHandlesRevokedJSON);
                 super.sendJsonResponse(200, result, resp);
-            } catch (StorageQueryException | TenantOrAppNotFoundException e) {
+            } catch (StorageQueryException | TenantOrAppNotFoundException | BadPermissionException e) {
                 throw new ServletException(e);
             }
         } else {

src/main/java/io/supertokens/webserver/api/session/SessionUserAPI.java

@@ -77,21 +77,15 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO
             String[] sessionHandles;
 
             if (fetchAcrossAllTenants) {
-                // when fetchAcrossAllTenants is true, and given that the backend SDK might pass tenant id
-                // we do not want to enforce public tenant here but behave as if this is an app specific API
-                // So instead of calling enforcePublicTenantAndGetAllStoragesForApp, we simply do all the logic
-                // here to fetch the storages and find the storage where `userId` exists. If user id does not
-                // exist, we use the storage for the tenantId passed in the request.
                 AppIdentifier appIdentifier = getAppIdentifier(req);
-                Storage[] storages = StorageLayer.getStoragesForApp(main, appIdentifier);
                 Storage storage;
                 try {
                     StorageAndUserIdMapping storageAndUserIdMapping =
-                            StorageLayer.findStorageAndUserIdMappingForUser(
-                            appIdentifier, storages, userId, UserIdType.ANY);
+                            enforcePublicTenantAndGetStorageAndUserIdMappingForAppSpecificApi(
+                            req, userId, UserIdType.ANY, false);
                     storage = storageAndUserIdMapping.storage;
                 } catch (UnknownUserIdException e) {
-                    storage = getTenantStorage(req);
+                    throw new IllegalStateException("should never happen");
                 }
                 sessionHandles = Session.getAllNonExpiredSessionHandlesForUser(
                         main, appIdentifier, storage, userId,
@@ -112,7 +106,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO
             result.add("sessionHandles", arr);
             super.sendJsonResponse(200, result, resp);
 
-        } catch (StorageQueryException | TenantOrAppNotFoundException e) {
+        } catch (StorageQueryException | TenantOrAppNotFoundException | BadPermissionException e) {
             throw new ServletException(e);
         }
     }

src/main/java/io/supertokens/webserver/api/totp/VerifyTotpAPI.java

@@ -46,9 +46,6 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I
         if (userId.isEmpty()) {
             throw new ServletException(new BadRequestException("userId cannot be empty"));
         }
-        if (totp.length() != 6) {
-            throw new ServletException(new BadRequestException("totp must be 6 characters long"));
-        }
 
         JsonObject result = new JsonObject();
 

src/main/java/io/supertokens/webserver/api/totp/VerifyTotpDeviceAPI.java

@@ -50,9 +50,6 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I
         if (deviceName.isEmpty()) {
             throw new ServletException(new BadRequestException("deviceName cannot be empty"));
         }
-        if (totp.length() != 6) {
-            throw new ServletException(new BadRequestException("totp must be 6 characters long"));
-        }
 
         JsonObject result = new JsonObject();
 

src/test/java/io/supertokens/test/multitenant/api/TestWithNonAuthRecipes.java

@@ -34,18 +34,24 @@
 import io.supertokens.pluginInterface.multitenancy.TenantIdentifier;
 import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException;
 import io.supertokens.pluginInterface.usermetadata.sqlStorage.UserMetadataSQLStorage;
+import io.supertokens.session.Session;
+import io.supertokens.session.info.SessionInformationHolder;
 import io.supertokens.storageLayer.StorageLayer;
 import io.supertokens.test.TestingProcessManager;
 import io.supertokens.test.Utils;
+import io.supertokens.test.httpRequest.HttpRequestForTesting;
 import io.supertokens.test.httpRequest.HttpResponseException;
 import io.supertokens.thirdparty.InvalidProviderConfigException;
 import io.supertokens.useridmapping.UserIdMapping;
+import io.supertokens.utils.SemVer;
 import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 
 import static org.junit.Assert.*;
 
@@ -375,4 +381,91 @@ public void testEmailVerificationWithUsersOnDifferentTenantStorages() throws Exc
         assertFalse(t1EvStorage.isEmailVerified(t0.toAppIdentifier(), user2.getSupertokensUserId(), "[email protected]")); // ensure t1 storage does not have user2's ev
         assertFalse(t0EvStorage.isEmailVerified(t0.toAppIdentifier(), user1.getSupertokensUserId(), "[email protected]")); // ensure t0 storage does not have user1's ev
     }
+
+    @Test
+    public void testSessionCannotGetAcrossAllStorageOrRevokedAcrossAllTenantsFromNonPublicTenant() throws Exception {
+        if (StorageLayer.getBaseStorage(process.getProcess()).getType() != STORAGE_TYPE.SQL) {
+            return;
+        }
+
+        if (StorageLayer.isInMemDb(process.getProcess())) {
+            return;
+        }
+
+        TenantIdentifier t0 = new TenantIdentifier(null, null, null);
+        Storage t0Storage = (StorageLayer.getStorage(t0, process.getProcess()));
+
+        TenantIdentifier t1 = new TenantIdentifier(null, null, "t1");
+        Storage t1Storage = (StorageLayer.getStorage(t1, process.getProcess()));
+
+        // Create users
+        AuthRecipeUserInfo user1 = EmailPassword.signUp(t0, t0Storage, process.getProcess(), "[email protected]", "password123");
+        AuthRecipeUserInfo user2 = EmailPassword.signUp(t1, t1Storage, process.getProcess(), "[email protected]", "password123");
+
+        UserIdMapping.populateExternalUserIdForUsers(t0.toAppIdentifier(), t0Storage, new AuthRecipeUserInfo[]{user1});
+        UserIdMapping.populateExternalUserIdForUsers(t1.toAppIdentifier(), t1Storage, new AuthRecipeUserInfo[]{user2});
+
+        SessionInformationHolder sess1 = Session.createNewSession(t0, t0Storage,
+                process.getProcess(), user1.getSupertokensUserId(), new JsonObject(), new JsonObject());
+        SessionInformationHolder sess2 = Session.createNewSession(t1, t1Storage,
+                process.getProcess(), user2.getSupertokensUserId(), new JsonObject(), new JsonObject());
+
+        {
+            Map<String, String> params = new HashMap<>();
+            params.put("fetchAcrossAllTenants", "true");
+            params.put("userId", user1.getSupertokensUserId());
+
+            JsonObject sessionResponse = HttpRequestForTesting.sendGETRequest(process.getProcess(), "",
+                    HttpRequestForTesting.getMultitenantUrl(t1, "/recipe/session/user"),
+                    params, 1000, 1000, null, SemVer.v4_0.get(),
+                    "session");
+            assertEquals("OK", sessionResponse.get("status").getAsString());
+            assertEquals(1, sessionResponse.get("sessionHandles").getAsJsonArray().size());
+            assertEquals(sess1.session.handle, sessionResponse.get("sessionHandles").getAsJsonArray().get(0).getAsString());
+        }
+
+        {
+            try {
+                Map<String, String> params = new HashMap<>();
+                params.put("fetchAcrossAllTenants", "true");
+                params.put("userId", user1.getSupertokensUserId());
+
+                JsonObject sessionResponse = HttpRequestForTesting.sendGETRequest(process.getProcess(), "",
+                        HttpRequestForTesting.getMultitenantUrl(t1, "/recipe/session/user"),
+                        params, 1000, 1000, null, SemVer.v5_0.get(),
+                        "session");
+                fail();
+            } catch (HttpResponseException e) {
+                assertEquals(403, e.statusCode);
+            }
+        }
+
+        {
+            try {
+                JsonObject requestBody = new JsonObject();
+                requestBody.addProperty("userId", user1.getSupertokensUserId());
+                requestBody.addProperty("revokeAcrossAllTenants", true);
+
+                JsonObject sessionResponse = HttpRequestForTesting.sendJsonPOSTRequest(process.getProcess(), "",
+                        HttpRequestForTesting.getMultitenantUrl(t1, "/recipe/session/remove"), requestBody,
+                        1000, 1000, null, SemVer.v5_0.get(),
+                        "session");
+                fail();
+            } catch (HttpResponseException e) {
+                assertEquals(403, e.statusCode);
+            }
+        }
+
+        {
+            JsonObject requestBody = new JsonObject();
+            requestBody.addProperty("userId", user1.getSupertokensUserId());
+            requestBody.addProperty("revokeAcrossAllTenants", true);
+
+            JsonObject sessionResponse = HttpRequestForTesting.sendJsonPOSTRequest(process.getProcess(), "",
+                    HttpRequestForTesting.getMultitenantUrl(t1, "/recipe/session/remove"), requestBody,
+                    1000, 1000, null, SemVer.v4_0.get(),
+                    "session");
+            assertEquals("OK", sessionResponse.get("status").getAsString());
+        }
+    }
 }

src/test/java/io/supertokens/test/totp/TOTPRecipeTest.java

@@ -348,13 +348,33 @@ public void rateLimitCooldownTest() throws Exception {
         // Wait for 1 second (Should cool down rate limiting):
         Thread.sleep(1000);
         // But again try with invalid code:
-        assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "user", "invalid0"));
+        InvalidTotpException invalidTotpException = assertThrows(InvalidTotpException.class,
+                () -> Totp.verifyCode(main, "user", "invalid0"));
+        assertEquals(1, invalidTotpException.currentAttempts);
+        invalidTotpException = assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "user", "invalid0"));
+        assertEquals(2, invalidTotpException.currentAttempts);
+        invalidTotpException = assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "user", "invalid0"));
+        assertEquals(3, invalidTotpException.currentAttempts);
+
         // This triggered rate limiting again. So even valid codes will fail for
         // another cooldown period:
-        assertThrows(LimitReachedException.class,
+        LimitReachedException limitReachedException = assertThrows(LimitReachedException.class,
                 () -> Totp.verifyCode(main, "user", generateTotpCode(main, device)));
+        assertEquals(3, limitReachedException.currentAttempts);
         // Wait for 1 second (Should cool down rate limiting):
         Thread.sleep(1000);
+
+        // test that after cool down, we can retry invalid codes N times again
+        invalidTotpException = assertThrows(InvalidTotpException.class,
+                () -> Totp.verifyCode(main, "user", "invalid0"));
+        assertEquals(1, invalidTotpException.currentAttempts);
+        invalidTotpException = assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "user", "invalid0"));
+        assertEquals(2, invalidTotpException.currentAttempts);
+        invalidTotpException = assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "user", "invalid0"));
+        assertEquals(3, invalidTotpException.currentAttempts);
+
+        Thread.sleep(1100);
+
         // Now try with valid code:
         Totp.verifyCode(main, "user", generateTotpCode(main, device));
         // Now invalid code shouldn't trigger rate limiting. Unless you do it N times:

src/test/java/io/supertokens/test/totp/api/VerifyTotpAPITest.java

@@ -23,6 +23,8 @@
 import org.junit.Test;
 import org.junit.rules.TestRule;
 
+import java.io.IOException;
+
 import static io.supertokens.test.totp.TOTPRecipeTest.generateTotpCode;
 import static org.junit.Assert.*;
 
@@ -56,6 +58,21 @@ private Exception verifyTotpCodeRequest(TestingProcessManager.TestingProcess pro
                         "totp"));
     }
 
+    private void verifyTotpRequestThatReturnsInvalidCode(TestingProcessManager.TestingProcess process, JsonObject body)
+            throws HttpResponseException, IOException {
+        JsonObject resp = HttpRequestForTesting.sendJsonPOSTRequest(
+                process.getProcess(),
+                "",
+                "http://localhost:3567/recipe/totp/verify",
+                body,
+                1000,
+                1000,
+                null,
+                Utils.getCdiVersionStringLatestForTests(),
+                "totp");
+        assertEquals("INVALID_TOTP_ERROR", resp.get("status").getAsString());
+    }
+
     private void checkFieldMissingErrorResponse(Exception ex, String fieldName) {
         assert ex instanceof HttpResponseException;
         HttpResponseException e = (HttpResponseException) ex;
@@ -149,18 +166,27 @@ public void testApi() throws Exception {
             checkResponseErrorContains(e, "userId cannot be empty"); // Note that this is not a field missing error
 
             body.addProperty("userId", device.userId);
-            e = verifyTotpCodeRequest(process, body);
-            checkResponseErrorContains(e, "totp must be 6 characters long");
+            verifyTotpRequestThatReturnsInvalidCode(process, body);
+
+            Thread.sleep(1100);
 
             // test totp of length 5:
             body.addProperty("totp", "12345");
-            e = verifyTotpCodeRequest(process, body);
-            checkResponseErrorContains(e, "totp must be 6 characters long");
+            verifyTotpRequestThatReturnsInvalidCode(process, body);
+
+            Thread.sleep(1100);
+
+            // test totp of alphabets:
+            body.addProperty("totp", "abcd");
+            verifyTotpRequestThatReturnsInvalidCode(process, body);
+
+            Thread.sleep(1100);
 
             // test totp of length 8:
             body.addProperty("totp", "12345678");
-            e = verifyTotpCodeRequest(process, body);
-            checkResponseErrorContains(e, "totp must be 6 characters long");
+            verifyTotpRequestThatReturnsInvalidCode(process, body);
+
+            Thread.sleep(1100);
 
             // but let's pass invalid code first
             body.addProperty("totp", "123456");