Skip to content

Require a session claim on access tokens (#660) - #767

Merged
thomasluizon merged 1 commit into
mainfrom
thomasluizon/ticket-660-require-session-claim
Oct 19, 2026
Merged

thomasluizon merged 1 commit into
mainfrom
thomasluizon/ticket-660-require-session-claim

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

A validated access token without orbit_session_id bypasses the session check. Remove the early return in src/Orbit.Api/Extensions/ServiceCollectionExtensions.cs so the existing parse guard produces an authentication failure. Replace the legacy-acceptance test in tests/Orbit.Infrastructure.Tests/Extensions/JwtSessionValidationTests.cs with a rejection assertion against the configured JWT event. Reusing the existing rejection path keeps missing, malformed, and inactive sessions under the same gate.

The WHY comment named in the ticket is already absent in this checkout. The build regenerated both OpenAPI documents without a diff.

Closes thomasluizon/orbit-tickets#660.

Deployment timing

Render service srv-d6tc2isr85hc739bf75g deployed PR #553's merge commit ad018bd2f869f8c09b2417df196519e98a58dedd as deploy dep-daquq8nf3r2c73cli720. Render reports completion at 2026-09-25T03:41:51.003508Z. The 168-hour cutoff was 2026-10-02T03:41:51.003508Z. At the read on 2026-10-19T02:17:18.902858Z, 574.59 hours had passed.

Test evidence

All commands ran with LC_ALL=en_US.UTF-8.

  1. Before either edit, ValidatedLegacyToken_WithoutSessionClaim_RemainsAccepted passed with the acceptance branch present:

    LC_ALL=en_US.UTF-8 dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~ValidatedLegacyToken_WithoutSessionClaim_RemainsAccepted --verbosity minimal

    Result: exit 0, 1 passed.

  2. After replacing the test and before changing production code, ValidatedToken_WithoutSessionClaim_IsRejected failed:

    LC_ALL=en_US.UTF-8 dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~ValidatedToken_WithoutSessionClaim_IsRejected --verbosity minimal

    Result: exit 1, 1 failed. Assertion: Expected context.Result to be Microsoft.AspNetCore.Authentication.AuthenticateResult, but found <null>.

  3. After removing the early return, the configured-session tests passed:

    LC_ALL=en_US.UTF-8 dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~JwtSessionValidationTests --verbosity minimal

    Result: exit 0, 4 passed, including the replacement test, active and inactive sessions, and logout followed by an HTTP 401 challenge.

  4. Final verification after commit: LC_ALL=en_US.UTF-8 dotnet build Orbit.slnx --verbosity minimal exited 0 with 0 errors. The full .NET suite passed across all four solution test projects, 10,037 tests total, with no failures or skips:

    Command (with LC_ALL=en_US.UTF-8) Result
    dotnet test tests/Orbit.Domain.Tests --verbosity minimal Exit 0, 736 passed
    dotnet test tests/Orbit.Application.Tests --verbosity minimal Exit 0, 5581 passed
    dotnet test tests/Orbit.Infrastructure.Tests --verbosity minimal Exit 0, 3688 passed
    dotnet test tests/Orbit.Analyzers.Tests --verbosity minimal Exit 0, 32 passed

External interface evidence

The rejection test invokes the real configured JwtBearerEvents.OnTokenValidated callback. Its passing assertion observes an actual AuthenticateResult with a non-null Failure; it does not supply a mock result for that external type.

The Render deployment fields above were read from the live response. Reproduce the read with an authorized Render token:

curl --fail --silent --show-error \
  --header "Authorization: Bearer ${RENDER_MCP_TOKEN}" \
  'https://api.render.com/v1/services/srv-d6tc2isr85hc739bf75g/deploys/dep-daquq8nf3r2c73cli720'

Complete response, with no credentials returned:

{"id":"dep-daquq8nf3r2c73cli720","commit":{"id":"ad018bd2f869f8c09b2417df196519e98a58dedd","message":"Fix #657 logout revocation across a session family (#553)\n\n* Start #657 session family fix\n\n* Revoke the full auth session after logout (#657)\n\n* Explain temporary legacy token allowance (#657)\n\n* Handle duplicate refresh history race (#657)","createdAt":"2026-09-25T03:40:16Z"},"status":"deactivated","trigger":"new_commit","createdAt":"2026-09-25T03:40:18.436165Z","updatedAt":"2026-09-25T04:28:47.330702Z","startedAt":"2026-09-25T03:40:18.412312Z","finishedAt":"2026-09-25T03:41:51.003508Z"}

@sonarqubecloud

Copy link
Copy Markdown

@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.

Reviewed changes Reviewed both changed files, token issuance and refresh paths, and the web/mobile clients' HTTP 401 recovery behavior.

  • Required session claim: Removes the legacy-token bypass so an absent orbit_session_id fails the existing session-validation guard without querying the session store.
  • Regression coverage: Replaces the legacy-acceptance assertion with a failure assertion against the configured JWT callback. All four JwtSessionValidationTests cases passed during this review; the full solution suite was not rerun.

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

@thomasluizon
thomasluizon merged commit 517ed1b into main Oct 19, 2026
22 checks passed
@thomasluizon
thomasluizon deleted the thomasluizon/ticket-660-require-session-claim branch October 19, 2026 02:44
thomasluizon added a commit that referenced this pull request Oct 19, 2026
…, slip contract and the Supabase sign-in removal (#768)

* Stop Stripe and Play renewals when account deletion is confirmed (#746)

* fix: stop subscription renewal when account deletion is confirmed

* fix: restrict renewal restoration to an unexpired paid period

* fix: keep free account deletion independent of billing services

* fix: bound billing cancellation delays during account erasure

* fix: preserve renewal intent during deletion reconciliation

(cherry picked from commit 85d8824)

* docs: name Render web services in version floor preconditions (#755)

(cherry picked from commit 21615aa)

* fix(infra): use Render for production database connections (#754)

(cherry picked from commit e2909b6)

* test(habits): cover end date reactivation allowance through handler (#759)

(cherry picked from commit 16203d9)

* test(mcp): verify Pro access for OAuth and API keys (#760)

(cherry picked from commit 0a90f67)

* Cover calendar auto-sync scheduling and import reconciliation (ORB-199) (#761)

* test(calendar): cover auto-sync scheduler eligibility and batching

* test(calendar): cover import suggestion reconciliation filters

* test(calendar): isolate free plan eligibility from expiry

(cherry picked from commit 664c756)

* Clarify controller action cross-references in the agent catalog (#762)

(cherry picked from commit d9cda52)

* refactor(calendar): share legacy match key for sync and suggestions (#763)

(cherry picked from commit 8e754c4)

* Contract habit log slip classification schema (#718) (#764)

(cherry picked from commit b0d3ff4)

* fix(calendar): gate recurring event reconciliation (#765)

(cherry picked from commit c900dba)

* Gate rolling schema contractions behind deployment barriers (#766)

* feat: require deployment barriers for rolling schema contractions

* fix: keep quoted SQL comment markers from hiding contractions

* test: record the deployed habit log contract barrier

* ci: run the rolling contract gate only when its inputs change

The gate builds the infrastructure test project, which costs minutes on every
pull request. It now runs only when a migration, the barrier file or the gate
itself changes; the step before it already reads the same diff.

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

* fix: require barriers for unclassified ALTER TABLE actions

* test: isolate barrier unit coverage from checkout history

---------

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

* fix: require a session claim on access tokens (#767)

(cherry picked from commit 517ed1b)

* Deprecate legacy Google auth routes (#1528, part 1) (#753)

* Deprecate legacy Google auth routes for #1528

* Explain deprecated Google auth routes and annotate legacy tests

(cherry picked from commit 21f810f)

* Remove deleted Supabase sign-in path (#1528) (#752)

* Remove deleted Supabase sign-in path (#1528)

* Remove unused access-token validation catalog entries (#1528)

(cherry picked from commit 141a4a2)

* fix: bind carried schema barrier to the deployed redesign

Record the observed staging deployment and its drained predecessors without changing the migration gate. Regenerate the tracked gating matrix for the combined source. Refs thomasluizon/orbit-tickets#746.

* fix: retain deletion cancellation ownership across provider retries (#770)

* fix: persist deletion cancellation ownership before provider updates

* test: cover deletion intent commit and account changes

* fix: identify deletion-owned Stripe cancellations

Reconcile provider metadata before restoring renewal or retaining ownership, reject open cancellation transactions, and log skipped erasure rechecks.

Refs thomasluizon/orbit-tickets#1547

(cherry picked from commit a7d1cf2)

* test: drop the deprecation assertions the legacy Google route removal supersedes

The base merge brought #769's assertions that POST /api/Auth/google and
POST /api/Auth/operations/google stay available and deprecated. This carry
removes both routes (main's #1528 part 2), so those assertions no longer
have a subject.

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

---------

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