From d3b8d70f08aee1e8a82842cf559c74921894ece2 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 3 Mar 2026 12:28:16 +0100 Subject: [PATCH] fix(subscription): harden quantity updates and proxy trust behavior Centralize min/max server limits in Stripe quantity updates and wire them into Livewire subscription actions with price preview/update handling. Also improve host/proxy middleware behavior by trusting loopback hosts when FQDN is set and auto-enabling secure session cookies for HTTPS requests behind proxies when session.secure is unset. Includes feature tests for loopback trust and secure cookie auto-detection. --- .../Stripe/UpdateSubscriptionQuantity.php | 9 +- app/Http/Middleware/TrustHosts.php | 7 + app/Http/Middleware/TrustProxies.php | 22 ++ app/Livewire/Subscription/Actions.php | 49 +++ app/Models/Subscription.php | 14 + .../components/modal-confirmation.blade.php | 2 +- .../livewire/subscription/actions.blade.php | 305 ++++++++++++------ .../Feature/SecureCookieAutoDetectionTest.php | 64 ++++ tests/Feature/TrustHostsMiddlewareTest.php | 50 +++ 9 files changed, 423 insertions(+), 99 deletions(-) create mode 100644 tests/Feature/SecureCookieAutoDetectionTest.php diff --git a/app/Actions/Stripe/UpdateSubscriptionQuantity.php b/app/Actions/Stripe/UpdateSubscriptionQuantity.php index f22f56ad0..c181e988d 100644 --- a/app/Actions/Stripe/UpdateSubscriptionQuantity.php +++ b/app/Actions/Stripe/UpdateSubscriptionQuantity.php @@ -8,6 +8,10 @@ class UpdateSubscriptionQuantity { + public const int MAX_SERVER_LIMIT = 100; + + public const int MIN_SERVER_LIMIT = 2; + private StripeClient $stripe; public function __construct(?StripeClient $stripe = null) @@ -60,6 +64,7 @@ public function fetchPricePreview(Team $team, int $quantity): array $taxDescription = $taxRate->display_name.' ('.$taxRate->jurisdiction.') '.$taxRate->percentage.'%'; } } + // Fallback tax percentage from invoice totals - use tax_rate details when available for accuracy if ($taxPercentage === 0.0 && ($upcomingInvoice->tax ?? 0) > 0 && ($upcomingInvoice->subtotal ?? 0) > 0) { $taxPercentage = round(($upcomingInvoice->tax / $upcomingInvoice->subtotal) * 100, 2); } @@ -110,8 +115,8 @@ public function fetchPricePreview(Team $team, int $quantity): array */ public function execute(Team $team, int $quantity): array { - if ($quantity < 2) { - return ['success' => false, 'error' => 'Minimum server limit is 2.']; + if ($quantity < self::MIN_SERVER_LIMIT) { + return ['success' => false, 'error' => 'Minimum server limit is '.self::MIN_SERVER_LIMIT.'.']; } $subscription = $team->subscription; diff --git a/app/Http/Middleware/TrustHosts.php b/app/Http/Middleware/TrustHosts.php index f0b9d67f2..5fca583d9 100644 --- a/app/Http/Middleware/TrustHosts.php +++ b/app/Http/Middleware/TrustHosts.php @@ -91,6 +91,13 @@ public function hosts(): array // Trust all subdomains of APP_URL as fallback $trustedHosts[] = $this->allSubdomainsOfApplicationUrl(); + // Always trust loopback addresses so local access works even when FQDN is configured + foreach (['localhost', '127.0.0.1', '[::1]'] as $localHost) { + if (! in_array($localHost, $trustedHosts, true)) { + $trustedHosts[] = $localHost; + } + } + return array_filter($trustedHosts); } } diff --git a/app/Http/Middleware/TrustProxies.php b/app/Http/Middleware/TrustProxies.php index 559dd2fc3..a4764047b 100644 --- a/app/Http/Middleware/TrustProxies.php +++ b/app/Http/Middleware/TrustProxies.php @@ -25,4 +25,26 @@ class TrustProxies extends Middleware Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO | Request::HEADER_X_FORWARDED_AWS_ELB; + + /** + * Handle the request. + * + * Wraps $next so that after proxy headers are resolved (X-Forwarded-Proto processed), + * the Secure cookie flag is auto-enabled when the request is over HTTPS. + * This ensures session cookies are correctly marked Secure when behind an HTTPS + * reverse proxy (Cloudflare Tunnel, nginx, etc.) even when SESSION_SECURE_COOKIE + * is not explicitly set in .env. + */ + public function handle($request, \Closure $next) + { + return parent::handle($request, function ($request) use ($next) { + // At this point proxy headers have been applied to the request, + // so $request->secure() correctly reflects the actual protocol. + if ($request->secure() && config('session.secure') === null) { + config(['session.secure' => true]); + } + + return $next($request); + }); + } } diff --git a/app/Livewire/Subscription/Actions.php b/app/Livewire/Subscription/Actions.php index 4ac95adfb..2d5392240 100644 --- a/app/Livewire/Subscription/Actions.php +++ b/app/Livewire/Subscription/Actions.php @@ -5,6 +5,7 @@ use App\Actions\Stripe\CancelSubscriptionAtPeriodEnd; use App\Actions\Stripe\RefundSubscription; use App\Actions\Stripe\ResumeSubscription; +use App\Actions\Stripe\UpdateSubscriptionQuantity; use App\Models\Team; use Illuminate\Support\Facades\Hash; use Livewire\Component; @@ -14,6 +15,14 @@ class Actions extends Component { public $server_limits = 0; + public int $quantity = UpdateSubscriptionQuantity::MIN_SERVER_LIMIT; + + public int $minServerLimit = UpdateSubscriptionQuantity::MIN_SERVER_LIMIT; + + public int $maxServerLimit = UpdateSubscriptionQuantity::MAX_SERVER_LIMIT; + + public ?array $pricePreview = null; + public bool $isRefundEligible = false; public int $refundDaysRemaining = 0; @@ -25,6 +34,46 @@ class Actions extends Component public function mount(): void { $this->server_limits = Team::serverLimit(); + $this->quantity = (int) $this->server_limits; + } + + public function loadPricePreview(int $quantity): void + { + $this->quantity = $quantity; + $result = (new UpdateSubscriptionQuantity)->fetchPricePreview(currentTeam(), $quantity); + $this->pricePreview = $result['success'] ? $result['preview'] : null; + } + + // Password validation is intentionally skipped for quantity updates. + // Unlike refunds/cancellations, changing the server limit is a + // non-destructive, reversible billing adjustment (prorated by Stripe). + public function updateQuantity(string $password = ''): bool + { + if ($this->quantity < UpdateSubscriptionQuantity::MIN_SERVER_LIMIT) { + $this->dispatch('error', 'Minimum server limit is '.UpdateSubscriptionQuantity::MIN_SERVER_LIMIT.'.'); + $this->quantity = UpdateSubscriptionQuantity::MIN_SERVER_LIMIT; + + return true; + } + + if ($this->quantity === (int) $this->server_limits) { + return true; + } + + $result = (new UpdateSubscriptionQuantity)->execute(currentTeam(), $this->quantity); + + if ($result['success']) { + $this->server_limits = $this->quantity; + $this->pricePreview = null; + $this->dispatch('success', 'Server limit updated to '.$this->quantity.'.'); + + return true; + } + + $this->dispatch('error', $result['error'] ?? 'Failed to update server limit.'); + $this->quantity = (int) $this->server_limits; + + return true; } public function loadRefundEligibility(): void diff --git a/app/Models/Subscription.php b/app/Models/Subscription.php index 00f85ced5..69d7cbf0d 100644 --- a/app/Models/Subscription.php +++ b/app/Models/Subscription.php @@ -20,6 +20,20 @@ public function team() return $this->belongsTo(Team::class); } + public function billingInterval(): string + { + if ($this->stripe_plan_id) { + $configKey = collect(config('subscription')) + ->search($this->stripe_plan_id); + + if ($configKey && str($configKey)->contains('yearly')) { + return 'yearly'; + } + } + + return 'monthly'; + } + public function type() { if (isStripe()) { diff --git a/resources/views/components/modal-confirmation.blade.php b/resources/views/components/modal-confirmation.blade.php index b14888040..e77b52076 100644 --- a/resources/views/components/modal-confirmation.blade.php +++ b/resources/views/components/modal-confirmation.blade.php @@ -35,7 +35,7 @@ $skipPasswordConfirmation = shouldSkipPasswordConfirmation(); if ($temporaryDisableTwoStepConfirmation) { $disableTwoStepConfirmation = false; - $skipPasswordConfirmation = false; + // Password confirmation requirement is not affected by temporary two-step disable } // When password step is skipped, Step 2 becomes final - change button text from "Continue" to "Confirm" $effectiveStep2ButtonText = ($skipPasswordConfirmation && $step2ButtonText === 'Continue') ? 'Confirm' : $step2ButtonText; diff --git a/resources/views/livewire/subscription/actions.blade.php b/resources/views/livewire/subscription/actions.blade.php index 2f33d4f70..4b276aaf6 100644 --- a/resources/views/livewire/subscription/actions.blade.php +++ b/resources/views/livewire/subscription/actions.blade.php @@ -1,7 +1,39 @@
@if (subscriptionProvider() === 'stripe') {{-- Plan Overview --}} -
+

Plan Overview

{{-- Current Plan Card --}} @@ -25,11 +57,12 @@
- {{-- Server Limit Card --}} -
+ {{-- Paid Servers Card --}} +
Paid Servers
-
{{ $server_limits }}
-
Included in your plan
+
+
Click to adjust
{{-- Active Servers Card --}} @@ -49,103 +82,183 @@ class="p-5 rounded border dark:bg-coolgray-100 bg-white border-neutral-200 dark: subscription. Excess servers will be deactivated. @endif + + {{-- Adjust Server Limit Modal --}} + - {{-- Manage Plan --}} + {{-- Billing, Refund & Cancellation --}}
-

Manage Plan

-
-
- - - - - Manage Billing on Stripe - -
-

Change your server quantity, update payment methods, or view - invoices.

+

Manage Subscription

+
+ {{-- Billing --}} + + + + + Manage Billing on Stripe + + + {{-- Resume or Cancel --}} + @if (currentTeam()->subscription->stripe_cancel_at_period_end) + Resume Subscription + @else + + + @endif + + {{-- Refund --}} + @if ($refundCheckLoading) + + @elseif ($isRefundEligible && !currentTeam()->subscription->stripe_cancel_at_period_end) + + @endif
+ + {{-- Contextual notes --}} + @if ($isRefundEligible && !currentTeam()->subscription->stripe_cancel_at_period_end) +

Eligible for a full refund — {{ $refundDaysRemaining }} days remaining.

+ @elseif ($refundAlreadyUsed) +

Refund already processed. Each team is eligible for one refund only.

+ @endif + @if (currentTeam()->subscription->stripe_cancel_at_period_end) +

Your subscription is set to cancel at the end of the billing period.

+ @endif
- {{-- Refund Section --}} - @if ($refundCheckLoading) -
-

Refund

- -
- @elseif ($isRefundEligible && !currentTeam()->subscription->stripe_cancel_at_period_end) -
-

Refund

-
-
- -
-

You are eligible for a full refund. - {{ $refundDaysRemaining }} days remaining - in the 30-day refund window.

-
-
- @elseif ($refundAlreadyUsed) -
-

Refund

-

A refund has already been processed for this team. Each team is - eligible for one refund only to prevent abuse.

-
- @endif - - {{-- Resume / Cancel Subscription Section --}} - @if (currentTeam()->subscription->stripe_cancel_at_period_end) -
-

Resume Subscription

-
-
- Resume Subscription -
-

Your subscription is set to cancel at the end of the billing - period. Resume to continue your plan.

-
-
- @else -
-

Cancel Subscription

-
-
- - -
-

Cancel your subscription immediately or at the end of the - current billing period.

-
-
- @endif -
Need help? Contact us. diff --git a/tests/Feature/SecureCookieAutoDetectionTest.php b/tests/Feature/SecureCookieAutoDetectionTest.php new file mode 100644 index 000000000..4db0a7681 --- /dev/null +++ b/tests/Feature/SecureCookieAutoDetectionTest.php @@ -0,0 +1,64 @@ + 0], ['fqdn' => null]); + // Ensure session.secure starts unconfigured for each test + config(['session.secure' => null]); +}); + +it('sets session.secure to true when request arrives over HTTPS via proxy', function () { + $this->get('/login', [ + 'X-Forwarded-Proto' => 'https', + 'X-Forwarded-For' => '1.2.3.4', + ]); + + expect(config('session.secure'))->toBeTrue(); +}); + +it('does not set session.secure for plain HTTP requests', function () { + $this->get('/login'); + + expect(config('session.secure'))->toBeNull(); +}); + +it('does not override explicit SESSION_SECURE_COOKIE=false for HTTPS requests', function () { + config(['session.secure' => false]); + + $this->get('/login', [ + 'X-Forwarded-Proto' => 'https', + 'X-Forwarded-For' => '1.2.3.4', + ]); + + // Explicit false must not be overridden — our check is `=== null` only + expect(config('session.secure'))->toBeFalse(); +}); + +it('does not override explicit SESSION_SECURE_COOKIE=true', function () { + config(['session.secure' => true]); + + $this->get('/login'); + + expect(config('session.secure'))->toBeTrue(); +}); + +it('marks session cookie with Secure flag when accessed over HTTPS proxy', function () { + $response = $this->get('/login', [ + 'X-Forwarded-Proto' => 'https', + 'X-Forwarded-For' => '1.2.3.4', + ]); + + $response->assertSuccessful(); + + $cookieName = config('session.cookie'); + $sessionCookie = collect($response->headers->all('set-cookie')) + ->first(fn ($c) => str_contains($c, $cookieName)); + + expect($sessionCookie)->not->toBeNull() + ->and(strtolower($sessionCookie))->toContain('; secure'); +}); diff --git a/tests/Feature/TrustHostsMiddlewareTest.php b/tests/Feature/TrustHostsMiddlewareTest.php index b745259fe..5c60b30d6 100644 --- a/tests/Feature/TrustHostsMiddlewareTest.php +++ b/tests/Feature/TrustHostsMiddlewareTest.php @@ -286,6 +286,56 @@ expect($response->status())->not->toBe(400); }); +it('trusts localhost when FQDN is configured', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + $middleware = new TrustHosts($this->app); + $hosts = $middleware->hosts(); + + expect($hosts)->toContain('localhost'); +}); + +it('trusts 127.0.0.1 when FQDN is configured', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + $middleware = new TrustHosts($this->app); + $hosts = $middleware->hosts(); + + expect($hosts)->toContain('127.0.0.1'); +}); + +it('trusts IPv6 loopback when FQDN is configured', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + $middleware = new TrustHosts($this->app); + $hosts = $middleware->hosts(); + + expect($hosts)->toContain('[::1]'); +}); + +it('allows local access via localhost when FQDN is configured and request uses localhost host header', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + $response = $this->get('/', [ + 'Host' => 'localhost', + ]); + + // Should NOT be rejected as untrusted host (would be 400) + expect($response->status())->not->toBe(400); +}); + it('skips host validation for webhook endpoints', function () { // All webhook routes are under /webhooks/* prefix (see RouteServiceProvider) // and use cryptographic signature validation instead of host validation