Repository navigation
Push registration: a deleted account's request still on its way stores nothing - #5219
Merged
Merged
Conversation
…s nothing The registration insert refuses an owner in account_deletions in the same statement as its cap and revocation checks, so a subscribe authorized before the deletion cannot write the phone back after it, whichever device deleted the account. The deletion suite holds an APNs and a Web Push registration at its insert across a real deletion, and the next account then registers the same phone.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
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
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.
Follow-up to #5215 (merged at
cae140de75a): a push registration already in flight could recreate push data after the account was deleted.The problem
A subscribe request that passed its session check before the player deleted their account could still write the phone's registration afterwards. Account deletion removes every push registration in its transaction (
account-deletion.ts), but the store's insert checked only the device cap and the id's revocation. The anonymiseduserrow stays, so the foreign key allowed it. The deleted account then had an APNs token or a browser endpoint again, andgameAlertDeviceslisted it. The sign-out on the registering page cannot be the guard: the account may be deleted from another device.The fix
push-subscription-store.ts: the registration's oneINSERT … SELECT … WHEREalso requires that the owner has no row inaccount_deletions. That table is written in the deletion's own transaction. So whichever lands first wins: a deletion first means the insert writes nothing and returnsdeleted(the route answers409 subscription_deleted); a registration first means the deletion's transaction removes it. No pre-insert check, client step or new state.Tests
account-deletion.test.ts, once per transport (APNs, Web Push): the real Worker signs the account in, the store's registration is held just before its insert, the account is deleted throughDELETE /api/accountwith its emailed code, and the insert is released. It returnsdeleted; no row, game-alert or chat-alert recipient is left. The same email then signs in to a new account, which registers the same phone. Both cases fail onnext(455085973c6) (expected 'registered' to be 'deleted').push-subscription-store.test.tstoheld-push-registrations.tsso both suites use one.pnpm --dir apps/realms test(350), realms typecheck,format:check,knipandwords:checkpass.src/pwa/notifications.mdnotes the refusal beside the revocation rule.