Skip to content

feat(members): deactivate sub-keyed members from the id_token sub fallback - #2987

Merged
mroderick merged 2 commits into
masterfrom
fix/subkeyed-member-cleanup
Oct 7, 2026
Merged

mroderick merged 2 commits into
masterfrom
fix/subkeyed-member-cleanup

Conversation

@mroderick

@mroderick mroderick commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

When the planner's OmniAuth strategy substituted payload['sub'] (the better-auth user id) for the member email on tokens without an email claim, 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: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, and counts 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 value). Re-runs detect nothing — repeatable and idempotent.

Usage

Makefile targets (same shape as #2809):

make detect_subkeyed_members              # list matches from the local production dump (read-only)
make fix_subkeyed_members                 # dry run on the dump: prints what would happen
make fix_subkeyed_members EXECUTE=1       # deactivate on the dump for real
make verify_subkeyed_members              # confirm the end state (exits 1 while work remains)

make detect_subkeyed_members_production   # list matches on the PRODUCTION database (prompts first)
make fix_subkeyed_members_production      # dry run on PRODUCTION (EXECUTE=1 executes)
make verify_subkeyed_members_production   # verify on PRODUCTION

The production targets connect from this machine directly to the Heroku
Postgres database: they fetch the live DATABASE_URL via
heroku config:get and print a WARNING: Connecting to REMOTE database
line before connecting. They require heroku login and outbound access
to the Heroku Postgres endpoint on 5432. Take a backup first
(make backup_production), and run production EXECUTE=1 only after
#2986 (the fail-closed strategy guard) is deployed.

The underlying rake tasks work the same way without make:

rake member:subkeyed:detect
DB_NAME=codebar_production_dump rake member:subkeyed:detect       # local dump
DB_URL=$(heroku config:get DATABASE_URL --app=codebar-production) rake member:subkeyed:detect   # production

Sequencing dependency: run production EXECUTE=1 only 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

  • 29 spec examples across the service and rake layers (the verify failure-path test contains its exit 1 inside 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 eight OWNED_DATA skip-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.
  • Full parallel suite: 1306 examples, 0 failures. RuboCop clean on touched files.
  • Proven against 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.

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
mroderick force-pushed the fix/subkeyed-member-cleanup branch 2 times, most recently from 7dc6589 to 1ea8cb7 Compare October 7, 2026 06:51
…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
mroderick force-pushed the fix/subkeyed-member-cleanup branch from 1ea8cb7 to bcf07db Compare October 7, 2026 07:09
@mroderick
mroderick marked this pull request as ready for review October 7, 2026 07:17
@mroderick
mroderick requested a review from olleolleolle October 7, 2026 07:17
@mroderick
mroderick enabled auto-merge October 7, 2026 18:54
@mroderick
mroderick merged commit 453e54b into master Oct 7, 2026
10 checks passed
@mroderick
mroderick deleted the fix/subkeyed-member-cleanup branch October 7, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants