From 1ecf114d4cecf89a69a98d46713f60fb6bd42f83 Mon Sep 17 00:00:00 2001 From: Tamas Soltesz Date: Wed, 9 Oct 2024 13:21:08 +0200 Subject: [PATCH] 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 --- CHANGELOG.md | 5 ++- build.gradle | 2 +- .../api/multitenancy/BaseCreateOrUpdate.java | 22 +++++++++ .../test/multitenant/api/TestApp5_1.java | 39 ++++++++++++++++ .../api/TestConnectionUriDomain5_1.java | 45 +++++++++++++++++++ .../test/multitenant/api/TestTenant5_1.java | 39 ++++++++++++++++ 6 files changed, 150 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 001af5c22..d2a4547f5 100644 --- 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 index c565f1bdc..7ede11ac5 100644 --- 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 index 2ee4aeb46..1ae57140d 100644 --- 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 index 320231971..623ce71fc 100644 --- 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 index 065c3b0d7..087e0ae8e 100644 --- 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 index 728e65625..1ef07d4f2 100644 --- 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,