diff --git a/src/main/java/io/supertokens/inmemorydb/Start.java b/src/main/java/io/supertokens/inmemorydb/Start.java index f3b57ae1b..6becde092 100644 --- a/src/main/java/io/supertokens/inmemorydb/Start.java +++ b/src/main/java/io/supertokens/inmemorydb/Start.java @@ -70,8 +70,8 @@ import io.supertokens.pluginInterface.totp.TOTPStorage; import io.supertokens.pluginInterface.totp.TOTPUsedCode; import io.supertokens.pluginInterface.totp.exception.DeviceAlreadyExistsException; -import io.supertokens.pluginInterface.totp.exception.TotpNotEnabledException; import io.supertokens.pluginInterface.totp.exception.UnknownDeviceException; +import io.supertokens.pluginInterface.totp.exception.UnknownTotpUserIdException; import io.supertokens.pluginInterface.totp.exception.UsedCodeAlreadyExistsException; import io.supertokens.pluginInterface.totp.sqlStorage.TOTPSQLStorage; import io.supertokens.pluginInterface.useridmapping.UserIdMapping; @@ -2601,29 +2601,40 @@ public void revokeExpiredSessions() throws StorageQueryException { // TOTP recipe: @Override public void createDevice(AppIdentifier appIdentifier, TOTPDevice device) - throws StorageQueryException, DeviceAlreadyExistsException, TenantOrAppNotFoundException { + throws DeviceAlreadyExistsException, TenantOrAppNotFoundException, StorageQueryException { try { - TOTPQueries.createDevice(this, appIdentifier, device); - } catch (StorageTransactionLogicException e) { - if (e.actualException instanceof SQLiteException) { - String errMsg = e.actualException.getMessage(); - - if (isPrimaryKeyError(errMsg, Config.getConfig(this).getTotpUserDevicesTable(), - new String[]{"app_id", "user_id", "device_name"})) { - throw new DeviceAlreadyExistsException(); - } else if (isForeignKeyConstraintError( - errMsg, - Config.getConfig(this).getAppsTable(), - new String[]{"app_id"}, - new Object[]{appIdentifier.getAppId()})) { - throw new TenantOrAppNotFoundException(appIdentifier); + startTransaction(con -> { + try { + createDevice_Transaction(con, new AppIdentifier(null, null), device); + } catch (DeviceAlreadyExistsException | TenantOrAppNotFoundException e) { + throw new StorageTransactionLogicException(e); } + return null; + }); + } catch (StorageTransactionLogicException e) { + if (e.actualException instanceof DeviceAlreadyExistsException) { + throw (DeviceAlreadyExistsException) e.actualException; + } else if (e.actualException instanceof TenantOrAppNotFoundException) { + throw (TenantOrAppNotFoundException) e.actualException; + } else if (e.actualException instanceof StorageQueryException) { + throw (StorageQueryException) e.actualException; } - - throw new StorageQueryException(e.actualException); } } + @Override + public TOTPDevice createDevice_Transaction(TransactionConnection con, AppIdentifier appIdentifier, TOTPDevice device) + throws DeviceAlreadyExistsException, TenantOrAppNotFoundException, StorageQueryException { + // TODO + return null; + } + + @Override + public TOTPDevice getDeviceByName_Transaction(TransactionConnection con, AppIdentifier appIdentifier, String userId, String deviceName) throws StorageQueryException { + // TODO + return null; + } + @Override public void markDeviceAsVerified(AppIdentifier appIdentifier, String userId, String deviceName) throws StorageQueryException, UnknownDeviceException { @@ -2717,7 +2728,7 @@ public TOTPDevice[] getDevices_Transaction(TransactionConnection con, AppIdentif @Override public void insertUsedCode_Transaction(TransactionConnection con, TenantIdentifier tenantIdentifier, TOTPUsedCode usedCodeObj) - throws StorageQueryException, TotpNotEnabledException, UsedCodeAlreadyExistsException, + throws StorageQueryException, UnknownTotpUserIdException, UsedCodeAlreadyExistsException, TenantOrAppNotFoundException { Connection sqlCon = (Connection) con.getConnection(); try { @@ -2732,7 +2743,7 @@ public void insertUsedCode_Transaction(TransactionConnection con, TenantIdentifi Config.getConfig(this).getTotpUsersTable(), new String[]{"app_id", "user_id"}, new Object[]{tenantIdentifier.getAppId(), usedCodeObj.userId})) { - throw new TotpNotEnabledException(); + throw new UnknownTotpUserIdException(); } else if (isForeignKeyConstraintError( e.getMessage(), diff --git a/src/main/java/io/supertokens/inmemorydb/queries/ActiveUsersQueries.java b/src/main/java/io/supertokens/inmemorydb/queries/ActiveUsersQueries.java index 559d1c7d4..3bb08ca1f 100644 --- a/src/main/java/io/supertokens/inmemorydb/queries/ActiveUsersQueries.java +++ b/src/main/java/io/supertokens/inmemorydb/queries/ActiveUsersQueries.java @@ -96,7 +96,7 @@ public static int countUsersEnabledTotpAndActiveSince(Start start, AppIdentifier } public static int countUsersEnabledMfa(Start start, AppIdentifier appIdentifier) throws SQLException, StorageQueryException { - String QUERY = "SELECT COUNT(*) as total FROM (SELECT DISTINCT user_id FROM " + Config.getConfig(start).getMfaUserFactorsTable() + "WHERE app_id = ?) AS app_mfa_users"; + String QUERY = "SELECT COUNT(*) as total FROM (SELECT DISTINCT user_id FROM " + Config.getConfig(start).getMfaUserFactorsTable() + " WHERE app_id = ?) AS app_mfa_users"; return execute(start, QUERY, pst -> { pst.setString(1, appIdentifier.getAppId()); @@ -127,7 +127,8 @@ public static int countUsersEnabledMfaAndActiveSince(Start start, AppIdentifier }); } - public static int updateUserLastActive(Start start, AppIdentifier appIdentifier, String userId) throws SQLException, StorageQueryException { + public static int updateUserLastActive(Start start, AppIdentifier appIdentifier, String userId) + throws SQLException, StorageQueryException { String QUERY = "INSERT INTO " + Config.getConfig(start).getUserLastActiveTable() + "(app_id, user_id, last_active_time) VALUES(?, ?, ?) ON CONFLICT(app_id, user_id) DO UPDATE SET " + diff --git a/src/main/java/io/supertokens/totp/Totp.java b/src/main/java/io/supertokens/totp/Totp.java index 307f49471..e5f715c3b 100644 --- a/src/main/java/io/supertokens/totp/Totp.java +++ b/src/main/java/io/supertokens/totp/Totp.java @@ -15,8 +15,8 @@ import io.supertokens.pluginInterface.totp.TOTPDevice; import io.supertokens.pluginInterface.totp.TOTPUsedCode; import io.supertokens.pluginInterface.totp.exception.DeviceAlreadyExistsException; -import io.supertokens.pluginInterface.totp.exception.TotpNotEnabledException; import io.supertokens.pluginInterface.totp.exception.UnknownDeviceException; +import io.supertokens.pluginInterface.totp.exception.UnknownTotpUserIdException; import io.supertokens.pluginInterface.totp.exception.UsedCodeAlreadyExistsException; import io.supertokens.pluginInterface.totp.sqlStorage.TOTPSQLStorage; import io.supertokens.storageLayer.StorageLayer; @@ -81,12 +81,11 @@ private static boolean isTotpEnabled(AppIdentifier appIdentifier, Main main) return false; } - @TestOnly public static TOTPDevice registerDevice(Main main, String userId, - String deviceName, int skew, int period) + String deviceName, int skew, int period) throws StorageQueryException, DeviceAlreadyExistsException, NoSuchAlgorithmException, - FeatureNotEnabledException { + FeatureNotEnabledException, StorageTransactionLogicException { try { return registerDevice(new AppIdentifierWithStorage(null, null, StorageLayer.getStorage(main)), main, userId, deviceName, skew, period); @@ -95,10 +94,37 @@ public static TOTPDevice registerDevice(Main main, String userId, } } + private static TOTPDevice createDevice(AppIdentifierWithStorage appIdentifierWithStorage, TOTPDevice device) + throws DeviceAlreadyExistsException, StorageQueryException { + TOTPSQLStorage totpStorage = appIdentifierWithStorage.getTOTPStorage(); + try { + return totpStorage.startTransaction(con -> { + try { + TOTPDevice existingDevice = totpStorage.getDeviceByName_Transaction(con, appIdentifierWithStorage, device.userId, device.deviceName); + if (existingDevice == null) { + return totpStorage.createDevice_Transaction(con, appIdentifierWithStorage, device); + } else if (!existingDevice.verified) { + totpStorage.deleteDevice_Transaction(con, appIdentifierWithStorage, device.userId, device.deviceName); + return totpStorage.createDevice_Transaction(con, appIdentifierWithStorage, device); + } else { + throw new StorageTransactionLogicException(new DeviceAlreadyExistsException()); + } + } catch (TenantOrAppNotFoundException | DeviceAlreadyExistsException e) { + throw new StorageTransactionLogicException(e); + } + }); + } catch (StorageTransactionLogicException e) { + if (e.actualException instanceof DeviceAlreadyExistsException) { + throw (DeviceAlreadyExistsException) e.actualException; + } + throw new StorageQueryException(e.actualException); + } + } + public static TOTPDevice registerDevice(AppIdentifierWithStorage appIdentifierWithStorage, Main main, String userId, - String deviceName, int skew, int period) + String deviceName, int skew, int period) throws StorageQueryException, DeviceAlreadyExistsException, NoSuchAlgorithmException, - FeatureNotEnabledException, TenantOrAppNotFoundException { + FeatureNotEnabledException, TenantOrAppNotFoundException, StorageTransactionLogicException { if (!isTotpEnabled(appIdentifierWithStorage, main)) { throw new FeatureNotEnabledException( @@ -106,19 +132,38 @@ public static TOTPDevice registerDevice(AppIdentifierWithStorage appIdentifierWi "feature."); } - TOTPSQLStorage totpStorage = appIdentifierWithStorage.getTOTPStorage(); - String secret = generateSecret(); TOTPDevice device = new TOTPDevice(userId, deviceName, secret, period, skew, false); - totpStorage.createDevice(appIdentifierWithStorage, device); + TOTPSQLStorage totpStorage = appIdentifierWithStorage.getTOTPStorage(); + + if (deviceName != null) { + return createDevice(appIdentifierWithStorage, device); + } - return device; + // Find number of existing devices to set device name + TOTPDevice[] devices = totpStorage.getDevices(appIdentifierWithStorage, userId); + int verifiedDevicesCount = Arrays.stream(devices).filter(d -> d.verified).toArray().length; + + while (true) { + try { + return createDevice(appIdentifierWithStorage, new TOTPDevice( + device.userId, + "TOTP Device " + verifiedDevicesCount, + device.secretKey, + device.period, + device.skew, + device.verified + )); + } catch (DeviceAlreadyExistsException e){ + } + verifiedDevicesCount++; + } } private static void checkAndStoreCode(TenantIdentifierWithStorage tenantIdentifierWithStorage, Main main, - String userId, TOTPDevice[] devices, - String code) - throws InvalidTotpException, TotpNotEnabledException, + String userId, TOTPDevice[] devices, + String code) + throws InvalidTotpException, UnknownTotpUserIdException, LimitReachedException, StorageQueryException, StorageTransactionLogicException, TenantOrAppNotFoundException { // Note that the TOTP cron runs every 1 hour, so all the expired tokens can stay @@ -169,10 +214,9 @@ private static void checkAndStoreCode(TenantIdentifierWithStorage tenantIdentifi // Count # of contiguous invalids in latest N attempts (stop at first valid): long invalidOutOfN = Arrays.stream(usedCodes).limit(N).takeWhile(usedCode -> !usedCode.isValid) .count(); - int rateLimitResetTimeInMs = - Config.getConfig(tenantIdentifierWithStorage, main).getTotpRateLimitCooldownTimeSec() * - 1000; // (Default - // 15 mins) + int rateLimitResetTimeInMs = Config.getConfig(tenantIdentifierWithStorage, main) + .getTotpRateLimitCooldownTimeSec() * + 1000; // (Default 15 mins) // Check if the user has been rate limited: if (invalidOutOfN == N) { @@ -230,9 +274,9 @@ private static void checkAndStoreCode(TenantIdentifierWithStorage tenantIdentifi .mapToInt(device -> device.period * (2 * device.skew + 1)) .max() .orElse(0); - int expireInSec = - (matchingDevice != null) ? matchingDevice.period * (2 * matchingDevice.skew + 1) - : maxUsedCodeExpiry; + int expireInSec = (matchingDevice != null) + ? matchingDevice.period * (2 * matchingDevice.skew + 1) + : maxUsedCodeExpiry; long now = System.currentTimeMillis(); TOTPUsedCode newCode = new TOTPUsedCode(userId, @@ -241,7 +285,7 @@ private static void checkAndStoreCode(TenantIdentifierWithStorage tenantIdentifi try { totpSQLStorage.insertUsedCode_Transaction(con, tenantIdentifierWithStorage, newCode); totpSQLStorage.commitTransaction(con); - } catch (UsedCodeAlreadyExistsException | TotpNotEnabledException e) { + } catch (UsedCodeAlreadyExistsException | UnknownTotpUserIdException e) { throw new StorageTransactionLogicException(e); } @@ -264,16 +308,10 @@ private static void checkAndStoreCode(TenantIdentifierWithStorage tenantIdentifi throw (LimitReachedException) e.actualException; } else if (e.actualException instanceof InvalidTotpException) { throw (InvalidTotpException) e.actualException; - } else if (e.actualException instanceof TotpNotEnabledException) { - throw (TotpNotEnabledException) e.actualException; + } else if (e.actualException instanceof UnknownTotpUserIdException) { + throw (UnknownTotpUserIdException) e.actualException; } else if (e.actualException instanceof UsedCodeAlreadyExistsException) { - // retry the transaction after a small delay: - int delayInMs = (int) (Math.random() * 10 + 1); - try { - Thread.sleep(delayInMs); - } catch (InterruptedException ignored) { - // ignore the error and retry - } + throw new InvalidTotpException(); } else { throw e; } @@ -283,8 +321,8 @@ private static void checkAndStoreCode(TenantIdentifierWithStorage tenantIdentifi @TestOnly public static boolean verifyDevice(Main main, - String userId, String deviceName, String code) - throws TotpNotEnabledException, UnknownDeviceException, InvalidTotpException, + String userId, String deviceName, String code) + throws UnknownDeviceException, InvalidTotpException, LimitReachedException, StorageQueryException, StorageTransactionLogicException { try { return verifyDevice(new TenantIdentifierWithStorage(null, null, null, StorageLayer.getStorage(main)), main, @@ -295,8 +333,8 @@ public static boolean verifyDevice(Main main, } public static boolean verifyDevice(TenantIdentifierWithStorage tenantIdentifierWithStorage, Main main, - String userId, String deviceName, String code) - throws TotpNotEnabledException, UnknownDeviceException, InvalidTotpException, + String userId, String deviceName, String code) + throws UnknownDeviceException, InvalidTotpException, LimitReachedException, StorageQueryException, StorageTransactionLogicException, TenantOrAppNotFoundException { // Here boolean return value tells whether the device has been @@ -312,7 +350,7 @@ public static boolean verifyDevice(TenantIdentifierWithStorage tenantIdentifierW // Check if the user has any devices: TOTPDevice[] devices = totpStorage.getDevices(tenantIdentifierWithStorage.toAppIdentifier(), userId); if (devices.length == 0) { - throw new TotpNotEnabledException(); + throw new UnknownDeviceException(); } // Check if the requested device exists: @@ -337,29 +375,31 @@ public static boolean verifyDevice(TenantIdentifierWithStorage tenantIdentifierW // verified in the devices table (because it was deleted/renamed). So the user // gets a UnknownDevceException. // This behaviour is okay so we can ignore it. - checkAndStoreCode(tenantIdentifierWithStorage, main, userId, new TOTPDevice[]{matchingDevice}, - code); + try { + checkAndStoreCode(tenantIdentifierWithStorage, main, userId, new TOTPDevice[] { matchingDevice }, code); + } catch (UnknownTotpUserIdException e) { + // User must have deleted the device in parallel. + throw new UnknownDeviceException(); + } // Will reach here only if the code is valid: totpStorage.markDeviceAsVerified(tenantIdentifierWithStorage.toAppIdentifier(), userId, deviceName); return true; // Newly verified } @TestOnly - public static void verifyCode(Main main, String userId, - String code, boolean allowUnverifiedDevices) - throws TotpNotEnabledException, InvalidTotpException, LimitReachedException, + public static void verifyCode(Main main, String userId, String code) + throws InvalidTotpException, UnknownTotpUserIdException, LimitReachedException, StorageQueryException, StorageTransactionLogicException, FeatureNotEnabledException { try { verifyCode(new TenantIdentifierWithStorage(null, null, null, StorageLayer.getStorage(main)), main, - userId, code, allowUnverifiedDevices); + userId, code); } catch (TenantOrAppNotFoundException e) { throw new IllegalStateException(e); } } - public static void verifyCode(TenantIdentifierWithStorage tenantIdentifierWithStorage, Main main, String userId, - String code, boolean allowUnverifiedDevices) - throws TotpNotEnabledException, InvalidTotpException, LimitReachedException, + public static void verifyCode(TenantIdentifierWithStorage tenantIdentifierWithStorage, Main main, String userId, String code) + throws InvalidTotpException, UnknownTotpUserIdException, LimitReachedException, StorageQueryException, StorageTransactionLogicException, FeatureNotEnabledException, TenantOrAppNotFoundException { @@ -374,25 +414,28 @@ public static void verifyCode(TenantIdentifierWithStorage tenantIdentifierWithSt // Check if the user has any devices: TOTPDevice[] devices = totpStorage.getDevices(tenantIdentifierWithStorage.toAppIdentifier(), userId); if (devices.length == 0) { - throw new TotpNotEnabledException(); + // No devices found. So we can't verify the code anyway. + throw new UnknownTotpUserIdException(); } // Filter out unverified devices: - if (!allowUnverifiedDevices) { - devices = Arrays.stream(devices).filter(device -> device.verified).toArray(TOTPDevice[]::new); - } + devices = Arrays.stream(devices).filter(device -> device.verified).toArray(TOTPDevice[]::new); // At this point, even if some of the devices are suddenly deleted/renamed by // another API call. We will still check the code against the updated set of // devices and store it in the used codes table. This behaviour is okay so we // can ignore it. + + // UnknownTotpUserIdException will be thrown when + // the User has deleted the device in parallel + // since they cannot un-verify a device (no API exists) checkAndStoreCode(tenantIdentifierWithStorage, main, userId, devices, code); } @TestOnly public static void removeDevice(Main main, String userId, - String deviceName) - throws StorageQueryException, UnknownDeviceException, TotpNotEnabledException, + String deviceName) + throws StorageQueryException, UnknownDeviceException, StorageTransactionLogicException { try { removeDevice(new AppIdentifierWithStorage(null, null, StorageLayer.getStorage(main)), @@ -406,8 +449,8 @@ public static void removeDevice(Main main, String userId, * Delete device and also delete the user if deleting the last device */ public static void removeDevice(AppIdentifierWithStorage appIdentifierWithStorage, String userId, - String deviceName) - throws StorageQueryException, UnknownDeviceException, TotpNotEnabledException, + String deviceName) + throws StorageQueryException, UnknownDeviceException, StorageTransactionLogicException, TenantOrAppNotFoundException { TOTPSQLStorage storage = appIdentifierWithStorage.getTOTPStorage(); @@ -432,12 +475,6 @@ public static void removeDevice(AppIdentifierWithStorage appIdentifierWithStorag return; } catch (StorageTransactionLogicException e) { if (e.actualException instanceof UnknownDeviceException) { - // Check if any device exists for the user: - TOTPDevice[] devices = storage.getDevices(appIdentifierWithStorage, userId); - if (devices.length == 0) { - throw new TotpNotEnabledException(); - } - throw (UnknownDeviceException) e.actualException; } @@ -447,9 +484,8 @@ public static void removeDevice(AppIdentifierWithStorage appIdentifierWithStorag @TestOnly public static void updateDeviceName(Main main, String userId, - String oldDeviceName, String newDeviceName) - throws StorageQueryException, DeviceAlreadyExistsException, UnknownDeviceException, - TotpNotEnabledException { + String oldDeviceName, String newDeviceName) + throws StorageQueryException, DeviceAlreadyExistsException, UnknownDeviceException { try { updateDeviceName(new AppIdentifierWithStorage(null, null, StorageLayer.getStorage(main)), userId, oldDeviceName, newDeviceName); @@ -459,26 +495,16 @@ public static void updateDeviceName(Main main, String userId, } public static void updateDeviceName(AppIdentifierWithStorage appIdentifierWithStorage, String userId, - String oldDeviceName, String newDeviceName) + String oldDeviceName, String newDeviceName) throws StorageQueryException, DeviceAlreadyExistsException, UnknownDeviceException, - TotpNotEnabledException, TenantOrAppNotFoundException { + TenantOrAppNotFoundException { TOTPSQLStorage totpStorage = appIdentifierWithStorage.getTOTPStorage(); - try { - totpStorage.updateDeviceName(appIdentifierWithStorage, userId, oldDeviceName, newDeviceName); - } catch (UnknownDeviceException e) { - // Check if any device exists for the user: - TOTPDevice[] devices = totpStorage.getDevices(appIdentifierWithStorage, userId); - if (devices.length == 0) { - throw new TotpNotEnabledException(); - } else { - throw e; - } - } + totpStorage.updateDeviceName(appIdentifierWithStorage, userId, oldDeviceName, newDeviceName); } @TestOnly public static TOTPDevice[] getDevices(Main main, String userId) - throws StorageQueryException, TotpNotEnabledException { + throws StorageQueryException { try { return getDevices(new AppIdentifierWithStorage(null, null, StorageLayer.getStorage(main)), userId); @@ -488,13 +514,10 @@ public static TOTPDevice[] getDevices(Main main, String userId) } public static TOTPDevice[] getDevices(AppIdentifierWithStorage appIdentifierWithStorage, String userId) - throws StorageQueryException, TotpNotEnabledException, TenantOrAppNotFoundException { + throws StorageQueryException, TenantOrAppNotFoundException { TOTPSQLStorage totpStorage = appIdentifierWithStorage.getTOTPStorage(); TOTPDevice[] devices = totpStorage.getDevices(appIdentifierWithStorage, userId); - if (devices.length == 0) { - throw new TotpNotEnabledException(); - } return devices; } diff --git a/src/main/java/io/supertokens/webserver/api/totp/CreateOrUpdateTotpDeviceAPI.java b/src/main/java/io/supertokens/webserver/api/totp/CreateOrUpdateTotpDeviceAPI.java index 3d1ad47aa..01bc2c28a 100644 --- a/src/main/java/io/supertokens/webserver/api/totp/CreateOrUpdateTotpDeviceAPI.java +++ b/src/main/java/io/supertokens/webserver/api/totp/CreateOrUpdateTotpDeviceAPI.java @@ -7,11 +7,11 @@ import io.supertokens.pluginInterface.RECIPE_ID; import io.supertokens.pluginInterface.emailpassword.exceptions.UnknownUserIdException; import io.supertokens.pluginInterface.exceptions.StorageQueryException; +import io.supertokens.pluginInterface.exceptions.StorageTransactionLogicException; import io.supertokens.pluginInterface.multitenancy.AppIdentifierWithStorage; import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException; import io.supertokens.pluginInterface.totp.TOTPDevice; import io.supertokens.pluginInterface.totp.exception.DeviceAlreadyExistsException; -import io.supertokens.pluginInterface.totp.exception.TotpNotEnabledException; import io.supertokens.pluginInterface.totp.exception.UnknownDeviceException; import io.supertokens.totp.Totp; import io.supertokens.useridmapping.UserIdType; @@ -42,7 +42,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I JsonObject input = InputParser.parseJsonObjectOrThrowError(req); String userId = InputParser.parseStringOrThrowError(input, "userId", false); - String deviceName = InputParser.parseStringOrThrowError(input, "deviceName", false); + String deviceName = InputParser.parseStringOrThrowError(input, "deviceName", true); Integer skew = InputParser.parseIntOrThrowError(input, "skew", false); Integer period = InputParser.parseIntOrThrowError(input, "period", false); @@ -52,7 +52,8 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I if (userId.isEmpty()) { throw new ServletException(new BadRequestException("userId cannot be empty")); } - if (deviceName.isEmpty()) { + if (deviceName != null && deviceName.isEmpty()) { + // Only Null or valid device name are allowed throw new ServletException(new BadRequestException("deviceName cannot be empty")); } if (skew < 0) { @@ -87,13 +88,14 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I TOTPDevice device = Totp.registerDevice(appIdentifierWithStorage, main, userId, deviceName, skew, period); result.addProperty("status", "OK"); + result.addProperty("deviceName", device.deviceName); result.addProperty("secret", device.secretKey); super.sendJsonResponse(200, result, resp); } catch (DeviceAlreadyExistsException e) { result.addProperty("status", "DEVICE_ALREADY_EXISTS_ERROR"); super.sendJsonResponse(200, result, resp); } catch (StorageQueryException | NoSuchAlgorithmException | FeatureNotEnabledException | - TenantOrAppNotFoundException e) { + TenantOrAppNotFoundException | StorageTransactionLogicException e) { throw new ServletException(e); } } @@ -143,9 +145,6 @@ protected void doPut(HttpServletRequest req, HttpServletResponse resp) throws IO result.addProperty("status", "OK"); super.sendJsonResponse(200, result, resp); - } catch (TotpNotEnabledException e) { - result.addProperty("status", "TOTP_NOT_ENABLED_ERROR"); - super.sendJsonResponse(200, result, resp); } catch (UnknownDeviceException e) { result.addProperty("status", "UNKNOWN_DEVICE_ERROR"); super.sendJsonResponse(200, result, resp); diff --git a/src/main/java/io/supertokens/webserver/api/totp/GetTotpDevicesAPI.java b/src/main/java/io/supertokens/webserver/api/totp/GetTotpDevicesAPI.java index 079daae94..98da43d5c 100644 --- a/src/main/java/io/supertokens/webserver/api/totp/GetTotpDevicesAPI.java +++ b/src/main/java/io/supertokens/webserver/api/totp/GetTotpDevicesAPI.java @@ -10,7 +10,6 @@ import io.supertokens.pluginInterface.multitenancy.AppIdentifierWithStorage; import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException; import io.supertokens.pluginInterface.totp.TOTPDevice; -import io.supertokens.pluginInterface.totp.exception.TotpNotEnabledException; import io.supertokens.totp.Totp; import io.supertokens.useridmapping.UserIdType; import io.supertokens.webserver.InputParser; @@ -80,9 +79,6 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO result.addProperty("status", "OK"); result.add("devices", devicesArray); super.sendJsonResponse(200, result, resp); - } catch (TotpNotEnabledException e) { - result.addProperty("status", "TOTP_NOT_ENABLED_ERROR"); - super.sendJsonResponse(200, result, resp); } catch (StorageQueryException | TenantOrAppNotFoundException e) { throw new ServletException(e); } diff --git a/src/main/java/io/supertokens/webserver/api/totp/RemoveTotpDeviceAPI.java b/src/main/java/io/supertokens/webserver/api/totp/RemoveTotpDeviceAPI.java index df3ea4801..d6b0c6f50 100644 --- a/src/main/java/io/supertokens/webserver/api/totp/RemoveTotpDeviceAPI.java +++ b/src/main/java/io/supertokens/webserver/api/totp/RemoveTotpDeviceAPI.java @@ -9,7 +9,6 @@ import io.supertokens.pluginInterface.exceptions.StorageTransactionLogicException; import io.supertokens.pluginInterface.multitenancy.AppIdentifierWithStorage; import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException; -import io.supertokens.pluginInterface.totp.exception.TotpNotEnabledException; import io.supertokens.pluginInterface.totp.exception.UnknownDeviceException; import io.supertokens.totp.Totp; import io.supertokens.useridmapping.UserIdType; @@ -75,9 +74,6 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I result.addProperty("status", "OK"); result.addProperty("didDeviceExist", true); super.sendJsonResponse(200, result, resp); - } catch (TotpNotEnabledException e) { - result.addProperty("status", "TOTP_NOT_ENABLED_ERROR"); - super.sendJsonResponse(200, result, resp); } catch (UnknownDeviceException e) { result.addProperty("status", "OK"); result.addProperty("didDeviceExist", false); diff --git a/src/main/java/io/supertokens/webserver/api/totp/VerifyTotpAPI.java b/src/main/java/io/supertokens/webserver/api/totp/VerifyTotpAPI.java index 7d3980d99..b413332df 100644 --- a/src/main/java/io/supertokens/webserver/api/totp/VerifyTotpAPI.java +++ b/src/main/java/io/supertokens/webserver/api/totp/VerifyTotpAPI.java @@ -4,7 +4,6 @@ import com.google.gson.JsonObject; -import io.supertokens.AppIdentifierWithStorageAndUserIdMapping; import io.supertokens.Main; import io.supertokens.TenantIdentifierWithStorageAndUserIdMapping; import io.supertokens.featureflag.exceptions.FeatureNotEnabledException; @@ -12,11 +11,9 @@ import io.supertokens.pluginInterface.emailpassword.exceptions.UnknownUserIdException; import io.supertokens.pluginInterface.exceptions.StorageQueryException; import io.supertokens.pluginInterface.exceptions.StorageTransactionLogicException; -import io.supertokens.pluginInterface.multitenancy.AppIdentifierWithStorage; import io.supertokens.pluginInterface.multitenancy.TenantIdentifierWithStorage; import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException; -import io.supertokens.pluginInterface.totp.exception.TotpNotEnabledException; -import io.supertokens.pluginInterface.useridmapping.UserIdMapping; +import io.supertokens.pluginInterface.totp.exception.UnknownTotpUserIdException; import io.supertokens.totp.Totp; import io.supertokens.totp.exceptions.InvalidTotpException; import io.supertokens.totp.exceptions.LimitReachedException; @@ -46,7 +43,6 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I String userId = InputParser.parseStringOrThrowError(input, "userId", false); String totp = InputParser.parseStringOrThrowError(input, "totp", false); - Boolean allowUnverifiedDevices = InputParser.parseBooleanOrThrowError(input, "allowUnverifiedDevices", false); if (userId.isEmpty()) { throw new ServletException(new BadRequestException("userId cannot be empty")); @@ -54,7 +50,6 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I if (totp.length() != 6) { throw new ServletException(new BadRequestException("totp must be 6 characters long")); } - // Already checked that allowUnverifiedDevices is not null. JsonObject result = new JsonObject(); @@ -76,16 +71,16 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I tenantIdentifierWithStorage = getTenantIdentifierWithStorageFromRequest(req); } - Totp.verifyCode(tenantIdentifierWithStorage, main, userId, totp, allowUnverifiedDevices); + Totp.verifyCode(tenantIdentifierWithStorage, main, userId, totp); result.addProperty("status", "OK"); super.sendJsonResponse(200, result, resp); - } catch (TotpNotEnabledException e) { - result.addProperty("status", "TOTP_NOT_ENABLED_ERROR"); - super.sendJsonResponse(200, result, resp); } catch (InvalidTotpException e) { result.addProperty("status", "INVALID_TOTP_ERROR"); super.sendJsonResponse(200, result, resp); + } catch (UnknownTotpUserIdException e) { + result.addProperty("status", "UNKNOWN_USER_ID_ERROR"); + super.sendJsonResponse(200, result, resp); } catch (LimitReachedException e) { result.addProperty("status", "LIMIT_REACHED_ERROR"); result.addProperty("retryAfterMs", e.retryAfterMs); diff --git a/src/main/java/io/supertokens/webserver/api/totp/VerifyTotpDeviceAPI.java b/src/main/java/io/supertokens/webserver/api/totp/VerifyTotpDeviceAPI.java index a068e07e4..da8c21c01 100644 --- a/src/main/java/io/supertokens/webserver/api/totp/VerifyTotpDeviceAPI.java +++ b/src/main/java/io/supertokens/webserver/api/totp/VerifyTotpDeviceAPI.java @@ -12,9 +12,7 @@ import io.supertokens.pluginInterface.exceptions.StorageTransactionLogicException; import io.supertokens.pluginInterface.multitenancy.TenantIdentifierWithStorage; import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException; -import io.supertokens.pluginInterface.totp.exception.TotpNotEnabledException; import io.supertokens.pluginInterface.totp.exception.UnknownDeviceException; -import io.supertokens.pluginInterface.useridmapping.UserIdMapping; import io.supertokens.totp.Totp; import io.supertokens.totp.exceptions.InvalidTotpException; import io.supertokens.totp.exceptions.LimitReachedException; @@ -80,9 +78,6 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I result.addProperty("status", "OK"); result.addProperty("wasAlreadyVerified", !isNewlyVerified); super.sendJsonResponse(200, result, resp); - } catch (TotpNotEnabledException e) { - result.addProperty("status", "TOTP_NOT_ENABLED_ERROR"); - super.sendJsonResponse(200, result, resp); } catch (UnknownDeviceException e) { result.addProperty("status", "UNKNOWN_DEVICE_ERROR"); super.sendJsonResponse(200, result, resp); diff --git a/src/test/java/io/supertokens/test/StorageLayerTest.java b/src/test/java/io/supertokens/test/StorageLayerTest.java index ec49fd89b..df64313bd 100644 --- a/src/test/java/io/supertokens/test/StorageLayerTest.java +++ b/src/test/java/io/supertokens/test/StorageLayerTest.java @@ -11,7 +11,9 @@ import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException; import io.supertokens.pluginInterface.totp.TOTPDevice; import io.supertokens.pluginInterface.totp.TOTPUsedCode; -import io.supertokens.pluginInterface.totp.exception.TotpNotEnabledException; +import io.supertokens.pluginInterface.totp.exception.DeviceAlreadyExistsException; +import io.supertokens.pluginInterface.totp.exception.UnknownDeviceException; +import io.supertokens.pluginInterface.totp.exception.UnknownTotpUserIdException; import io.supertokens.pluginInterface.totp.exception.UsedCodeAlreadyExistsException; import io.supertokens.pluginInterface.totp.sqlStorage.TOTPSQLStorage; import io.supertokens.storageLayer.StorageLayer; @@ -47,7 +49,7 @@ public static void insertUsedCodeUtil(TOTPSQLStorage storage, TOTPUsedCode usedC storage.insertUsedCode_Transaction(con, new TenantIdentifier(null, null, null), usedCode); storage.commitTransaction(con); return null; - } catch (TotpNotEnabledException | UsedCodeAlreadyExistsException e) { + } catch (UnknownTotpUserIdException | UsedCodeAlreadyExistsException e) { throw new StorageTransactionLogicException(e); } catch (TenantOrAppNotFoundException e) { throw new IllegalStateException(e); @@ -55,7 +57,7 @@ public static void insertUsedCodeUtil(TOTPSQLStorage storage, TOTPUsedCode usedC }); } catch (StorageTransactionLogicException e) { Exception actual = e.actualException; - if (actual instanceof TotpNotEnabledException || actual instanceof UsedCodeAlreadyExistsException) { + if (actual instanceof UnknownDeviceException || actual instanceof UsedCodeAlreadyExistsException) { throw actual; } else { throw e; @@ -84,7 +86,7 @@ public void totpCodeLengthTest() throws Exception { TOTPDevice d1 = new TOTPDevice("user", "d1", "secret", 30, 1, false); storage.createDevice(new AppIdentifier(null, null), d1); - + // Try code with length > 8 try { TOTPUsedCode code = new TOTPUsedCode("user", "123456789", true, nextDay, now); diff --git a/src/test/java/io/supertokens/test/mfa/api/MfaUserIdMappingTest.java b/src/test/java/io/supertokens/test/mfa/api/MfaUserIdMappingTest.java index 467a56215..ec60ff654 100644 --- a/src/test/java/io/supertokens/test/mfa/api/MfaUserIdMappingTest.java +++ b/src/test/java/io/supertokens/test/mfa/api/MfaUserIdMappingTest.java @@ -19,7 +19,6 @@ import com.google.gson.JsonObject; import io.supertokens.Main; import io.supertokens.emailpassword.EmailPassword; - import io.supertokens.pluginInterface.authRecipe.AuthRecipeUserInfo; import io.supertokens.test.mfa.MfaTestBase; import io.supertokens.useridmapping.UserIdMapping; diff --git a/src/test/java/io/supertokens/test/multitenant/TestAppData.java b/src/test/java/io/supertokens/test/multitenant/TestAppData.java index f6c4c679e..5d3b75039 100644 --- a/src/test/java/io/supertokens/test/multitenant/TestAppData.java +++ b/src/test/java/io/supertokens/test/multitenant/TestAppData.java @@ -154,8 +154,10 @@ public void testThatDeletingAppDeleteDataFromAllTables() throws Exception { TOTPDevice totpDevice = Totp.registerDevice(appWithStorage.toAppIdentifierWithStorage(), process.getProcess(), epUser.getSupertokensUserId(), "test", 1, 3); + Totp.verifyDevice(appWithStorage, process.getProcess(), epUser.getSupertokensUserId(), totpDevice.deviceName, + generateTotpCode(process.getProcess(), totpDevice, -1)); Totp.verifyCode(appWithStorage, process.getProcess(), epUser.getSupertokensUserId(), - generateTotpCode(process.getProcess(), totpDevice, 0), true); + generateTotpCode(process.getProcess(), totpDevice, 0)); ActiveUsers.updateLastActive(appWithStorage.toAppIdentifierWithStorage(), process.getProcess(), epUser.getSupertokensUserId()); diff --git a/src/test/java/io/supertokens/test/totp/TOTPRecipeTest.java b/src/test/java/io/supertokens/test/totp/TOTPRecipeTest.java index 0af4feaf5..88db473ae 100644 --- a/src/test/java/io/supertokens/test/totp/TOTPRecipeTest.java +++ b/src/test/java/io/supertokens/test/totp/TOTPRecipeTest.java @@ -23,19 +23,16 @@ import io.supertokens.cronjobs.deleteExpiredTotpTokens.DeleteExpiredTotpTokens; import io.supertokens.featureflag.EE_FEATURES; import io.supertokens.featureflag.FeatureFlagTestContent; -import io.supertokens.featureflag.exceptions.InvalidLicenseKeyException; -import io.supertokens.httpRequest.HttpResponseException; import io.supertokens.pluginInterface.STORAGE_TYPE; import io.supertokens.pluginInterface.exceptions.StorageQueryException; import io.supertokens.pluginInterface.exceptions.StorageTransactionLogicException; import io.supertokens.pluginInterface.multitenancy.TenantIdentifier; -import io.supertokens.pluginInterface.STORAGE_TYPE; import io.supertokens.pluginInterface.totp.TOTPDevice; import io.supertokens.pluginInterface.totp.TOTPStorage; import io.supertokens.pluginInterface.totp.TOTPUsedCode; import io.supertokens.pluginInterface.totp.exception.DeviceAlreadyExistsException; -import io.supertokens.pluginInterface.totp.exception.TotpNotEnabledException; import io.supertokens.pluginInterface.totp.exception.UnknownDeviceException; +import io.supertokens.pluginInterface.totp.exception.UnknownTotpUserIdException; import io.supertokens.pluginInterface.totp.sqlStorage.TOTPSQLStorage; import io.supertokens.storageLayer.StorageLayer; import io.supertokens.test.TestingProcessManager; @@ -114,7 +111,7 @@ public static String generateTotpCode(Main main, TOTPDevice device) /** * Generates TOTP code similar to apps like Google Authenticator and Authy */ - private static String generateTotpCode(Main main, TOTPDevice device, int step) + public static String generateTotpCode(Main main, TOTPDevice device, int step) throws InvalidKeyException, StorageQueryException { final TimeBasedOneTimePasswordGenerator totp = new TimeBasedOneTimePasswordGenerator( Duration.ofSeconds(device.period)); @@ -150,6 +147,10 @@ public void createDeviceTest() throws Exception { TOTPDevice device = Totp.registerDevice(main, "user", "device1", 1, 30); assert !Objects.equals(device.secretKey, ""); + // Verify device + String validTotp = TOTPRecipeTest.generateTotpCode(main, device); + Totp.verifyDevice(main, "user", "device1", validTotp); + // Create same device again (should fail) assertThrows(DeviceAlreadyExistsException.class, () -> Totp.registerDevice(main, "user", "device1", 1, 30)); @@ -163,76 +164,84 @@ public void createDeviceAndVerifyCodeTest() throws Exception { } Main main = result.process.getProcess(); - // Create device + // Create devices TOTPDevice device = Totp.registerDevice(main, "user", "device1", 1, 1); + TOTPDevice unverifiedDevice = Totp.registerDevice(main, "user", "unverified-device", 1, 1); + + // Verify device: + Totp.verifyDevice(main, "user", device.deviceName, generateTotpCode(main, device, -1)); // Try login with non-existent user: - assertThrows(TotpNotEnabledException.class, - () -> Totp.verifyCode(main, "non-existent-user", "any-code", true)); + assertThrows(UnknownTotpUserIdException.class, + () -> Totp.verifyCode(main, "non-existent-user", "any-code")); - // {Code: [INVALID, VALID]} * {Devices: [VERIFIED_ONLY, ALL]} + // {Code: [INVALID, VALID]} * {Devices: [verified, unverfied]} - // Invalid code & allowUnverifiedDevice = true: + // Invalid code & unverified device: assertThrows(InvalidTotpException.class, - () -> Totp.verifyCode(main, "user", "invalid", true)); + () -> Totp.verifyCode(main, "user", "invalid")); - // Invalid code & allowUnverifiedDevice = false: + // Invalid code & verified device: assertThrows(InvalidTotpException.class, - () -> Totp.verifyCode(main, "user", "invalid", false)); + () -> Totp.verifyCode(main, "user", "invalid")); - // Valid code & allowUnverifiedDevice = false: + // Valid code & unverified device: assertThrows( InvalidTotpException.class, - () -> Totp.verifyCode(main, "user", generateTotpCode(main, device), false)); + () -> Totp.verifyCode(main, "user", generateTotpCode(main, unverifiedDevice))); - // Valid code & allowUnverifiedDevice = true (Success): + // Valid code & verified device (Success) String validCode = generateTotpCode(main, device); - Totp.verifyCode(main, "user", validCode, true); + Totp.verifyCode(main, "user", validCode); // Now try again with same code: assertThrows( InvalidTotpException.class, - () -> Totp.verifyCode(main, "user", validCode, true)); + () -> Totp.verifyCode(main, "user", validCode)); // Sleep for 1s so that code changes. Thread.sleep(1000); // Use a new valid code: String newValidCode = generateTotpCode(main, device); - Totp.verifyCode(main, "user", newValidCode, true); + Totp.verifyCode(main, "user", newValidCode); // Reuse the same code and use it again (should fail): assertThrows(InvalidTotpException.class, - () -> Totp.verifyCode(main, "user", newValidCode, true)); + () -> Totp.verifyCode(main, "user", newValidCode)); // Use a code from next period: String nextValidCode = generateTotpCode(main, device, 1); - Totp.verifyCode(main, "user", nextValidCode, true); + Totp.verifyCode(main, "user", nextValidCode); // Use previous period code (should fail coz validCode has been used): String previousCode = generateTotpCode(main, device, -1); assert previousCode.equals(validCode); - assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "user", previousCode, true)); + assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "user", previousCode)); // Create device with skew = 0, check that it only works with the current code TOTPDevice device2 = Totp.registerDevice(main, "user", "device2", 0, 1); assert !Objects.equals(device2.secretKey, device.secretKey); + Totp.verifyDevice(main, "user", device2.deviceName, generateTotpCode(main, device2)); + + // Sleep because code was used for verifying the device + Thread.sleep(1000); String nextValidCode2 = generateTotpCode(main, device2, 1); assertThrows(InvalidTotpException.class, - () -> Totp.verifyCode(main, "user", nextValidCode2, true)); + () -> Totp.verifyCode(main, "user", nextValidCode2)); String previousValidCode2 = generateTotpCode(main, device2, -1); assertThrows(InvalidTotpException.class, - () -> Totp.verifyCode(main, "user", previousValidCode2, true)); + () -> Totp.verifyCode(main, "user", previousValidCode2)); String currentValidCode2 = generateTotpCode(main, device2); - Totp.verifyCode(main, "user", currentValidCode2, true); + Totp.verifyCode(main, "user", currentValidCode2); // Submit invalid code and check that it's expiry time is correct // created - expiryTime = max of ((2 * skew + 1) * period) for all devices assertThrows(InvalidTotpException.class, - () -> Totp.verifyCode(main, "user", "invalid", true)); + () -> Totp.verifyCode(main, "user", "invalid")); TOTPUsedCode[] usedCodes = getAllUsedCodesUtil(result.storage, "user"); TOTPUsedCode latestCode = usedCodes[0]; @@ -247,20 +256,18 @@ public void createDeviceAndVerifyCodeTest() throws Exception { Totp.verifyDevice(main, "user", device2.deviceName, generateTotpCode(main, device2)); // device1: unverified, device2: verified - // Valid code & allowUnverifiedDevice = false: - assertThrows( - InvalidTotpException.class, - () -> Totp.verifyCode(main, "user", generateTotpCode(main, device), false)); + // Valid code & verified device: + Totp.verifyCode(main, "user", generateTotpCode(main, device)); Thread.sleep(1000); - Totp.verifyCode(main, "user", generateTotpCode(main, device2), false); + Totp.verifyCode(main, "user", generateTotpCode(main, device2)); // Valid code & allowUnverifiedDevice = true: Thread.sleep(1000); - Totp.verifyCode(main, "user", generateTotpCode(main, device), true); + Totp.verifyCode(main, "user", generateTotpCode(main, device)); Thread.sleep(1000); - Totp.verifyCode(main, "user", generateTotpCode(main, device2), true); + Totp.verifyCode(main, "user", generateTotpCode(main, device2)); } /* @@ -277,20 +284,20 @@ public int triggerAndCheckRateLimit(Main main, TOTPDevice device) throws Excepti String code = "ic-" + i; // ic = invalid code assertThrows( InvalidTotpException.class, - () -> Totp.verifyCode(main, "user", code, true)); + () -> Totp.verifyCode(main, "user", code)); } // Any kind of attempt after this should fail with rate limiting error. // This should happen until rate limiting cooldown happens: assertThrows( LimitReachedException.class, - () -> Totp.verifyCode(main, "user", "icN+1", true)); + () -> Totp.verifyCode(main, "user", "icN+1")); assertThrows( LimitReachedException.class, - () -> Totp.verifyCode(main, "user", generateTotpCode(main, device), true)); + () -> Totp.verifyCode(main, "user", generateTotpCode(main, device))); assertThrows( LimitReachedException.class, - () -> Totp.verifyCode(main, "user", "icN+2", true)); + () -> Totp.verifyCode(main, "user", "icN+2")); return N; } @@ -318,6 +325,7 @@ public void rateLimitCooldownTest() throws Exception { // Create device TOTPDevice device = Totp.registerDevice(main, "user", "deviceName", 1, 1); + Totp.verifyDevice(main, "user", device.deviceName, generateTotpCode(main, device, -1)); // Trigger rate limiting and fix it with a correct code after some time: int attemptsRequired = triggerAndCheckRateLimit(main, device); @@ -325,17 +333,17 @@ 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", true)); + assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "user", "invalid0")); // This triggered rate limiting again. So even valid codes will fail for // another cooldown period: assertThrows(LimitReachedException.class, - () -> Totp.verifyCode(main, "user", generateTotpCode(main, device), true)); + () -> Totp.verifyCode(main, "user", generateTotpCode(main, device))); // Wait for 1 second (Should cool down rate limiting): Thread.sleep(1000); // Now try with valid code: - Totp.verifyCode(main, "user", generateTotpCode(main, device), true); + Totp.verifyCode(main, "user", generateTotpCode(main, device)); // Now invalid code shouldn't trigger rate limiting. Unless you do it N times: - assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "user", "invaldd", true)); + assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "user", "invaldd")); } @Test @@ -361,9 +369,9 @@ public void cronRemovesCodesDuringRateLimitTest() throws Exception { // This removal shouldn't affect rate limiting. User must remain rate limited. assertThrows(LimitReachedException.class, - () -> Totp.verifyCode(main, "user", generateTotpCode(main, device), true)); + () -> Totp.verifyCode(main, "user", generateTotpCode(main, device))); assertThrows(LimitReachedException.class, - () -> Totp.verifyCode(main, "user", "yet-ic", true)); + () -> Totp.verifyCode(main, "user", "yet-ic")); } @Test @@ -378,7 +386,7 @@ public void createAndVerifyDeviceTest() throws Exception { TOTPDevice device = Totp.registerDevice(main, "user", "deviceName", 1, 30); // Try verify non-existent user: - assertThrows(TotpNotEnabledException.class, + assertThrows(UnknownDeviceException.class, () -> Totp.verifyDevice(main, "non-existent-user", "deviceName", "XXXX")); // Try verify non-existent device @@ -424,20 +432,23 @@ public void removeDeviceTest() throws Exception { TOTPDevice device1 = Totp.registerDevice(main, "user", "device1", 1, 30); TOTPDevice device2 = Totp.registerDevice(main, "user", "device2", 1, 30); + Totp.verifyDevice(main, "user", "device1", generateTotpCode(main, device1, -1)); + Totp.verifyDevice(main, "user", "device2", generateTotpCode(main, device2, -1)); + TOTPDevice[] devices = Totp.getDevices(main, "user"); assert (devices.length == 2); // Try to delete device for non-existent user: - assertThrows(TotpNotEnabledException.class, () -> Totp.removeDevice(main, "non-existent-user", "device1")); + assertThrows(UnknownDeviceException.class, () -> Totp.removeDevice(main, "non-existent-user", "device1")); // Try to delete non-existent device: assertThrows(UnknownDeviceException.class, () -> Totp.removeDevice(main, "user", "non-existent-device")); // Delete one of the devices { - assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "user", "ic0", true)); - Totp.verifyCode(main, "user", generateTotpCode(main, device1), true); - Totp.verifyCode(main, "user", generateTotpCode(main, device2), true); + assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "user", "ic0")); + Totp.verifyCode(main, "user", generateTotpCode(main, device1)); + Totp.verifyCode(main, "user", generateTotpCode(main, device2)); // Delete device1 Totp.removeDevice(main, "user", "device1"); @@ -447,7 +458,7 @@ public void removeDeviceTest() throws Exception { // 1 device still remain so all codes should still be still there: TOTPUsedCode[] usedCodes = getAllUsedCodesUtil(storage, "user"); - assert (usedCodes.length == 3); + assert (usedCodes.length == 5); // 2 for device verification and 3 for code verification } // Deleting the last device of a user should delete all related codes: @@ -456,14 +467,15 @@ public void removeDeviceTest() throws Exception { // Create another user to test that other users aren't affected: TOTPDevice otherUserDevice = Totp.registerDevice(main, "other-user", "device", 1, 30); - Totp.verifyCode(main, "other-user", generateTotpCode(main, otherUserDevice), true); - assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "other-user", "ic1", true)); + Totp.verifyDevice(main, "other-user", "device", generateTotpCode(main, otherUserDevice, -1)); + Totp.verifyCode(main, "other-user", generateTotpCode(main, otherUserDevice)); + assertThrows(InvalidTotpException.class, () -> Totp.verifyCode(main, "other-user", "ic1")); // Delete device2 Totp.removeDevice(main, "user", "device2"); - // TOTP has ben disabled for the user: - assertThrows(TotpNotEnabledException.class, () -> Totp.getDevices(main, "user")); + // No more devices are left for the user: + assert (Totp.getDevices(main, "user").length == 0); // No device left so all codes of the user should be deleted: TOTPUsedCode[] usedCodes = getAllUsedCodesUtil(storage, "user"); @@ -474,7 +486,7 @@ public void removeDeviceTest() throws Exception { assert (otherUserDevices.length == 1); usedCodes = getAllUsedCodesUtil(storage, "other-user"); - assert (usedCodes.length == 2); + assert (usedCodes.length == 3); // 1 for device verification and 2 for code verification } } @@ -490,7 +502,7 @@ public void updateDeviceNameTest() throws Exception { Totp.registerDevice(main, "user", "device2", 1, 30); // Try update non-existent user: - assertThrows(TotpNotEnabledException.class, + assertThrows(UnknownDeviceException.class, () -> Totp.updateDeviceName(main, "non-existent-user", "device1", "new-device-name")); // Try update non-existent device: @@ -526,7 +538,7 @@ public void getDevicesTest() throws Exception { Main main = result.process.getProcess(); // Try get devices for non-existent user: - assertThrows(TotpNotEnabledException.class, () -> Totp.getDevices(main, "non-existent-user")); + assert (Totp.getDevices(main, "non-existent-user").length == 0); TOTPDevice device1 = Totp.registerDevice(main, "user", "device1", 2, 30); TOTPDevice device2 = Totp.registerDevice(main, "user", "device2", 1, 10); @@ -548,4 +560,15 @@ public void deleteExpiredTokensCronIntervalTest() throws Exception { assert DeleteExpiredTotpTokens.getInstance(main).getIntervalTimeSeconds() == 60 * 60; } + @Test + public void testRegisterDeviceWithSameNameAsAnUnverifiedDevice() throws Exception { + TestSetupResult result = defaultInit(); + if (result == null) { + return; + } + Main main = result.process.getProcess(); + + Totp.registerDevice(main, "user", "device1", 1, 30); + Totp.registerDevice(main, "user", "device1", 1, 30); + } } diff --git a/src/test/java/io/supertokens/test/totp/TOTPStorageTest.java b/src/test/java/io/supertokens/test/totp/TOTPStorageTest.java index ebd2bf133..e03169d2a 100644 --- a/src/test/java/io/supertokens/test/totp/TOTPStorageTest.java +++ b/src/test/java/io/supertokens/test/totp/TOTPStorageTest.java @@ -14,8 +14,8 @@ import io.supertokens.pluginInterface.totp.TOTPStorage; import io.supertokens.pluginInterface.totp.TOTPUsedCode; import io.supertokens.pluginInterface.totp.exception.DeviceAlreadyExistsException; -import io.supertokens.pluginInterface.totp.exception.TotpNotEnabledException; import io.supertokens.pluginInterface.totp.exception.UnknownDeviceException; +import io.supertokens.pluginInterface.totp.exception.UnknownTotpUserIdException; import io.supertokens.pluginInterface.totp.exception.UsedCodeAlreadyExistsException; import io.supertokens.pluginInterface.totp.sqlStorage.TOTPSQLStorage; import io.supertokens.storageLayer.StorageLayer; @@ -27,8 +27,6 @@ import org.junit.Test; import org.junit.rules.TestRule; -import java.io.IOException; - import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThrows; @@ -89,7 +87,7 @@ private static TOTPUsedCode[] getAllUsedCodesUtil(TOTPStorage storage, String us } public static void insertUsedCodesUtil(TOTPSQLStorage storage, TOTPUsedCode[] usedCodes) - throws StorageQueryException, StorageTransactionLogicException, TotpNotEnabledException, + throws StorageQueryException, StorageTransactionLogicException, UnknownDeviceException, UsedCodeAlreadyExistsException { try { storage.startTransaction(con -> { @@ -97,7 +95,7 @@ public static void insertUsedCodesUtil(TOTPSQLStorage storage, TOTPUsedCode[] us for (TOTPUsedCode usedCode : usedCodes) { storage.insertUsedCode_Transaction(con, new TenantIdentifier(null, null, null), usedCode); } - } catch (TotpNotEnabledException | UsedCodeAlreadyExistsException e) { + } catch (UnknownTotpUserIdException | UsedCodeAlreadyExistsException e) { throw new StorageTransactionLogicException(e); } catch (TenantOrAppNotFoundException e) { throw new IllegalStateException(e); @@ -108,8 +106,8 @@ public static void insertUsedCodesUtil(TOTPSQLStorage storage, TOTPUsedCode[] us }); } catch (StorageTransactionLogicException e) { Exception actual = e.actualException; - if (actual instanceof TotpNotEnabledException) { - throw (TotpNotEnabledException) actual; + if (actual instanceof UnknownDeviceException) { + throw (UnknownDeviceException) actual; } else if (actual instanceof UsedCodeAlreadyExistsException) { throw (UsedCodeAlreadyExistsException) actual; } @@ -404,11 +402,13 @@ public void insertUsedCodeTest() throws Exception { // Try to insert code when user doesn't have any device (i.e. TOTP not enabled) { - assertThrows(TotpNotEnabledException.class, + StorageTransactionLogicException e = assertThrows(StorageTransactionLogicException.class, () -> insertUsedCodesUtil(storage, new TOTPUsedCode[]{ new TOTPUsedCode("new-user-without-totp", "1234", true, nextDay, System.currentTimeMillis()) })); + + // assert e.actualException instanceof UnknownDeviceException } // Try to insert code after user has atleast one device (i.e. TOTP enabled) @@ -423,11 +423,13 @@ public void insertUsedCodeTest() throws Exception { } // Try to insert code when user doesn't exist: - assertThrows(TotpNotEnabledException.class, + StorageTransactionLogicException e = assertThrows(StorageTransactionLogicException.class, () -> insertUsedCodesUtil(storage, new TOTPUsedCode[]{ new TOTPUsedCode("non-existent-user", "1234", true, nextDay, System.currentTimeMillis()) })); + + // assert e.actualException instanceof UnknownDeviceException; } @Test diff --git a/src/test/java/io/supertokens/test/totp/TotpLicenseTest.java b/src/test/java/io/supertokens/test/totp/TotpLicenseTest.java index a46ca392e..d612bd40e 100644 --- a/src/test/java/io/supertokens/test/totp/TotpLicenseTest.java +++ b/src/test/java/io/supertokens/test/totp/TotpLicenseTest.java @@ -99,7 +99,7 @@ public void testTotpWithoutLicense() throws Exception { }); // Verify code assertThrows(FeatureNotEnabledException.class, () -> { - Totp.verifyCode(main, "user", "device1", true); + Totp.verifyCode(main, "user", "device1"); }); // Try to create device via API: @@ -133,7 +133,6 @@ public void testTotpWithoutLicense() throws Exception { JsonObject body2 = new JsonObject(); body2.addProperty("userId", "user-id"); body2.addProperty("totp", "123456"); - body2.addProperty("allowUnverifiedDevices", true); HttpResponseException e2 = assertThrows( @@ -169,9 +168,12 @@ public void testTotpWithLicense() throws Exception { // Create device TOTPDevice device = Totp.registerDevice(main, "user", "device1", 1, 30); + // Verify device + String code = generateTotpCode(main, device, 0); + Totp.verifyDevice(main, device.userId, device.deviceName, code); // Verify code - String code = generateTotpCode(main, device); - Totp.verifyCode(main, "user", code, true); + String nextCode = generateTotpCode(main, device, 1); + Totp.verifyCode(main, "user", nextCode); } diff --git a/src/test/java/io/supertokens/test/totp/api/CreateTotpDeviceAPITest.java b/src/test/java/io/supertokens/test/totp/api/CreateTotpDeviceAPITest.java index 3c181fbaa..4aa922e2d 100644 --- a/src/test/java/io/supertokens/test/totp/api/CreateTotpDeviceAPITest.java +++ b/src/test/java/io/supertokens/test/totp/api/CreateTotpDeviceAPITest.java @@ -6,12 +6,15 @@ import io.supertokens.featureflag.FeatureFlag; import io.supertokens.featureflag.FeatureFlagTestContent; import io.supertokens.pluginInterface.STORAGE_TYPE; +import io.supertokens.pluginInterface.totp.TOTPDevice; 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.test.totp.TOTPRecipeTest; import io.supertokens.test.totp.TotpLicenseTest; +import io.supertokens.totp.Totp; import org.junit.AfterClass; import org.junit.Before; import org.junit.Rule; @@ -88,17 +91,15 @@ public void testApi() throws Exception { JsonObject body = new JsonObject(); - // Missing userId/deviceName/skew/period + // Missing userId/skew/period { Exception e = createDeviceRequest(process, body); checkFieldMissingErrorResponse(e, "userId"); + + body.addProperty("deviceName", ""); body.addProperty("userId", ""); e = createDeviceRequest(process, body); - checkFieldMissingErrorResponse(e, "deviceName"); - - body.addProperty("deviceName", ""); - e = createDeviceRequest(process, body); checkFieldMissingErrorResponse(e, "skew"); body.addProperty("skew", -1); @@ -138,8 +139,10 @@ public void testApi() throws Exception { Utils.getCdiVersionStringLatestForTests(), "totp"); assert res.get("status").getAsString().equals("OK"); + assert res.get("deviceName").getAsString().equals("d1"); - // try again with same device: + // try again with same device name: + // This should replace the previous device JsonObject res2 = HttpRequestForTesting.sendJsonPOSTRequest( process.getProcess(), "", @@ -150,7 +153,102 @@ public void testApi() throws Exception { null, Utils.getCdiVersionStringLatestForTests(), "totp"); + assert res2.get("status").getAsString().equals("OK"); + assert res.get("deviceName").getAsString().equals("d1"); + + // verify d1 + { + TOTPDevice device = Totp.getDevices(process.getProcess(), "user-id" )[0]; + String validTotp = TOTPRecipeTest.generateTotpCode(process.getProcess(), device); + Totp.verifyDevice(process.getProcess(), "user-id", "d1", validTotp); + } + + // try again with same device name: + res2 = HttpRequestForTesting.sendJsonPOSTRequest( + process.getProcess(), + "", + "http://localhost:3567/recipe/totp/device", + body, + 1000, + 1000, + null, + Utils.getCdiVersionStringLatestForTests(), + "totp"); assert res2.get("status").getAsString().equals("DEVICE_ALREADY_EXISTS_ERROR"); + assert res.get("deviceName").getAsString().equals("d1"); + + // try without passing deviceName: + body.remove("deviceName"); + JsonObject res3 = HttpRequestForTesting.sendJsonPOSTRequest( + process.getProcess(), + "", + "http://localhost:3567/recipe/totp/device", + body, + 1000, + 1000, + null, + Utils.getCdiVersionStringLatestForTests(), + "totp"); + assert res3.get("status").getAsString().equals("OK"); + assert res3.get("deviceName").getAsString().equals("TOTP Device 1"); + String attempt1Secret = res3.get("secret").getAsString(); + + // try again without passing deviceName: + // should re-create the device since "TOTP Device 1" wasn't verified + JsonObject res4 = HttpRequestForTesting.sendJsonPOSTRequest( + process.getProcess(), + "", + "http://localhost:3567/recipe/totp/device", + body, + 1000, + 1000, + null, + Utils.getCdiVersionStringLatestForTests(), + "totp"); + assert res4.get("status").getAsString().equals("OK"); + assert res3.get("deviceName").getAsString().equals("TOTP Device 1"); + String attempt2Secret = res4.get("secret").getAsString(); + assert !attempt1Secret.equals(attempt2Secret); + + // verify the device: + TOTPDevice device = new TOTPDevice( + "user-id", + "TOTP Device 1", + attempt2Secret, + 30, + 0, + false + ); + JsonObject verifyDeviceBody = new JsonObject(); + verifyDeviceBody.addProperty("userId", device.userId); + verifyDeviceBody.addProperty("deviceName", device.deviceName); + verifyDeviceBody.addProperty("totp", TOTPRecipeTest.generateTotpCode(process.getProcess(), device)); + JsonObject res5 = HttpRequestForTesting.sendJsonPOSTRequest( + process.getProcess(), + "", + "http://localhost:3567/recipe/totp/device/verify", + verifyDeviceBody, + 1000, + 1000, + null, + Utils.getCdiVersionStringLatestForTests(), + "totp"); + assert res5.get("status").getAsString().equals("OK"); + + // now try to create a device: + // "TOTP Device 1" has been verified, it won't replace it + JsonObject res6 = HttpRequestForTesting.sendJsonPOSTRequest( + process.getProcess(), + "", + "http://localhost:3567/recipe/totp/device", + body, + 1000, + 1000, + null, + Utils.getCdiVersionStringLatestForTests(), + "totp"); + assert res6.get("status").getAsString().equals("OK"); + assert res6.get("deviceName").getAsString().equals("TOTP Device 2"); } process.kill(); diff --git a/src/test/java/io/supertokens/test/totp/api/GetTotpDevicesAPITest.java b/src/test/java/io/supertokens/test/totp/api/GetTotpDevicesAPITest.java index 5450be943..0ea0f8387 100644 --- a/src/test/java/io/supertokens/test/totp/api/GetTotpDevicesAPITest.java +++ b/src/test/java/io/supertokens/test/totp/api/GetTotpDevicesAPITest.java @@ -153,7 +153,8 @@ public void testApi() throws Exception { null, Utils.getCdiVersionStringLatestForTests(), "totp"); - assert res2.get("status").getAsString().equals("TOTP_NOT_ENABLED_ERROR"); + assert res2.get("status").getAsString().equals("OK"); + assert res2.get("devices").getAsJsonArray().size() == 0; } process.kill(); diff --git a/src/test/java/io/supertokens/test/totp/api/MultitenantAPITest.java b/src/test/java/io/supertokens/test/totp/api/MultitenantAPITest.java index 3479cd320..df8cac9fb 100644 --- a/src/test/java/io/supertokens/test/totp/api/MultitenantAPITest.java +++ b/src/test/java/io/supertokens/test/totp/api/MultitenantAPITest.java @@ -37,6 +37,7 @@ import io.supertokens.test.httpRequest.HttpResponseException; import io.supertokens.test.totp.TOTPRecipeTest; import io.supertokens.thirdparty.InvalidProviderConfigException; +import io.supertokens.totp.Totp; import io.supertokens.utils.SemVer; import org.junit.After; import org.junit.AfterClass; @@ -249,9 +250,12 @@ public void testDevicesWorkAppWide() throws Exception { int userCount = 1; for (TenantIdentifier tenant1 : tenants) { createDevice(tenant1, "user" + userCount); + TOTPDevice device = Totp.getDevices(t1.withStorage(StorageLayer.getStorage(tenant1, process.getProcess())).toAppIdentifierWithStorage(), "user" + userCount)[0]; + String validTotp = TOTPRecipeTest.generateTotpCode(process.getProcess(), device); + verifyDevice(tenant1, "user" + userCount, validTotp); for (TenantIdentifier tenant2 : tenants) { - createDeviceAlreadyExists(tenant2, "user1"); + createDeviceAlreadyExists(tenant2, "user" + userCount); } userCount++; diff --git a/src/test/java/io/supertokens/test/totp/api/RemoveTotpDeviceAPITest.java b/src/test/java/io/supertokens/test/totp/api/RemoveTotpDeviceAPITest.java index 845b42852..bb4a13a53 100644 --- a/src/test/java/io/supertokens/test/totp/api/RemoveTotpDeviceAPITest.java +++ b/src/test/java/io/supertokens/test/totp/api/RemoveTotpDeviceAPITest.java @@ -180,7 +180,8 @@ public void testApi() throws Exception { null, Utils.getCdiVersionStringLatestForTests(), "totp"); - assert res3.get("status").getAsString().equals("TOTP_NOT_ENABLED_ERROR"); + assert res3.get("status").getAsString().equals("OK"); + assert res3.get("didDeviceExist").getAsBoolean() == false; } process.kill(); diff --git a/src/test/java/io/supertokens/test/totp/api/TotpUserIdMappingTest.java b/src/test/java/io/supertokens/test/totp/api/TotpUserIdMappingTest.java index 7e135958c..112cd0c14 100644 --- a/src/test/java/io/supertokens/test/totp/api/TotpUserIdMappingTest.java +++ b/src/test/java/io/supertokens/test/totp/api/TotpUserIdMappingTest.java @@ -61,7 +61,7 @@ public void testExternalUserIdTranslation() throws Exception { body.addProperty("userId", externalUserId); body.addProperty("deviceName", "d1"); - body.addProperty("skew", 0); + body.addProperty("skew", 1); body.addProperty("period", 30); // Register 1st device @@ -77,7 +77,7 @@ public void testExternalUserIdTranslation() throws Exception { "totp"); assert res1.get("status").getAsString().equals("OK"); String d1Secret = res1.get("secret").getAsString(); - TOTPDevice device1 = new TOTPDevice(externalUserId, "deviceName", d1Secret, 30, 0, false); + TOTPDevice device1 = new TOTPDevice(externalUserId, "deviceName", d1Secret, 30, 1, false); body.addProperty("deviceName", "d2"); @@ -93,14 +93,14 @@ public void testExternalUserIdTranslation() throws Exception { "totp"); assert res2.get("status").getAsString().equals("OK"); String d2Secret = res2.get("secret").getAsString(); - TOTPDevice device2 = new TOTPDevice(externalUserId, "deviceName", d2Secret, 30, 0, false); + TOTPDevice device2 = new TOTPDevice(externalUserId, "d2", d2Secret, 30, 1, false); // Verify d1 but not d2: JsonObject verifyD1Input = new JsonObject(); verifyD1Input.addProperty("userId", externalUserId); - String d1Totp = TOTPRecipeTest.generateTotpCode(process.getProcess(), device1); + String d1VerifyTotp = TOTPRecipeTest.generateTotpCode(process.getProcess(), device1); verifyD1Input.addProperty("deviceName", "d1"); - verifyD1Input.addProperty("totp", d1Totp ); + verifyD1Input.addProperty("totp", d1VerifyTotp); JsonObject verifyD1Res = HttpRequestForTesting.sendJsonPOSTRequest( process.getProcess(), @@ -116,25 +116,43 @@ public void testExternalUserIdTranslation() throws Exception { assert verifyD1Res.get("status").getAsString().equals("OK"); assert verifyD1Res.get("wasAlreadyVerified").getAsBoolean() == false; - // use d2 to login in totp: - JsonObject loginInput = new JsonObject(); - loginInput.addProperty("userId", externalUserId); - String d2Totp = TOTPRecipeTest.generateTotpCode(process.getProcess(), device2); - loginInput.addProperty("totp", d2Totp); // use code from d2 which is unverified - loginInput.addProperty("allowUnverifiedDevices", true); + // use d2 to login in totp: (should fail coz it's not verified) + JsonObject d2LoginInput = new JsonObject(); + d2LoginInput.addProperty("userId", externalUserId); + String d2Totp = TOTPRecipeTest.generateTotpCode(process.getProcess(), device2, 1); + d2LoginInput.addProperty("totp", d2Totp); // use code from d2 which is unverified - JsonObject loginRes = HttpRequestForTesting.sendJsonPOSTRequest( + JsonObject d2LoginRes = HttpRequestForTesting.sendJsonPOSTRequest( process.getProcess(), "", "http://localhost:3567/recipe/totp/verify", - loginInput, + d2LoginInput, 1000, 1000, null, Utils.getCdiVersionStringLatestForTests(), "totp"); - assert loginRes.get("status").getAsString().equals("OK"); + assert d2LoginRes.get("status").getAsString().equals("INVALID_TOTP_ERROR"); + + // use d1 to login in totp: (should pass) + JsonObject d1LoginInput = new JsonObject(); + d1LoginInput.addProperty("userId", externalUserId); + String d1Totp = TOTPRecipeTest.generateTotpCode(process.getProcess(), device1, 1); + d1LoginInput.addProperty("totp", d1Totp); // use code from d2 which is unverified + + JsonObject d1LoginRes = HttpRequestForTesting.sendJsonPOSTRequest( + process.getProcess(), + "", + "http://localhost:3567/recipe/totp/verify", + d1LoginInput, + 1000, + 1000, + null, + Utils.getCdiVersionStringLatestForTests(), + "totp"); + + assert d1LoginRes.get("status").getAsString().equals("OK"); // Change the name of d1 to d3: JsonObject updateDeviceNameInput = new JsonObject(); diff --git a/src/test/java/io/supertokens/test/totp/api/UpdateTotpDeviceAPITest.java b/src/test/java/io/supertokens/test/totp/api/UpdateTotpDeviceAPITest.java index c94d72ff0..29be8c12c 100644 --- a/src/test/java/io/supertokens/test/totp/api/UpdateTotpDeviceAPITest.java +++ b/src/test/java/io/supertokens/test/totp/api/UpdateTotpDeviceAPITest.java @@ -199,7 +199,7 @@ public void testApi() throws Exception { null, Utils.getCdiVersionStringLatestForTests(), "totp"); - assert res4.get("status").getAsString().equals("TOTP_NOT_ENABLED_ERROR"); + assert res4.get("status").getAsString().equals("UNKNOWN_DEVICE_ERROR"); } process.kill(); diff --git a/src/test/java/io/supertokens/test/totp/api/VerifyTotpAPITest.java b/src/test/java/io/supertokens/test/totp/api/VerifyTotpAPITest.java index 57d765a51..08c836586 100644 --- a/src/test/java/io/supertokens/test/totp/api/VerifyTotpAPITest.java +++ b/src/test/java/io/supertokens/test/totp/api/VerifyTotpAPITest.java @@ -23,6 +23,7 @@ import org.junit.Test; import org.junit.rules.TestRule; +import static io.supertokens.test.totp.TOTPRecipeTest.generateTotpCode; import static org.junit.Assert.*; public class VerifyTotpAPITest { @@ -40,7 +41,7 @@ public void beforeEach() { Utils.reset(); } - private Exception updateDeviceRequest(TestingProcessManager.TestingProcess process, JsonObject body) { + private Exception verifyTotpCodeRequest(TestingProcessManager.TestingProcess process, JsonObject body) { return assertThrows( io.supertokens.test.httpRequest.HttpResponseException.class, () -> HttpRequestForTesting.sendJsonPOSTRequest( @@ -93,7 +94,7 @@ public void testApi() throws Exception { JsonObject createDeviceReq = new JsonObject(); createDeviceReq.addProperty("userId", "user-id"); createDeviceReq.addProperty("deviceName", "deviceName"); - createDeviceReq.addProperty("period", 30); + createDeviceReq.addProperty("period", 2); createDeviceReq.addProperty("skew", 0); JsonObject createDeviceRes = HttpRequestForTesting.sendJsonPOSTRequest( @@ -109,44 +110,56 @@ public void testApi() throws Exception { assertEquals(createDeviceRes.get("status").getAsString(), "OK"); String secretKey = createDeviceRes.get("secret").getAsString(); - TOTPDevice device = new TOTPDevice("user-id", "deviceName", secretKey, 30, 0, false); + TOTPDevice device = new TOTPDevice("user-id", "deviceName", secretKey, 2, 0, false); - // Start the actual tests for update device API: + JsonObject verifyDeviceReq = new JsonObject(); + verifyDeviceReq.addProperty("userId", device.userId); + verifyDeviceReq.addProperty("deviceName", device.deviceName); + verifyDeviceReq.addProperty("totp", generateTotpCode(process.getProcess(), device)); + JsonObject verifyDeviceRes = HttpRequestForTesting.sendJsonPOSTRequest( + process.getProcess(), + "", + "http://localhost:3567/recipe/totp/device/verify", + verifyDeviceReq, + 1000, + 1000, + null, + Utils.getCdiVersionStringLatestForTests(), + "totp"); + assertEquals(verifyDeviceRes.get("status").getAsString(), "OK"); + + // Start the actual tests for update device API: JsonObject body = new JsonObject(); // Missing userId/deviceName/skew/period { - Exception e = updateDeviceRequest(process, body); + Exception e = verifyTotpCodeRequest(process, body); checkFieldMissingErrorResponse(e, "userId"); body.addProperty("userId", ""); - e = updateDeviceRequest(process, body); + e = verifyTotpCodeRequest(process, body); checkFieldMissingErrorResponse(e, "totp"); - - body.addProperty("totp", ""); - e = updateDeviceRequest(process, body); - checkFieldMissingErrorResponse(e, "allowUnverifiedDevices"); } // Invalid userId/deviceName/skew/period { - body.addProperty("allowUnverifiedDevices", true); - Exception e = updateDeviceRequest(process, body); + body.addProperty("totp", ""); + Exception e = verifyTotpCodeRequest(process, body); checkResponseErrorContains(e, "userId cannot be empty"); // Note that this is not a field missing error body.addProperty("userId", device.userId); - e = updateDeviceRequest(process, body); + e = verifyTotpCodeRequest(process, body); checkResponseErrorContains(e, "totp must be 6 characters long"); // test totp of length 5: body.addProperty("totp", "12345"); - e = updateDeviceRequest(process, body); + e = verifyTotpCodeRequest(process, body); checkResponseErrorContains(e, "totp must be 6 characters long"); // test totp of length 8: body.addProperty("totp", "12345678"); - e = updateDeviceRequest(process, body); + e = verifyTotpCodeRequest(process, body); checkResponseErrorContains(e, "totp must be 6 characters long"); // but let's pass invalid code first @@ -178,10 +191,10 @@ public void testApi() throws Exception { assert res3.get("retryAfterMs") != null; // wait for cooldown to end (1s) - Thread.sleep(1000); + Thread.sleep(1200); // should pass now on valid code - String validTotp = TOTPRecipeTest.generateTotpCode(process.getProcess(), device); + String validTotp = generateTotpCode(process.getProcess(), device); body.addProperty("totp", validTotp); JsonObject res = HttpRequestForTesting.sendJsonPOSTRequest( process.getProcess(), @@ -210,7 +223,7 @@ public void testApi() throws Exception { assert res2.get("status").getAsString().equals("INVALID_TOTP_ERROR"); // Try with a new valid code during rate limiting: - body.addProperty("totp", TOTPRecipeTest.generateTotpCode(process.getProcess(), device)); + body.addProperty("totp", generateTotpCode(process.getProcess(), device)); res = HttpRequestForTesting.sendJsonPOSTRequest( process.getProcess(), "", @@ -235,7 +248,7 @@ public void testApi() throws Exception { null, Utils.getCdiVersionStringLatestForTests(), "totp"); - assert res5.get("status").getAsString().equals("TOTP_NOT_ENABLED_ERROR"); + assert res5.get("status").getAsString().equals("UNKNOWN_USER_ID_ERROR"); } process.kill(); diff --git a/src/test/java/io/supertokens/test/totp/api/VerifyTotpDeviceAPITest.java b/src/test/java/io/supertokens/test/totp/api/VerifyTotpDeviceAPITest.java index 6df604603..ca5e1c43b 100644 --- a/src/test/java/io/supertokens/test/totp/api/VerifyTotpDeviceAPITest.java +++ b/src/test/java/io/supertokens/test/totp/api/VerifyTotpDeviceAPITest.java @@ -238,7 +238,7 @@ public void testApi() throws Exception { null, Utils.getCdiVersionStringLatestForTests(), "totp"); - assert res5.get("status").getAsString().equals("TOTP_NOT_ENABLED_ERROR"); + assert res5.get("status").getAsString().equals("UNKNOWN_DEVICE_ERROR"); } process.kill();