From 9a8d2a4e735414779389477c64382fc1cc8c47b3 Mon Sep 17 00:00:00 2001 From: senke Date: Fri, 17 Apr 2026 12:21:53 +0200 Subject: [PATCH] =?UTF-8?q?chore(release):=20v1.0.6.2=20=E2=80=94=20subscr?= =?UTF-8?q?iption=20payment-gate=20bypass=20hotfix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes a bypass surfaced by the 2026-04 audit probe (axis-1 Q2): any authenticated user could POST /api/v1/subscriptions/subscribe on a paid plan and receive 201 active without the payment provider ever being invoked. The resulting row satisfied `checkEligibility()` in the distribution service via `can_sell_on_marketplace=true` on the Creator plan — effectively free access to /api/v1/distribution/submit, which dispatches to external partners. Fix is centralised in `GetUserSubscription` so there is no code path that can grant subscription-gated access without routing through the payment check. Effective-payment = free plan OR unexpired trial OR invoice with non-empty hyperswitch_payment_id. Migration 980 sweeps pre-existing fantôme rows into `expired`, preserving the tuple in a dated audit table for support outreach. Subscribe and subscribeToFreePlan treat the new ErrSubscriptionNoPayment as equivalent to ErrNoActiveSubscription so re-subscription works cleanly post-cleanup. GET /me/subscription surfaces needs_payment=true with a support-contact message rather than a misleading "you're on free" or an opaque 500. TODO(v1.0.7-item-G) annotation marks where the `if s.paymentProvider != nil` short-circuit needs to become a mandatory pending_payment state. Probe script `scripts/probes/subscription-unpaid-activation.sh` kept as a versioned regression test — dry-run by default, --destructive logs in and attempts the exploit against a live backend with automatic cleanup. 8-case unit test matrix covers the full hasEffectivePayment predicate. Smoke validated end-to-end against local v1.0.6.2: POST /subscribe returns 201 (by design — item G closes the creation path), but GET /me/subscription returns subscription=null + needs_payment=true, distribution eligibility returns false. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 59 ++++++ VERSION | 2 +- .../probes/subscription-unpaid-activation.sh | 189 ++++++++++++++++++ .../internal/core/distribution/service.go | 5 +- .../internal/core/subscription/gate_test.go | 169 ++++++++++++++++ .../internal/core/subscription/service.go | 63 +++++- .../internal/handlers/subscription_handler.go | 21 +- .../980_void_unpaid_subscriptions.sql | 60 ++++++ .../980_void_unpaid_subscriptions_down.sql | 13 ++ 9 files changed, 571 insertions(+), 10 deletions(-) create mode 100755 scripts/probes/subscription-unpaid-activation.sh create mode 100644 veza-backend-api/internal/core/subscription/gate_test.go create mode 100644 veza-backend-api/migrations/980_void_unpaid_subscriptions.sql create mode 100644 veza-backend-api/migrations/rollback/980_void_unpaid_subscriptions_down.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 04df11cfc..53d849bce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,64 @@ # Changelog - Veza +## [v1.0.6.2] - 2026-04-17 + +### Hotfix — subscription payment-gate bypass + +Discovered during the 2026-04 audit probe (ops question Q2, "are paid +subscriptions actually gated server-side?"). An authenticated user could +POST `/api/v1/subscriptions/subscribe` with a paid plan and receive +HTTP 201 with `status=active` — with the payment provider never +invoked when `HYPERSWITCH_ENABLED=false` (or unset). The resulting +row satisfied `checkEligibility()` in the distribution service, which +returns `sub.Plan.HasDistribution || sub.Plan.CanSellOnMarketplace`. +The Creator plan carries `can_sell_on_marketplace=true`, so any user +could reach `/api/v1/distribution/submit` — a paid feature that +dispatches to external distribution partners — without paying. + +Fix — `GetUserSubscription` now filters out active/trialing rows that +lack an effective payment linkage. "Effective" means: on a free plan, +or in an unexpired trial, or at least one attached invoice carries a +PSP payment intent (`hyperswitch_payment_id` non-empty). This is the +sole centralised gate; all paid-feature eligibility paths (distribution +and anything added later) route through it. + + * `ErrSubscriptionNoPayment` added to `internal/core/subscription`. + `GetUserSubscription` returns it when a row sits in active/trialing + but fails the payment-effective predicate. Callers treat it as + ineligible (distribution returns `false, nil`; subscription HTTP + handlers return 404 "Active subscription" for cancel/reactivate/ + billing-cycle paths; `GET /me/subscription` returns an explicit + `needs_payment=true` payload so honest-path users who landed here + via a broken flow get actionable information, not a misleading + "you're on free" or an opaque 500). + * `Subscribe` and `subscribeToFreePlan` also treat the new error as + "no existing active subscription" so a user can re-subscribe + cleanly once migration 980 has voided their fantôme row. + * Migration `980_void_unpaid_subscriptions.sql` sweeps all + pre-v1.0.6.2 fantôme rows into `status='expired'`, capturing the + `(subscription_id, user_id, plan_id, previous_status)` tuple in a + dated audit table (`voided_subscriptions_20260417`) so support can + notify any honest-path user who landed there by mistake. + * Probe script `scripts/probes/subscription-unpaid-activation.sh` + kept as a versioned regression test. `--dry-run` lists plans; + `--destructive` logs in and attempts the exploit, cleaning up + after itself. Exit 0 = no bypass; exit 1 = bypass detected. + * Unit test `gate_test.go` covers the 8-branch matrix of the + `hasEffectivePayment` predicate (free pass, paid with/without + invoice, paid with empty vs populated `hyperswitch_payment_id`, + trial variants with future/past/nil `trial_end`, no row at all). + * `TODO(v1.0.7-item-G)` annotation on the `if s.paymentProvider != + nil` short-circuit in `createNewSubscription` so the v1.0.7 work + that replaces it with a mandatory `pending_payment` state retains + the audit trail. + +### Security + +Closes a subscription-gate bypass affecting distribution eligibility. +Internal audit finding; no external report. Axis-1 correctness item +P1.7 will be reclassified to P0 and item G added to the v1.0.7 plan +in a follow-up commit. + ## [v1.0.6.1] - 2026-04-17 ### Hotfix — partial UNIQUE on refunds.hyperswitch_refund_id diff --git a/VERSION b/VERSION index 91ddda75e..96943aeff 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.0.6.1 +1.0.6.2 diff --git a/scripts/probes/subscription-unpaid-activation.sh b/scripts/probes/subscription-unpaid-activation.sh new file mode 100755 index 000000000..e7eb10657 --- /dev/null +++ b/scripts/probes/subscription-unpaid-activation.sh @@ -0,0 +1,189 @@ +#!/usr/bin/env bash +# ----------------------------------------------------------------------------- +# scripts/probes/subscription-unpaid-activation.sh +# +# Regression probe for the v1.0.6.2 subscription payment-gate hotfix. +# +# Before v1.0.6.2, POST /api/v1/subscriptions/subscribe on a paid plan would +# return HTTP 201 with a user_subscription row in 'active' status, WITHOUT +# the payment provider ever being invoked. This enabled any authenticated +# user to satisfy the distribution-eligibility gate (checkEligibility()) for +# free. v1.0.6.2 filters these rows out of GetUserSubscription and voids the +# pre-existing ones via migration 980. +# +# This probe verifies that the bypass is closed. It must return REFUSED +# after v1.0.6.2 is deployed. Kept as a versioned regression test against +# any refactor of the subscription flow. +# +# Modes: +# --dry-run (default) Read-only. Lists plans + describes what the +# destructive probe would do. No rows written. +# --destructive Actually logs in, POSTs /subscribe, observes the +# outcome, then rolls back (deletes the sub row and +# invoice rows it created, if any). Requires a real +# running backend and a test account. +# +# Exit codes: +# 0 Probe did not reveal a bypass (expected post-v1.0.6.2). +# 1 Probe revealed the bypass (active sub + paid plan + no PSP linkage). +# 2 Probe could not run (missing deps, auth failed, backend unreachable). +# +# Env overrides: +# API_BASE_URL default http://localhost:8080 +# PROBE_EMAIL default smoke.0416@veza.local +# PROBE_PASSWORD default Str0ng!Probe#2026 +# DB_CONTAINER default veza_postgres (for --destructive rollback) +# DB_USER default veza +# DB_NAME default veza +# ----------------------------------------------------------------------------- + +set -euo pipefail + +API_BASE_URL="${API_BASE_URL:-http://localhost:8080}" +PROBE_EMAIL="${PROBE_EMAIL:-smoke.0416@veza.local}" +PROBE_PASSWORD="${PROBE_PASSWORD:-Str0ng!Probe#2026}" +DB_CONTAINER="${DB_CONTAINER:-veza_postgres}" +DB_USER="${DB_USER:-veza}" +DB_NAME="${DB_NAME:-veza}" + +MODE="dry-run" +for arg in "$@"; do + case "$arg" in + --destructive) MODE="destructive" ;; + --dry-run) MODE="dry-run" ;; + -h|--help) + sed -n '2,40p' "$0" + exit 0 + ;; + *) echo "unknown arg: $arg" >&2 ; exit 2 ;; + esac +done + +log() { printf '[probe] %s\n' "$*"; } + +need() { + command -v "$1" >/dev/null 2>&1 || { echo "missing dep: $1" >&2 ; exit 2 ; } +} + +need curl +need jq + +# ----------------------------------------------------------------------------- +# Phase 0 — reachability (both modes) +# ----------------------------------------------------------------------------- +log "target: $API_BASE_URL" +if ! curl -sf --max-time 3 "$API_BASE_URL/health" > /dev/null 2>&1; then + log "backend unreachable — start it first" + exit 2 +fi +log "backend reachable" + +PLANS_JSON=$(curl -sf "$API_BASE_URL/api/v1/subscriptions/plans") +# Response wraps in {success, data:{plans:[...]}} (v1.0.6.2) or {data:[...]} fallback. +CREATOR_ID=$(jq -r '(.data.plans // .data // [])[] | select(.name == "creator") | .id' <<<"$PLANS_JSON" || echo "") +if [ -z "$CREATOR_ID" ] || [ "$CREATOR_ID" = "null" ]; then + log "creator plan not found in /plans response" + exit 2 +fi +log "creator plan id: $CREATOR_ID" + +if [ "$MODE" = "dry-run" ]; then + log "dry-run — would now: login as $PROBE_EMAIL, POST /subscribe with plan_id=$CREATOR_ID." + log "to run the exploit attempt: $0 --destructive" + exit 0 +fi + +# ----------------------------------------------------------------------------- +# Phase 1 — authenticate (destructive only) +# ----------------------------------------------------------------------------- +COOKIE_JAR=$(mktemp) +trap 'rm -f "$COOKIE_JAR"' EXIT + +LOGIN_STATUS=$(curl -s -o /tmp/probe_login.json -w '%{http_code}' \ + -X POST "$API_BASE_URL/api/v1/auth/login" \ + -H 'Content-Type: application/json' \ + -c "$COOKIE_JAR" \ + -d "{\"email\":\"$PROBE_EMAIL\",\"password\":\"$PROBE_PASSWORD\"}") + +if [ "$LOGIN_STATUS" != "200" ]; then + log "login failed with HTTP $LOGIN_STATUS — probe account may not exist" + cat /tmp/probe_login.json >&2 + exit 2 +fi +log "login OK" + +CSRF_STATUS=$(curl -s -o /tmp/probe_csrf.json -w '%{http_code}' \ + "$API_BASE_URL/api/v1/csrf-token" \ + -b "$COOKIE_JAR" -c "$COOKIE_JAR") +if [ "$CSRF_STATUS" != "200" ]; then + log "csrf fetch failed with HTTP $CSRF_STATUS" + exit 2 +fi +CSRF=$(jq -r '.data.csrf_token // .csrf_token // empty' /tmp/probe_csrf.json) +if [ -z "$CSRF" ]; then + log "csrf token not found in /auth/csrf response" + exit 2 +fi + +# ----------------------------------------------------------------------------- +# Phase 2 — attempt the exploit +# +# v1.0.6.2 design note: POST /subscribe still returns 201 when the payment +# provider is unreachable (the creation path itself is item G in v1.0.7). +# What v1.0.6.2 closes is the *gate* — `GetUserSubscription` must now +# filter the fantôme row and surface `needs_payment: true`. So the probe +# tests the observable consequence: after subscribing, does the server +# still grant access? If GET /me/subscription returns a valid subscription +# object for this user, the gate failed and the bypass is open. +# ----------------------------------------------------------------------------- +log "POST /subscribe { plan_id: creator, billing_cycle: monthly }" +SUBSCRIBE_STATUS=$(curl -s -o /tmp/probe_sub.json -w '%{http_code}' \ + -X POST "$API_BASE_URL/api/v1/subscriptions/subscribe" \ + -H 'Content-Type: application/json' \ + -H "X-CSRF-Token: $CSRF" \ + -b "$COOKIE_JAR" \ + -d "{\"plan_id\":\"$CREATOR_ID\",\"billing_cycle\":\"monthly\"}") + +SUB_ID=$(jq -r '.data.subscription.id // .subscription.id // empty' /tmp/probe_sub.json) +SUB_STATUS=$(jq -r '.data.subscription.status // .subscription.status // empty' /tmp/probe_sub.json) + +log "subscribe: HTTP $SUBSCRIBE_STATUS row_status=$SUB_STATUS row_id=$SUB_ID" + +log "GET /api/v1/subscriptions/me — testing whether the gate grants access" +curl -s -o /tmp/probe_me.json \ + "$API_BASE_URL/api/v1/subscriptions/me" \ + -b "$COOKIE_JAR" + +ME_SUB=$(jq -r '.data.subscription // .subscription' /tmp/probe_me.json) +ME_NEEDS_PAYMENT=$(jq -r '.data.needs_payment // .needs_payment // false' /tmp/probe_me.json) + +log "me: subscription=$ME_SUB needs_payment=$ME_NEEDS_PAYMENT" + +BYPASS_DETECTED=0 +# Bypass is open when /me/subscription returns a valid subscription object +# (non-null) to a user whose /subscribe path never reached the PSP. Under +# the v1.0.6.2 filter, the row is surfaced as null + needs_payment=true. +if [ "$ME_SUB" != "null" ] && [ -n "$ME_SUB" ] && [ "$ME_NEEDS_PAYMENT" != "true" ]; then + BYPASS_DETECTED=1 +fi + +# Roll back any row we created so the probe is idempotent. +if [ -n "$SUB_ID" ] && [ "$SUB_ID" != "null" ]; then + if command -v docker >/dev/null 2>&1 && docker ps --format '{{.Names}}' | grep -qx "$DB_CONTAINER"; then + log "rolling back subscription row $SUB_ID" + docker exec "$DB_CONTAINER" psql -U "$DB_USER" -d "$DB_NAME" -q \ + -c "DELETE FROM subscription_invoices WHERE subscription_id='$SUB_ID';" \ + -c "DELETE FROM user_subscriptions WHERE id='$SUB_ID';" \ + >/dev/null 2>&1 || log "rollback encountered an error (non-fatal)" + else + log "db container not accessible — manual cleanup may be needed for row $SUB_ID" + fi +fi + +if [ "$BYPASS_DETECTED" = "1" ]; then + log "BYPASS DETECTED — paid subscription created without PSP linkage" + exit 1 +fi + +log "REFUSED as expected (no bypass)" +exit 0 diff --git a/veza-backend-api/internal/core/distribution/service.go b/veza-backend-api/internal/core/distribution/service.go index 047ac91b3..505a85c7a 100644 --- a/veza-backend-api/internal/core/distribution/service.go +++ b/veza-backend-api/internal/core/distribution/service.go @@ -244,7 +244,10 @@ func (s *Service) checkEligibility(ctx context.Context, userID uuid.UUID) (bool, sub, err := s.subscriptionService.GetUserSubscription(ctx, userID) if err != nil { - if errors.Is(err, subscription.ErrNoActiveSubscription) { + // v1.0.6.2: ErrSubscriptionNoPayment means a row exists in + // active/trialing but has no effective payment linkage. Treat as + // ineligible, same blast radius as no subscription at all. + if errors.Is(err, subscription.ErrNoActiveSubscription) || errors.Is(err, subscription.ErrSubscriptionNoPayment) { return false, nil } return false, err diff --git a/veza-backend-api/internal/core/subscription/gate_test.go b/veza-backend-api/internal/core/subscription/gate_test.go new file mode 100644 index 000000000..aaede8269 --- /dev/null +++ b/veza-backend-api/internal/core/subscription/gate_test.go @@ -0,0 +1,169 @@ +package subscription + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "gorm.io/driver/sqlite" + "gorm.io/gorm" +) + +// TestGetUserSubscription_PaymentGate exercises the v1.0.6.2 hotfix: a row in +// active/trialing state that lacks an effective payment linkage must not be +// returned as a valid subscription. A single test covers the full branch +// matrix of hasEffectivePayment so a regression in any clause is caught. +func TestGetUserSubscription_PaymentGate(t *testing.T) { + ctx := context.Background() + + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate(&Plan{}, &UserSubscription{}, &Invoice{})) + + svc := NewService(db, zap.NewNop()) + + freePlan := Plan{ID: uuid.New(), Name: PlanFree, DisplayName: "Free", PriceMonthly: 0, IsActive: true} + paidPlan := Plan{ID: uuid.New(), Name: PlanCreator, DisplayName: "Creator", PriceMonthly: 999, IsActive: true} + require.NoError(t, db.Create(&freePlan).Error) + require.NoError(t, db.Create(&paidPlan).Error) + + now := time.Now() + future := now.Add(24 * time.Hour) + past := now.Add(-24 * time.Hour) + + tests := []struct { + name string + prepare func(t *testing.T, userID uuid.UUID) // sets up the row(s) for this user + expectErr error + }{ + { + name: "no subscription row returns ErrNoActiveSubscription", + prepare: func(t *testing.T, userID uuid.UUID) { + // no-op + }, + expectErr: ErrNoActiveSubscription, + }, + { + name: "free plan active always passes", + prepare: func(t *testing.T, userID uuid.UUID) { + sub := UserSubscription{ + UserID: userID, PlanID: freePlan.ID, Status: StatusActive, + BillingCycle: BillingMonthly, + CurrentPeriodStart: now, CurrentPeriodEnd: future, + } + require.NoError(t, db.Create(&sub).Error) + }, + expectErr: nil, + }, + { + name: "paid plan active with PSP payment intent on invoice passes", + prepare: func(t *testing.T, userID uuid.UUID) { + sub := UserSubscription{ + UserID: userID, PlanID: paidPlan.ID, Status: StatusActive, + BillingCycle: BillingMonthly, + CurrentPeriodStart: now, CurrentPeriodEnd: future, + } + require.NoError(t, db.Create(&sub).Error) + inv := Invoice{ + SubscriptionID: sub.ID, UserID: userID, AmountCents: 999, + Currency: "USD", Status: InvoicePending, + BillingPeriodStart: now, BillingPeriodEnd: future, + HyperswitchPaymentID: "pay_12345", + } + require.NoError(t, db.Create(&inv).Error) + }, + expectErr: nil, + }, + { + name: "paid plan active with invoice but empty hs_payment_id returns ErrSubscriptionNoPayment", + prepare: func(t *testing.T, userID uuid.UUID) { + sub := UserSubscription{ + UserID: userID, PlanID: paidPlan.ID, Status: StatusActive, + BillingCycle: BillingMonthly, + CurrentPeriodStart: now, CurrentPeriodEnd: future, + } + require.NoError(t, db.Create(&sub).Error) + inv := Invoice{ + SubscriptionID: sub.ID, UserID: userID, AmountCents: 999, + Currency: "USD", Status: InvoicePending, + BillingPeriodStart: now, BillingPeriodEnd: future, + HyperswitchPaymentID: "", + } + require.NoError(t, db.Create(&inv).Error) + }, + expectErr: ErrSubscriptionNoPayment, + }, + { + name: "paid plan active with no invoice at all returns ErrSubscriptionNoPayment", + prepare: func(t *testing.T, userID uuid.UUID) { + sub := UserSubscription{ + UserID: userID, PlanID: paidPlan.ID, Status: StatusActive, + BillingCycle: BillingMonthly, + CurrentPeriodStart: now, CurrentPeriodEnd: future, + } + require.NoError(t, db.Create(&sub).Error) + }, + expectErr: ErrSubscriptionNoPayment, + }, + { + name: "paid plan trialing with future trial_end passes", + prepare: func(t *testing.T, userID uuid.UUID) { + sub := UserSubscription{ + UserID: userID, PlanID: paidPlan.ID, Status: StatusTrialing, + BillingCycle: BillingMonthly, + CurrentPeriodStart: now, CurrentPeriodEnd: future, + TrialStart: &now, TrialEnd: &future, + } + require.NoError(t, db.Create(&sub).Error) + }, + expectErr: nil, + }, + { + name: "paid plan trialing with past trial_end and no payment returns ErrSubscriptionNoPayment", + prepare: func(t *testing.T, userID uuid.UUID) { + sub := UserSubscription{ + UserID: userID, PlanID: paidPlan.ID, Status: StatusTrialing, + BillingCycle: BillingMonthly, + CurrentPeriodStart: now, CurrentPeriodEnd: future, + TrialStart: &past, TrialEnd: &past, + } + require.NoError(t, db.Create(&sub).Error) + }, + expectErr: ErrSubscriptionNoPayment, + }, + { + name: "paid plan trialing with nil trial_end and no payment returns ErrSubscriptionNoPayment", + prepare: func(t *testing.T, userID uuid.UUID) { + sub := UserSubscription{ + UserID: userID, PlanID: paidPlan.ID, Status: StatusTrialing, + BillingCycle: BillingMonthly, + CurrentPeriodStart: now, CurrentPeriodEnd: future, + } + require.NoError(t, db.Create(&sub).Error) + }, + expectErr: ErrSubscriptionNoPayment, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + userID := uuid.New() + tc.prepare(t, userID) + + sub, err := svc.GetUserSubscription(ctx, userID) + if tc.expectErr == nil { + require.NoError(t, err) + require.NotNil(t, sub) + require.Equal(t, userID, sub.UserID) + return + } + require.Error(t, err) + require.True(t, errors.Is(err, tc.expectErr), + "expected error %v, got %v", tc.expectErr, err) + }) + } +} diff --git a/veza-backend-api/internal/core/subscription/service.go b/veza-backend-api/internal/core/subscription/service.go index 62eaf7feb..487caf2fe 100644 --- a/veza-backend-api/internal/core/subscription/service.go +++ b/veza-backend-api/internal/core/subscription/service.go @@ -20,6 +20,15 @@ var ( ErrNoActiveSubscription = errors.New("no active subscription found") ErrInvalidBillingCycle = errors.New("invalid billing cycle: must be 'monthly' or 'yearly'") ErrFreePlanNoBilling = errors.New("free plan does not require billing") + // ErrSubscriptionNoPayment: a subscription row exists in active/trialing but no + // effective payment signal is attached (no PSP payment intent on any invoice, and + // neither free plan nor active trial). Introduced in v1.0.6.2 to close a bypass + // where rows could be created as 'active' without the payment provider ever + // being invoked (e.g., HYPERSWITCH_ENABLED=false). Callers that gate features + // by subscription should treat this as ineligible. The /me/subscription + // handler surfaces a specific message so honest-path users know to contact + // support. + ErrSubscriptionNoPayment = errors.New("subscription has no effective payment linkage") ) // PaymentProvider defines the interface for subscription payments @@ -94,6 +103,12 @@ func (s *Service) GetPlanByName(ctx context.Context, name PlanName) (*Plan, erro } // GetUserSubscription returns the user's current active/trialing subscription +// IFF it has an effective payment linkage. A row that sits in active/trialing +// but was never linked to a PSP payment intent (and is not on a free plan or +// in an unexpired trial) returns ErrSubscriptionNoPayment. This is the sole +// gate for feature eligibility — any caller routing to a paid-feature check +// goes through here, by design, so there is no code path that can grant +// access to a subscription that never paid. See v1.0.6.2 hotfix. func (s *Service) GetUserSubscription(ctx context.Context, userID uuid.UUID) (*UserSubscription, error) { var sub UserSubscription err := s.db.WithContext(ctx). @@ -106,9 +121,33 @@ func (s *Service) GetUserSubscription(ctx context.Context, userID uuid.UUID) (*U } return nil, fmt.Errorf("failed to get user subscription: %w", err) } + if !s.hasEffectivePayment(ctx, &sub) { + return nil, ErrSubscriptionNoPayment + } return &sub, nil } +// hasEffectivePayment returns true if the subscription represents a legitimate +// claim on paid features. True when: the plan is free, OR the subscription is +// in an unexpired trial, OR at least one invoice carries a PSP payment intent +// (hyperswitch_payment_id populated). The last branch raises the bar from +// "anyone can POST /subscribe" to "someone must have actually reached the +// PSP". It does not prove the charge succeeded — item G in v1.0.7 tightens +// this further by requiring invoice.status='paid' via webhook. +func (s *Service) hasEffectivePayment(ctx context.Context, sub *UserSubscription) bool { + if sub.Plan.PriceMonthly == 0 { + return true + } + if sub.Status == StatusTrialing && sub.TrialEnd != nil && time.Now().Before(*sub.TrialEnd) { + return true + } + var count int64 + s.db.WithContext(ctx).Model(&Invoice{}). + Where("subscription_id = ? AND hyperswitch_payment_id IS NOT NULL AND hyperswitch_payment_id <> ''", sub.ID). + Count(&count) + return count > 0 +} + // GetUserSubscriptionHistory returns all subscriptions for a user (including canceled/expired) func (s *Service) GetUserSubscriptionHistory(ctx context.Context, userID uuid.UUID, limit, offset int) ([]UserSubscription, error) { if limit <= 0 || limit > 100 { @@ -160,11 +199,18 @@ func (s *Service) Subscribe(ctx context.Context, userID uuid.UUID, req Subscribe return s.subscribeToFreePlan(ctx, userID, plan) } - // Check for existing active subscription + // Check for existing active subscription. A row in active/trialing without + // effective payment (ErrSubscriptionNoPayment, v1.0.6.2) is treated as + // "no active subscription" for the purpose of re-subscribing — the + // cleanup migration 980 voids those rows at deploy time, so in practice + // this branch is only hit during the brief deploy window. existing, err := s.GetUserSubscription(ctx, userID) - if err != nil && !errors.Is(err, ErrNoActiveSubscription) { + if err != nil && !errors.Is(err, ErrNoActiveSubscription) && !errors.Is(err, ErrSubscriptionNoPayment) { return nil, err } + if errors.Is(err, ErrSubscriptionNoPayment) { + existing = nil + } if existing != nil && existing.PlanID == req.PlanID { return nil, ErrAlreadySubscribed @@ -180,9 +226,11 @@ func (s *Service) Subscribe(ctx context.Context, userID uuid.UUID, req Subscribe // subscribeToFreePlan assigns the free plan without payment func (s *Service) subscribeToFreePlan(ctx context.Context, userID uuid.UUID, plan *Plan) (*SubscribeResponse, error) { - // Cancel any existing subscription first + // Cancel any existing subscription first. A no-payment fantôme row + // (v1.0.6.2 filter) is treated as "nothing to cancel" — the cleanup + // migration handles it at deploy time. existing, err := s.GetUserSubscription(ctx, userID) - if err != nil && !errors.Is(err, ErrNoActiveSubscription) { + if err != nil && !errors.Is(err, ErrNoActiveSubscription) && !errors.Is(err, ErrSubscriptionNoPayment) { return nil, err } if existing != nil { @@ -275,7 +323,12 @@ func (s *Service) createNewSubscription(ctx context.Context, userID uuid.UUID, p return fmt.Errorf("failed to create invoice: %w", err) } - // Initiate payment if provider is configured + // TODO(v1.0.7-item-G): make payment provider mandatory for paid plans. + // Today `if s.paymentProvider != nil` short-circuits silently when + // Hyperswitch is disabled, leaving the row `active` with no PSP + // linkage. Item G replaces this with a mandatory pending_payment + // state + webhook-driven activation. Until then, v1.0.6.2 + // compensates via the `GetUserSubscription` filter. if s.paymentProvider != nil { var err error paymentID, clientSecret, err = s.paymentProvider.CreateSubscriptionPayment( diff --git a/veza-backend-api/internal/handlers/subscription_handler.go b/veza-backend-api/internal/handlers/subscription_handler.go index ebe189938..12b19e7a9 100644 --- a/veza-backend-api/internal/handlers/subscription_handler.go +++ b/veza-backend-api/internal/handlers/subscription_handler.go @@ -70,6 +70,19 @@ func (h *SubscriptionHandler) GetMySubscription(c *gin.Context) { RespondSuccess(c, http.StatusOK, gin.H{"subscription": nil, "plan": "free"}) return } + // v1.0.6.2: a subscription row exists but has no payment linkage. + // Surface a specific payload so honest-path users who landed here + // via a broken flow (payment never completed) get a clear message + // rather than "you're on free" (misleading) or a 500. + if errors.Is(err, subscription.ErrSubscriptionNoPayment) { + RespondSuccess(c, http.StatusOK, gin.H{ + "subscription": nil, + "plan": "free", + "needs_payment": true, + "message": "Your subscription is not linked to a payment. Please contact support to resolve.", + }) + return + } RespondWithAppError(c, apperrors.NewInternalErrorWrap("Failed to get subscription", err)) return } @@ -117,7 +130,8 @@ func (h *SubscriptionHandler) CancelSubscription(c *gin.Context) { sub, err := h.service.CancelSubscription(c.Request.Context(), userID) if err != nil { switch { - case errors.Is(err, subscription.ErrNoActiveSubscription): + case errors.Is(err, subscription.ErrNoActiveSubscription), + errors.Is(err, subscription.ErrSubscriptionNoPayment): RespondWithAppError(c, apperrors.NewNotFoundError("Active subscription")) case errors.Is(err, subscription.ErrFreePlanNoBilling): RespondWithAppError(c, apperrors.NewValidationError("Free plan cannot be canceled")) @@ -142,7 +156,7 @@ func (h *SubscriptionHandler) ReactivateSubscription(c *gin.Context) { sub, err := h.service.ReactivateSubscription(c.Request.Context(), userID) if err != nil { - if errors.Is(err, subscription.ErrNoActiveSubscription) { + if errors.Is(err, subscription.ErrNoActiveSubscription) || errors.Is(err, subscription.ErrSubscriptionNoPayment) { RespondWithAppError(c, apperrors.NewNotFoundError("Active subscription")) return } @@ -171,7 +185,8 @@ func (h *SubscriptionHandler) ChangeBillingCycle(c *gin.Context) { sub, err := h.service.ChangeBillingCycle(c.Request.Context(), userID, req.BillingCycle) if err != nil { switch { - case errors.Is(err, subscription.ErrNoActiveSubscription): + case errors.Is(err, subscription.ErrNoActiveSubscription), + errors.Is(err, subscription.ErrSubscriptionNoPayment): RespondWithAppError(c, apperrors.NewNotFoundError("Active subscription")) case errors.Is(err, subscription.ErrInvalidBillingCycle): RespondWithAppError(c, apperrors.NewValidationError("Invalid billing cycle: must be 'monthly' or 'yearly'")) diff --git a/veza-backend-api/migrations/980_void_unpaid_subscriptions.sql b/veza-backend-api/migrations/980_void_unpaid_subscriptions.sql new file mode 100644 index 000000000..ea8986919 --- /dev/null +++ b/veza-backend-api/migrations/980_void_unpaid_subscriptions.sql @@ -0,0 +1,60 @@ +-- v1.0.6.2: Void subscription rows that sit in active/trialing state without +-- any effective payment linkage. Introduced to compensate for a bypass where +-- POST /subscribe could create 'active' rows on paid plans without invoking +-- the payment provider (e.g., when HYPERSWITCH_ENABLED=false or provider +-- unset). The runtime filter in GetUserSubscription (service.go) closes the +-- feature bypass going forward; this migration cleans up the rows already +-- written to the database pre-v1.0.6.2. +-- +-- Fantôme selection criteria: +-- 1. status IN ('active', 'trialing') +-- 2. plan is paid (subscription_plans.price_monthly_cents > 0) +-- 3. no invoice attached carries a hyperswitch_payment_id (= PSP never reached) +-- 4. not a currently-valid trial (trial_end > NOW()) +-- +-- Audit table is dated so a future rerun doesn't collide. Rows here can be +-- used to notify affected users (if any were honest-path). + +CREATE TABLE IF NOT EXISTS voided_subscriptions_20260417 ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + subscription_id UUID NOT NULL, + user_id UUID NOT NULL, + plan_id UUID NOT NULL, + previous_status VARCHAR(30) NOT NULL, + voided_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +CREATE INDEX IF NOT EXISTS idx_voided_subscriptions_20260417_user + ON voided_subscriptions_20260417(user_id); + +INSERT INTO voided_subscriptions_20260417 (subscription_id, user_id, plan_id, previous_status) +SELECT us.id, us.user_id, us.plan_id, us.status +FROM user_subscriptions us +JOIN subscription_plans sp ON sp.id = us.plan_id +WHERE us.status IN ('active', 'trialing') + AND sp.price_monthly_cents > 0 + AND NOT EXISTS ( + SELECT 1 + FROM subscription_invoices si + WHERE si.subscription_id = us.id + AND si.hyperswitch_payment_id IS NOT NULL + AND si.hyperswitch_payment_id <> '' + ) + AND NOT ( + us.status = 'trialing' + AND us.trial_end IS NOT NULL + AND us.trial_end > NOW() + ); + +UPDATE user_subscriptions + SET status = 'expired', + canceled_at = COALESCE(canceled_at, NOW()), + updated_at = NOW() + WHERE id IN (SELECT subscription_id FROM voided_subscriptions_20260417); + +DO $$ +DECLARE v_count INTEGER; +BEGIN + SELECT COUNT(*) INTO v_count FROM voided_subscriptions_20260417; + RAISE NOTICE 'v1.0.6.2: voided % pre-existing unpaid subscription row(s)', v_count; +END $$; diff --git a/veza-backend-api/migrations/rollback/980_void_unpaid_subscriptions_down.sql b/veza-backend-api/migrations/rollback/980_void_unpaid_subscriptions_down.sql new file mode 100644 index 000000000..feb068acd --- /dev/null +++ b/veza-backend-api/migrations/rollback/980_void_unpaid_subscriptions_down.sql @@ -0,0 +1,13 @@ +-- Rollback for v1.0.6.2 void-unpaid cleanup. +-- Restores the previous status for every row recorded in the audit table, +-- then drops the audit table itself. + +UPDATE user_subscriptions us + SET status = vs.previous_status, + canceled_at = NULL, + updated_at = NOW() + FROM voided_subscriptions_20260417 vs + WHERE us.id = vs.subscription_id + AND us.status = 'expired'; + +DROP TABLE IF EXISTS voided_subscriptions_20260417;