Skip to content

fix(customer_seat): reject seat claim when linked member is soft-deleted - #14170

Merged
psincraian merged 1 commit into
mainfrom
detail/bug-fix/fix-customer-seat-reject-seat-claim-when-linked-me-04882f
Sep 8, 2026
Merged

fix(customer_seat): reject seat claim when linked member is soft-deleted#14170
psincraian merged 1 commit into
mainfrom
detail/bug-fix/fix-customer-seat-reject-seat-claim-when-linked-me-04882f

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Summary

Related Issue: polarsource/feedback#436

Customer-portal seat claims with a soft-deleted linked member in member_model_enabled organizations issued a polar_cst_ customer-portal session for the billing/purchaser customer instead of the seat-scoped member — a privilege escalation that granted the seat-claimer customer_portal_write as the purchaser and bypassed the owner/billing_manager role gate on billing endpoints (cancel/update/revoke subscriptions, invoice/retry orders, request email change).

What

server/polar/customer_seat/service.py claim_seat was rewritten in #9983 to dispatch the session type on seat.member_id presence and fall back to create_customer_session(session, session_customer) when the linked member was missing. In member-model mode session_customer is the billing customer, so that fallback minted a purchaser-scope session. Restored the member_model_enabled and outer guard and replaced the fallback create_customer_session call with raise InvalidInvitationToken(invitation_token) when the linked member is not found. Legacy mode is unchanged — there session_customer is the seat-holder's own customer, so the fallback remains a self-session (not an escalation).

Added two regression tests in server/tests/customer_seat/test_service.py::TestClaimSeat that lock in the asymmetry the fix deliberately creates:

  • member-model + soft-deleted member → claim rejected, no session of either kind minted
  • legacy + soft-deleted member → seat-holder's own polar_cst_ self-session (not the purchaser's)

Why

The escalation was introduced in #9983 (commit 96c3c17), which dropped the member_model_enabled and guard that was previously present in #9350 and replaced the safe raise InvalidInvitationToken with the create_customer_session fallback. The fallback looks benign but upgrades scope in the member-model branch because the only customer available there is the purchaser. Exploitation only needs an admin to soft-delete a member while its seat's 24h invitation is still outstanding and the async customer_seat.revoke_seats_for_member job has not yet run — a routine post-delete race window.

How

Restored the exact control flow from #9350: if member_model_enabled and seat.member_id is not None: branches into the member lookup; a found member issues a polar_mst_ member session, a missing member raises InvalidInvitationToken with a comment explaining why (issuing a customer session here would hand the seat-claimer purchaser-scope access). The else branch keeps the legacy create_customer_session(session, session_customer) path unchanged.

Checklist

  • This PR addresses a single concern (one bug fix)
  • The diff is reasonably sized and easy to review
  • New functionality is covered by tests (2 regression tests guarding the member-model/legacy asymmetry)
  • Linting and type checking pass (uv run task lint && uv run task lint_types)
  • No unrelated changes or drive-by fixes are included

Testing

  • New regression tests: member-model + soft-deleted member rejects the claim with no session minted (fails without the fix — verified by stashing the service change); legacy-mode + soft-deleted member still issues the seat-holder's self-session (passes both ways as a non-regression guard).
  • tests/customer_seat/, tests/member/, tests/customer_session/, tests/member_session/, tests/customer_portal/ all pass (157 + 194 + 282 tests) — no collateral regression in the seat, session, or billing-endpoint blast radius.
  • ruff format --check, ruff check, mypy (2-file and full task lint_types over 1631 files), and the lint_org_scope/lint_subquery/lint_ast AST linters all pass.
  • OpenAPI contract byte-identical with vs without the fix (app.openapi() md5 match) — endpoint shape, status codes, and schemas unchanged; alembic check reports no schema drift (no migration required).
  • Live server E2E: started the dev API server, built the exploit scenario in the dev DB (member-model org, seat-based product, billing customer, pending seat with valid invitation_token whose member was then soft-deleted — the post-delete / pre-revoke-job race window), then curl POST /v1/customer-seats/claim returned HTTP 400 InvalidInvitationToken with no session token. (Verified at the HTTP layer; an equivalent in-process HTTP regression test was written and excluded from the final diff as duplicative of the service-level test.)
  • Pre-revoke re-claim check: not exercised against a live worker (no worker running in the dev stack); the invariant that the revocation job clears invitation_token is covered by the existing TestRevokeSeat suite and tests/member/ async-job tests.

Automatic Fixes PRs can be configured here.

Review in cubic

@detail-app
detail-app Bot requested a review from psincraian as a code owner September 7, 2026 01:23
@vercel

vercel Bot commented Sep 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
orbit Ready Ready Preview Sep 7, 2026 1:27am UTC
polar-test Ready Ready Preview Sep 7, 2026 1:27am UTC

Request Review

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

OpenAPI Changes

No changes detected in the OpenAPI schema.

@psincraian
psincraian added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit a916e59 Sep 8, 2026
34 of 44 checks passed
@psincraian
psincraian deleted the detail/bug-fix/fix-customer-seat-reject-seat-claim-when-linked-me-04882f branch September 8, 2026 14:58
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