feat: add user locking to isolation reservation table #290
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bump version to 9.3.1 and update CHANGELOG.
- Add LockedUserImpl with connection validation - Add UserLockingQueries with FOR UPDATE locking - Implement UserLockingStorage and AccountInfoStorage in Start.java - Use consistent ordering (TreeSet) to prevent deadlocks - Re-verify primary user mapping under lock Co-Authored-By: Claude Opus 4.5 <[email protected]>
…oForLinking - Add new reserveAccountInfoForLinking_Transaction overload that accepts LockedUser - Update linkAccounts_Transaction to acquire locks via lockUsersForLinking - Validate primary/recipe user status using LockedUser methods - Skip advisory lock since row-level locks handle concurrency Co-Authored-By: Claude Opus 4.5 <[email protected]>
…_Transaction - Add new updateAccountInfo_Transaction method with LockedUser parameter to AccountInfoQueries.java - Update storage methods to acquire locks before calling account info update: - updateUsersEmail_Transaction (EmailPassword) - updateUserEmail_Transaction (ThirdParty) - updateUserEmailAndPhone_Transaction (Passwordless) - updateUserEmail_Transaction (WebAuthN) - Remove old updateAccountInfo_Transaction method that took String userId - Convert UserNotFoundForLockingException to UnknownUserIdException Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ountInfo - Add LockedUser overload of addPrimaryUserAccountInfo_Transaction - Update makePrimaryUser_Transaction to acquire lock before calling method - Validate via LockedUser state (isLinked/isPrimary) before DB operations - Remove advisory lock since row-level locking is now used via LockedUser Co-Authored-By: Claude Opus 4.5 <[email protected]>
…nForPrimaryUserForUnlinking Implement the AccountInfoStorage.removeAccountInfoReservationForPrimaryUserForUnlinking_Transaction method that requires LockedUser parameters. The implementation: - Validates the recipe user is part of the primary user group (linked or is the primary itself) - Delegates to the existing string-based implementation after validation Also updates Start.java to implement the AccountInfoStorage interface method. Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ransaction - Add LockedUser.isValidForConnection() validation at method start - Remove redundant AppIdentifier parameter (derive from TenantIdentifier) - Update callers to use simplified signature Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Fix UserLockingQueries to use app_id_to_user_id table instead of
all_auth_recipe_users, allowing users removed from all tenants to be locked
- Fix readPrimaryUserId to check is_linked_or_is_a_primary_user flag,
distinguishing standalone users from primary/linked users
- Fix lockUsers to handle empty string (standalone user) vs null (not found)
- Fix reserveAccountInfoForLinking_Transaction validation:
- Allow linked users as "primary" parameter (getPrimaryUserId returns their
actual primary)
- Add check for recipe user being a primary user (cannot link a primary
as recipe user)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
…aryUser_Transaction Update addTenantIdToPrimaryUser_Transaction to accept LockedUser instead of String userId: - Change method signature in AccountInfoQueries and Start - Add isPrimary() validation to ensure the user is actually a primary user - Extract supertokensUserId from LockedUser for query execution This ensures proper row-level locking is acquired before adding account info reservations when associating a primary user with a tenant. Co-Authored-By: Claude Opus 4.5 <[email protected]>
UserLockingQueries now fetches recipe_id from app_id_to_user_id during lock acquisition and passes it to LockedUserImpl. This ensures recipe ID is available even for users removed from all tenants. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Update getRecipeIdForUser_Transaction to take LockedUser instead of userId string. The method now simply returns lockedUser.getRecipeId() since the recipe ID is fetched during lock acquisition from app_id_to_user_id. This removes the need for a separate query and ensures the recipe ID is available even for users not in any tenant (users removed from all tenants). Callers now: 1. First acquire lock via UserLockingQueries.lockUser() 2. Get recipe ID from LockedUser instead of separate query Co-Authored-By: Claude Opus 4.5 <[email protected]>
…level locks to core - Delete lockThirdPartyInfoAndTenant_Transaction methods (unused) - Delete String-based addPrimaryUserAccountInfo_Transaction (LockedUser version exists) - Delete String-based reserveAccountInfoForLinking_Transaction (LockedUser version exists) - Rename String-based removeAccountInfoReservationForPrimaryUserForUnlinking_Transaction to doRemoveAccountInfoReservationForUnlinking (internal helper) - Remove lock call from listPrimaryUsersByThirdPartyInfo_Transaction (locking now in core) - Keep getAllPasswordResetTokenInfoForUser_Transaction (used for token-level locking) Co-Authored-By: Claude Opus 4.5 <[email protected]>
porcellus
changed the title
Migrate plugin-level locks to core pattern by removing FOR UPDATE from: - TOTPQueries.getDevices_Transaction: caller should obtain lock via UserLockingStorage - UserMetadataQueries.getUserMetadata_Transaction: advisory lock already provides protection - UserMetadataQueries.getMultipleUsersMetadatas_Transaction: caller should obtain locks Co-Authored-By: Claude Opus 4.5 <[email protected]>
Convert removeAccountInfoForRecipeUserWhileRemovingTenant_Transaction and removeAccountInfoReservationForPrimaryUserWhileRemovingTenant_Transaction to accept LockedUser instead of String userId. This provides compile-time safety and eliminates redundant subqueries for primary_user_id lookup since it's already available from LockedUser. Co-Authored-By: Claude Opus 4.5 <[email protected]>
…rvation-table-locking
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of change
(A few sentences about this PR)
Related issues
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your
changes work. Bonus points for screenshots and videos!)
Documentation changes
(If relevant, please create a PR in our docs repo, or create a checklist here
highlighting the necessary changes)
Checklist for important updates
pluginInterfaceSupported.jsonfile has been updated (if needed)build.gradlebuild.gradle, please make sure to add themin
implementationDependencies.json.git tag) in the formatvX.Y.Z, and then find thelatest branch (
git branch --all) whoseX.Yis greater than the latest released tag.OneMillionUsersTestRemaining TODOs for this PR