From d3ab41b23ec7cb6ee166c0118df82e828ddc7294 Mon Sep 17 00:00:00 2001 From: Sattvik Chakravarthy Date: Tue, 17 Oct 2023 13:56:27 +0530 Subject: [PATCH] fix: mfa cleanup (#164) * fix: mfa cleanup * fix: pr comments --- .../supertokens/storage/postgresql/Start.java | 84 +----------- .../postgresql/config/PostgreSQLConfig.java | 4 - .../queries/ActiveUsersQueries.java | 28 +--- .../postgresql/queries/GeneralQueries.java | 11 +- .../postgresql/queries/MfaQueries.java | 126 ------------------ 5 files changed, 8 insertions(+), 245 deletions(-) delete mode 100644 src/main/java/io/supertokens/storage/postgresql/queries/MfaQueries.java diff --git a/src/main/java/io/supertokens/storage/postgresql/Start.java b/src/main/java/io/supertokens/storage/postgresql/Start.java index e35995bd..fdaca7c9 100644 --- a/src/main/java/io/supertokens/storage/postgresql/Start.java +++ b/src/main/java/io/supertokens/storage/postgresql/Start.java @@ -48,8 +48,6 @@ import io.supertokens.pluginInterface.jwt.JWTSigningKeyInfo; import io.supertokens.pluginInterface.jwt.exceptions.DuplicateKeyIdException; import io.supertokens.pluginInterface.jwt.sqlstorage.JWTRecipeSQLStorage; -import io.supertokens.pluginInterface.mfa.MfaStorage; -import io.supertokens.pluginInterface.mfa.sqlStorage.MfaSQLStorage; import io.supertokens.pluginInterface.multitenancy.*; import io.supertokens.pluginInterface.multitenancy.exceptions.DuplicateClientTypeException; import io.supertokens.pluginInterface.multitenancy.exceptions.DuplicateTenantException; @@ -108,8 +106,8 @@ public class Start implements SessionSQLStorage, EmailPasswordSQLStorage, EmailVerificationSQLStorage, ThirdPartySQLStorage, JWTRecipeSQLStorage, PasswordlessSQLStorage, UserMetadataSQLStorage, UserRolesSQLStorage, UserIdMappingStorage, - UserIdMappingSQLStorage, MultitenancyStorage, MultitenancySQLStorage, DashboardSQLStorage, TOTPSQLStorage, MfaStorage, - MfaSQLStorage, ActiveUsersStorage, ActiveUsersSQLStorage, AuthRecipeSQLStorage { + UserIdMappingSQLStorage, MultitenancyStorage, MultitenancySQLStorage, DashboardSQLStorage, TOTPSQLStorage, + ActiveUsersStorage, ActiveUsersSQLStorage, AuthRecipeSQLStorage { // these configs are protected from being modified / viewed by the dev using the SuperTokens // SaaS. If the core is not running in SuperTokens SaaS, this array has no effect. @@ -760,13 +758,6 @@ public boolean isUserIdBeingUsedInNonAuthRecipe(AppIdentifier appIdentifier, Str return false; } else if (className.equals(ActiveUsersStorage.class.getName())) { return ActiveUsersQueries.getLastActiveByUserId(this, appIdentifier, userId) != null; - } else if (className.equals(MfaStorage.class.getName())) { - try { - MultitenancyQueries.getAllTenants(this); - return MfaQueries.listFactors(this, appIdentifier, userId).length > 0; - } catch (SQLException e) { - throw new StorageQueryException(e); - } } else { throw new IllegalStateException("ClassName: " + className + " is not part of NonAuthRecipeStorage"); } @@ -864,12 +855,6 @@ public void addInfoToNonAuthRecipesBasedOnUserId(TenantIdentifier tenantIdentifi } catch (SQLException e) { throw new StorageQueryException(e); } - } else if (className.equals(MfaStorage.class.getName())) { - try { - MfaQueries.enableFactor(this, tenantIdentifier, userId, "emailpassword"); - } catch (SQLException e) { - throw new StorageQueryException(e); - } } else { throw new IllegalStateException("ClassName: " + className + " is not part of NonAuthRecipeStorage"); } @@ -2838,71 +2823,6 @@ public int removeExpiredCodes(TenantIdentifier tenantIdentifier, long expiredBef } } - // MFA recipe: - @Override - public boolean enableFactor(TenantIdentifier tenantIdentifier, String userId, String factor) - throws StorageQueryException { - try { - int insertedCount = MfaQueries.enableFactor(this, tenantIdentifier, userId, factor); - if (insertedCount == 0) { - return false; - } - return true; - } catch (SQLException e) { - throw new StorageQueryException(e); - } - } - - @Override - public String[] listFactors(TenantIdentifier tenantIdentifier, String userId) - throws StorageQueryException { - try { - return MfaQueries.listFactors(this, tenantIdentifier, userId); - } catch (SQLException e) { - throw new StorageQueryException(e); - } - } - - @Override - public boolean disableFactor(TenantIdentifier tenantIdentifier, String userId, String factor) - throws StorageQueryException { - try { - int deletedCount = MfaQueries.disableFactor(this, tenantIdentifier, userId, factor); - if (deletedCount == 0) { - return false; - } - return true; - } catch (SQLException e) { - throw new StorageQueryException(e); - } - } - - @Override - public boolean deleteMfaInfoForUser_Transaction(TransactionConnection con, AppIdentifier appIdentifier, String userId) throws StorageQueryException { - try { - int deletedCount = MfaQueries.deleteUser_Transaction(this, (Connection) con.getConnection(), appIdentifier, userId); - if (deletedCount == 0) { - return false; - } - return true; - } catch (SQLException e) { - throw new StorageQueryException(e); - } - } - - @Override - public boolean deleteMfaInfoForUser(TenantIdentifier tenantIdentifier, String userId) throws StorageQueryException { - try { - int deletedCount = MfaQueries.deleteUser(this, tenantIdentifier, userId); - if (deletedCount == 0) { - return false; - } - return true; - } catch (SQLException e) { - throw new StorageQueryException(e); - } - } - @Override public Set getValidFieldsInConfig() { return PostgreSQLConfig.getValidFields(); diff --git a/src/main/java/io/supertokens/storage/postgresql/config/PostgreSQLConfig.java b/src/main/java/io/supertokens/storage/postgresql/config/PostgreSQLConfig.java index 2a7bd5db..e8bc81d6 100644 --- a/src/main/java/io/supertokens/storage/postgresql/config/PostgreSQLConfig.java +++ b/src/main/java/io/supertokens/storage/postgresql/config/PostgreSQLConfig.java @@ -302,10 +302,6 @@ public String getTotpUsedCodesTable() { return addSchemaAndPrefixToTableName("totp_used_codes"); } - public String getMfaUserFactorsTable() { - return addSchemaAndPrefixToTableName("mfa_user_factors"); - } - private String addSchemaAndPrefixToTableName(String tableName) { return addSchemaToTableName(postgresql_table_names_prefix + tableName); } diff --git a/src/main/java/io/supertokens/storage/postgresql/queries/ActiveUsersQueries.java b/src/main/java/io/supertokens/storage/postgresql/queries/ActiveUsersQueries.java index cf1ad814..3faeda33 100644 --- a/src/main/java/io/supertokens/storage/postgresql/queries/ActiveUsersQueries.java +++ b/src/main/java/io/supertokens/storage/postgresql/queries/ActiveUsersQueries.java @@ -106,35 +106,11 @@ public static int countUsersEnabledTotpAndActiveSince(Start start, AppIdentifier } public static int countUsersEnabledMfa(Start start, AppIdentifier appIdentifier) throws SQLException, StorageQueryException { - String QUERY = "SELECT COUNT(*) as total FROM (SELECT DISTINCT user_id FROM " + Config.getConfig(start).getMfaUserFactorsTable() + " WHERE app_id = ?) AS app_mfa_users"; - - return execute(start, QUERY, pst -> { - pst.setString(1, appIdentifier.getAppId()); - }, result -> { - if (result.next()) { - return result.getInt("total"); - } - return 0; - }); + return 0; // TODO } public static int countUsersEnabledMfaAndActiveSince(Start start, AppIdentifier appIdentifier, long sinceTime) throws SQLException, StorageQueryException { - // Find unique users from mfa_user_factors table and join with user_last_active table - String QUERY = "SELECT COUNT(*) as total FROM (SELECT DISTINCT user_id FROM " + Config.getConfig(start).getMfaUserFactorsTable() + ") AS mfa_users " - + "INNER JOIN " + Config.getConfig(start).getUserLastActiveTable() + " AS user_last_active " - + "ON mfa_users.user_id = user_last_active.user_id " - + "WHERE user_last_active.app_id = ? " - + "AND user_last_active.last_active_time >= ?"; - - return execute(start, QUERY, pst -> { - pst.setString(1, appIdentifier.getAppId()); - pst.setLong(2, sinceTime); - }, result -> { - if (result.next()) { - return result.getInt("total"); - } - return 0; - }); + return 0; // TODO } public static int updateUserLastActive(Start start, AppIdentifier appIdentifier, String userId) diff --git a/src/main/java/io/supertokens/storage/postgresql/queries/GeneralQueries.java b/src/main/java/io/supertokens/storage/postgresql/queries/GeneralQueries.java index 8be26c54..dcd36eec 100644 --- a/src/main/java/io/supertokens/storage/postgresql/queries/GeneralQueries.java +++ b/src/main/java/io/supertokens/storage/postgresql/queries/GeneralQueries.java @@ -28,6 +28,7 @@ import io.supertokens.storage.postgresql.ConnectionPool; import io.supertokens.storage.postgresql.Start; import io.supertokens.storage.postgresql.config.Config; +import io.supertokens.storage.postgresql.queries.GeneralQueries.AccountLinkingInfo; import io.supertokens.storage.postgresql.utils.Utils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -516,11 +517,6 @@ public static void createTablesIfNotExists(Start start) throws SQLException, Sto update(start, TOTPQueries.getQueryToCreateTenantIdIndexForUsedCodesTable(start), NO_OP_SETTER); } - if (!doesTableExists(start, Config.getConfig(start).getMfaUserFactorsTable())) { - getInstance(start).addState(CREATING_NEW_TABLE, null); - update(start, MfaQueries.getQueryToCreateUserFactorsTable(start), NO_OP_SETTER); - } - } catch (Exception e) { if (e.getMessage().contains("schema") && e.getMessage().contains("does not exist") && numberOfRetries < 1) { @@ -589,8 +585,9 @@ public static void deleteAllTables(Start start) throws SQLException, StorageQuer + getConfig(start).getUserRolesTable() + "," + getConfig(start).getDashboardUsersTable() + "," + getConfig(start).getDashboardSessionsTable() + "," - + getConfig(start).getTotpUsedCodesTable() + "," + getConfig(start).getTotpUserDevicesTable() + "," - + getConfig(start).getTotpUsersTable() + "," + getConfig(start).getMfaUserFactorsTable(); + + getConfig(start).getTotpUsedCodesTable() + "," + + getConfig(start).getTotpUserDevicesTable() + "," + + getConfig(start).getTotpUsersTable(); update(start, DROP_QUERY, NO_OP_SETTER); } } diff --git a/src/main/java/io/supertokens/storage/postgresql/queries/MfaQueries.java b/src/main/java/io/supertokens/storage/postgresql/queries/MfaQueries.java deleted file mode 100644 index 2815043f..00000000 --- a/src/main/java/io/supertokens/storage/postgresql/queries/MfaQueries.java +++ /dev/null @@ -1,126 +0,0 @@ -/* - * Copyright (c) 2023, VRAI Labs and/or its affiliates. All rights reserved. - * - * This software is licensed under the Apache License, Version 2.0 (the - * "License") as published by the Apache Software Foundation. - * - * You may not use this file except in compliance with the License. You may - * obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations - * under the License. - */ - -package io.supertokens.storage.postgresql.queries; - -import io.supertokens.pluginInterface.multitenancy.AppIdentifier; -import io.supertokens.pluginInterface.multitenancy.TenantIdentifier; -import io.supertokens.storage.postgresql.Start; -import io.supertokens.storage.postgresql.config.Config; -import io.supertokens.pluginInterface.exceptions.StorageQueryException; - -import java.sql.Connection; -import java.sql.SQLException; -import java.util.ArrayList; -import java.util.List; - -import static io.supertokens.storage.postgresql.QueryExecutorTemplate.execute; -import static io.supertokens.storage.postgresql.QueryExecutorTemplate.update; - -public class MfaQueries { - public static String getQueryToCreateUserFactorsTable(Start start) { - return "CREATE TABLE IF NOT EXISTS " + Config.getConfig(start).getMfaUserFactorsTable() + " (" - + "app_id VARCHAR(64) DEFAULT 'public'," - + "tenant_id VARCHAR(64) DEFAULT 'public'," - + "user_id VARCHAR(128) NOT NULL," - + "factor_id VARCHAR(64) NOT NULL," - + "PRIMARY KEY (app_id, tenant_id, user_id, factor_id)," - + "FOREIGN KEY (app_id, tenant_id)" - + "REFERENCES " + Config.getConfig(start).getTenantsTable() + " (app_id, tenant_id) ON DELETE CASCADE);"; - } - - public static int enableFactor(Start start, TenantIdentifier tenantIdentifier, String userId, String factorId) - throws StorageQueryException, SQLException { - String QUERY = "INSERT INTO " + Config.getConfig(start).getMfaUserFactorsTable() + " (app_id, tenant_id, user_id, factor_id) VALUES (?, ?, ?, ?) ON CONFLICT DO NOTHING"; - - return update(start, QUERY, pst -> { - pst.setString(1, tenantIdentifier.getAppId()); - pst.setString(2, tenantIdentifier.getTenantId()); - pst.setString(3, userId); - pst.setString(4, factorId); - }); - } - - - public static String[] listFactors(Start start, TenantIdentifier tenantIdentifier, String userId) - throws StorageQueryException, SQLException { - String QUERY = "SELECT factor_id FROM " + Config.getConfig(start).getMfaUserFactorsTable() + " WHERE app_id = ? AND tenant_id = ? AND user_id = ?"; - - return execute(start, QUERY, pst -> { - pst.setString(1, tenantIdentifier.getAppId()); - pst.setString(2, tenantIdentifier.getTenantId()); - pst.setString(3, userId); - }, result -> { - List factors = new ArrayList<>(); - while (result.next()) { - factors.add(result.getString("factor_id")); - } - - return factors.toArray(String[]::new); - }); - } - - public static String[] listFactors(Start start, AppIdentifier appIdentifier, String userId) - throws StorageQueryException, SQLException { - String QUERY = "SELECT factor_id FROM " + Config.getConfig(start).getMfaUserFactorsTable() + " WHERE app_id = ? AND user_id = ?"; - - return execute(start, QUERY, pst -> { - pst.setString(1, appIdentifier.getAppId()); - pst.setString(2, userId); - }, result -> { - List factors = new ArrayList<>(); - while (result.next()) { - factors.add(result.getString("factor_id")); - } - - return factors.toArray(String[]::new); - }); - } - - public static int disableFactor(Start start, TenantIdentifier tenantIdentifier, String userId, String factorId) - throws StorageQueryException, SQLException { - String QUERY = "DELETE FROM " + Config.getConfig(start).getMfaUserFactorsTable() + " WHERE app_id = ? AND tenant_id = ? AND user_id = ? AND factor_id = ?"; - - return update(start, QUERY, pst -> { - pst.setString(1, tenantIdentifier.getAppId()); - pst.setString(2, tenantIdentifier.getTenantId()); - pst.setString(3, userId); - pst.setString(4, factorId); - }); - } - - public static int deleteUser_Transaction(Start start, Connection sqlCon, AppIdentifier appIdentifier, String userId) - throws StorageQueryException, SQLException { - String QUERY = "DELETE FROM " + Config.getConfig(start).getMfaUserFactorsTable() + " WHERE app_id = ? AND user_id = ?"; - - return update(sqlCon, QUERY, pst -> { - pst.setString(1, appIdentifier.getAppId()); - pst.setString(2, userId); - }); - } - - public static int deleteUser(Start start, TenantIdentifier tenantIdentifier, String userId) - throws StorageQueryException, SQLException { - String QUERY = "DELETE FROM " + Config.getConfig(start).getMfaUserFactorsTable() + " WHERE app_id = ? AND tenant_id = ? AND user_id = ?"; - - return update(start, QUERY, pst -> { - pst.setString(1, tenantIdentifier.getAppId()); - pst.setString(2, tenantIdentifier.getTenantId()); - pst.setString(3, userId); - }); - } - -}