From ab7d2bf62b2bcb98fc2f551cdf5d8472143fc54b Mon Sep 17 00:00:00 2001
From: Ankit Tiwari <ankucodes@gmail.com>
Date: Wed, 28 Feb 2024 15:14:40 +0530
Subject: [PATCH] fix: Make period and skew optional

---
 .../bulkimport/BulkImportUserUtils.java       | 24 +++++++++++------
 .../apis/AddBulkImportUsersTest.java          | 26 +++++++++++++++++--
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/src/main/java/io/supertokens/bulkimport/BulkImportUserUtils.java b/src/main/java/io/supertokens/bulkimport/BulkImportUserUtils.java
index e94ce5883..c006c3640 100644
--- a/src/main/java/io/supertokens/bulkimport/BulkImportUserUtils.java
+++ b/src/main/java/io/supertokens/bulkimport/BulkImportUserUtils.java
@@ -95,8 +95,8 @@ private static List<TotpDevice> getParsedTotpDevices(JsonObject userData, List<S
             JsonObject jsonTotpDevice = jsonTotpDeviceEl.getAsJsonObject();
 
             String secretKey = parseAndValidateFieldType(jsonTotpDevice, "secretKey", ValueType.STRING, true, String.class, errors, " for a totp device.");
-            Integer period = parseAndValidateFieldType(jsonTotpDevice, "period", ValueType.INTEGER, true, Integer.class, errors, " for a totp device.");
-            Integer skew = parseAndValidateFieldType(jsonTotpDevice, "skew", ValueType.INTEGER, true, Integer.class, errors, " for a totp device.");
+            Integer period = parseAndValidateFieldType(jsonTotpDevice, "period", ValueType.INTEGER, false, Integer.class, errors, " for a totp device.");
+            Integer skew = parseAndValidateFieldType(jsonTotpDevice, "skew", ValueType.INTEGER, false, Integer.class, errors, " for a totp device.");
             String deviceName = parseAndValidateFieldType(jsonTotpDevice, "deviceName", ValueType.STRING, false, String.class, errors, " for a totp device.");
 
             secretKey = validateAndNormaliseTotpSecretKey(secretKey, errors);
@@ -220,21 +220,29 @@ private static String validateAndNormaliseTotpSecretKey(String secretKey, List<S
     }
 
     private static Integer validateAndNormaliseTotpPeriod(Integer period, List<String> errors) {
-        // We don't perform any normalisation on the period in ImportTotpDeviceAPI.java other than checking if it is > 0
-        if (period != null && period.intValue() < 1) {
+        // We default to 30 if period is null
+        if (period == null) {
+            return 30;
+        }
+
+        if (period.intValue() < 1) {
             errors.add("period should be > 0 for a totp device.");
             return null;
         }
-        return period != null ? period.intValue() : null;
+        return period;
     }
 
     private static Integer validateAndNormaliseTotpSkew(Integer skew, List<String> errors) {
-        // We don't perform any normalisation on the period in ImportTotpDeviceAPI.java other than checking if it is >= 0
-        if (skew != null && skew.intValue() < 0) {
+        // We default to 1 if skew is null
+        if (skew == null) {
+            return 1;
+        }
+
+        if (skew.intValue() < 0) {
             errors.add("skew should be >= 0 for a totp device.");
             return null;
         }
-        return skew != null ? skew.intValue() : null;
+        return skew;
     }
 
     private static String validateAndNormaliseTotpDeviceName(String deviceName, List<String> errors) {
diff --git a/src/test/java/io/supertokens/test/bulkimport/apis/AddBulkImportUsersTest.java b/src/test/java/io/supertokens/test/bulkimport/apis/AddBulkImportUsersTest.java
index 709455605..b3e37cbf3 100644
--- a/src/test/java/io/supertokens/test/bulkimport/apis/AddBulkImportUsersTest.java
+++ b/src/test/java/io/supertokens/test/bulkimport/apis/AddBulkImportUsersTest.java
@@ -152,6 +152,20 @@ public void shouldThrow400Error() throws Exception {
                 assertEquals(responseString,
                         "{\"error\":\"" + genericErrMsg +  "\",\"users\":[{\"index\":0,\"errors\":[\"externalUserId should be of type string.\",\"userRoles should be of type array of string.\",\"totpDevices should be of type array of object.\",\"loginMethods is required.\"]}]}");
             }
+            // secretKey is required in totpDevices
+            try {
+                JsonObject request = new JsonParser()
+                        .parse("{\"users\":[{\"totpDevices\":[{\"secret\": \"secret\"}]}]}")
+                        .getAsJsonObject();
+                HttpRequestForTesting.sendJsonPOSTRequest(process.getProcess(), "",
+                        "http://localhost:3567/bulk-import/users",
+                        request, 1000, 1000, null, Utils.getCdiVersionStringLatestForTests(), null);
+            } catch (io.supertokens.test.httpRequest.HttpResponseException e) {
+                String responseString = getResponseMessageFromError(e.getMessage());
+                assertEquals(400, e.statusCode);
+                assertEquals(responseString,
+                        "{\"error\":\"" + genericErrMsg +  "\",\"users\":[{\"index\":0,\"errors\":[\"secretKey is required for a totp device.\",\"loginMethods is required.\"]}]}");
+            }
             // Invalid role (does not exist)
             try {
                 JsonObject request = new JsonParser()
@@ -444,8 +458,16 @@ public void shouldNormaliseFields() throws Exception {
         assertEquals(1, bulkImportUsers.size());
 
         JsonObject bulkImportUserJson = bulkImportUsers.get(0).getAsJsonObject();
-        JsonArray loginMethods = bulkImportUserJson.getAsJsonArray("loginMethods");
 
+        // Test if default values were set in totpDevices
+        JsonArray totpDevices = bulkImportUserJson.getAsJsonArray("totpDevices");
+        for (int i = 0; i < totpDevices.size(); i++) {
+            JsonObject totpDevice = totpDevices.get(i).getAsJsonObject();
+            assertEquals(30, totpDevice.get("period").getAsInt());
+            assertEquals(1, totpDevice.get("skew").getAsInt());
+        }
+
+        JsonArray loginMethods = bulkImportUserJson.getAsJsonArray("loginMethods");
         for (int i = 0; i < loginMethods.size(); i++) {
             JsonObject loginMethod = loginMethods.get(i).getAsJsonObject();
             if (loginMethod.has("email")) {
@@ -474,7 +496,7 @@ public static JsonObject generateUsersJson(int numberOfUsers) {
             user.addProperty("externalUserId", UUID.randomUUID().toString());
             user.add("userMetadata", parser.parse("{\"key1\":\"value1\",\"key2\":{\"key3\":\"value3\"}}"));
             user.add("userRoles", parser.parse("[\"role1\", \"role2\"]"));
-            user.add("totpDevices", parser.parse("[{\"secretKey\":\"secretKey\",\"period\": 30,\"skew\":1,\"deviceName\":\"deviceName\"}]"));
+            user.add("totpDevices", parser.parse("[{\"secretKey\":\"secretKey\",\"deviceName\":\"deviceName\"}]"));
 
             String email = " johndoe+" + i + "@gmail.com ";