Skip to content

fix(auth): equalize account provisioning work (ORB-96) - #744

Merged
thomasluizon merged 7 commits into
mainfrom
thomasluizon/ticket-102-enumeration-timing
Oct 18, 2026
Merged

thomasluizon merged 7 commits into
mainfrom
thomasluizon/ticket-102-enumeration-timing

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Oct 18, 2026 •

Copy link
Copy Markdown
Owner

Successful email and Google sign-ins did an extra user insert/commit and signup work only for new accounts. Both account states now follow the same transaction, provisioning write/read, account save, session creation and background-job enqueue sequence.

Closes thomasluizon/orbit-tickets#102. Implements ORB-96.

Approach

  • AuthUserProvisioning builds and validates the same domain candidate for every authenticated email. The new IAuthUserStore and Infrastructure AuthUserStore perform a parameterized email-keyed upsert, followed by a tracked read. Conflicts update only the email to itself, preserving the stored profile, trial, language, handle, preferences and Google tokens. Candidate identity distinguishes the insert winner without reading a database-specific result field.
  • VerifyCodeCommandHandler and GoogleSignInFlow execute provisioning and session persistence inside the existing unit-of-work transaction. The upsert holds the account row lock until commit; the tracked read preserves PostgreSQL's concurrency token. Marking email modified makes the subsequent account save write in both branches, including an otherwise unchanged returning account.
  • Both flows enqueue CompleteSignupJob after the transaction completes. Returning-account jobs exit immediately. Signup analytics, welcome mail and referral processing run with fresh Hangfire-scoped services. Email referral processing consequently moves out of the request; Google referral processing was already background work. The existing Hangfire registration gains this job, and the service registrations gain the store.
  • Existing validators and domain factory guards remain authoritative. Response contracts, route mappings, error texts, status mappings and headers are unchanged. OpenAPI generation ran during the build and produced no contract diff.

This implements the final run decision to remove branch-dependent work. It makes no exact constant-time claim and adds no response floor or wall-clock timing assertions. PostgreSQL's INSERT documentation describes the atomic insert-or-update guarantee used here.

Path audit

The defect was present on starting head b802f9e4: AuthUserProvisioning.cs:24 returned after a lookup for known emails, whereas :31 created, added and committed new accounts. VerifyCodeCommand.cs:99 also processed new-user referrals inline and :112 captured signup analytics; GoogleSignInFlow.cs:33 scheduled signup effects and :53 captured analytics only for new accounts.

Current path Evidence and treatment
Email send, including operations and OAuth wrappers src/Orbit.Application/Auth/Commands/SendCodeCommand.cs:16 accepts only cache and job-client collaborators; :59 enqueues verification mail. No user lookup or registration-dependent branch. Production code unchanged; constructor and existing cache/enqueue tests guard the property.
Email verify, including operations and OAuth wrappers src/Orbit.Application/Auth/Commands/VerifyCodeCommand.cs:28 rejects invalid codes before persistence; :33 begins the common sign-in transaction and :63 always enqueues the signup decision after success. The provisioning, referral and analytics split above is removed.
Google access-token sign-in and operations wrapper src/Orbit.Application/Auth/Commands/GoogleAuthCommand.cs:21 validates the provider token before calling the shared flow. src/Orbit.Application/Auth/Commands/GoogleSignInFlow.cs:22 now uses the same transaction/save/session/enqueue collaborators for both registration states. No change to provider-token parsing or the wrapper.
Google code sign-in and operations wrapper src/Orbit.Application/Auth/Commands/GoogleCodeAuthCommand.cs:23 exchanges the code once; :28 dispatches CompleteGoogleSignInCommand, whose handler calls the shared Google flow. It inherits the provisioning fix. Code exchange and retry boundaries are unchanged.
Google OAuth callback src/Orbit.Api/Controllers/OAuthController.cs:275 already dispatches GoogleCodeAuthCommand with calendar-token persistence disabled. There is no GoogleAuth action or duplicated FindOrCreateGoogleUserAsync on this head. No controller production change was needed. The new composition test drives callback, code exchange, completion and shared provisioning, comparing collaborator order and email-provisioning language/handle defaults.
Session creation and refresh src/Orbit.Infrastructure/Services/AuthSessionService.cs:32 creates and saves a session for every successful sign-in. Refresh at :54 consumes an existing refresh credential and reads the associated user by ID at :68; it never chooses work by email registration. Unchanged.
OAuth token exchange src/Orbit.Api/Controllers/OAuthController.cs:342 consumes an already-issued authorization code, queries prior keys, adds a new key and saves for every accepted exchange. It neither accepts an email nor provisions a user. Per-key revocation depends on credential history, not a registration lookup. Unchanged.
Other user creation The repository-wide User.Create search finds only shared auth provisioning, the domain factory and src/Orbit.Api/Seed/StagingSeedCommand.cs:137. The latter is a staging-only seed command guarded at :50, with no public response to probe. Unchanged.

Test evidence

  1. Before changing implementation or tests, ran dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~VerifyCodeCommandHandlerTests|FullyQualifiedName~GoogleAuthCommandHandlerTests' --verbosity quiet on the defective head: exit 0, 22 passed. Existing success/error tests did not compare branch work.
  2. Added AuthRegistrationWorkTests.SignIn_RegistrationDoesNotChangeCollaboratorOrder and ran dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~AuthRegistrationWorkTests --verbosity quiet while implementation was unchanged: exit 1, both email and Google cases failed. The comparison exposed the signup-only insert/save/analytics calls. This red regression is preserved in commit dfb5b180 before the implementation commit.
  3. After the fix, dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~Commands.Auth' --verbosity quiet: exit 0, 84 passed. The branch-order test now also covers referral and Google-token inputs and asserts enqueue follows transaction completion.
  4. dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~AuthUserStoreTests|FullyQualifiedName~GoogleAuthFirstLoginRaceTests|FullyQualifiedName~OAuthControllerTests|FullyQualifiedName~AuthControllerTests' --verbosity quiet: exit 0, 118 passed. Actual SQL sequences match between insertion and conflict; candidate defaults persist; existing account state survives. Callback tests reach the shared command composition. Anonymous verification tests compare serialized body, status and headers for absent, expired, wrong and throttled codes, with zero persistence/session/job calls. Wrong-code tests assert one failed attempt in each registration state.
  5. CompleteSignupJobTests execute signup effects and returning-account no-ops directly, including referral and analytics failures. Existing code-exchange retry tests still prove a consumed Google code is not redeemed again.

Broader checks:

  • dotnet build Orbit.slnx --verbosity quiet: exit 0, zero errors.
  • dotnet test tests/Orbit.Domain.Tests --verbosity quiet: exit 0, 671 passed.
  • dotnet test tests/Orbit.Application.Tests --verbosity quiet: exit 0, 5,494 passed.
  • dotnet test tests/Orbit.Infrastructure.Tests --verbosity quiet: exit 0, 3,364 passed.
  • dotnet test tests/Orbit.Analyzers.Tests --verbosity quiet: exit 0, 32 passed, covering ORBIT0001 through ORBIT0005.

Existing package-version, API-versioning and obsolete-member warnings remain. Git hooks passed after retrying a transient formatter restore failure; hooks were never bypassed. No browser, server, live database harness or mutation testing was started.

  • Unchanged infrastructure tests: 3 passed using:
    dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~AuthUserStoreTests|FullyQualifiedName~GoogleAuthFirstLoginRaceTests' --verbosity minimal
  • Required-column regression: temporarily removing Name from the insert columns and values made Insert_IncludesEveryRequiredMappedColumnWithoutDatabaseDefault fail with NOT NULL constraint failed: Users.Name. Command:
    dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~Insert_IncludesEveryRequiredMappedColumnWithoutDatabaseDefault' --verbosity minimal
    Restored the column; final infrastructure focus: 5 passed.
  • Concurrent regression initially failed with cannot start a transaction within a transaction using the shared SQLite connection. Independent connections to a temporary SQLite file fixed the harness. Both simultaneous callers now receive one account, with two sessions and one signup.
  • Unchanged signup/referral tests: 10 passed using:
    dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~CompleteSignupJobTests|FullyQualifiedName~ProcessReferralCodeCommandHandlerTests' --verbosity minimal
  • Strengthened email-retry test failed because analytics was captured twice:
    dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~CompleteSignupJobTests' --verbosity minimal
    After splitting jobs, the combined focus passed 12 tests, including one analytics capture, one referral invocation, and successful email retry.

Post-commit verification:

Command Result
dotnet build Orbit.slnx --verbosity minimal 0 errors
Both focused commands above with --no-build 17 passed
dotnet test tests/Orbit.Domain.Tests --no-build --verbosity minimal 671 passed
dotnet test tests/Orbit.Application.Tests --no-build --verbosity minimal 5,496 passed
dotnet test tests/Orbit.Infrastructure.Tests --no-build --verbosity minimal 3,366 passed
dotnet test tests/Orbit.Analyzers.Tests --no-build --verbosity minimal 32 passed

Pre-commit gates passed. OpenAPI regeneration produced no changes. Job tests execute actual Hangfire-produced Job.Type, Method, and Args; the schema guard reads actual EF metadata.

  • Existing tests with defect present: 27 passed.
  • New regressions before fix: five failed as intended; all pass after fix.
  • Final build: zero errors.
  • Final suites: Domain 677, Application 5,502, Infrastructure 3,372, Analyzers 32 passed. All exit codes zero.
  • Unchanged baseline: dotnet test tests/Orbit.Application.Tests --no-build --filter 'FullyQualifiedName~CompleteSignupJobTests' passed all 7 tests with the defect present. After the fix, all 7 passed again.
  • Red: dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~CompleteSignupReferralTests' exited 1. CouponFailure_RollsBackReferralAndAllowsRetryWithoutRepeatingOtherSignupJobs failed because ReferredByUserId persisted after coupon failure. Both terminal-failure cases passed.
  • Green: dotnet test tests/Orbit.Infrastructure.Tests --no-build --filter 'FullyQualifiedName~CompleteSignupReferralTests' exited 0; all 3 passed.
  • Final dotnet build: exit 0, zero errors.
  • git diff --exit-code -- src/Orbit.Api/openapi.json: exit 0.
  • Repository hooks passed; no bare narration comments added.
Full suite command Passed Exit
dotnet test tests/Orbit.Domain.Tests --no-build 716 0
dotnet test tests/Orbit.Application.Tests --no-build 5,520 0
dotnet test tests/Orbit.Infrastructure.Tests --no-build 3,511 0
dotnet test tests/Orbit.Analyzers.Tests --no-build 32 0

Interface evidence

  • AuthUserStoreTests.NewAndExistingAccounts_ExecuteIdenticalSqlInOrder captures real EF commands executed against the repository's existing SQLite fixture, rather than mocking an invented upsert result. The observed sequence is INSERT ... ON CONFLICT ("Email") DO UPDATE, SELECT, UPDATE for both states. The test also exercises an already tracked account and verifies all persisted domain values. Conflict_PreservesDeactivatedAccountAndItsPreferences observes the real tracked entry's modified-email flag. The SQL execution return value is ignored; no affected-row count or undocumented database field is read.
  • The auth tests capture Hangfire's actual Enqueue expression output at its IBackgroundJobClient.Create port: job type CompleteSignupJob, arguments [userId, isNewUser, language, referralCode], enqueued state. Tests execute the real job separately. No new Google or Supabase response fields are read, and their existing HTTP parsing is untouched. Production PostgreSQL/provider calls were not made during these unit checks.

Assumptions

  • Followed the final run decision's uniform-work implementation instead of the superseded floor/configuration/p95-measurement proposal.
  • Applied the Google fix in the current shared GoogleSignInFlow instead of recreating the removed OAuth Google action and duplicate provisioning helper.
  • Preserved account-specific success values such as stored name and WasReactivated, as the ticket explicitly excludes changing them, instead of homogenizing those values.
  • Used xUnit and the repository's existing SQLite unit-test fixture instead of the generic Vitest language and a new live-Postgres measurement harness.
  • Enqueued a signup decision for every successful login instead of enqueueing only for new accounts, which would retain registration-dependent request work.
  • Used the explicitly permitted column-coverage guard instead of replacing the parameterized upsert with model-derived SQL.
  • Used independent SQLite file connections instead of the shared in-memory connection for concurrent callers.

Legacy accounts without signup metadata retain their historical signup behavior. Original serialized Hangfire job signatures remain compatible.

Placed the relational regression in Infrastructure.Tests to reuse its existing SQLite fixture, instead of introducing a database harness in Application.Tests.

Manual steps

None. No new environment variables, secrets, console settings, migrations, or backfills are required.

Normal deployment through GitHub Actions > Release API > Run workflow applies the included migration through Database:MigrateOnStartup. Verify the deployed commit and migration entry in __EFMigrationsHistory. No new settings or backfill required.

None introduced by this round.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

A signup-job scheduling failure now turns committed email and Google sign-ins into HTTP 500 responses and can permanently skip new-account signup work.

Reviewed changes Read all 19 changed files and traced provisioning, transaction/session persistence, signup dispatch, and the affected authentication entry points.

  • Uniform provisioning: Replaces lookup/insert branching with candidate creation and an email-keyed upsert, followed by a fresh tracked read and forced account update.
  • Transactional authentication: Moves account provisioning, reactivation, Google-token persistence, and session creation into a common transaction sequence.
  • Background signup effects: Enqueues CompleteSignupJob for every successful sign-in and moves referral processing, signup analytics, and welcome mail into scoped background execution.
  • Regression coverage: Adds collaborator-order, SQL-shape/state-preservation, callback-composition, and anonymous-error parity tests. All 202 focused Application and Infrastructure auth tests passed locally; no live PostgreSQL concurrency or timing test was run.

Optional cross-model verification was unavailable: /bin/sh: 1: codex: not found.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread src/Orbit.Application/Auth/Commands/GoogleSignInFlow.cs Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

No new Critical or High findings in this delta. The existing post-commit signup-dispatch finding remains unresolved, so this is not an approval.

Reviewed changes Reviewed 1ea91154 against the prior review at 84d481fb, with the full PR diff as context.

  • Isolated welcome-email retries: Split referral processing, signup analytics, and welcome delivery into separate jobs so an email-delivery retry does not repeat the other effects.
  • Strengthened provisioning coverage: Added required-column checks against EF metadata and simultaneous same-email sign-ins using independent SQLite connections.
  • Verified retry and replay behavior: Added welcome-email retry and completed-referral replay regressions. All 36 focused Application and Infrastructure tests passed locally, including shared SQLite query tests; no live PostgreSQL or timing test was run.

Pullfrog  | Fix it ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

A new referral failure path commits partial referral state and marks the effect complete without creating the user's discount. This needs correction before merging.

Reviewed changes Reviewed 3a445d05 against the prior Pullfrog review at 1ea91154, with the full PR diff as context.

  • Preserved signup intent in nullable account fields so later sign-ins recover the original language and referral after dispatch failure.
  • Isolated dispatch failures behind SignupJobDispatcher, preserving committed email and Google authentication responses and logging the request identifier.
  • Serialized signup checkpoints under a user-row lock and added an additive migration to persist per-effect completion.
  • Verified recovery and replay coverage with 98 Application, 129 Infrastructure, and 6 Domain tests passing. A separate temporary SQLite diagnostic reproduced the partial-referral commit described inline; no live PostgreSQL or timing test was run.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread src/Orbit.Application/Auth/Jobs/CompleteSignupJob.cs Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new issues found. Both prior findings are fixed and settled; no Critical or High concerns remain.

Reviewed changes Reviewed 3a445d05..f375e8e8 against the prior Pullfrog review, with the complete PR diff as context.

  • Removed referral exception swallowing so failed coupon creation rolls back tracked referral mutations and leaves the completion checkpoint unset for retry.
  • Added real-handler regression coverage proving rollback, successful retry in a fresh context, independent welcome and analytics completion, and checkpointing of terminal referral rejections.
  • Updated migration-model assertions after merging main to distinguish the existing accountability-pair cascade migration from the additive signup fields.
  • Verified 115 focused tests: 98 Application and 17 Infrastructure tests passed locally. No full-suite, live PostgreSQL, or timing test was run in this review.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

@sonarqubecloud

Copy link
Copy Markdown

@thomasluizon
thomasluizon merged commit 1bd0d26 into main Oct 18, 2026
25 checks passed
@thomasluizon
thomasluizon deleted the thomasluizon/ticket-102-enumeration-timing branch October 18, 2026 19:21
thomasluizon added a commit that referenced this pull request Oct 18, 2026
* fix: pin analyzer test Pkcs dependency to patched version (#728)

(cherry picked from commit 61b0090)

* fix: shard pull request domain mutation testing (#729)

(cherry picked from commit bcfd81e)

* Promote the API redesign with production compatibility checks (#723)

* Bring the API redesign tree onto main for #1413

* Preserve calendar connection detection for shipped clients

* Prove API and migrated schema compatibility with shipped clients

* Trim whitespace in the archived main schema proof

* Require the production health response to report the released commit

* Bind initial approval previews to stored pending operations

* fix: return the bulk skip refresh outcome and split the undo skip query

SonarCloud failed the quality gate on two reliability findings in code this pull request brings to main:
the bulk skip conflict check read a flag only a lambda wrote (S2583), and the undo skip check loaded
Tags and Goals in one query (S8733). The transaction now returns whether every habit refreshed, and the
undo skip check splits its query. A new case covers the concurrent change conflict; it failed with the
refresh outcome forced to true and passes with the fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 5526c9b)

* Add per-account feature flag targeting and admin controls (#724)

* feat: resolve feature flags per account

* feat: administer account feature flags through MCP and REST

* test: exclude feature flag targets from rate limit fixture

* fix: reconcile account flag tests and migration metadata with redesign

(cherry picked from commit 1b886bd)

* Add header API versioning for v1 and v2 (#733)

* Add header API versioning with v1 compatibility

* Verify versioned OpenAPI and browser header compatibility

* Preserve skip undo path contract with optional version header

(cherry picked from commit ac8b34a)

* fix: expose the config response in OpenAPI (#734)

(cherry picked from commit a9d8dd5)

* Route production redesign accounts through Cloudflare (#735)

* Add cookie-based routing for the production redesign web service

* Document web routing rollout, permissions, and rollback checks

* Fix web routing coverage and disable infra install scripts

* chore: regenerate the v2 OpenAPI document on main

orbit-api#733 added openapi_v2.json and orbit-api#734 typed the /api/config
response in parallel, so main carries a v2 document without the typed config
schema and the Build check's stale-document step fails on every merge with
main. dotnet build src/Orbit.Api regenerates it; openapi.json is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 9c64770)

* Fix habit edit gating, end dates, and reminder updates (#736)

* fix: validate habit updates with the resulting time range

* fix: preserve end dates when creating root habits

* fix: gate slip alert activation and preserve unchanged reminders

* fix: compare effective reminder rules across stored formats

(cherry picked from commit 7472203)

* Reduce mutation and guard CI jobs (#737)

* Select mutation shards from changed projects

* Consolidate guards and filter artifact generation

* Skip project documentation in mutation selection

* Validate mutation base refs and restrict Git PATH

* Read mutation shard changes from standard input

(cherry picked from commit b802f9e)

* test: strengthen habit and goal ownership coverage (#739)

(cherry picked from commit 0e32628)

* test: verify checkout session user metadata (#740)

(cherry picked from commit da2f53a)

* fix: protect OAuth code delivery with Turnstile (#741)

(cherry picked from commit 1149606)

* fix: enforce chat interaction scope before loading context (#743)

(cherry picked from commit 98faf4e)

* fix: erase accountability pairs during account deletion and reset (#745)

* fix: erase accountability pairs during account deletion and reset

* test: preserve migration compatibility checks after accountability cleanup

(cherry picked from commit a9e5fb9)

* Fix goal sync dates and deadline reminder counts (#747)

* fix: compare streak sync dates in the user timezone

* fix: show actual days remaining in goal deadline reminders

* test: configure timezone for streak scheduler fixtures

(cherry picked from commit 0e63842)

* Manage staging landing routing in Terraform (#742)

(cherry picked from commit abdd738)

* Restrict Web Push endpoints to supported push services (#1503) (#751)

Web Push subscriptions accept only HTTPS endpoints on the FCM, Mozilla, Apple and WNS push hosts, and the push HTTP client follows no redirects. Replaces orbit-api#748 as one commit so no commit carries the high-entropy sample fixture.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit d9f847a)

* Erase account uploads before permanent deletion (#1507) (#750)

* fix: erase uploaded objects before completing account deletion

* fix: refuse upload reads after permanent account deletion

* fix: grant uploads bucket deletion permissions and verify provider wiring

* test: align storage startup checks with erasure requirements

* fix: retire deleted Supabase upload storage

* test: configure upload erasure providers in deletion fixtures

Share the existing object-store fake across account deletion and scheduler fixtures after merging main. Preserve accountability assertions and production provider requirements for thomasluizon/orbit-tickets#1507.

(cherry picked from commit fa33a94)

* fix: limit anonymous auth and waitlist by email and source IP (ORB-108) (#749)

* fix: enforce source IP ceilings alongside anonymous email limits

* test: cover both rate limit dimensions before bot verification

* fix: isolate BFF email rate limits by verified client IP

* fix: generate and persist BFF forwarding secrets

(cherry picked from commit de0f42f)

* Limit access tokens to fifteen minutes (#738)

* fix(auth): limit access tokens to fifteen minutes

* fix(auth): align managed JWT lifetime and prove logout rejection

(cherry picked from commit ade7016)

* fix(auth): equalize account provisioning work (ORB-96) (#744)

* test(auth): expose registration dependent sign-in work

* fix(auth): perform uniform registration work before sign-in responses

* test(auth): cover expired codes and tracked upsert conflicts

* fix(auth): isolate signup retries and restore provisioning guards

* fix(auth): recover signup work after dispatch failures

* fix(auth): roll back failed signup referrals before retry

(cherry picked from commit 1bd0d26)

* fix: expose redesign controllers in both API versions

Apply the carried version declarations to the Notion and proactive controllers and regenerate the v2 contract. Refs thomasluizon/orbit-tickets#746.

* fix: honor account flags in proactive engine

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant