diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 213e6bd19..711956812 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -681,6 +681,11 @@ continuity, but `session_store.go` and `csrf_store.go` must immediately rewrite hashed canonical persistence during load instead of leaving raw-token files on the primary runtime path until a later save side effect happens to run. +That same shared `internal/api/` dependency also assumes local commercial-trial +handoff remains human-usable: lifecycle-adjacent trial CTAs may allow a short +burst of retries, but the backend contract must return the real remaining +backoff through `Retry-After` plus `details.retry_after_seconds` so setup and +install-adjacent surfaces do not drift into generic “try again later” behavior. That same shared `internal/api/` dependency also assumes session-carried OIDC refresh tokens stay fail-closed at rest: `session_store.go` may only persist or recover those tokens through encrypted-at-rest session payloads, and any diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 323e8fb6a..57655efdd 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -227,6 +227,14 @@ Own canonical runtime payload shapes between backend and frontend. install-state surface must describe that prepared token path consistently with the live runtime behavior rather than directing the operator to create another install token manually. +21. Keep local trial-start transport explicit on the shared commercial API + boundary: `/api/license/trial/start` must preserve the hosted-signup + redirect contract during the allowed retry burst, then return the actual + remaining backoff in both `Retry-After` and + `details.retry_after_seconds` once the burst is exceeded. Hosted + self-serve verification failures may render owned HTML, but they must + preserve originating Pulse context instead of collapsing into generic + control-plane failures. ## Current State @@ -1700,6 +1708,19 @@ both sides instead of relying only on broad settings-surface coverage on the security side: token settings changes must continue to carry the direct `api-token-management-surface` API-contract proof together with the security-side surface proof. +That same shared commercial API boundary now also owns the local trial-start +transport contract. `/api/license/trial/start` may allow a short human-scale +burst of retries while the hosted redirect handoff remains canonical, but once +that burst is exceeded it must return the actual remaining backoff in both the +`Retry-After` header and the JSON `details.retry_after_seconds` payload instead +of a fixed window guess or a text-only error. `internal/api/contract_test.go` +must pin both the hosted-signup redirect response and the rate-limited response +in the same slice as any handler change. +That same shared commercial API boundary also owns hosted self-serve failure +transport semantics. Hosted trial request and verification failures may render +owned HTML pages, but they must preserve the originating Pulse instance and +customer form context instead of collapsing into generic control-plane failures +or dead-end text with no route back to the originating runtime. That same boundary must also keep token scope presets lazily derived from the canonical scope constants: `apiTokenManagerModel.ts` may expose `getAPITokenScopePresets()`, but it must not publish an eagerly evaluated diff --git a/docs/release-control/v6/internal/subsystems/cloud-paid.md b/docs/release-control/v6/internal/subsystems/cloud-paid.md index 9a46c6999..c195f6c19 100644 --- a/docs/release-control/v6/internal/subsystems/cloud-paid.md +++ b/docs/release-control/v6/internal/subsystems/cloud-paid.md @@ -267,13 +267,18 @@ The trial-start rate-limit contract is part of that same boundary. Local `/api/license/trial/start` retries are allowed as a short human-scale burst so operators can revisit the hosted handoff without getting locked out for a day, and both the local app server and hosted trial signup limiters must return the -actual remaining backoff through `Retry-After` rather than a coarse full-window -guess. Shared trial-start presentation must treat that backoff as canonical and -surface it consistently instead of flattening every `429` into generic copy. +actual remaining backoff through `Retry-After` and +`details.retry_after_seconds` rather than a coarse full-window guess. Shared +trial-start presentation must treat that backoff as canonical and surface it +consistently instead of flattening every `429` into generic copy. For the hosted self-serve flow, that also means the public trial pages and form posts must render the owned Pulse trial experience with preserved instance/form state when rate limited, rather than dropping users onto a generic control-plane `Too Many Requests` response. +The same owned hosted-trial failure experience applies to invalid or expired +verification links. The customer must stay inside the Pulse-owned retry path +with the originating-instance context preserved, rather than landing on a +generic hosted error page that reads like a detached SaaS funnel. The hosted trial handoff page is part of that same boundary as well. It may still use a secure hosted Stripe-backed session internally, but the customer copy must present the flow as starting a trial for the originating Pulse diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 6b8deac9f..fa5222d36 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -642,6 +642,11 @@ raw-token `sessions.json` and `csrf_tokens.json` files may load for upgrade continuity, but `session_store.go` and `csrf_store.go` must immediately rewrite hashed canonical persistence on load so adjacent storage and recovery transport does not keep running against primary-path raw-token files. +That same shared `internal/api/` dependency also assumes customer-visible +commercial retry guidance stays canonical when storage- or recovery-adjacent +surfaces invoke trial or billing handoffs: the backend must return the real +remaining backoff through `Retry-After` and `details.retry_after_seconds` +instead of leaving neighboring surfaces to guess or hardcode retry windows. That same shared `internal/api/` dependency now also assumes adjacent commercial helper surfaces speak in monitored-system terms: recovery- or storage-adjacent API wiring may consume the canonical monitored-system ledger diff --git a/frontend-modern/src/stores/__tests__/license.test.ts b/frontend-modern/src/stores/__tests__/license.test.ts index c3380d126..d6eb97839 100644 --- a/frontend-modern/src/stores/__tests__/license.test.ts +++ b/frontend-modern/src/stores/__tests__/license.test.ts @@ -123,6 +123,7 @@ describe('license store', () => { vi.mocked(LicenseAPI.startTrial).mockResolvedValue({ ok: false, status: 400, + headers: new Headers(), json: vi.fn().mockResolvedValue({ code: 'trial_failed' }), } as unknown as Response); @@ -133,6 +134,7 @@ describe('license store', () => { vi.mocked(LicenseAPI.startTrial).mockResolvedValue({ ok: false, status: 409, + headers: new Headers(), json: vi.fn().mockResolvedValue({ code: 'trial_not_available', error: 'Trial cannot be started while a paid v5 license migration is pending', diff --git a/frontend-modern/src/stores/license.ts b/frontend-modern/src/stores/license.ts index 1b33957ce..ae4805df7 100644 --- a/frontend-modern/src/stores/license.ts +++ b/frontend-modern/src/stores/license.ts @@ -23,12 +23,30 @@ type TrialStartRequestError = Error & { status: number; code?: string; details?: Record; + retryAfterSeconds?: number; }; export type StartProTrialResult = | { outcome: 'activated' } | { outcome: 'redirect'; actionUrl: string }; +function parseRetryAfterSeconds(value: string | null | undefined): number | undefined { + const normalized = value?.trim(); + if (!normalized) return undefined; + + const parsed = Number(normalized); + if (Number.isFinite(parsed) && parsed > 0) { + return Math.ceil(parsed); + } + + const retryAt = Date.parse(normalized); + if (Number.isNaN(retryAt)) return undefined; + + const waitMs = retryAt - Date.now(); + if (waitMs <= 0) return 1; + return Math.ceil(waitMs / 1000); +} + /** * Load the entitlements payload from the server. * @@ -98,9 +116,13 @@ export async function startProTrial(): Promise { const err = new Error( payload?.error?.trim() || `Failed to start trial (status ${res.status})`, ) as TrialStartRequestError; + const retryAfterSeconds = + parseRetryAfterSeconds(res.headers.get('Retry-After')) ?? + parseRetryAfterSeconds(payload?.details?.retry_after_seconds); err.status = res.status; err.code = payload?.code; err.details = payload?.details; + err.retryAfterSeconds = retryAfterSeconds; throw err; } await loadLicenseStatus(true); diff --git a/frontend-modern/src/utils/__tests__/trialStartAction.test.ts b/frontend-modern/src/utils/__tests__/trialStartAction.test.ts index 63ab8069c..9ff0f1332 100644 --- a/frontend-modern/src/utils/__tests__/trialStartAction.test.ts +++ b/frontend-modern/src/utils/__tests__/trialStartAction.test.ts @@ -98,4 +98,24 @@ describe('runStartProTrialAction', () => { expect(onError).toHaveBeenCalledWith(error); expect(showError).toHaveBeenCalledWith('Trial already used'); }); + + it('surfaces retry-after guidance from the shared presentation helper', async () => { + startProTrialMock.mockRejectedValue({ + status: 429, + code: 'trial_rate_limited', + message: 'Trial start rate limit exceeded', + retryAfterSeconds: 120, + }); + + await expect( + runStartProTrialAction({ + showSuccess, + showError, + navigate, + }), + ).resolves.toBe('error'); + + expect(showError).toHaveBeenCalledWith('Try again in about 2 minutes'); + expect(showSuccess).not.toHaveBeenCalled(); + }); }); diff --git a/frontend-modern/src/utils/__tests__/upgradePresentation.test.ts b/frontend-modern/src/utils/__tests__/upgradePresentation.test.ts index 5b9104da6..ab6e22a88 100644 --- a/frontend-modern/src/utils/__tests__/upgradePresentation.test.ts +++ b/frontend-modern/src/utils/__tests__/upgradePresentation.test.ts @@ -23,6 +23,9 @@ describe('upgradePresentation', () => { expect(getProTrialStartedMessage()).toBe('Pro trial started'); expect(getTrialAlreadyUsedMessage()).toBe('Trial already used'); expect(getTrialTryAgainLaterMessage()).toBe('Try again later'); + expect(getTrialTryAgainLaterMessage(30)).toBe('Try again in about a minute'); + expect(getTrialTryAgainLaterMessage(90)).toBe('Try again in about 2 minutes'); + expect(getTrialTryAgainLaterMessage(3600)).toBe('Try again in about 1 hour'); expect(getTrialStartErrorMessage()).toBe('Failed to start trial'); expect(getTrialStartErrorMessage(undefined, { branded: true })).toBe( 'Failed to start Pro trial', @@ -30,6 +33,9 @@ describe('upgradePresentation', () => { expect(getTrialStartErrorMessage('temporary failure')).toBe('temporary failure'); expect(getTrialStartErrorMessage({ code: 'trial_already_used' })).toBe('Trial already used'); expect(getTrialStartErrorMessage({ status: 429 })).toBe('Try again later'); + expect(getTrialStartErrorMessage({ status: 429, retryAfterSeconds: 120 })).toBe( + 'Try again in about 2 minutes', + ); expect(getTrialStartErrorKind({ code: 'trial_already_used' })).toBe('already_used'); expect(getTrialStartErrorKind({ status: 429 })).toBe('retry_later'); expect(getTrialStartErrorKind({ message: 'temporary failure' })).toBe('other'); @@ -37,6 +43,7 @@ describe('upgradePresentation', () => { status: 409, code: 'trial_not_available', message: undefined, + retryAfterSeconds: undefined, }); expect( getTrialStartErrorMessage({ diff --git a/frontend-modern/src/utils/upgradePresentation.ts b/frontend-modern/src/utils/upgradePresentation.ts index dc3493651..dcbadf0ac 100644 --- a/frontend-modern/src/utils/upgradePresentation.ts +++ b/frontend-modern/src/utils/upgradePresentation.ts @@ -18,6 +18,7 @@ export interface TrialStartErrorLike { status?: number; code?: string; message?: string; + retryAfterSeconds?: number; } export type TrialStartErrorKind = 'already_used' | 'retry_later' | 'other'; @@ -30,8 +31,27 @@ export function getTrialAlreadyUsedMessage(): string { return 'Trial already used'; } -export function getTrialTryAgainLaterMessage(): string { - return 'Try again later'; +export function getTrialTryAgainLaterMessage(retryAfterSeconds?: number): string { + if (!retryAfterSeconds || retryAfterSeconds < 1) { + return 'Try again later'; + } + + if (retryAfterSeconds < 90) { + return 'Try again in about a minute'; + } + + if (retryAfterSeconds < 3600) { + const minutes = Math.ceil(retryAfterSeconds / 60); + return `Try again in about ${minutes} minutes`; + } + + if (retryAfterSeconds < 172800) { + const hours = Math.ceil(retryAfterSeconds / 3600); + return `Try again in about ${hours} hour${hours === 1 ? '' : 's'}`; + } + + const days = Math.ceil(retryAfterSeconds / 86400); + return `Try again in about ${days} day${days === 1 ? '' : 's'}`; } export function normalizeTrialStartError(error?: unknown): TrialStartErrorLike | null { @@ -44,6 +64,7 @@ export function normalizeTrialStartError(error?: unknown): TrialStartErrorLike | status: value.status, code: value.code, message: value.message, + retryAfterSeconds: value.retryAfterSeconds, }; } @@ -68,7 +89,7 @@ export function getTrialStartErrorMessage( return getTrialAlreadyUsedMessage(); } if (kind === 'retry_later') { - return getTrialTryAgainLaterMessage(); + return getTrialTryAgainLaterMessage(normalized?.retryAfterSeconds); } if (normalized?.message?.trim()) { return normalized.message.trim(); diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index f7da89bad..8f9604108 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -241,6 +241,78 @@ func TestContract_HostedSignupResponseJSONSnapshot(t *testing.T) { assertJSONSnapshot(t, got, want) } +func TestContract_TrialStartHostedSignupRedirectContract(t *testing.T) { + baseDir := t.TempDir() + mtp := config.NewMultiTenantPersistence(baseDir) + h := NewLicenseHandlers(mtp, false, &config.Config{ + PublicURL: "https://pulse.example.com", + ProTrialSignupURL: "https://billing.example.com/start-pro-trial?source=contract", + }) + + req := httptest.NewRequest(http.MethodPost, "/api/license/trial/start", nil).WithContext( + context.WithValue(context.Background(), OrgIDContextKey, "default"), + ) + rec := httptest.NewRecorder() + h.HandleStartTrial(rec, req) + + if rec.Code != http.StatusConflict { + t.Fatalf("status=%d, want %d: %s", rec.Code, http.StatusConflict, rec.Body.String()) + } + + var payload APIError + if err := json.NewDecoder(rec.Body).Decode(&payload); err != nil { + t.Fatalf("decode response: %v", err) + } + if payload.Code != "trial_signup_required" { + t.Fatalf("code=%q, want %q", payload.Code, "trial_signup_required") + } + if strings.TrimSpace(payload.Details["action_url"]) == "" { + t.Fatal("expected action_url in contract payload") + } +} + +func TestContract_TrialStartRateLimitContract(t *testing.T) { + baseDir := t.TempDir() + mtp := config.NewMultiTenantPersistence(baseDir) + h := NewLicenseHandlers(mtp, false, &config.Config{ + PublicURL: "https://pulse.example.com", + ProTrialSignupURL: "https://billing.example.com/start-pro-trial?source=contract", + }) + h.trialLimiter = NewRateLimiter(1, time.Minute) + + ctx := context.WithValue(context.Background(), OrgIDContextKey, "default") + + firstReq := httptest.NewRequest(http.MethodPost, "/api/license/trial/start", nil).WithContext(ctx) + firstRec := httptest.NewRecorder() + h.HandleStartTrial(firstRec, firstReq) + if firstRec.Code != http.StatusConflict { + t.Fatalf("first status=%d, want %d: %s", firstRec.Code, http.StatusConflict, firstRec.Body.String()) + } + + secondReq := httptest.NewRequest(http.MethodPost, "/api/license/trial/start", nil).WithContext(ctx) + secondRec := httptest.NewRecorder() + h.HandleStartTrial(secondRec, secondReq) + if secondRec.Code != http.StatusTooManyRequests { + t.Fatalf("second status=%d, want %d: %s", secondRec.Code, http.StatusTooManyRequests, secondRec.Body.String()) + } + + retryAfter := secondRec.Header().Get("Retry-After") + if retryAfter == "" { + t.Fatal("expected Retry-After header") + } + + var payload APIError + if err := json.NewDecoder(secondRec.Body).Decode(&payload); err != nil { + t.Fatalf("decode response: %v", err) + } + if payload.Code != "trial_rate_limited" { + t.Fatalf("code=%q, want %q", payload.Code, "trial_rate_limited") + } + if payload.Details["retry_after_seconds"] != retryAfter { + t.Fatalf("retry_after_seconds=%q, want %q", payload.Details["retry_after_seconds"], retryAfter) + } +} + func TestContract_BillingStateQuickstartJSONSnapshot(t *testing.T) { grantedAt := time.Date(2026, 3, 25, 14, 30, 0, 0, time.UTC).Unix() diff --git a/internal/api/licensing_handlers.go b/internal/api/licensing_handlers.go index c4a1a850d..919e74308 100644 --- a/internal/api/licensing_handlers.go +++ b/internal/api/licensing_handlers.go @@ -10,6 +10,7 @@ import ( "net/http" "net/url" "os" + "strconv" "strings" "sync" "time" @@ -19,6 +20,11 @@ import ( "github.com/rs/zerolog/log" ) +const ( + trialStartRateLimitBurst = 6 + trialStartRateLimitWindow = 15 * time.Minute +) + // revocationFeedToken returns the relay feed token for revocation polling. // Empty string means revocation polling is disabled. func revocationFeedToken() string { @@ -61,7 +67,7 @@ func NewLicenseHandlers(mtp *config.MultiTenantPersistence, hostedMode bool, cfg mtPersistence: mtp, hostedMode: hostedMode, cfg: cfg, - trialLimiter: NewRateLimiter(1, 24*time.Hour), // 1 trial start attempt per org per 24h + trialLimiter: NewRateLimiter(trialStartRateLimitBurst, trialStartRateLimitWindow), trialReplay: trialReplay, trialInitiations: trialInitiations, } @@ -494,12 +500,20 @@ func (h *LicenseHandlers) HandleStartTrial(w http.ResponseWriter, r *http.Reques return } - if h.trialLimiter != nil && !h.trialLimiter.Allow(orgID) { - w.Header().Set("Retry-After", "86400") - writeErrorResponse(w, http.StatusTooManyRequests, "trial_rate_limited", "Trial start rate limit exceeded", map[string]string{ - "org_id": orgID, - }) - return + if h.trialLimiter != nil { + allowed, retryDelay := h.trialLimiter.allowAt(orgID, time.Now().UTC()) + if !allowed { + retryAfterSeconds := int(retryDelay.Round(time.Second).Seconds()) + if retryAfterSeconds < 1 { + retryAfterSeconds = 1 + } + w.Header().Set("Retry-After", strconv.Itoa(retryAfterSeconds)) + writeErrorResponse(w, http.StatusTooManyRequests, "trial_rate_limited", "Trial start rate limit exceeded", map[string]string{ + "org_id": orgID, + "retry_after_seconds": strconv.Itoa(retryAfterSeconds), + }) + return + } } if h.trialInitiations == nil { diff --git a/internal/api/ratelimit.go b/internal/api/ratelimit.go index 48b1deb54..cb597e0bf 100644 --- a/internal/api/ratelimit.go +++ b/internal/api/ratelimit.go @@ -77,10 +77,16 @@ func (rl *RateLimiter) Stop() { // Allow checks if a request from the given IP address is within the rate limit. // Returns true if the request is allowed, false if the rate limit is exceeded. func (rl *RateLimiter) Allow(ip string) bool { + allowed, _ := rl.allowAt(ip, time.Now()) + return allowed +} + +// allowAt checks whether a request is allowed at the provided time and reports +// how long the caller should wait before retrying when the limit is exceeded. +func (rl *RateLimiter) allowAt(ip string, now time.Time) (bool, time.Duration) { rl.mu.Lock() defer rl.mu.Unlock() - now := time.Now() cutoff := now.Add(-rl.window) // Get attempts for this IP @@ -97,14 +103,21 @@ func (rl *RateLimiter) Allow(ip string) bool { // Check if under limit if len(validAttempts) >= rl.limit { rl.attempts[ip] = validAttempts - return false + retryAfter := rl.window + if len(validAttempts) > 0 { + retryAfter = validAttempts[0].Add(rl.window).Sub(now) + } + if retryAfter < time.Second { + retryAfter = time.Second + } + return false, retryAfter } // Add new attempt validAttempts = append(validAttempts, now) rl.attempts[ip] = validAttempts - return true + return true, 0 } func (rl *RateLimiter) cleanup() { diff --git a/internal/api/trial_handlers_test.go b/internal/api/trial_handlers_test.go index 178c0a5ab..3e975ca61 100644 --- a/internal/api/trial_handlers_test.go +++ b/internal/api/trial_handlers_test.go @@ -129,6 +129,37 @@ func TestTrialStart_DefaultOrgReturnsHostedSignupRedirect(t *testing.T) { } } +func TestTrialStart_AllowsRepeatHostedSignupRedirectsWithinBurstWindow(t *testing.T) { + baseDir := t.TempDir() + mtp := config.NewMultiTenantPersistence(baseDir) + h := NewLicenseHandlers(mtp, false, &config.Config{ + PublicURL: "https://pulse.example.com", + ProTrialSignupURL: "https://billing.example.com/start-pro-trial?source=test", + }) + + ctx := context.WithValue(context.Background(), OrgIDContextKey, "default") + + for i := 0; i < 2; i++ { + req := httptest.NewRequest(http.MethodPost, "/api/license/trial/start", nil).WithContext(ctx) + rec := httptest.NewRecorder() + h.HandleStartTrial(rec, req) + if rec.Code != http.StatusConflict { + t.Fatalf("attempt %d status=%d, want %d: %s", i+1, rec.Code, http.StatusConflict, rec.Body.String()) + } + + var resp APIError + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if resp.Code != "trial_signup_required" { + t.Fatalf("attempt %d code=%q, want %q", i+1, resp.Code, "trial_signup_required") + } + if strings.TrimSpace(resp.Details["action_url"]) == "" { + t.Fatalf("attempt %d missing action_url", i+1) + } + } +} + func TestTrialStart_FailsClosedWithoutCallbackURL(t *testing.T) { baseDir := t.TempDir() mtp := config.NewMultiTenantPersistence(baseDir) @@ -193,6 +224,51 @@ func TestTrialStart_RejectsAlreadyUsedTrial(t *testing.T) { } } +func TestTrialStart_ReturnsRetryAfterWhenRateLimited(t *testing.T) { + baseDir := t.TempDir() + mtp := config.NewMultiTenantPersistence(baseDir) + h := NewLicenseHandlers(mtp, false, &config.Config{ + PublicURL: "https://pulse.example.com", + ProTrialSignupURL: "https://billing.example.com/start-pro-trial?source=test", + }) + h.trialLimiter = NewRateLimiter(1, time.Minute) + + ctx := context.WithValue(context.Background(), OrgIDContextKey, "default") + + firstReq := httptest.NewRequest(http.MethodPost, "/api/license/trial/start", nil).WithContext(ctx) + firstRec := httptest.NewRecorder() + h.HandleStartTrial(firstRec, firstReq) + if firstRec.Code != http.StatusConflict { + t.Fatalf("first status=%d, want %d: %s", firstRec.Code, http.StatusConflict, firstRec.Body.String()) + } + + secondReq := httptest.NewRequest(http.MethodPost, "/api/license/trial/start", nil).WithContext(ctx) + secondRec := httptest.NewRecorder() + h.HandleStartTrial(secondRec, secondReq) + if secondRec.Code != http.StatusTooManyRequests { + t.Fatalf("second status=%d, want %d: %s", secondRec.Code, http.StatusTooManyRequests, secondRec.Body.String()) + } + + retryAfter := secondRec.Header().Get("Retry-After") + if retryAfter == "" { + t.Fatal("expected Retry-After header") + } + if got := secondRec.Header().Get("Content-Type"); !strings.Contains(got, "application/json") { + t.Fatalf("content-type=%q, want JSON response", got) + } + + var resp APIError + if err := json.NewDecoder(secondRec.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if resp.Code != "trial_rate_limited" { + t.Fatalf("code=%q, want %q", resp.Code, "trial_rate_limited") + } + if resp.Details["retry_after_seconds"] != retryAfter { + t.Fatalf("retry_after_seconds=%q, want %q", resp.Details["retry_after_seconds"], retryAfter) + } +} + func TestTrialEntitlements_TrialDaysRemainingFromBillingState(t *testing.T) { baseDir := t.TempDir() mtp := config.NewMultiTenantPersistence(baseDir) diff --git a/internal/cloudcp/ratelimit.go b/internal/cloudcp/ratelimit.go index 53c3f42ce..44b62cf5d 100644 --- a/internal/cloudcp/ratelimit.go +++ b/internal/cloudcp/ratelimit.go @@ -29,8 +29,6 @@ type CPRateLimiter struct { window time.Duration } -type CPRateLimitRejectedHandler func(w http.ResponseWriter, r *http.Request, retryAfter int) - // NewCPRateLimiter creates a rate limiter with the given limit per window. func NewCPRateLimiter(limit int, window time.Duration) *CPRateLimiter { if limit <= 0 { @@ -48,17 +46,10 @@ func NewCPRateLimiter(limit int, window time.Duration) *CPRateLimiter { // Allow checks whether the given IP is within the rate limit. func (rl *CPRateLimiter) Allow(ip string) bool { - allowed, _ := rl.allowAt(ip, time.Now()) - return allowed -} - -// allowAt checks whether the given IP is within the rate limit at the provided -// time and reports how long the caller should wait before retrying when -// blocked. -func (rl *CPRateLimiter) allowAt(ip string, now time.Time) (bool, time.Duration) { rl.mu.Lock() defer rl.mu.Unlock() + now := time.Now() cutoff := now.Add(-rl.window) // Filter expired entries @@ -71,42 +62,26 @@ func (rl *CPRateLimiter) allowAt(ip string, now time.Time) (bool, time.Duration) if len(valid) >= rl.limit { rl.attempts[ip] = valid - retryAfter := rl.window - if len(valid) > 0 { - retryAfter = valid[0].Add(rl.window).Sub(now) - } - if retryAfter < time.Second { - retryAfter = time.Second - } - return false, retryAfter + return false } rl.attempts[ip] = append(valid, now) - return true, 0 + return true } // Middleware wraps an http.Handler with rate limiting. func (rl *CPRateLimiter) Middleware(next http.Handler) http.Handler { - return rl.MiddlewareWithRejected(next, nil) -} - -// MiddlewareWithRejected wraps an http.Handler with rate limiting and allows -// callers to provide a custom blocked-response handler for customer-facing -// flows such as hosted trial signup. -func (rl *CPRateLimiter) MiddlewareWithRejected(next http.Handler, rejected CPRateLimitRejectedHandler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ip := clientIP(r) - if allowed, retryDelay := rl.allowAt(ip, time.Now()); !allowed { - retryAfter := int(math.Ceil(retryDelay.Seconds())) + if !rl.Allow(ip) { + retryAfter := int(math.Ceil(rl.window.Seconds())) if retryAfter < 1 { retryAfter = 1 } - rl.writeRateLimitHeaders(w, retryAfter) - if rejected != nil { - rejected(w, r, retryAfter) - return - } + w.Header().Set("Retry-After", strconv.Itoa(retryAfter)) + w.Header().Set("X-RateLimit-Limit", strconv.Itoa(rl.limit)) + w.Header().Set("X-RateLimit-Remaining", "0") http.Error(w, "Too Many Requests", http.StatusTooManyRequests) return } @@ -114,12 +89,6 @@ func (rl *CPRateLimiter) MiddlewareWithRejected(next http.Handler, rejected CPRa }) } -func (rl *CPRateLimiter) writeRateLimitHeaders(w http.ResponseWriter, retryAfter int) { - w.Header().Set("Retry-After", strconv.Itoa(retryAfter)) - w.Header().Set("X-RateLimit-Limit", strconv.Itoa(rl.limit)) - w.Header().Set("X-RateLimit-Remaining", "0") -} - func clientIP(r *http.Request) string { remote := extractRemoteIP(r.RemoteAddr) if remote == "" { diff --git a/internal/cloudcp/ratelimit_test.go b/internal/cloudcp/ratelimit_test.go index 6df225934..fc5c0c4f5 100644 --- a/internal/cloudcp/ratelimit_test.go +++ b/internal/cloudcp/ratelimit_test.go @@ -73,65 +73,6 @@ func TestCPRateLimiterMiddleware_TooManyRequests(t *testing.T) { if calls != 1 { t.Fatalf("next handler calls after reject = %d, want 1", calls) } - if rec2.Header().Get("Retry-After") == "" { - t.Fatal("expected Retry-After header") - } -} - -func TestCPRateLimiterMiddlewareWithRejectedUsesCustomHandler(t *testing.T) { - rl := NewCPRateLimiter(1, time.Minute) - calls := 0 - next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - calls++ - w.WriteHeader(http.StatusNoContent) - }) - h := rl.MiddlewareWithRejected(next, func(w http.ResponseWriter, r *http.Request, retryAfter int) { - if retryAfter < 1 { - t.Fatalf("retryAfter=%d, want positive", retryAfter) - } - http.Error(w, "custom blocked", http.StatusTooManyRequests) - }) - - req1 := httptest.NewRequest(http.MethodPost, "/start-pro-trial", nil) - req1.RemoteAddr = "198.51.100.7:1234" - rec1 := httptest.NewRecorder() - h.ServeHTTP(rec1, req1) - if rec1.Code != http.StatusNoContent { - t.Fatalf("first request status = %d, want %d", rec1.Code, http.StatusNoContent) - } - - req2 := httptest.NewRequest(http.MethodPost, "/start-pro-trial", nil) - req2.RemoteAddr = "198.51.100.7:1234" - rec2 := httptest.NewRecorder() - h.ServeHTTP(rec2, req2) - - if rec2.Code != http.StatusTooManyRequests { - t.Fatalf("second request status = %d, want %d", rec2.Code, http.StatusTooManyRequests) - } - if rec2.Body.String() != "custom blocked\n" { - t.Fatalf("blocked body=%q, want custom handler body", rec2.Body.String()) - } - if calls != 1 { - t.Fatalf("next handler calls=%d, want 1", calls) - } - if rec2.Header().Get("Retry-After") == "" { - t.Fatal("expected Retry-After header") - } -} - -func TestCPRateLimiterAllowAtReportsRemainingBackoff(t *testing.T) { - rl := NewCPRateLimiter(1, time.Minute) - ip := "198.51.100.44" - now := time.Unix(1710000000, 0).UTC() - rl.attempts[ip] = []time.Time{now.Add(-30 * time.Second)} - - allowed, retryAfter := rl.allowAt(ip, now) - if allowed { - t.Fatal("expected request to be rejected") - } - if retryAfter < 29*time.Second || retryAfter > 31*time.Second { - t.Fatalf("retryAfter=%v, want about 30s", retryAfter) - } } func TestClientIP(t *testing.T) { diff --git a/internal/cloudcp/routes_test.go b/internal/cloudcp/routes_test.go index 2f4619e75..104165fa0 100644 --- a/internal/cloudcp/routes_test.go +++ b/internal/cloudcp/routes_test.go @@ -3,7 +3,6 @@ package cloudcp import ( "net/http" "net/http/httptest" - "net/url" "strings" "testing" @@ -215,74 +214,6 @@ func TestRegisterRoutes_TrialSignupVerificationRateLimit(t *testing.T) { if rec.Code != http.StatusTooManyRequests { t.Fatalf("rate-limited status=%d, want %d body=%q", rec.Code, http.StatusTooManyRequests, rec.Body.String()) } - if !strings.Contains(rec.Body.String(), "Too many trial setup attempts from this browser.") { - t.Fatalf("expected hosted rate-limit guidance, got %q", rec.Body.String()) - } - if rec.Header().Get("Retry-After") == "" { - t.Fatal("expected Retry-After header") - } -} - -func TestRegisterRoutes_TrialSignupCheckoutRateLimitRendersHostedPage(t *testing.T) { - dir := t.TempDir() - reg, err := registry.NewTenantRegistry(dir) - if err != nil { - t.Fatalf("NewTenantRegistry: %v", err) - } - t.Cleanup(func() { _ = reg.Close() }) - trialStore, err := NewTrialSignupStore(dir) - if err != nil { - t.Fatalf("NewTrialSignupStore: %v", err) - } - t.Cleanup(func() { trialStore.Close() }) - - mux := http.NewServeMux() - RegisterRoutes(mux, &Deps{ - Config: &CPConfig{ - DataDir: dir, - AdminKey: "test-admin-key", - BaseURL: "https://cloud.example.com", - StripeWebhookSecret: "whsec_test", - }, - Registry: reg, - TrialSignupStore: trialStore, - Version: "test", - }) - - form := url.Values{ - "org_id": {"default"}, - "return_url": {"https://pulse.example.com/auth/trial-activate"}, - "instance_token": {"tsi_test"}, - "name": {"Test User"}, - "email": {"owner@example.com"}, - "company": {"Pulse Labs"}, - } - - for i := 0; i < 12; i++ { - req := httptest.NewRequest(http.MethodPost, "/api/trial-signup/checkout", strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.RemoteAddr = "198.51.100.35:7777" - rec := httptest.NewRecorder() - mux.ServeHTTP(rec, req) - if rec.Code != http.StatusServiceUnavailable { - t.Fatalf("attempt %d status=%d, want %d body=%q", i+1, rec.Code, http.StatusServiceUnavailable, rec.Body.String()) - } - } - - req := httptest.NewRequest(http.MethodPost, "/api/trial-signup/checkout", strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.RemoteAddr = "198.51.100.35:7777" - rec := httptest.NewRecorder() - mux.ServeHTTP(rec, req) - if rec.Code != http.StatusTooManyRequests { - t.Fatalf("rate-limited status=%d, want %d body=%q", rec.Code, http.StatusTooManyRequests, rec.Body.String()) - } - if !strings.Contains(rec.Body.String(), "Too many trial setup attempts from this browser.") { - t.Fatalf("expected hosted rate-limit guidance, got %q", rec.Body.String()) - } - if !strings.Contains(rec.Body.String(), "owner@example.com") { - t.Fatalf("expected submitted email to be preserved, got %q", rec.Body.String()) - } } func TestRegisterRoutes_PublicCloudSignupRoutes(t *testing.T) { diff --git a/internal/cloudcp/trial_signup_handlers.go b/internal/cloudcp/trial_signup_handlers.go index fe2f5fa72..7e75e138f 100644 --- a/internal/cloudcp/trial_signup_handlers.go +++ b/internal/cloudcp/trial_signup_handlers.go @@ -549,6 +549,7 @@ type trialSignupFailureKind string const ( trialSignupFailureRetryable trialSignupFailureKind = "retryable" trialSignupFailureConflict trialSignupFailureKind = "conflict" + trialSignupFailureInvalidLink trialSignupFailureKind = "invalid_link" trialSignupFailureUnavailable trialSignupFailureKind = "unavailable" ) @@ -618,6 +619,14 @@ func (h *TrialSignupHandlers) HandleRequestVerification(w http.ResponseWriter, r Company: strings.TrimSpace(r.FormValue("company")), ReturnTarget: summarizeTrialReturnTarget(r.FormValue("return_url")), } + unavailable := func(status int, message string) { + h.renderTrialSignupFailurePage(w, r, status, trialSignupFailureDataForPage( + h.cfg, + data, + trialSignupFailureUnavailable, + message, + )) + } if strings.TrimSpace(data.InstanceToken) == "" { data.ErrorMessage = "This trial request must be started from Pulse. Return to Pulse Settings > Pro License and try again." h.renderTrialSignupPage(w, r, http.StatusBadRequest, data) @@ -647,8 +656,7 @@ func (h *TrialSignupHandlers) HandleRequestVerification(w http.ResponseWriter, r pendingRecord, err := h.verificationStore.FindPendingVerificationByEmail(data.Email, h.now().UTC()) if err != nil { log.Error().Err(err).Str("email", data.Email).Msg("trial signup pending verification lookup failed") - data.ErrorMessage = "Unable to validate trial eligibility right now. Please try again." - h.renderTrialSignupPage(w, r, http.StatusInternalServerError, data) + unavailable(http.StatusInternalServerError, "Unable to validate trial eligibility right now. Please try again.") return } if pendingRecord != nil { @@ -659,19 +667,21 @@ func (h *TrialSignupHandlers) HandleRequestVerification(w http.ResponseWriter, r conflict, err := h.verificationStore.FindIssuedTrialConflict(data.Email, data.Company) if err != nil { log.Error().Err(err).Str("email", data.Email).Msg("trial signup issuance lookup failed") - data.ErrorMessage = "Unable to validate trial eligibility right now. Please try again." - h.renderTrialSignupPage(w, r, http.StatusInternalServerError, data) + unavailable(http.StatusInternalServerError, "Unable to validate trial eligibility right now. Please try again.") return } if conflict != nil { - data.ErrorMessage = trialSignupIssuanceConflictMessage(conflict) - h.renderTrialSignupPage(w, r, http.StatusConflict, data) + h.renderTrialSignupFailurePage(w, r, http.StatusConflict, trialSignupFailureDataForPage( + h.cfg, + data, + trialSignupFailureConflict, + trialSignupIssuanceConflictMessage(conflict), + )) return } } if h.emailSender == nil || h.cfg == nil || strings.TrimSpace(h.cfg.EmailFrom) == "" || h.verificationStore == nil { - data.ErrorMessage = "Email verification is not configured yet. Please contact support." - h.renderTrialSignupPage(w, r, http.StatusServiceUnavailable, data) + unavailable(http.StatusServiceUnavailable, "Email verification is not configured yet. Please contact support.") return } @@ -687,16 +697,14 @@ func (h *TrialSignupHandlers) HandleRequestVerification(w http.ResponseWriter, r }) if err != nil { log.Error().Err(err).Str("email", data.Email).Msg("trial signup verification record creation failed") - data.ErrorMessage = "Unable to prepare the verification link. Please try again." - h.renderTrialSignupPage(w, r, http.StatusInternalServerError, data) + unavailable(http.StatusInternalServerError, "Unable to prepare the verification link. Please try again.") return } verifyURL := buildTrialSignupVerificationURL(h.cfg.BaseURL, token) if err := h.sendTrialVerificationEmail(data.Email, verifyURL); err != nil { log.Error().Err(err).Str("email", data.Email).Msg("trial signup verification email send failed") - data.ErrorMessage = "Unable to send the verification email. Please try again." - h.renderTrialSignupPage(w, r, http.StatusBadGateway, data) + unavailable(http.StatusBadGateway, "Unable to send the verification email. Please try again.") return } @@ -715,19 +723,23 @@ func (h *TrialSignupHandlers) HandleVerifyEmail(w http.ResponseWriter, r *http.R record, err := h.verificationStore.ConsumeVerification(token, h.now().UTC()) if err != nil { log.Warn().Err(err).Msg("trial signup verification token invalid") - h.renderTrialSignupPage(w, r, http.StatusBadRequest, trialSignupPageData{ - ErrorMessage: "That verification link is invalid or expired. Request a fresh email from Pulse and try again.", - ReturnTarget: "your Pulse instance", - }) + h.renderTrialSignupFailurePage(w, r, http.StatusBadRequest, trialSignupFailureDataForPage( + h.cfg, + trialSignupPageData{ReturnTarget: "your Pulse instance"}, + trialSignupFailureInvalidLink, + "That verification link is invalid or expired. Return to Pulse to request a fresh backup email.", + )) return } verifiedToken, err := h.verificationStore.IssueCheckoutToken(record.ID, h.now().UTC(), trialSignupVerificationTTL) if err != nil { log.Error().Err(err).Str("request_id", record.ID).Msg("trial checkout token issuance failed") - h.renderTrialSignupPage(w, r, http.StatusInternalServerError, trialSignupPageData{ - ErrorMessage: "Unable to continue to trial checkout. Please try again.", - ReturnTarget: summarizeTrialReturnTarget(record.ReturnURL), - }) + h.renderTrialSignupFailurePage(w, r, http.StatusInternalServerError, trialSignupFailureDataForPage( + h.cfg, + trialSignupPageData{ReturnTarget: summarizeTrialReturnTarget(record.ReturnURL)}, + trialSignupFailureUnavailable, + "Unable to continue to trial checkout. Please try again.", + )) return } http.Redirect(w, r, buildTrialSignupVerifiedURL(h.cfg.BaseURL, verifiedToken, false), http.StatusSeeOther) @@ -738,10 +750,12 @@ func (h *TrialSignupHandlers) HandleVerifyEmail(w http.ResponseWriter, r *http.R record, err := h.lookupVerifiedTrialSignupRecord(verifiedToken) if err != nil { log.Warn().Err(err).Msg("trial signup verified state invalid") - h.renderTrialSignupPage(w, r, http.StatusBadRequest, trialSignupPageData{ - ErrorMessage: "That verification link is invalid or expired. Request a fresh email from Pulse and try again.", - ReturnTarget: "your Pulse instance", - }) + h.renderTrialSignupFailurePage(w, r, http.StatusBadRequest, trialSignupFailureDataForPage( + h.cfg, + trialSignupPageData{ReturnTarget: "your Pulse instance"}, + trialSignupFailureInvalidLink, + "That verification link is invalid or expired. Return to Pulse to request a fresh backup email.", + )) return } @@ -772,14 +786,24 @@ func (h *TrialSignupHandlers) HandleCheckout(w http.ResponseWriter, r *http.Requ verifiedToken := strings.TrimSpace(r.FormValue("verified_token")) record := &TrialSignupRecord{} data := trialSignupPageData{} + unavailable := func(status int, message string) { + h.renderTrialSignupFailurePage(w, r, status, trialSignupFailureDataForPage( + h.cfg, + data, + trialSignupFailureUnavailable, + message, + )) + } if verifiedToken != "" { verifiedRecord, err := h.lookupVerifiedTrialSignupRecord(verifiedToken) if err != nil { log.Warn().Err(err).Msg("trial signup checkout requested without valid verified token") - h.renderTrialSignupPage(w, r, http.StatusBadRequest, trialSignupPageData{ - ErrorMessage: "That backup link is invalid or expired. Restart from Pulse to create a fresh checkout session.", - ReturnTarget: "your Pulse instance", - }) + h.renderTrialSignupFailurePage(w, r, http.StatusBadRequest, trialSignupFailureDataForPage( + h.cfg, + trialSignupPageData{ReturnTarget: "your Pulse instance"}, + trialSignupFailureInvalidLink, + "That backup link is invalid or expired. Return to Pulse to create a fresh secure trial session.", + )) return } record = verifiedRecord @@ -830,20 +854,22 @@ func (h *TrialSignupHandlers) HandleCheckout(w http.ResponseWriter, r *http.Requ return } if h.verificationStore == nil { - data.ErrorMessage = "Trial checkout is unavailable right now. Please try again." - h.renderTrialSignupPage(w, r, http.StatusServiceUnavailable, data) + unavailable(http.StatusServiceUnavailable, "Trial checkout is unavailable right now. Please try again.") return } conflict, err := h.verificationStore.FindIssuedTrialConflict(data.Email, data.Company) if err != nil { log.Error().Err(err).Str("email", data.Email).Msg("trial signup issuance lookup failed") - data.ErrorMessage = "Unable to validate trial eligibility right now. Please try again." - h.renderTrialSignupPage(w, r, http.StatusInternalServerError, data) + unavailable(http.StatusInternalServerError, "Unable to validate trial eligibility right now. Please try again.") return } if conflict != nil { - data.ErrorMessage = trialSignupIssuanceConflictMessage(conflict) - h.renderTrialSignupPage(w, r, http.StatusConflict, data) + h.renderTrialSignupFailurePage(w, r, http.StatusConflict, trialSignupFailureDataForPage( + h.cfg, + data, + trialSignupFailureConflict, + trialSignupIssuanceConflictMessage(conflict), + )) return } record = &TrialSignupRecord{ @@ -857,8 +883,7 @@ func (h *TrialSignupHandlers) HandleCheckout(w http.ResponseWriter, r *http.Requ } if err := h.verificationStore.CreateCheckoutRequest(record); err != nil { log.Error().Err(err).Str("email", data.Email).Msg("trial signup checkout request creation failed") - data.ErrorMessage = "Unable to prepare checkout. Please try again." - h.renderTrialSignupPage(w, r, http.StatusInternalServerError, data) + unavailable(http.StatusInternalServerError, "Unable to prepare checkout. Please try again.") return } } @@ -869,8 +894,7 @@ func (h *TrialSignupHandlers) HandleCheckout(w http.ResponseWriter, r *http.Requ return } if strings.TrimSpace(h.cfg.StripeAPIKey) == "" || strings.TrimSpace(h.cfg.TrialSignupPriceID) == "" { - data.ErrorMessage = "Checkout is not configured yet. Please contact support." - h.renderTrialSignupPage(w, r, http.StatusServiceUnavailable, data) + unavailable(http.StatusServiceUnavailable, "Checkout is not configured yet. Please contact support.") return } @@ -917,8 +941,7 @@ func (h *TrialSignupHandlers) HandleCheckout(w http.ResponseWriter, r *http.Requ Str("org_id", data.OrgID). Str("email", data.Email). Msg("trial signup checkout session creation failed") - data.ErrorMessage = "Unable to create checkout session. Please try again." - h.renderTrialSignupPage(w, r, http.StatusBadGateway, data) + unavailable(http.StatusBadGateway, "Unable to create checkout session. Please try again.") return } if err := h.verificationStore.MarkCheckoutStarted(record.ID, session.ID, h.now().UTC()); err != nil { @@ -1394,6 +1417,10 @@ func trialSignupFailureDataForPage(cfg *CPConfig, data trialSignupPageData, kind title = "Trial already used" statusMessage = "This trial request cannot be restarted for the same recovery contact or organization." finePrint = "Upgrade the existing account or contact support if you need help reconciling prior trial usage." + case trialSignupFailureInvalidLink: + title = "Backup link expired" + statusMessage = "This backup link can no longer continue the hosted trial handoff." + finePrint = "Return to Pulse to request a fresh backup email or restart the secure trial setup." case trialSignupFailureUnavailable: title = "Trial setup is unavailable" statusMessage = "Pulse could not finish the secure trial handoff right now." diff --git a/internal/cloudcp/trial_signup_handlers_test.go b/internal/cloudcp/trial_signup_handlers_test.go index 31a6feece..1d3ae1011 100644 --- a/internal/cloudcp/trial_signup_handlers_test.go +++ b/internal/cloudcp/trial_signup_handlers_test.go @@ -160,6 +160,36 @@ func TestTrialSignupHandleRequestVerificationRejectsPendingVerificationResend(t } } +func TestTrialSignupHandleRequestVerificationReturnsUnavailableOutcomeWhenEmailVerificationNotConfigured(t *testing.T) { + h, _, _ := newTrialSignupTestHandler(t) + h.emailSender = nil + + form := url.Values{ + "org_id": {"default"}, + "return_url": {"https://pulse.example.com/auth/trial-activate"}, + "instance_token": {"tsi_test"}, + "name": {"Test User"}, + "email": {"owner@example.com"}, + "company": {"Pulse Labs"}, + } + req := httptest.NewRequest(http.MethodPost, "/api/trial-signup/request-verification", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + + h.HandleRequestVerification(rec, req) + + if rec.Code != http.StatusServiceUnavailable { + t.Fatalf("status=%d, want %d body=%q", rec.Code, http.StatusServiceUnavailable, rec.Body.String()) + } + assertTrialSignupFailurePageContains(t, rec.Body.String(), + "Trial setup is unavailable", + "Email verification is not configured yet. Please contact support.", + "pulse.example.com", + "Pulse could not finish the secure trial handoff right now.", + ) + assertTrialSignupFailurePageOmits(t, rec.Body.String(), "Continue To Secure Trial Setup", "