Repository navigation
feat(members): deactivate sub-keyed members from the id_token sub fallback - #2987
Merged
Merged
Conversation
mroderick
added a commit
that referenced
this pull request
Oct 7, 2026
…laim When the codebar OmniAuth strategy received an id_token without an `email` claim, it substituted `payload['sub']` (the better-auth user id) for the member email, so AuthServicesController created a member keyed on an opaque user id with no subscriptions or roles — the duplicate-member mechanism behind the 2026-10-02 incident (members 31450 and 31452). The strategy now fails the callback with `:missing_email` before building the auth hash. No Member, AuthService, or activity row is created; the standard OmniAuth failure redirect to `/auth/failure` and the generic "Authentication failed" flash surface the failure. The guard also covers a present-but-blank email claim. The successful-callback spec's token now carries a distinct `sub` and an `email` claim, so `uid` proves to come from the email, not the sub fallback — that spec previously passed only because of the fallback. New specs pin the guard, the middleware 302 to `/auth/failure?`, and the blank-claim boundary. Related: codebar/auth#83 restores the claims provider-side. This is the planner-side integrity guard; the sub-keyed cleanup tooling ships in #2987 and its production run waits for this deploy.
mroderick
force-pushed
the
fix/subkeyed-member-cleanup
branch
2 times, most recently
from
October 7, 2026 06:51
7dc6589 to
1ea8cb7
Compare
…lback When a planner id_token arrived without an `email` claim, the OmniAuth strategy substituted the better-auth user id (`payload['sub']`) as the member email, creating garbage members with no subscriptions or roles — the duplicate-member mechanism behind the 2026-10-02 incident (members 31450 and 31452). The existing duplicate heuristics cannot see these members: they share no name, email, or activity evidence with the real account. Adds `member:subkeyed` rake tasks (`detect` / `deactivate` / `verify`) backed by `SubkeyedMemberCleanup`. Detection is deterministic and planner-side: the member's email equals its own codebar auth service uid, that shared value contains no `@`, and the member was created after the 2026-08-06 auth-flow cutoff. Cleanup is deactivate-only: no merge target is knowable from the planner database, and guessing merges is the harm the duplicate tooling guards against. A detected member owning subscriptions, invitations, roles, bans, feedbacks, or notes is skipped and reported, never touched; skipped members count as handled, so `verify` reaches a clean PASS even while skips remain. Runs default to a dry read; `EXECUTE=1` writes one transaction per member (auth services removed, email renamed `subkeyed.<id>.deactivated@codebar.io`, an audit note recording the original email/uid). Re-runs detect nothing — repeatable and idempotent. Pattern and deactivate flow adapted from the temporary duplicate-merge tooling on branch fix/codebar-auth-duplicate-members (draft PR #2809), which stays temporary and unmerged. Sequencing dependency: run production EXECUTE=1 only after #2986 (the fail-closed strategy guard) deploys.
mroderick
force-pushed
the
fix/subkeyed-member-cleanup
branch
from
October 7, 2026 07:09
1ea8cb7 to
bcf07db
Compare
mroderick
marked this pull request as ready for review
October 7, 2026 07:17
KimberleyCook
approved these changes
Oct 7, 2026
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.
When the planner's OmniAuth strategy substituted
payload['sub'](the better-auth user id) for the member email on tokens without anemailclaim, members got keyed on that opaque id — the garbage-member mechanism behind the 2026-10-02 incident (members 31450, 31452). The existing duplicate heuristics cannot see these members: they share no name, email, or activity evidence with the real account.Adds
member:subkeyedrake tasks (detect/deactivate/verify) backed bySubkeyedMemberCleanup. Detection is deterministic and planner-side: the member's email equals its own codebar auth service uid, that shared value contains no@, and the member was created after the 2026-08-06 auth-flow cutoff. Cleanup is deactivate-only — no merge target is knowable from the planner database, and guessing merges is the harm the duplicate tooling guards against. A detected member owning subscriptions, invitations, roles, bans, feedbacks, or notes is skipped and reported, never touched, and counts as handled, soverifyreaches a clean PASS even while skips remain. Runs default to a dry read;EXECUTE=1writes one transaction per member (auth services removed, email renamedsubkeyed.<id>.deactivated@codebar.io, an audit note recording the original value). Re-runs detect nothing — repeatable and idempotent.Usage
Makefile targets (same shape as #2809):
The production targets connect from this machine directly to the Heroku
Postgres database: they fetch the live
DATABASE_URLviaheroku config:getand print aWARNING: Connecting to REMOTE databaseline before connecting. They require
heroku loginand outbound accessto the Heroku Postgres endpoint on 5432. Take a backup first
(
make backup_production), and run productionEXECUTE=1only after#2986 (the fail-closed strategy guard) is deployed.
The underlying rake tasks work the same way without make:
Sequencing dependency: run production
EXECUTE=1only after #2986 (the fail-closed strategy guard) is deployed — while the old fallback is live, a deactivated user's next sign-in re-creates the garbage member. The deactivate flow's pattern is adapted from the temporary tooling on #2809, which stays temporary and unmerged.Testing
exit 1inside a capturing rescue — rspec-core re-raises SystemExit without recording a failure, and an escaped exit poisoned the parallel-test child's exit code even when every example was green; this was the deterministic CI failure, fixed in this PR), including: all eightOWNED_DATAskip-rule relations, the full audit-note contract (original value, timestamp, sequencing clause), the verify gate pinned to exit status 1 with its operator output, idempotency, and the prefix-exclusion chain proven by an inclusion/exclusion control pair.codebar_production_dump: detection matched exactly the incident members; dry-run wrote nothing; EXECUTE deactivated both; re-detect and verify PASSed; a second run was a no-op.Code review: ce-code-review receipt run 20261006-u2-subkeyed — Ready with fixes, all 4 validator-confirmed findings applied.