feat(api): native Google Play Billing verify + RTDN with unified Pro source - #194
Conversation
…source Add a Play-purchase verify endpoint (Play Developer API subscriptionsv2.get + server-side acknowledge) and an RTDN Pub/Sub webhook that re-verifies authoritative state, behind a new IPlayBillingService seam. The User entitlement gains SubscriptionSource (stripe|play) + PlayPurchaseToken; PayGate is unchanged and the web Stripe flow is untouched. Refs thomasluizon/orbit-ui-mobile#132 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Paired UI PR (closes the issue): thomasluizon/orbit-ui-mobile#147 — merge this api PR first. |
There was a problem hiding this comment.
One correctness bug that needs a fix before this lands.
StripeCoversLaterPeriod early-return skips both acknowledgement and token persistence (VerifyPlayPurchaseCommand.cs:42-46)
When a user with an active Stripe subscription (expiring later) verifies a Play purchase, the handler returns early without:
-
Acknowledging the purchase. Google's 3-day window applies to every new subscription purchase regardless of local entitlement logic. Skipping
AcknowledgeAsynchere means Google auto-refunds the user and voids the subscription — the app shows success but the purchase evaporates silently. -
Saving
PlayPurchaseToken. Every future RTDN for that token (renewals, cancellations) will find no matching user, so the Play subscription is permanently invisible to the backend. When Stripe later expires and its webhook firesCancelSubscription(), the user drops to free even though they are still paying for Play.
Fix: move AcknowledgeAsync outside (before) the StripeCoversLaterPeriod branch, and add a User.LinkPlayPurchaseToken domain method that saves the token without altering PlanExpiresAt, called unconditionally before returning in that branch. The existing test for this path should also assert both behaviors.
Everything else in the PR is well-structured: the OIDC verification chain for RTDN, the EntitledStates set (CANCELED = still-active-until-period-end is correct per the Play v2 API), the migration backfill, the IPlayBillingService seam, and test coverage for all other handler paths are solid.
… period The StripeCoversLaterPeriod early return skipped purchase acknowledgement (Google auto-refunds unacknowledged purchases within 3 days) and PlayPurchaseToken persistence (leaving RTDN unable to match the user). When Stripe later cancelled, the user would drop to free despite an active Play subscription. Acknowledge unconditionally before the branch and link the token via a new User.LinkPlayPurchaseToken that preserves Stripe's later expiry and source. Refs thomasluizon/orbit-ui-mobile#132 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed in 52e30e5.
Test renamed to Build clean; handler tests 7/7, domain 347/347. |
There was a problem hiding this comment.
The fix in commit 52e30e5 correctly addresses both issues I flagged: AcknowledgeAsync is now called unconditionally before the StripeCoversLaterPeriod branch (lines 42-52 of VerifyPlayPurchaseCommand.cs), and user.LinkPlayPurchaseToken is called with SaveChangesAsync in the early-return path so RTDN can always match the token. The updated test Handle_StripeCoversLaterPeriod_LinksTokenAndAcknowledgesWithoutShorteningExpiry asserts all three behaviors (acknowledge called, token persisted, Stripe expiry preserved). The rest of the PR — OIDC verification chain for RTDN, EntitledStates set, migration backfill, IPlayBillingService seam, and full handler/domain/integration test coverage — remains solid. Good to merge.
Split CancelSubscription into CancelStripeSubscription and CancelPlaySubscription so a cancellation from one provider no longer wipes the other provider's still-active entitlement. Each clears the shared Pro slot only when it owns the active SubscriptionSource, and never clears the other source's token: the Stripe webhook cancelling a web subscription now preserves an active Play subscription, and even in the linked-token window it keeps PlayPurchaseToken so the next RTDN self-heals. Routes the Stripe webhook to CancelStripeSubscription and the Play RTDN inactive path to CancelPlaySubscription. Refs thomasluizon/orbit-ui-mobile#132 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Follow-up in 48c3c8e — closed the dual-pay edge I flagged on the prior fix. Source-aware cancellation.
Net effect: a web→Android migrator who cancels Stripe while Play is active no longer drops to free. Domain 350/350 (2 tests renamed + 3 added for the dual-pay matrix), all Subscriptions handler/query tests 77/77, build clean. |
There was a problem hiding this comment.
The source-aware cancellation commit (48c3c8e) correctly resolves the dual-pay edge case. CancelSubscription() is split symmetrically into CancelStripeSubscription() / CancelPlaySubscription(): each clears its own provider's token unconditionally, and delegates to the private ClearEntitlement() only when it owns the active SubscriptionSource. The private helper clears the shared entitlement fields (Plan, PlanExpiresAt, SubscriptionInterval, SubscriptionSource) but intentionally never touches provider tokens, which is exactly right — it means a linked Play token survives a Stripe cancellation for later RTDN self-healing, and vice versa. All five dual-pay matrix scenarios are covered by domain tests. Good to merge.
Code Review: Native Google Play Billing — orbit-api #194 + orbit-ui-mobile #147Scope: both PRs (one cross-repo feature). Focus: best-practice implementation + security (this is a payments/entitlement surface). Severity summary
CriticalC1 — A purchase token is never bound to the calling user → "buy once, share the token, get Pro on unlimited accounts"Files:
Contrast the Stripe path: Exploit: any authenticated user who obtains any active Fix (defense in depth, do both):
HighH1 — No DB uniqueness on
|
| Check | Result |
|---|---|
orbit-api dotnet build |
PASS |
| orbit-api Domain.Tests | PASS (350) |
| orbit-api Subscriptions tests | PASS (77) |
| mobile type-check + lint + vitest | PASS (9 hook tests; 398 total this session) |
| shared type-check + vitest | PASS (852) |
| Parity / i18n / contract | PASS (mirrored; allowed adapter divergence) |
Recommendation
Land C1 + H1 together (token→user binding + unique index) before relying on Play revenue — they're the same attack from the app and DB angles, and now (zero existing Play purchases) is the cheapest time to add obfuscatedAccountId binding. Then make RTDN fail-closed (M1) and add message-id idempotency (M2). M3 + the Low items are follow-ups. The mobile PR (#147) is approvable as-is; its only items are minor UX polish.
…closed auth Security hardening from the review of #194 (paywall integrity + RTDN): - Purchase-token binding (C1/H1): the mobile client stamps the buyer's user id as Play's obfuscatedAccountId; VerifyAsync surfaces it and the verify handler rejects a token whose account id does not match the JWT caller (PLAY_PURCHASE_ACCOUNT_MISMATCH). A unique filtered index on Users.PlayPurchaseToken makes one-token-one-account atomic. Together these close 'buy once, share the token, get Pro on many accounts.' - RTDN fail-closed (M1): /play/rtdn always requires a valid Google OIDC push token (no RtdnAudience-empty bypass), and startup validation throws when the GooglePlay section is absent instead of booting unauthenticated. - RTDN idempotency (M2): a ProcessedPlayNotification dedup table + Pub/Sub messageId short-circuit make at-least-once redelivery safe. Refs thomasluizon/orbit-ui-mobile#132 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Full hardening implemented in bda6b24 — all four selected findings addressed. C1 + H1 — token bound to the buyer (paywall fix). The mobile client now stamps the signed-in user's id as Play's M1 — RTDN fail-closed. M2 — RTDN idempotency. New Validation: build clean · Domain 350 · Subscriptions 79 (incl. new account-mismatch + dedup tests) · Play integration 2/2 (migration applies + binding grants end-to-end). Paired mobile change: thomasluizon/orbit-ui-mobile#147 (f996942). Not in this pass (non-blocking follow-ups): M3 (5xx→400 mapping), L1–L4. |
There was a problem hiding this comment.
The final commit (bda6b24) addresses every security and correctness concern from the previous review cycle cleanly. The RTDN endpoint is now fully fail-closed: the !string.IsNullOrEmpty bypass is gone, startup validation can no longer skip on a null config object, and the OIDC check is always enforced. The ObfuscatedAccountId field is propagated from the Play API all the way to the verify handler, where it is compared against the authenticated user's GUID before any grant — correctly handling the unparseable-ID case as a hard mismatch. RTDN idempotency is backed by a ProcessedPlayNotifications table with a unique index on MessageId, written atomically with the user-state change in a single SaveChangesAsync so there is no window for partial state. All handler paths (grant, cancel, linked-token, duplicate, verify-throws, account-mismatch) are covered by unit tests; the integration test now threads the real _userId into the stub state so the ownership check passes end-to-end. Good to merge.
…on dedup Security review follow-up on #194: - Paywall integrity: VerifyAsync's IsActive only meant "an active subscription exists on the package" -- the configured ProductId and a recognized base plan were never checked, so any active subscription under the package (or one with a null/unrecognized base plan) granted full Pro. Gate both the verify handler and the RTDN handler on a shared PlaySubscriptionState.GrantsOrbitPro rule (active AND the configured product AND a monthly/yearly base plan). - RTDN idempotency: the AnyAsync-then-AddAsync dedup raced under concurrent redelivery, surfacing the loser as an unhandled 500. Catch the unique-index DbUpdateException and treat a confirmed-already-recorded messageId as success. Refs thomasluizon/orbit-ui-mobile#132 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Security re-review (pass #2) — fixes pushed in
|
There was a problem hiding this comment.
The final commit (5f98924) rounds out the hardening cycle cleanly. GrantsOrbitPro consolidates the entitlement rule into one testable place — IsActive && Interval is not null && ProductId == settings.ProductId — so neither the verify handler nor the RTDN handler can grant Pro for an unrecognized base plan or a different product under the same package, even with a valid Google-issued token. The race-safe dedup fix is also correct: the DbUpdateException catch is scoped to !string.IsNullOrEmpty(decoded.MessageId), then re-confirms the messageId is recorded before swallowing — a different DB error still surfaces as an exception, and a legitimate concurrent duplicate correctly returns success because the winning concurrent request already applied the state change. All five commits together ship a complete, defense-in-depth Play Billing integration: server-side verify + acknowledge, RTDN fail-closed with OIDC, token→buyer binding at both app and DB layers, source-aware cancellation, product/plan gating, and idempotent dedup. Good to merge.
…rd, dedup bound+prune, OIDC seam) Implements every deferred item from the security review: - Verify token TOCTOU: a unique-index violation on PlayPurchaseToken now maps to a clean PLAY_PURCHASE_ACCOUNT_MISMATCH instead of a 500 (re-checks ownership in the catch, no Npgsql leak). - RTDN defense-in-depth: skip a notification whose verified ObfuscatedAccountId does not match the resolved user, so the token->user binding is re-asserted rather than trusted. - Dedup table bounded + pruned: ProcessedPlayNotification.MessageId is now varchar(255) and a daily PlayNotificationCleanupService purges records older than 30 days (wired into the health check). - Webhook auth boundary extracted to an injectable IPlayPushTokenValidator (Google OIDC verification moved out of the controller), enabling full coverage: controller 401/200 wiring, the validator's fail-closed paths, and an end-to-end /play/rtdn integration test (envelope -> verify -> cancel). Refs thomasluizon/orbit-ui-mobile#132 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
All deferred follow-ups implemented —
|
There was a problem hiding this comment.
Play Billing integration lands cleanly. The verify + RTDN pair forms a solid, fail-closed pair: the verify flow enforces strict account binding (obfuscated GUID must match the JWT user, else the request is rejected), while the RTDN re-verifies authoritative state from Google rather than trusting the push payload. A few things I specifically checked and found correct:
- RTDN auth:
[AllowAnonymous]is intentional — the JWT middleware can't validate Google's OIDC tokens, soIPlayPushTokenValidatortakes over. The validator checks signature, audience, email, andEmailVerified, and the implementation falls closed on anyInvalidJwtException. - Idempotency race: The optimistic pre-check + atomic commit +
DbUpdateExceptioncatch on duplicateMessageIdis the right pattern. If two concurrent deliveries race past the pre-check, the unique index catches them and the losing thread returnsResult.Success()correctly. - Stripe → Play entitlement hand-off:
CancelStripeSubscriptionandCancelPlaySubscriptionare source-aware — cancelling one channel can't accidentally wipe the other.ClearEntitlementis only called when the cancelled source matches the currentSubscriptionSource. The "Stripe covers later period" shortcut in the verify handler is also correct: it links the token without downgrading the Stripe period. - Migration backfill:
WHERE "StripeSubscriptionId" IS NOT NULLcorrectly identifies currently-active Stripe users (cancelled subs null the ID viaCancelSubscription), back-filling them tosource = 0 (Stripe). The unique filtered index onPlayPurchaseTokenis the right constraint for TOCTOU protection. - API contract: Both new response fields (
SourceonSubscriptionStatusResponseandProfileResponse) are additive nullable strings — no breaking change for existing mobile/web clients.
Issue
Refs thomasluizon/orbit-ui-mobile#132 — cross-repo, paired with the orbit-ui-mobile PR. Merge this one first (mobile calls
/play/verify).Summary
Adds native Google Play Billing server support so the Android app can sell Orbit Pro through the native Play purchase sheet (Play-policy compliant), while the web app keeps Stripe. The backend remains the single source of truth for one Pro entitlement, now carrying a purchase
source(stripe|play).What's included
POST /api/subscriptions/play/verify— verifies a Play purchase via the Play Developer API (purchases.subscriptionsv2.get), grants Pro, and acknowledges server-side within Google's 3-day window. Reconciles the "has both" case (skips a downgrade when an active Stripe sub covers a later period).POST /api/subscriptions/play/rtdn— Real-time Developer Notifications webhook (Pub/Sub push, OIDC-verified whenRtdnAudienceis set). Decodes the envelope only to learn which purchase token changed, then re-verifies authoritative state (never trusts the payload) and grants/cancels accordingly.IPlayBillingServiceseam (Application) +GooglePlayBillingService(Infrastructure,Google.Apis.AndroidPublisher.v3), mirroring the StripeIBillingServicepattern.UsergainsSubscriptionSource+PlayPurchaseToken(EF migrationAddPlayBillingFields, back-fills existing Stripe subscribers tosource=stripe).PayGateis unchanged.sourceis surfaced on the profile + subscription-status DTOs.Tests
Domain (
SetPlaySubscription/CancelSubscription), verify-handler (grant/ack/has-both/inactive/error), RTDN-handler (active/inactive/linked-token/malformed/no-user/error), validator, andPlayBillingIntegrationTestsvia aCapturingPlayBillingService. All 3,100 unit tests pass; the feature's 2 integration tests pass. (10 unrelated, pre-existing integration failures in the AI + api-key suites — live-OpenAI non-determinism and a Pro-gating setup issue — documented in the report.)Deploy / manual
Set
GooglePlay__*on Render (package, service-account JSON, product/base-plan ids, RTDN audience + push SA email) and create the Pub/Sub push subscription →/api/subscriptions/play/rtdn.🤖 Generated with Claude Code