fix: validating firstFactors not to contain special chars (#1050)

* fix: validating firstFactors not to contain special chars * fix: review fixes * fix: review fix: allowing . (dot) * feat: tests for the validation --------- Co-authored-by: Sattvik Chakravarthy <[email protected]>
supertokens · Oct 9, 2024 · 1ecf114 · 1ecf114
1 parent 6f9568a
commit 1ecf114
Show file tree

Hide file tree

Showing 6 changed files with 150 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,7 +5,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres
 to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## Unreleased
+## [9.2.3] - 2024-10-09
+
+- Adds validation to firstFactors and requiredSecondaryFactors names while creating tenants/apps/etc. to not allow 
+  special chars.
 
 ## [9.2.2] - 2024-09-04
 

diff --git a/build.gradle b/build.gradle
@@ -19,7 +19,7 @@ compileTestJava { options.encoding = "UTF-8" }
 //    }
 //}
 
-version = "9.2.2"
+version = "9.2.3"
 
 
 repositories {

diff --git a/src/main/java/io/supertokens/webserver/api/multitenancy/BaseCreateOrUpdate.java b/src/main/java/io/supertokens/webserver/api/multitenancy/BaseCreateOrUpdate.java
@@ -102,6 +102,7 @@ protected void handle(HttpServletRequest req, HttpServletResponse resp, TenantId
 
             // Apply updates based on CDI version
             tenantConfig = applyTenantUpdates(tenantConfig, getVersionFromRequest(req), isV2, input);
+            validateFactorsName(tenantConfig);
 
             // Write tenant config to db
             createOrUpdate(req, sourceTenantIdentifier, tenantConfig);
@@ -938,6 +939,27 @@ private static TenantConfig applyTenantUpdates_5_0(TenantConfig tenantConfig, Js
         return tenantConfig;
     }
 
+    private static void validateFactorsName(TenantConfig tenantConfig) throws ServletException{
+        if(!areFactorNamesValid(tenantConfig.firstFactors)){
+            throw new ServletException(new BadRequestException("firstFactors should contain only 0-9,a-z,A-Z,_,.,- characters"));
+        }
+        if(!areFactorNamesValid(tenantConfig.requiredSecondaryFactors)){
+            throw new ServletException(new BadRequestException("requiredSecondaryFactors should contain only 0-9,a-z,A-Z,_,.,- characters"));
+        }
+    }
+
+    private static boolean areFactorNamesValid(String[] factors) {
+        if(factors != null && factors.length > 0) {
+            String allowedPattern = "^[0-9a-zA-Z_(\\.)-]+$";
+            for(String factor: factors){
+                if(factor != null && !factor.matches(allowedPattern)){
+                    return false;
+                }
+            }
+        }
+        return true;
+    }
+
     private static TenantConfig applyV2TenantUpdates_5_1(TenantConfig tenantConfig, JsonObject input)
             throws ServletException {
         if (input.has("emailPasswordEnabled")) {

diff --git a/src/test/java/io/supertokens/test/multitenant/api/TestApp5_1.java b/src/test/java/io/supertokens/test/multitenant/api/TestApp5_1.java
@@ -861,6 +861,45 @@ public void testDuplicateValuesInFirstFactorsAndRequiredSecondaryFactors() throw
         }
     }
 
+    @Test
+    public void testInvalidValuesInFirstFactorsAndRequiredSecondaryFactors() throws Exception {
+        if (StorageLayer.getStorage(process.getProcess()).getType() != STORAGE_TYPE.SQL) {
+            return;
+        }
+
+        JsonObject config = new JsonObject();
+        StorageLayer.getBaseStorage(process.getProcess()).modifyConfigToAddANewUserPoolForTesting(config, 1);
+
+        String[] factors = new String[]{"not!okay", "some-special-@charz", "i*dont*know", "custom//"};
+        try {
+            createApp(
+                    process.getProcess(),
+                    new TenantIdentifier(null, null, null),
+                    "a1", true, factors, false, null,
+                    config);
+            fail();
+        } catch (HttpResponseException e) {
+            assertEquals(400, e.statusCode);
+            assertEquals(
+                    "Http error. Status Code: 400. Message: firstFactors should contain only 0-9,a-z,A-Z,_,.,- characters",
+                    e.getMessage());
+        }
+
+        try {
+            createApp(
+                    process.getProcess(),
+                    new TenantIdentifier(null, null, null),
+                    "a1", false, null, true, factors,
+                    config);
+            fail();
+        } catch (HttpResponseException e) {
+            assertEquals(400, e.statusCode);
+            assertEquals(
+                    "Http error. Status Code: 400. Message: requiredSecondaryFactors should contain only 0-9,a-z,A-Z,_,.,- characters",
+                    e.getMessage());
+        }
+    }
+
     private static JsonObject createApp(Main main, TenantIdentifier sourceTenant, String appId,
                                         boolean setFirstFactors, String[] firstFactors,
                                         boolean setRequiredSecondaryFactors, String[] requiredSecondaryFactors,

diff --git a/src/test/java/io/supertokens/test/multitenant/api/TestConnectionUriDomain5_1.java b/src/test/java/io/supertokens/test/multitenant/api/TestConnectionUriDomain5_1.java
@@ -745,6 +745,51 @@ public void testDuplicateValuesInFirstFactorsAndRequiredSecondaryFactors() throw
         }
     }
 
+    @Test
+    public void testInvalidValuesInFirstFactorsAndRequiredSecondaryFactors() throws Exception {
+        if (StorageLayer.getStorage(process.getProcess()).getType() != STORAGE_TYPE.SQL) {
+            return;
+        }
+
+        if (StorageLayer.isInMemDb(process.getProcess())) {
+            return;
+        }
+
+        JsonObject config = new JsonObject();
+        StorageLayer.getBaseStorage(process.getProcess()).modifyConfigToAddANewUserPoolForTesting(config, 1);
+
+        String[] factors = new String[]{"inv@lid", "email?password", "cus!tom"};
+        try {
+            createConnectionUriDomain(
+                    process.getProcess(),
+                    new TenantIdentifier(null, null, null),
+                    "127.0.0.1",
+                    true, factors, false, null,
+                    config);
+            fail();
+        } catch (HttpResponseException e) {
+            assertEquals(400, e.statusCode);
+            assertEquals(
+                    "Http error. Status Code: 400. Message: firstFactors should contain only 0-9,a-z,A-Z,_,.,- characters",
+                    e.getMessage());
+        }
+
+        try {
+            createConnectionUriDomain(
+                    process.getProcess(),
+                    new TenantIdentifier(null, null, null),
+                    "127.0.0.1",
+                    false, null, true, factors,
+                    config);
+            fail();
+        } catch (HttpResponseException e) {
+            assertEquals(400, e.statusCode);
+            assertEquals(
+                    "Http error. Status Code: 400. Message: requiredSecondaryFactors should contain only 0-9,a-z,A-Z,_,.,- characters",
+                    e.getMessage());
+        }
+    }
+
     private static JsonObject createConnectionUriDomain(Main main, TenantIdentifier sourceTenant,
                                                         String connectionUriDomain,
                                                         boolean setFirstFactors, String[] firstFactors,

diff --git a/src/test/java/io/supertokens/test/multitenant/api/TestTenant5_1.java b/src/test/java/io/supertokens/test/multitenant/api/TestTenant5_1.java
@@ -612,6 +612,45 @@ public void testDuplicateValuesInFirstFactorsAndRequiredSecondaryFactors() throw
         }
     }
 
+    @Test
+    public void testInvalidValuesInFirstFactorsAndRequiredSecondaryFactors() throws Exception {
+        if (StorageLayer.getStorage(process.getProcess()).getType() != STORAGE_TYPE.SQL) {
+            return;
+        }
+
+        JsonObject config = new JsonObject();
+        StorageLayer.getBaseStorage(process.getProcess()).modifyConfigToAddANewUserPoolForTesting(config, 1);
+
+        String[] factors = new String[]{"~notvalid", "it's-also-not", "almost.good!1!", "itsok"};
+        try {
+            createTenant(
+                    process.getProcess(),
+                    new TenantIdentifier(null, null, null),
+                    "t1", true, factors, false, null,
+                    config);
+            fail();
+        } catch (HttpResponseException e) {
+            assertEquals(400, e.statusCode);
+            assertEquals(
+                    "Http error. Status Code: 400. Message: firstFactors should contain only 0-9,a-z,A-Z,_,.,- characters",
+                    e.getMessage());
+        }
+
+        try {
+            createTenant(
+                    process.getProcess(),
+                    new TenantIdentifier(null, null, null),
+                    "t1", false, null, true, factors,
+                    config);
+            fail();
+        } catch (HttpResponseException e) {
+            assertEquals(400, e.statusCode);
+            assertEquals(
+                    "Http error. Status Code: 400. Message: requiredSecondaryFactors should contain only 0-9,a-z,A-Z,_,.,- characters",
+                    e.getMessage());
+        }
+    }
+
     private static JsonObject createTenant(Main main, TenantIdentifier sourceTenant, String tenantId,
                                            boolean setFirstFactors, String[] firstFactors,
                                            boolean setRequiredSecondaryFactors, String[] requiredSecondaryFactors,
-Original file line number
+Diff line change
@@ Expand Up / @@ -19,7 +19,7 @@ compileTestJava { options.encoding = "UTF-8" } @@
     //    }
     //}
-    version = "9.2.2"
+    version = "9.2.3"
     repositories {
@@ Expand Down @@