refactor: Replace TOTP_NOT_ENABLED_ERROR status and make deviceName optional

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

@@ -284,7 +284,7 @@ private static void checkAndStoreCode(TenantIdentifierWithStorage tenantIdentifi
     @TestOnly
     public static boolean verifyDevice(Main main,
                                        String userId, String deviceName, String code)
-            throws TotpNotEnabledException, UnknownDeviceException, InvalidTotpException,
+            throws UnknownDeviceException, InvalidTotpException,
             LimitReachedException, StorageQueryException, StorageTransactionLogicException {
         try {
             return verifyDevice(new TenantIdentifierWithStorage(null, null, null, StorageLayer.getStorage(main)), main,
@@ -296,7 +296,7 @@ public static boolean verifyDevice(Main main,
 
     public static boolean verifyDevice(TenantIdentifierWithStorage tenantIdentifierWithStorage, Main main,
                                        String userId, String deviceName, String code)
-            throws TotpNotEnabledException, UnknownDeviceException, InvalidTotpException,
+            throws UnknownDeviceException, InvalidTotpException,
             LimitReachedException, StorageQueryException, StorageTransactionLogicException,
             TenantOrAppNotFoundException {
         // Here boolean return value tells whether the device has been
@@ -312,7 +312,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,8 +337,12 @@ 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 (TotpNotEnabledException 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
@@ -347,7 +351,7 @@ public static boolean verifyDevice(TenantIdentifierWithStorage tenantIdentifierW
     @TestOnly
     public static void verifyCode(Main main, String userId,
                                   String code, boolean allowUnverifiedDevices)
-            throws TotpNotEnabledException, InvalidTotpException, LimitReachedException,
+            throws InvalidTotpException, LimitReachedException,
             StorageQueryException, StorageTransactionLogicException, FeatureNotEnabledException {
         try {
             verifyCode(new TenantIdentifierWithStorage(null, null, null, StorageLayer.getStorage(main)), main,
@@ -359,7 +363,7 @@ public static void verifyCode(Main main, String userId,
 
     public static void verifyCode(TenantIdentifierWithStorage tenantIdentifierWithStorage, Main main, String userId,
                                   String code, boolean allowUnverifiedDevices)
-            throws TotpNotEnabledException, InvalidTotpException, LimitReachedException,
+            throws InvalidTotpException, LimitReachedException,
             StorageQueryException, StorageTransactionLogicException, FeatureNotEnabledException,
             TenantOrAppNotFoundException {
 
@@ -374,7 +378,8 @@ 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 InvalidTotpException();
         }
 
         // Filter out unverified devices:
@@ -386,13 +391,19 @@ public static void verifyCode(TenantIdentifierWithStorage tenantIdentifierWithSt
         // 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.
-        checkAndStoreCode(tenantIdentifierWithStorage, main, userId, devices, code);
+        try{
+            checkAndStoreCode(tenantIdentifierWithStorage, main, userId, devices, code);
+        } catch (TotpNotEnabledException e) {
+            // User must have deleted the device in parallel
+            // since they cannot un-verify a device (no API exists)
+            throw new InvalidTotpException();
+        }
     }
 
     @TestOnly
     public static void removeDevice(Main main, String userId,
                                     String deviceName)
-            throws StorageQueryException, UnknownDeviceException, TotpNotEnabledException,
+            throws StorageQueryException, UnknownDeviceException,
             StorageTransactionLogicException {
         try {
             removeDevice(new AppIdentifierWithStorage(null, null, StorageLayer.getStorage(main)),
@@ -407,7 +418,7 @@ public static void removeDevice(Main main, String userId,
      */
     public static void removeDevice(AppIdentifierWithStorage appIdentifierWithStorage, String userId,
                                     String deviceName)
-            throws StorageQueryException, UnknownDeviceException, TotpNotEnabledException,
+            throws StorageQueryException, UnknownDeviceException,
             StorageTransactionLogicException, TenantOrAppNotFoundException {
         TOTPSQLStorage storage = appIdentifierWithStorage.getTOTPStorage();
 
@@ -432,12 +443,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;
             }
 
@@ -448,8 +453,7 @@ public static void removeDevice(AppIdentifierWithStorage appIdentifierWithStorag
     @TestOnly
     public static void updateDeviceName(Main main, String userId,
                                         String oldDeviceName, String newDeviceName)
-            throws StorageQueryException, DeviceAlreadyExistsException, UnknownDeviceException,
-            TotpNotEnabledException {
+            throws StorageQueryException, DeviceAlreadyExistsException, UnknownDeviceException {
         try {
             updateDeviceName(new AppIdentifierWithStorage(null, null, StorageLayer.getStorage(main)),
                     userId, oldDeviceName, newDeviceName);
@@ -460,25 +464,14 @@ public static void updateDeviceName(Main main, String userId,
 
     public static void updateDeviceName(AppIdentifierWithStorage appIdentifierWithStorage, String userId,
                                         String oldDeviceName, String newDeviceName)
-            throws StorageQueryException, DeviceAlreadyExistsException, UnknownDeviceException,
-            TotpNotEnabledException, TenantOrAppNotFoundException {
+            throws StorageQueryException, DeviceAlreadyExistsException, UnknownDeviceException, 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 +481,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;
     }
 

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

@@ -11,7 +11,6 @@
 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;
@@ -143,9 +142,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);

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);
         }

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,12 +74,9 @@ 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);
+            result.addProperty("didDeviceExist", false); // Pattern not consistent with PUT API. We should fix?
             super.sendJsonResponse(200, result, resp);
         } catch (StorageQueryException | StorageTransactionLogicException | TenantOrAppNotFoundException e) {
             throw new ServletException(e);

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

@@ -4,19 +4,15 @@
 
 import com.google.gson.JsonObject;
 
-import io.supertokens.AppIdentifierWithStorageAndUserIdMapping;
 import io.supertokens.Main;
 import io.supertokens.TenantIdentifierWithStorageAndUserIdMapping;
 import io.supertokens.featureflag.exceptions.FeatureNotEnabledException;
 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.TenantIdentifierWithStorage;
 import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException;
-import io.supertokens.pluginInterface.totp.exception.TotpNotEnabledException;
-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 +76,6 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I
 
             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);

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);

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

@@ -167,7 +167,7 @@ public void createDeviceAndVerifyCodeTest() throws Exception {
         TOTPDevice device = Totp.registerDevice(main, "user", "device1", 1, 1);
 
         // Try login with non-existent user:
-        assertThrows(TotpNotEnabledException.class,
+        assertThrows(InvalidTotpException.class, // TODO: Can we throw a better error?
                 () -> Totp.verifyCode(main, "non-existent-user", "any-code", true));
 
         // {Code: [INVALID, VALID]} * {Devices: [VERIFIED_ONLY, ALL]}
@@ -378,7 +378,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
@@ -428,7 +428,7 @@ public void removeDeviceTest() throws Exception {
         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"));
@@ -462,8 +462,8 @@ public void removeDeviceTest() throws Exception {
             // 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");
@@ -490,7 +490,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 +526,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);

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();

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();