This repository was archived by the owner on Oct 5, 2026. It is now read-only.
Repository navigation
Make the kill switch stop the writes, not only the session - #40
Merged
Merged
Conversation
Closes #37, and it is a one-place fix only because the shared step moved into `auth-core::federated` in #39. Before that it would have been three copies of the same edit, which is the argument that change was making. `users.status` was already honoured where it counts: `issue` refuses a disabled account and no session was ever handed out. But `complete` carried out the linking outcome *first*, and every branch of it writes. A disabled account signing in with Google, Apple or Facebook still recorded a login on its identity; a verified-address match still inserted a new identity row and had the caller announce it. So an account someone had switched off went on accruing rows and events on every attempt, and the switch stopped short of the thing it is for. The status is now read before the outcome is acted on, in one place that every federated method goes through. `NewUser` is exempt because it cannot name an account that already exists; the other outcomes all resolve to a user id, which is what makes the check possible this early. Same shape as the passkey sign-count bug from #13/#14 — a write that happens before the check that should have refused the request — and far lower impact, which is why it was filed rather than rushed. Two tests, both verified to fail with the fix removed: - a repeat sign-in on a disabled account leaves `last_login_at` alone; - a verified-address match onto a disabled account inserts no identity. The first of those passed for the wrong reason on its first attempt: I advanced the test clock an hour to make the timestamp visibly different, which expired the fake's ID token, so the callback failed at verification and never reached the linking rules at all. It now advances a minute, and the comment says why. 329 workspace tests, clippy clean, rustfmt clean, wasm32 clean, cargo deny passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8K9TBtmnf29dV6uEP5oM3
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
Closes #37.
A one-place fix, and it is one place only because the shared step moved into
auth-core::federatedin #39. Before that it would have been three copies of the same edit — which is the argument that change was making.What was wrong
users.statuswas already honoured where it counts:issuerefuses a disabled account, and no session was ever handed out. Butcompletecarried out the linking outcome first, and every branch of it writes:Knownrecords a login timestamp on the identity;AutoLinkedinserts a new identity row and has the caller announce it.So an account someone had switched off went on accruing rows and events on every sign-in attempt, and the kill switch stopped short of the thing it is for.
The fix
The status is read before the outcome is acted on, in one place every federated method goes through.
NewUseris exempt because it cannot name an account that already exists; the other outcomes all resolve to a user id, which is what makes the check possible this early.Same shape as the passkey sign-count bug from #13/#14 — a write that happens before the check that should have refused the request — and far lower impact, which is why it was filed rather than rushed.
Tests
Two, both verified to fail with the fix removed:
last_login_atalone;The first passed for the wrong reason on its first attempt. I advanced the test clock an hour to make the timestamp visibly different, which expired the fake's ID token — so the callback failed at token verification and never reached the linking rules at all. It now advances a minute, and the comment says why. That is the third time in this repo a test has passed while proving nothing, so the check-that-it-fails step is earning its cost.
329 workspace tests, clippy clean, rustfmt clean, wasm32 clean, cargo deny passes.
🤖 Generated with Claude Code
https://claude.ai/code/session_01V8K9TBtmnf29dV6uEP5oM3