From 4bf90e0a037cf5312b8f6e3aed76258c6fbcf061 Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Tue, 5 Mar 2024 13:09:25 +0530 Subject: [PATCH] fix: fixes storage handling for non-auth recipes (#203) * fix: tests * fix: user role table constraint * fix: pr comments * fix: according to updated interface * fix: user roles * fix: version and changelog * fix: plugin interface version --- CHANGELOG.md | 11 ++++++++ build.gradle | 2 +- pluginInterfaceSupported.json | 2 +- .../supertokens/storage/postgresql/Start.java | 15 ++++++++--- .../postgresql/queries/UserRolesQueries.java | 18 ++++++++++--- .../postgresql/test/AccountLinkingTests.java | 4 +-- .../postgresql/test/DbConnectionPoolTest.java | 9 ++++--- .../test/multitenancy/StorageLayerTest.java | 6 ++--- .../TestUserPoolIdChangeBehaviour.java | 27 ++++++++++--------- 9 files changed, 63 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f3aa6c8..8508712a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [6.0.0] - 2024-03-05 + +- Implements `deleteAllUserRoleAssociationsForRole` +- Drops `(app_id, role)` foreign key constraint on `user_roles` table + +### Migration + +```sql +ALTER TABLE user_roles DROP CONSTRAINT IF EXISTS user_roles_role_fkey; +``` + ## [5.0.8] - 2024-02-19 - Fixes vulnerabilities in dependencies diff --git a/build.gradle b/build.gradle index 713fefbe..baafed34 100644 --- a/build.gradle +++ b/build.gradle @@ -2,7 +2,7 @@ plugins { id 'java-library' } -version = "5.0.8" +version = "6.0.0" repositories { mavenCentral() diff --git a/pluginInterfaceSupported.json b/pluginInterfaceSupported.json index a5fdc62c..e9d4c148 100644 --- a/pluginInterfaceSupported.json +++ b/pluginInterfaceSupported.json @@ -1,6 +1,6 @@ { "_comment": "contains a list of plugin interfaces branch names that this core supports", "versions": [ - "4.0" + "5.0" ] } \ No newline at end of file diff --git a/src/main/java/io/supertokens/storage/postgresql/Start.java b/src/main/java/io/supertokens/storage/postgresql/Start.java index 86a7e876..796d3027 100644 --- a/src/main/java/io/supertokens/storage/postgresql/Start.java +++ b/src/main/java/io/supertokens/storage/postgresql/Start.java @@ -1930,7 +1930,7 @@ public int deleteUserMetadata(AppIdentifier appIdentifier, String userId) throws @Override public void addRoleToUser(TenantIdentifier tenantIdentifier, String userId, String role) - throws StorageQueryException, UnknownRoleException, DuplicateUserRoleMappingException, + throws StorageQueryException, DuplicateUserRoleMappingException, TenantOrAppNotFoundException { try { UserRolesQueries.addRoleToUser(this, tenantIdentifier, userId, role); @@ -1938,9 +1938,6 @@ public void addRoleToUser(TenantIdentifier tenantIdentifier, String userId, Stri if (e instanceof PSQLException) { PostgreSQLConfig config = Config.getConfig(this); ServerErrorMessage serverErrorMessage = ((PSQLException) e).getServerErrorMessage(); - if (isForeignKeyConstraintError(serverErrorMessage, config.getUserRolesTable(), "role")) { - throw new UnknownRoleException(); - } if (isPrimaryKeyError(serverErrorMessage, config.getUserRolesTable())) { throw new DuplicateUserRoleMappingException(); } @@ -2012,6 +2009,16 @@ public boolean deleteRole(AppIdentifier appIdentifier, String role) throws Stora } } + @Override + public boolean deleteAllUserRoleAssociationsForRole(AppIdentifier appIdentifier, String role) + throws StorageQueryException { + try { + return UserRolesQueries.deleteAllUserRoleAssociationsForRole(this, appIdentifier, role); + } catch (SQLException e) { + throw new StorageQueryException(e); + } + } + @Override public String[] getRoles(AppIdentifier appIdentifier) throws StorageQueryException { try { diff --git a/src/main/java/io/supertokens/storage/postgresql/queries/UserRolesQueries.java b/src/main/java/io/supertokens/storage/postgresql/queries/UserRolesQueries.java index 549cac86..10fcb1a7 100644 --- a/src/main/java/io/supertokens/storage/postgresql/queries/UserRolesQueries.java +++ b/src/main/java/io/supertokens/storage/postgresql/queries/UserRolesQueries.java @@ -17,8 +17,10 @@ package io.supertokens.storage.postgresql.queries; import io.supertokens.pluginInterface.exceptions.StorageQueryException; +import io.supertokens.pluginInterface.exceptions.StorageTransactionLogicException; import io.supertokens.pluginInterface.multitenancy.AppIdentifier; import io.supertokens.pluginInterface.multitenancy.TenantIdentifier; +import io.supertokens.pluginInterface.sqlStorage.TransactionConnection; import io.supertokens.storage.postgresql.Start; import io.supertokens.storage.postgresql.config.Config; import io.supertokens.storage.postgresql.utils.Utils; @@ -91,9 +93,6 @@ public static String getQueryToCreateUserRolesTable(Start start) { + "role VARCHAR(255) NOT NULL," + "CONSTRAINT " + Utils.getConstraintName(schema, tableName, null, "pkey") + " PRIMARY KEY(app_id, tenant_id, user_id, role)," - + "CONSTRAINT " + Utils.getConstraintName(schema, tableName, "role", "fkey") - + " FOREIGN KEY(app_id, role)" - + " REFERENCES " + getConfig(start).getRolesTable() + "(app_id, role) ON DELETE CASCADE," + "CONSTRAINT " + Utils.getConstraintName(schema, tableName, "tenant_id", "fkey") + " FOREIGN KEY (app_id, tenant_id)" + " REFERENCES " + Config.getConfig(start).getTenantsTable() + "(app_id, tenant_id) ON DELETE CASCADE" @@ -142,7 +141,8 @@ public static void addPermissionToRoleOrDoNothingIfExists_Transaction(Start star }); } - public static boolean deleteRole(Start start, AppIdentifier appIdentifier, String role) + public static boolean deleteRole(Start start, AppIdentifier appIdentifier, + String role) throws SQLException, StorageQueryException { String QUERY = "DELETE FROM " + getConfig(start).getRolesTable() + " WHERE app_id = ? AND role = ? ;"; @@ -353,4 +353,14 @@ public static int deleteAllRolesForUser_Transaction(Connection con, Start start, pst.setString(2, userId); }); } + + public static boolean deleteAllUserRoleAssociationsForRole(Start start, AppIdentifier appIdentifier, String role) + throws SQLException, StorageQueryException { + String QUERY = "DELETE FROM " + getConfig(start).getUserRolesTable() + + " WHERE app_id = ? AND role = ? ;"; + return update(start, QUERY, pst -> { + pst.setString(1, appIdentifier.getAppId()); + pst.setString(2, role); + }) >= 1; + } } diff --git a/src/test/java/io/supertokens/storage/postgresql/test/AccountLinkingTests.java b/src/test/java/io/supertokens/storage/postgresql/test/AccountLinkingTests.java index 4f26a52c..580cc049 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/AccountLinkingTests.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/AccountLinkingTests.java @@ -97,7 +97,7 @@ public void canLinkFailsIfTryingToLinkUsersAcrossDifferentStorageLayers() throws AuthRecipe.createPrimaryUser(process.main, user1.getSupertokensUserId()); AuthRecipeUserInfo user2 = EmailPassword.signUp( - tenantIdentifier.withStorage(StorageLayer.getStorage(tenantIdentifier, process.main)), + tenantIdentifier, (StorageLayer.getStorage(tenantIdentifier, process.main)), process.getProcess(), "test2@example.com", "abcd1234"); try { @@ -135,7 +135,7 @@ public void canLinkFailsIfTryingToLinkUsersAcrossDifferentStorageLayers() throws ); AuthRecipeUserInfo user3 = EmailPassword.signUp( - tenantIdentifier.withStorage(StorageLayer.getStorage(tenantIdentifier, process.main)), + tenantIdentifier, (StorageLayer.getStorage(tenantIdentifier, process.main)), process.getProcess(), "test2@example.com", "abcd1234"); Map params = new HashMap<>(); diff --git a/src/test/java/io/supertokens/storage/postgresql/test/DbConnectionPoolTest.java b/src/test/java/io/supertokens/storage/postgresql/test/DbConnectionPoolTest.java index cdf0c28c..c89a5ac4 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/DbConnectionPoolTest.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/DbConnectionPoolTest.java @@ -24,6 +24,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import io.supertokens.pluginInterface.Storage; import io.supertokens.pluginInterface.multitenancy.*; import org.junit.AfterClass; import org.junit.Before; @@ -152,8 +153,8 @@ public void testDownTimeWhenChangingConnectionPoolSize() throws Exception { es.execute(() -> { try { TenantIdentifier t1 = new TenantIdentifier(null, null, "t1"); - TenantIdentifierWithStorage t1WithStorage = t1.withStorage(StorageLayer.getStorage(t1, process.getProcess())); - ThirdParty.signInUp(t1WithStorage, process.getProcess(), "google", "googleid"+ finalI, "user" + + Storage t1Storage = (StorageLayer.getStorage(t1, process.getProcess())); + ThirdParty.signInUp(t1, t1Storage, process.getProcess(), "google", "googleid"+ finalI, "user" + finalI + "@example.com"); if (firstErrorTime.get() != -1 && successAfterErrorTime.get() == -1) { @@ -353,8 +354,8 @@ public void testIdleConnectionTimeout() throws Exception { es.execute(() -> { try { TenantIdentifier t1 = new TenantIdentifier(null, null, "t1"); - TenantIdentifierWithStorage t1WithStorage = t1.withStorage(StorageLayer.getStorage(t1, process.getProcess())); - ThirdParty.signInUp(t1WithStorage, process.getProcess(), "google", "googleid"+ finalI, "user" + + Storage t1Storage = (StorageLayer.getStorage(t1, process.getProcess())); + ThirdParty.signInUp(t1, t1Storage, process.getProcess(), "google", "googleid"+ finalI, "user" + finalI + "@example.com"); } catch (StorageQueryException e) { diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/StorageLayerTest.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/StorageLayerTest.java index afce4e11..7bca0a99 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/StorageLayerTest.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/StorageLayerTest.java @@ -788,7 +788,7 @@ public void testTenantCreationAndThenDbDownDbThrowsErrorInRecipesAndDoesntAffect MultitenancyHelper.getInstance(process.getProcess()).refreshTenantsInCoreBasedOnChangesInCoreConfigOrIfTenantListChanged(true); try { - EmailPassword.signIn(tid.withStorage(StorageLayer.getStorage(tid, process.getProcess())), + EmailPassword.signIn(tid, (StorageLayer.getStorage(tid, process.getProcess())), process.getProcess(), "", ""); fail(); } catch (StorageQueryException e) { @@ -801,7 +801,7 @@ public void testTenantCreationAndThenDbDownDbThrowsErrorInRecipesAndDoesntAffect // we do this again just to check that if this function is called again, it fails again and there is no // side effect of calling the above function try { - EmailPassword.signIn(tid.withStorage(StorageLayer.getStorage(tid, process.getProcess())), + EmailPassword.signIn(tid, (StorageLayer.getStorage(tid, process.getProcess())), process.getProcess(), "", ""); fail(); } catch (StorageQueryException e) { @@ -830,7 +830,7 @@ public void testTenantCreationAndThenDbDownDbThrowsErrorInRecipesAndDoesntAffect TenantIdentifier tid = new TenantIdentifier("abc", null, null); try { - EmailPassword.signIn(tid.withStorage(StorageLayer.getStorage(tid, process.getProcess())), + EmailPassword.signIn(tid, (StorageLayer.getStorage(tid, process.getProcess())), process.getProcess(), "", ""); fail(); } catch (StorageQueryException e) { diff --git a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestUserPoolIdChangeBehaviour.java b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestUserPoolIdChangeBehaviour.java index 5a1d7a1f..bc5a791e 100644 --- a/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestUserPoolIdChangeBehaviour.java +++ b/src/test/java/io/supertokens/storage/postgresql/test/multitenancy/TestUserPoolIdChangeBehaviour.java @@ -25,6 +25,7 @@ import io.supertokens.multitenancy.Multitenancy; import io.supertokens.multitenancy.exception.BadPermissionException; import io.supertokens.multitenancy.exception.CannotModifyBaseConfigException; +import io.supertokens.pluginInterface.Storage; import io.supertokens.pluginInterface.authRecipe.AuthRecipeUserInfo; import io.supertokens.pluginInterface.exceptions.InvalidConfigException; import io.supertokens.pluginInterface.exceptions.StorageQueryException; @@ -86,13 +87,13 @@ public void testUsersWorkAfterUserPoolIdChanges() throws Exception { coreConfig ), false); - TenantIdentifierWithStorage tenantIdentifierWithStorage = tenantIdentifier.withStorage( + Storage storage = ( StorageLayer.getStorage(tenantIdentifier, process.getProcess())); - String userPoolId = tenantIdentifierWithStorage.getStorage().getUserPoolId(); + String userPoolId = storage.getUserPoolId(); AuthRecipeUserInfo userInfo = EmailPassword.signUp( - tenantIdentifierWithStorage, process.getProcess(), "user@example.com", "password"); + tenantIdentifier, storage, process.getProcess(), "user@example.com", "password"); coreConfig.addProperty("postgresql_host", "127.0.0.1"); Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( @@ -103,12 +104,13 @@ public void testUsersWorkAfterUserPoolIdChanges() throws Exception { coreConfig ), false); - tenantIdentifierWithStorage = tenantIdentifier.withStorage( + storage = ( StorageLayer.getStorage(tenantIdentifier, process.getProcess())); - String userPoolId2 = tenantIdentifierWithStorage.getStorage().getUserPoolId(); + String userPoolId2 = storage.getUserPoolId(); assertNotEquals(userPoolId, userPoolId2); - AuthRecipeUserInfo user2 = EmailPassword.signIn(tenantIdentifierWithStorage, process.getProcess(), + AuthRecipeUserInfo user2 = EmailPassword.signIn( + tenantIdentifier, storage, process.getProcess(), "user@example.com", "password"); assertEquals(userInfo, user2); @@ -130,13 +132,13 @@ public void testUsersWorkAfterUserPoolIdChangesAndServerRestart() throws Excepti coreConfig ), false); - TenantIdentifierWithStorage tenantIdentifierWithStorage = tenantIdentifier.withStorage( + Storage storage = ( StorageLayer.getStorage(tenantIdentifier, process.getProcess())); - String userPoolId = tenantIdentifierWithStorage.getStorage().getUserPoolId(); + String userPoolId = storage.getUserPoolId(); AuthRecipeUserInfo userInfo = EmailPassword.signUp( - tenantIdentifierWithStorage, process.getProcess(), "user@example.com", "password"); + tenantIdentifier, storage, process.getProcess(), "user@example.com", "password"); coreConfig.addProperty("postgresql_host", "127.0.0.1"); Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig( @@ -153,12 +155,13 @@ public void testUsersWorkAfterUserPoolIdChangesAndServerRestart() throws Excepti this.process = TestingProcessManager.start(args); assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); - tenantIdentifierWithStorage = tenantIdentifier.withStorage( + storage = ( StorageLayer.getStorage(tenantIdentifier, process.getProcess())); - String userPoolId2 = tenantIdentifierWithStorage.getStorage().getUserPoolId(); + String userPoolId2 = storage.getUserPoolId(); assertNotEquals(userPoolId, userPoolId2); - AuthRecipeUserInfo user2 = EmailPassword.signIn(tenantIdentifierWithStorage, process.getProcess(), + AuthRecipeUserInfo user2 = EmailPassword.signIn( + tenantIdentifier, storage, process.getProcess(), "user@example.com", "password");