From 68120428658af99f58d2436faaa89ca230315891 Mon Sep 17 00:00:00 2001 From: tamassoltesz Date: Thu, 22 May 2025 23:13:37 +0200 Subject: [PATCH 1/4] fix: add error handling for multiple email verification --- .../supertokens/storage/postgresql/Start.java | 55 ++++++++++++++----- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/src/main/java/io/supertokens/storage/postgresql/Start.java b/src/main/java/io/supertokens/storage/postgresql/Start.java index 03933584..1cc061a8 100644 --- a/src/main/java/io/supertokens/storage/postgresql/Start.java +++ b/src/main/java/io/supertokens/storage/postgresql/Start.java @@ -1270,6 +1270,15 @@ public void updateIsEmailVerified_Transaction(AppIdentifier appIdentifier, Trans } } + /** + * Update the isEmailVerified column for multiple users in the email verification table. This method is used in the + * Bulk Migration process. + * Important note: this method expects a Map of email to userId, but if there is an error in the batch processing, + * it will throw a BulkImportBatchInsertException with a map of userid to Exception, based on the position of the + * erroneous item in the batch. + * This means, that the underlying map implementation must be one that preserves iteration order (LinkedHashMap + * is a good choice) and this is the responsibility of the caller to ensure that the passed map is such. + */ @Override public void updateMultipleIsEmailVerified_Transaction(AppIdentifier appIdentifier, TransactionConnection con, Map emailToUserId, boolean isEmailVerified) @@ -1279,23 +1288,36 @@ public void updateMultipleIsEmailVerified_Transaction(AppIdentifier appIdentifie EmailVerificationQueries.updateMultipleUsersIsEmailVerified_Transaction(this, sqlCon, appIdentifier, emailToUserId, isEmailVerified); } catch (SQLException e) { - if (e instanceof PSQLException) { - PostgreSQLConfig config = Config.getConfig(this); - ServerErrorMessage serverMessage = ((PSQLException) e).getServerErrorMessage(); + if (e instanceof BatchUpdateException) { + BatchUpdateException batchUpdateException = (BatchUpdateException) e; + SQLException nextException = batchUpdateException.getNextException(); + Map errorByPosition = new HashMap<>(); + while (nextException != null) { - if (isForeignKeyConstraintError(serverMessage, config.getEmailVerificationTable(), "app_id")) { - throw new TenantOrAppNotFoundException(appIdentifier); - } - } + if (nextException instanceof PSQLException) { + PostgreSQLConfig config = Config.getConfig(this); + ServerErrorMessage serverMessage = ((PSQLException) nextException).getServerErrorMessage(); - boolean isPSQLPrimKeyError = e instanceof PSQLException && isPrimaryKeyError( - ((PSQLException) e).getServerErrorMessage(), - Config.getConfig(this).getEmailVerificationTable()); + int position = getErroneousEntryPosition(batchUpdateException); + String userid = ((Map.Entry) emailToUserId.entrySet().toArray()[position]).getKey(); + if (isNullConstraintError(serverMessage, config.getEmailVerificationTable(), "email")) { + errorByPosition.put(userid, new NullPointerException("email is null")); + } else if (isPrimaryKeyError(serverMessage, config.getEmailVerificationTable())) { + errorByPosition.put(userid, + new DuplicateEmailException()); + } + if (isForeignKeyConstraintError(serverMessage, config.getEmailVerificationTable(), + "app_id")) { + throw new TenantOrAppNotFoundException(appIdentifier); + } + } - if (!isEmailVerified || !isPSQLPrimKeyError) { + nextException = nextException.getNextException(); + } + throw new StorageQueryException( + new BulkImportBatchInsertException("emailverification errors", errorByPosition)); + } throw new StorageQueryException(e); - } - // we do not throw an error since the email is already verified } } @@ -1768,6 +1790,13 @@ private boolean isForeignKeyConstraintError(ServerErrorMessage serverMessage, St && serverMessage.getConstraint().equals(tableName + "_" + columnName + "_fkey"); } + private boolean isNullConstraintError(ServerErrorMessage serverMessage, String tableName, String columnName) { + String[] tableNameParts = tableName.split("\\."); + tableName = tableNameParts[tableNameParts.length - 1]; + return serverMessage.getSQLState().equals("23502") + && serverMessage.getMessage().contains("null value in column \"" + columnName + "\" of relation \"" + tableName + "\" violates not-null constraint"); + } + private boolean isPrimaryKeyError(ServerErrorMessage serverMessage, String tableName) { String[] tableNameParts = tableName.split("\\."); tableName = tableNameParts[tableNameParts.length - 1]; From f84d30d3dd777ca2ad078e9d6766983b30528493 Mon Sep 17 00:00:00 2001 From: tamassoltesz Date: Tue, 27 May 2025 11:20:47 +0200 Subject: [PATCH 2/4] fix: proper checking of batchupdateexception --- .../postgresql/BulkImportProxyStorage.java | 2 +- .../supertokens/storage/postgresql/Start.java | 80 +++++++++---------- .../queries/EmailPasswordQueries.java | 2 +- 3 files changed, 38 insertions(+), 46 deletions(-) diff --git a/src/main/java/io/supertokens/storage/postgresql/BulkImportProxyStorage.java b/src/main/java/io/supertokens/storage/postgresql/BulkImportProxyStorage.java index 12eeff8a..0f28bf10 100644 --- a/src/main/java/io/supertokens/storage/postgresql/BulkImportProxyStorage.java +++ b/src/main/java/io/supertokens/storage/postgresql/BulkImportProxyStorage.java @@ -98,7 +98,7 @@ public void commitTransactionForBulkImportProxyStorage() throws StorageQueryExce if (this.connection != null) { this.connection.commitForBulkImportProxyStorage(); } - } catch (SQLException e) { + } catch (SQLException e) {; throw new StorageQueryException(e); } } diff --git a/src/main/java/io/supertokens/storage/postgresql/Start.java b/src/main/java/io/supertokens/storage/postgresql/Start.java index 1cc061a8..1f37a1ed 100644 --- a/src/main/java/io/supertokens/storage/postgresql/Start.java +++ b/src/main/java/io/supertokens/storage/postgresql/Start.java @@ -1045,10 +1045,9 @@ public void signUpMultipleViaBulkImport_Transaction(TransactionConnection connec try { Connection sqlConnection = (Connection) connection.getConnection(); EmailPasswordQueries.signUpMultipleForBulkImport_Transaction(this, sqlConnection, users); - } catch (StorageQueryException | SQLException | StorageTransactionLogicException e) { + } catch (StorageQueryException | StorageTransactionLogicException e) { Throwable actual = e.getCause(); - if (actual instanceof BatchUpdateException) { - BatchUpdateException batchUpdateException = (BatchUpdateException) actual; + if (actual instanceof BatchUpdateException batchUpdateException) { Map errorByPosition = new HashMap<>(); SQLException nextException = batchUpdateException.getNextException(); while (nextException != null) { @@ -1288,8 +1287,7 @@ public void updateMultipleIsEmailVerified_Transaction(AppIdentifier appIdentifie EmailVerificationQueries.updateMultipleUsersIsEmailVerified_Transaction(this, sqlCon, appIdentifier, emailToUserId, isEmailVerified); } catch (SQLException e) { - if (e instanceof BatchUpdateException) { - BatchUpdateException batchUpdateException = (BatchUpdateException) e; + if (e instanceof BatchUpdateException batchUpdateException) { SQLException nextException = batchUpdateException.getNextException(); Map errorByPosition = new HashMap<>(); while (nextException != null) { @@ -1520,9 +1518,7 @@ public void importThirdPartyUsers_Transaction(TransactionConnection con, Connection sqlCon = (Connection) con.getConnection(); ThirdPartyQueries.importUser_Transaction(this, sqlCon, usersToImport); } catch (SQLException e) { - Throwable actual = e.getCause(); - if (actual instanceof BatchUpdateException) { - BatchUpdateException batchUpdateException = (BatchUpdateException) actual; + if (e instanceof BatchUpdateException batchUpdateException) { Map errorByPosition = new HashMap<>(); SQLException nextException = batchUpdateException.getNextException(); while (nextException != null) { @@ -2126,50 +2122,46 @@ public void importPasswordlessUsers_Transaction(TransactionConnection con, Connection sqlCon = (Connection) con.getConnection(); PasswordlessQueries.importUsers_Transaction(sqlCon, this, users); } catch (SQLException e) { - if (e instanceof BatchUpdateException) { - Throwable actual = e.getCause(); - if (actual instanceof BatchUpdateException) { - BatchUpdateException batchUpdateException = (BatchUpdateException) actual; - Map errorByPosition = new HashMap<>(); - SQLException nextException = batchUpdateException.getNextException(); - while (nextException != null) { + if (e instanceof BatchUpdateException batchUpdateException) { + Map errorByPosition = new HashMap<>(); + SQLException nextException = batchUpdateException.getNextException(); + while (nextException != null) { - if (nextException instanceof PSQLException) { - PostgreSQLConfig config = Config.getConfig(this); - ServerErrorMessage serverMessage = ((PSQLException) nextException).getServerErrorMessage(); + if (nextException instanceof PSQLException) { + PostgreSQLConfig config = Config.getConfig(this); + ServerErrorMessage serverMessage = ((PSQLException) nextException).getServerErrorMessage(); - int position = getErroneousEntryPosition(batchUpdateException); + int position = getErroneousEntryPosition(batchUpdateException); - if (isPrimaryKeyError(serverMessage, config.getPasswordlessUsersTable()) - || isPrimaryKeyError(serverMessage, config.getUsersTable()) - || isPrimaryKeyError(serverMessage, config.getPasswordlessUserToTenantTable()) - || isPrimaryKeyError(serverMessage, config.getAppIdToUserIdTable())) { - errorByPosition.put(users.get(position).userId, new DuplicateUserIdException()); - } - if (isUniqueConstraintError(serverMessage, config.getPasswordlessUserToTenantTable(), - "email")) { - errorByPosition.put(users.get(position).userId, new DuplicateEmailException()); + if (isPrimaryKeyError(serverMessage, config.getPasswordlessUsersTable()) + || isPrimaryKeyError(serverMessage, config.getUsersTable()) + || isPrimaryKeyError(serverMessage, config.getPasswordlessUserToTenantTable()) + || isPrimaryKeyError(serverMessage, config.getAppIdToUserIdTable())) { + errorByPosition.put(users.get(position).userId, new DuplicateUserIdException()); + } + if (isUniqueConstraintError(serverMessage, config.getPasswordlessUserToTenantTable(), + "email")) { + errorByPosition.put(users.get(position).userId, new DuplicateEmailException()); - } else if (isUniqueConstraintError(serverMessage, config.getPasswordlessUserToTenantTable(), - "phone_number")) { - errorByPosition.put(users.get(position).userId, new DuplicatePhoneNumberException()); + } else if (isUniqueConstraintError(serverMessage, config.getPasswordlessUserToTenantTable(), + "phone_number")) { + errorByPosition.put(users.get(position).userId, new DuplicatePhoneNumberException()); - } else if (isForeignKeyConstraintError(serverMessage, config.getAppIdToUserIdTable(), - "app_id")) { - throw new TenantOrAppNotFoundException(users.get(position).tenantIdentifier.toAppIdentifier()); + } else if (isForeignKeyConstraintError(serverMessage, config.getAppIdToUserIdTable(), + "app_id")) { + throw new TenantOrAppNotFoundException(users.get(position).tenantIdentifier.toAppIdentifier()); - } else if (isForeignKeyConstraintError(serverMessage, config.getUsersTable(), - "tenant_id")) { - throw new TenantOrAppNotFoundException(users.get(position).tenantIdentifier.toAppIdentifier()); - } + } else if (isForeignKeyConstraintError(serverMessage, config.getUsersTable(), + "tenant_id")) { + throw new TenantOrAppNotFoundException(users.get(position).tenantIdentifier.toAppIdentifier()); } - nextException = nextException.getNextException(); } - throw new StorageQueryException( - new BulkImportBatchInsertException("passwordless errors", errorByPosition)); + nextException = nextException.getNextException(); } - throw new StorageQueryException(e); + throw new StorageQueryException( + new BulkImportBatchInsertException("passwordless errors", errorByPosition)); } + throw new StorageQueryException(e); } } @@ -3655,8 +3647,7 @@ public void addBulkImportUsers(AppIdentifier appIdentifier, List try { BulkImportQueries.insertBulkImportUsers_Transaction(this, (Connection) con.getConnection(), appIdentifier, users); } catch (SQLException e) { - if (e instanceof BatchUpdateException) { - BatchUpdateException batchUpdateException = (BatchUpdateException) e; + if (e instanceof BatchUpdateException batchUpdateException) { SQLException nextException = batchUpdateException.getNextException(); if(nextException instanceof PSQLException){ ServerErrorMessage serverErrorMessage = ((PSQLException) nextException).getServerErrorMessage(); @@ -3784,6 +3775,7 @@ public List deleteBulkImportUsers(AppIdentifier appIdentifier, @Nonnull try { return BulkImportQueries.deleteBulkImportUsers(this, appIdentifier, bulkImportUserIds); } catch (SQLException e) { + Logging.error(this, "Error deleting bulk import users", true, e); throw new StorageQueryException(e); } } diff --git a/src/main/java/io/supertokens/storage/postgresql/queries/EmailPasswordQueries.java b/src/main/java/io/supertokens/storage/postgresql/queries/EmailPasswordQueries.java index f937227d..a88aac19 100644 --- a/src/main/java/io/supertokens/storage/postgresql/queries/EmailPasswordQueries.java +++ b/src/main/java/io/supertokens/storage/postgresql/queries/EmailPasswordQueries.java @@ -346,7 +346,7 @@ public static AuthRecipeUserInfo signUp(Start start, TenantIdentifier tenantIden } public static void signUpMultipleForBulkImport_Transaction(Start start, Connection sqlCon, List usersToSignUp) - throws StorageQueryException, StorageTransactionLogicException, SQLException { + throws StorageQueryException, StorageTransactionLogicException { try { String app_id_to_user_id_QUERY = "INSERT INTO " + getConfig(start).getAppIdToUserIdTable() + "(app_id, user_id, primary_or_recipe_user_id, recipe_id)" + " VALUES(?, ?, ?, ?)"; From d4aa460ea84579dfa80c807675f2f76609675a3d Mon Sep 17 00:00:00 2001 From: tamassoltesz Date: Tue, 27 May 2025 11:23:18 +0200 Subject: [PATCH 3/4] chore: changelog and build version update --- CHANGELOG.md | 4 ++++ build.gradle | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72fdbab2..590a2842 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [9.0.2] + +- Fixes BatchUpdateException checks and error handling to prevent bulk import users stuck in `PROCESSING` state + ## [9.0.1] - Upgrades the embedded tomcat 11.0.6 and logback classic to 1.5.13 because of security vulnerabilities diff --git a/build.gradle b/build.gradle index 48f8350a..ad08669f 100644 --- a/build.gradle +++ b/build.gradle @@ -2,7 +2,7 @@ plugins { id 'java-library' } -version = "9.0.1" +version = "9.0.2" repositories { mavenCentral() From 50517feb35088a1476b2ddbca6053120849f6726 Mon Sep 17 00:00:00 2001 From: tamassoltesz Date: Tue, 27 May 2025 13:05:14 +0200 Subject: [PATCH 4/4] fix: remove accidental ; --- .../supertokens/storage/postgresql/BulkImportProxyStorage.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/supertokens/storage/postgresql/BulkImportProxyStorage.java b/src/main/java/io/supertokens/storage/postgresql/BulkImportProxyStorage.java index 0f28bf10..12eeff8a 100644 --- a/src/main/java/io/supertokens/storage/postgresql/BulkImportProxyStorage.java +++ b/src/main/java/io/supertokens/storage/postgresql/BulkImportProxyStorage.java @@ -98,7 +98,7 @@ public void commitTransactionForBulkImportProxyStorage() throws StorageQueryExce if (this.connection != null) { this.connection.commitForBulkImportProxyStorage(); } - } catch (SQLException e) {; + } catch (SQLException e) { throw new StorageQueryException(e); } }