From 280fa7a44532d56e719cc18453e61811d606a4f5 Mon Sep 17 00:00:00 2001 From: tamassoltesz Date: Fri, 27 Sep 2024 17:46:32 +0200 Subject: [PATCH 1/4] fix: validating firstFactors not to contain special chars --- CHANGELOG.md | 2 ++ build.gradle | 2 +- .../api/multitenancy/BaseCreateOrUpdate.java | 12 ++++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 001af5c22..b9e2869fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased +- Adds validation to firstFactors name while creating tenants/apps/etc. to not allow special chars. + ## [9.2.2] - 2024-09-04 - Adds index on `last_active_time` for `user_last_active` table to improve the performance of MAU computation. 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..e0eb9fb13 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); + validateFirstFactorsName(tenantConfig); // Write tenant config to db createOrUpdate(req, sourceTenantIdentifier, tenantConfig); @@ -938,6 +939,17 @@ private static TenantConfig applyTenantUpdates_5_0(TenantConfig tenantConfig, Js return tenantConfig; } + private static void validateFirstFactorsName(TenantConfig tenantConfig) throws ServletException { + if(tenantConfig.firstFactors != null && tenantConfig.firstFactors.length > 0) { + String allowedPattern = "^[0-9a-z-]+$"; + for(String firstFactor: tenantConfig.firstFactors){ + if(firstFactor != null && !firstFactor.matches(allowedPattern)){ + throw new ServletException(new BadRequestException("firstFactors should not contain only 0-9,a-z,- characters")); + } + } + } + } + private static TenantConfig applyV2TenantUpdates_5_1(TenantConfig tenantConfig, JsonObject input) throws ServletException { if (input.has("emailPasswordEnabled")) { From 18393671e0cb7a181dc7e1f12a65942306f6cfd9 Mon Sep 17 00:00:00 2001 From: tamassoltesz Date: Thu, 3 Oct 2024 14:12:41 +0200 Subject: [PATCH 2/4] fix: review fixes --- CHANGELOG.md | 3 ++- .../api/multitenancy/BaseCreateOrUpdate.java | 24 +++++++++++++------ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9e2869fa..eea9a1011 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased -- Adds validation to firstFactors name while creating tenants/apps/etc. to not allow special chars. +- 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/src/main/java/io/supertokens/webserver/api/multitenancy/BaseCreateOrUpdate.java b/src/main/java/io/supertokens/webserver/api/multitenancy/BaseCreateOrUpdate.java index e0eb9fb13..52d96136d 100644 --- a/src/main/java/io/supertokens/webserver/api/multitenancy/BaseCreateOrUpdate.java +++ b/src/main/java/io/supertokens/webserver/api/multitenancy/BaseCreateOrUpdate.java @@ -102,7 +102,7 @@ protected void handle(HttpServletRequest req, HttpServletResponse resp, TenantId // Apply updates based on CDI version tenantConfig = applyTenantUpdates(tenantConfig, getVersionFromRequest(req), isV2, input); - validateFirstFactorsName(tenantConfig); + validateFactorsName(tenantConfig); // Write tenant config to db createOrUpdate(req, sourceTenantIdentifier, tenantConfig); @@ -939,15 +939,25 @@ private static TenantConfig applyTenantUpdates_5_0(TenantConfig tenantConfig, Js return tenantConfig; } - private static void validateFirstFactorsName(TenantConfig tenantConfig) throws ServletException { - if(tenantConfig.firstFactors != null && tenantConfig.firstFactors.length > 0) { - String allowedPattern = "^[0-9a-z-]+$"; - for(String firstFactor: tenantConfig.firstFactors){ - if(firstFactor != null && !firstFactor.matches(allowedPattern)){ - throw new ServletException(new BadRequestException("firstFactors should not contain only 0-9,a-z,- characters")); + 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) From b90c3ef877e1b09dbeb96852506001bca6c07e32 Mon Sep 17 00:00:00 2001 From: tamassoltesz Date: Wed, 9 Oct 2024 09:06:12 +0200 Subject: [PATCH 3/4] fix: review fix: allowing . (dot) --- CHANGELOG.md | 2 +- .../webserver/api/multitenancy/BaseCreateOrUpdate.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eea9a1011..d2a4547f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ 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. 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 52d96136d..1ae57140d 100644 --- a/src/main/java/io/supertokens/webserver/api/multitenancy/BaseCreateOrUpdate.java +++ b/src/main/java/io/supertokens/webserver/api/multitenancy/BaseCreateOrUpdate.java @@ -941,16 +941,16 @@ private static TenantConfig applyTenantUpdates_5_0(TenantConfig tenantConfig, Js 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")); + 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")); + 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_-]+$"; + String allowedPattern = "^[0-9a-zA-Z_(\\.)-]+$"; for(String factor: factors){ if(factor != null && !factor.matches(allowedPattern)){ return false; From 2d97adc38831470596498644f3fac6dd4933cbce Mon Sep 17 00:00:00 2001 From: tamassoltesz Date: Wed, 9 Oct 2024 10:04:52 +0200 Subject: [PATCH 4/4] feat: tests for the validation --- .../test/multitenant/api/TestApp5_1.java | 39 ++++++++++++++++ .../api/TestConnectionUriDomain5_1.java | 45 +++++++++++++++++++ .../test/multitenant/api/TestTenant5_1.java | 39 ++++++++++++++++ 3 files changed, 123 insertions(+) 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,