diff --git a/.ai/lessons.md b/.ai/lessons.md index 01d0d48671..cefe025782 100644 --- a/.ai/lessons.md +++ b/.ai/lessons.md @@ -39,6 +39,7 @@ - Put destructive actions on the footer's left and primary actions last on the right. - Use shared section, helper, tooltip, and icon-button components instead of local variants. - Keep validation, preview, and save controls in a fixed footer when the body is large. +- `x-modal-confirmation` parses `submitAction` with `resources/js/modal-confirmation.js`: quoted arguments lose their quotes, unquoted ones arrive as text. Quote text that can contain commas or parentheses. ## Verify layered UI behavior visually - Inspect the real layout with all conditional elements visible, especially compound status badges. diff --git a/app/Livewire/Concerns/InteractsWithDnsProviders.php b/app/Livewire/Concerns/InteractsWithDnsProviders.php index 2107e44984..a82a152818 100644 --- a/app/Livewire/Concerns/InteractsWithDnsProviders.php +++ b/app/Livewire/Concerns/InteractsWithDnsProviders.php @@ -11,13 +11,17 @@ use App\Services\Dns\CloudflareDnsProvider; use App\Services\Dns\ManagedDnsRecordCleanup; use Illuminate\Contracts\Cache\LockTimeoutException; use Illuminate\Database\Eloquent\Model; +use Livewire\Attributes\Locked; +use RuntimeException; trait InteractsWithDnsProviders { public bool $showDnsProviderModal = false; + #[Locked] public array $dnsProviderProposals = []; + #[Locked] public array $dnsProviderConflicts = []; public bool $deleteManagedDns = true; @@ -42,10 +46,11 @@ trait InteractsWithDnsProviders public function createManagedDnsRecord(string $hostname, int $zoneId, ?string $content = null): void { $this->authorizeDnsProviderChange(); + $hostname = strtolower($hostname); $cloudflare = app(CloudflareDnsProvider::class); - $zone = $this->findTeamZone($zoneId); - $content ??= $this->serverIp; - if ($zone === null || blank($content) || filter_var($content, FILTER_VALIDATE_IP) === false) { + $zone = $this->findTeamZone($zoneId, $hostname); + $content = $this->dnsRecordContent($content); + if ($zone === null || $content === null) { $this->dispatch('error', 'No connected DNS provider or public server IP is available for this domain.'); return; @@ -80,7 +85,7 @@ trait InteractsWithDnsProviders return collect($urls)->contains(function (string $url) use ($provider): bool { $hostname = parse_url($url, PHP_URL_HOST); - return is_string($hostname) && $provider->findZones(currentTeam()->id, $hostname)->isNotEmpty(); + return is_string($hostname) && $provider->findZones($this->dnsTeamId(), $hostname)->isNotEmpty(); }); } @@ -124,7 +129,7 @@ trait InteractsWithDnsProviders $resource = $this->dnsResourceForHostname($proposal['hostname']); ConfigureDnsRecordJob::dispatch( - currentTeam()->id, + $this->dnsTeamId(), $zone->id, $resource?->getMorphClass(), $resource?->getKey(), @@ -147,11 +152,12 @@ trait InteractsWithDnsProviders public function replaceManagedDnsRecord(string $hostname, int $zoneId, string $password = ''): void { $this->authorizeDnsProviderChange(); + $hostname = strtolower($hostname); $key = $hostname.'|'.$zoneId; $conflict = $this->dnsProviderConflicts[$key] ?? null; - $zone = $this->findTeamZone($zoneId); - $content = $this->serverIp; - if ($conflict === null || $zone === null || blank($content) || filter_var($content, FILTER_VALIDATE_IP) === false) { + $zone = $this->findTeamZone($zoneId, $hostname); + $content = $this->dnsRecordContent($conflict['proposed'] ?? null); + if ($conflict === null || $zone === null || $content === null) { $this->dispatch('error', 'The DNS conflict is no longer available. Check the record again.'); return; @@ -181,8 +187,9 @@ trait InteractsWithDnsProviders { $provider = app(CloudflareDnsProvider::class); $hostnames ??= $this->allDomainHostnames(); - $managed = ManagedDnsRecord::query()->where('team_id', currentTeam()->id)->whereIn('name', $hostnames)->pluck('id', 'name'); - $this->dnsProviderProposals = collect($hostnames)->flatMap(fn (string $hostname) => $provider->findZones(currentTeam()->id, $hostname) + $teamId = $this->dnsTeamId(); + $managed = ManagedDnsRecord::query()->where('team_id', $teamId)->whereIn('name', $hostnames)->pluck('id', 'name'); + $this->dnsProviderProposals = collect($hostnames)->flatMap(fn (string $hostname) => $provider->findZones($teamId, $hostname) ->map(fn (DnsProviderZone $zone) => [ 'hostname' => $hostname, 'zone_id' => $zone->id, 'zone' => $zone->name, 'credential' => $zone->integrationToken->name, 'target' => (string) $this->serverIp, @@ -250,7 +257,7 @@ trait InteractsWithDnsProviders return; } - $result = app(ManagedDnsRecordCleanup::class)->releaseHostname($resource, $hostname, currentTeam()->id, $deleteRecord); + $result = app(ManagedDnsRecordCleanup::class)->releaseHostname($resource, $hostname, $this->dnsTeamId(), $deleteRecord); $this->reportManagedDnsRelease($result, 'removed'); } @@ -273,7 +280,7 @@ trait InteractsWithDnsProviders */ protected function releaseManagedDnsForEditedDomains(Model $resource, array $previousHostnames): void { - $result = app(ManagedDnsRecordCleanup::class)->releaseRemovedHostnames($resource, $previousHostnames, currentTeam()->id); + $result = app(ManagedDnsRecordCleanup::class)->releaseRemovedHostnames($resource, $previousHostnames, $this->dnsTeamId()); $this->reportManagedDnsRelease($result, 'updated'); } @@ -314,13 +321,46 @@ trait InteractsWithDnsProviders protected function authorizeDnsProviderChange(): void { - $this->authorize('update', property_exists($this, 'application') ? $this->application : $this->service); + $this->authorize('update', $this->dnsResource()); } - protected function findTeamZone(int $zoneId): ?DnsProviderZone + /** The application or service whose domains this component manages. */ + protected function dnsResource(): Model { + return property_exists($this, 'application') ? $this->application : $this->service; + } + + /** + * Team of the resource. The session team can differ: a user can switch teams in another tab + * while this component is still open. + */ + protected function dnsTeamId(): int + { + return $this->dnsResource()->team()?->id ?? throw new RuntimeException('The resource has no team.'); + } + + /** + * Zone of the resource team. With a hostname, only a zone that manages that hostname. + */ + protected function findTeamZone(int $zoneId, ?string $hostname = null): ?DnsProviderZone + { + if ($hostname !== null) { + return app(CloudflareDnsProvider::class)->findZones($this->dnsTeamId(), $hostname)->firstWhere('id', $zoneId); + } + return DnsProviderZone::query()->whereKey($zoneId) - ->whereHas('integrationToken', fn ($query) => $query->where('team_id', currentTeam()->id))->first(); + ->whereHas('integrationToken', fn ($query) => $query->where('team_id', $this->dnsTeamId()))->first(); + } + + /** + * Content of a DNS record for this resource: one of its server addresses (IPv4 or IPv6), + * the first one when none is requested. Any other address is rejected. + */ + protected function dnsRecordContent(?string $requested = null): ?string + { + $addresses = array_values(array_filter($this->serverIpsForDnsHints())); + + return $requested === null ? ($addresses[0] ?? null) : (in_array($requested, $addresses, true) ? $requested : null); } abstract protected function persistDomainDnsStatuses(): void; diff --git a/app/Livewire/Project/Application/Domains.php b/app/Livewire/Project/Application/Domains.php index 6803e0fd97..fef335ade0 100644 --- a/app/Livewire/Project/Application/Domains.php +++ b/app/Livewire/Project/Application/Domains.php @@ -17,6 +17,7 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; +use Livewire\Attributes\Locked; use Livewire\Component; class Domains extends Component @@ -122,9 +123,11 @@ class Domains extends Component public bool $dnsValidationEnabled = true; /** Resolved or literal IP users should point DNS at. */ + #[Locked] public ?string $serverIp = null; /** Raw server IP/hostname as configured (may be a hostname). */ + #[Locked] public ?string $serverIpConfigured = null; protected $listeners = [ diff --git a/app/Livewire/Project/Service/Domains.php b/app/Livewire/Project/Service/Domains.php index d51e5c6642..dfa7f3af60 100644 --- a/app/Livewire/Project/Service/Domains.php +++ b/app/Livewire/Project/Service/Domains.php @@ -18,6 +18,7 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; +use Livewire\Attributes\Locked; use Livewire\Component; class Domains extends Component @@ -110,8 +111,10 @@ class Domains extends Component public bool $dnsValidationEnabled = true; + #[Locked] public ?string $serverIp = null; + #[Locked] public ?string $serverIpConfigured = null; /** Pending save payload after conflict/port confirmation */ diff --git a/resources/js/app.js b/resources/js/app.js index 13a2b6778d..4a4691e06f 100644 --- a/resources/js/app.js +++ b/resources/js/app.js @@ -4,6 +4,10 @@ import { initializeSettingsSidebarAccordionComponent } from './settings-sidebar- import { initializeTerminalComponent } from './terminal.js'; import './traffic-globe.js'; import { registerLivewireRequestFailureHandler } from './livewire-request-failure.js'; +import { parseSubmitAction } from './modal-confirmation.js'; + +// Used by the modal-confirmation Blade component to call its submitAction. +window.parseModalSubmitAction = parseSubmitAction; document.addEventListener('livewire:init', () => { registerLivewireRequestFailureHandler(window.Livewire); diff --git a/resources/js/modal-confirmation.js b/resources/js/modal-confirmation.js new file mode 100644 index 0000000000..c2bad2b578 --- /dev/null +++ b/resources/js/modal-confirmation.js @@ -0,0 +1,51 @@ +/** + * Splits the submitAction of the modal-confirmation component, such as + * `replaceManagedDnsRecord('app.example.com', 4)`, into the Livewire method and its arguments. + * Quoted arguments ('…' or "…") lose their quotes; other arguments stay text, as before. + */ +export function parseSubmitAction(submitAction) { + const open = submitAction.indexOf('('); + if (open === -1) { + return { method: submitAction.trim(), params: [] }; + } + + const close = submitAction.lastIndexOf(')'); + const argumentText = submitAction.slice(open + 1, close > open ? close : undefined); + const params = []; + let current = ''; + let quote = null; + let quoted = false; + + const finish = () => { + const value = quoted ? current : current.trim(); + if (value !== '' || quoted) { + params.push(value); + } + current = ''; + quoted = false; + }; + + for (let index = 0; index < argumentText.length; index++) { + const character = argumentText[index]; + if (quote) { + if (character === '\\' && index + 1 < argumentText.length) { + current += argumentText[++index]; + } else if (character === quote) { + quote = null; + } else { + current += character; + } + } else if ((character === "'" || character === '"') && current.trim() === '') { + quote = character; + quoted = true; + current = ''; + } else if (character === ',') { + finish(); + } else if (!quoted) { + current += character; + } + } + finish(); + + return { method: submitAction.slice(0, open).trim(), params }; +} diff --git a/resources/js/modal-confirmation.test.js b/resources/js/modal-confirmation.test.js new file mode 100644 index 0000000000..7613da7029 --- /dev/null +++ b/resources/js/modal-confirmation.test.js @@ -0,0 +1,36 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { parseSubmitAction } from './modal-confirmation.js'; + +test('keeps unquoted arguments as text, as the modal always passed them', () => { + assert.deepEqual(parseSubmitAction('delete(5)'), { method: 'delete', params: ['5'] }); + assert.deepEqual(parseSubmitAction('changeSource(3, App\\Models\\GithubApp)'), { + method: 'changeSource', + params: ['3', 'App\\Models\\GithubApp'], + }); + assert.deepEqual(parseSubmitAction('resetDefaultLabels(true)'), { method: 'resetDefaultLabels', params: ['true'] }); +}); + +test('removes the quotes of quoted arguments', () => { + assert.deepEqual(parseSubmitAction("generateNginxConfiguration('spa')"), { + method: 'generateNginxConfiguration', + params: ['spa'], + }); + assert.deepEqual(parseSubmitAction("replaceManagedDnsRecord('app.example.com', 4)"), { + method: 'replaceManagedDnsRecord', + params: ['app.example.com', '4'], + }); + assert.deepEqual(parseSubmitAction('rename("staging")'), { method: 'rename', params: ['staging'] }); +}); + +test('keeps commas, parentheses and escaped quotes inside quoted arguments', () => { + assert.deepEqual(parseSubmitAction("save('a, b', 'c)d', 'it\\'s')"), { + method: 'save', + params: ['a, b', 'c)d', "it's"], + }); +}); + +test('passes no arguments for empty parentheses or a bare method name', () => { + assert.deepEqual(parseSubmitAction('cleanupDeleted()'), { method: 'cleanupDeleted', params: [] }); + assert.deepEqual(parseSubmitAction('delete'), { method: 'delete', params: [] }); +}); diff --git a/resources/views/components/modal-confirmation.blade.php b/resources/views/components/modal-confirmation.blade.php index 6b0624a3bb..427e309eb2 100644 --- a/resources/views/components/modal-confirmation.blade.php +++ b/resources/views/components/modal-confirmation.blade.php @@ -102,9 +102,7 @@ return Promise.resolve(true); } - const methodName = this.submitAction.split('(')[0]; - const paramsMatch = this.submitAction.match(/\((.*?)\)/); - const params = paramsMatch ? paramsMatch[1].split(',').map(param => param.trim()) : []; + const { method: methodName, params } = window.parseModalSubmitAction(this.submitAction); // Always pass password parameter (empty string if password confirmation is skipped) // This ensures consistent method signature for backend Livewire methods diff --git a/resources/views/livewire/project/shared/dns-provider-management.blade.php b/resources/views/livewire/project/shared/dns-provider-management.blade.php index b33d073e13..e41d0f3ab2 100644 --- a/resources/views/livewire/project/shared/dns-provider-management.blade.php +++ b/resources/views/livewire/project/shared/dns-provider-management.blade.php @@ -28,7 +28,7 @@ @php($conflict = $dnsProviderConflicts[$key])
Currently {{ $conflict['current'] }} diff --git a/tests/Feature/ApplicationDomainsTest.php b/tests/Feature/ApplicationDomainsTest.php index 41c30d8926..6ccdf8aa53 100644 --- a/tests/Feature/ApplicationDomainsTest.php +++ b/tests/Feature/ApplicationDomainsTest.php @@ -1358,55 +1358,42 @@ it('hides dns check buttons from members', function () { ->assertDontSee('Check DNS'); }); +function openDnsProviderModalWithZone(Team $team, Application $application): mixed +{ + $token = IntegrationToken::factory()->for($team)->create(['provider' => 'cloudflare', 'name' => 'Cloudflare', 'token' => 'secret']); + DnsProviderZone::factory()->for($token)->create(['provider_zone_id' => 'zone-1', 'name' => 'example.com']); + $application->update(['fqdn' => 'https://app.example.com']); + + return Livewire::test(Domains::class, ['application' => $application->fresh()])->call('openDnsProviderModal'); +} + it('disables create dns record for members and hides replace confirmation', function () { - $this->team->members()->updateExistingPivot($this->user->id, ['role' => 'member']); - $this->actingAs($this->user->fresh()); + Http::fake(['https://api.cloudflare.com/client/v4/zones/zone-1/dns_records?*' => Http::response([ + 'success' => true, + 'result' => [['id' => 'rec-1', 'type' => 'A', 'name' => 'app.example.com', 'content' => '198.51.100.10']], + ])]); + $setRole = function (string $role): void { + $this->team->members()->updateExistingPivot($this->user->id, ['role' => $role]); + $this->actingAs($this->user->fresh()); + }; - $proposal = [ - 'hostname' => 'app.example.com', - 'zone_id' => 1, - 'zone' => 'example.com', - 'credential' => 'Cloudflare', - 'target' => '203.0.113.10', - 'managed' => false, - ]; - - $createHtml = Livewire::test(Domains::class, ['application' => $this->application->fresh()]) - ->set('showDnsProviderModal', true) - ->set('dnsProviderProposals', [$proposal]) - ->html(); - - expect($createHtml)->toContain('Create DNS record') + // The owner opens the DNS dialog, then becomes a member while the page is still open. + $createPage = openDnsProviderModalWithZone($this->team, $this->application); + $setRole('member'); + expect($createPage->call('$refresh')->html())->toContain('Create DNS record') ->toMatch('/]*\sdisabled(?:[=\s>])[^>]*>.*?Create DNS record/s'); - $replaceHtml = Livewire::test(Domains::class, ['application' => $this->application->fresh()]) - ->set('showDnsProviderModal', true) - ->set('dnsProviderProposals', [$proposal]) - ->set('dnsProviderConflicts', [ - 'app.example.com|1' => [ - 'record_id' => 'rec-1', - 'current' => '198.51.100.10', - 'proposed' => '203.0.113.10', - ], - ]) - ->html(); - - expect($replaceHtml)->toContain('Currently 198.51.100.10') + $setRole('owner'); + $replacePage = Livewire::test(Domains::class, ['application' => $this->application->fresh()]) + ->call('openDnsProviderModal') + ->call('createManagedDnsRecord', 'app.example.com', DnsProviderZone::query()->value('id')); + $setRole('member'); + expect($replacePage->call('$refresh')->html())->toContain('Currently 198.51.100.10') ->toMatch('/]*\sdisabled(?:[=\s>])[^>]*>.*?Replace record/s'); }); it('shows create dns record enabled for owners', function () { - $html = Livewire::test(Domains::class, ['application' => $this->application->fresh()]) - ->set('showDnsProviderModal', true) - ->set('dnsProviderProposals', [[ - 'hostname' => 'app.example.com', - 'zone_id' => 1, - 'zone' => 'example.com', - 'credential' => 'Cloudflare', - 'target' => '203.0.113.10', - 'managed' => false, - ]]) - ->html(); + $html = openDnsProviderModalWithZone($this->team, $this->application)->html(); expect($html)->toContain('Create DNS record') ->not->toMatch('/]*\sdisabled(?:[=\s>])[^>]*>.*?Create DNS record/s'); diff --git a/tests/Feature/DnsProviderManagementTest.php b/tests/Feature/DnsProviderManagementTest.php index 04f1d9d331..2e88e4d123 100644 --- a/tests/Feature/DnsProviderManagementTest.php +++ b/tests/Feature/DnsProviderManagementTest.php @@ -28,6 +28,7 @@ use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Queue; use Illuminate\Support\Str; +use Livewire\Features\SupportLockedProperties\CannotUpdateLockedPropertyException; use Livewire\Livewire; uses(RefreshDatabase::class); @@ -604,13 +605,10 @@ test('replacing a managed dns record uses the server ip and live cloudflare reco ]); Livewire::test(Domains::class, ['application' => $application->fresh()]) - ->set('dnsProviderConflicts', [ - 'app.example.com|'.$zone->id => [ - 'record_id' => 'record-1', - 'current' => '198.51.100.50', - 'proposed' => '198.51.100.1', - ], - ]) + ->call('createManagedDnsRecord', 'app.example.com', $zone->id) + ->assertSet('dnsProviderConflicts', ['app.example.com|'.$zone->id => [ + 'record_id' => 'record-1', 'current' => '198.51.100.50', 'proposed' => '203.0.113.10', + ]]) ->call('replaceManagedDnsRecord', 'app.example.com', $zone->id) ->assertDispatched('success', 'DNS record replaced for app.example.com.'); @@ -620,34 +618,46 @@ test('replacing a managed dns record uses the server ip and live cloudflare reco expect(ManagedDnsRecord::query()->where('name', 'app.example.com')->where('content', '203.0.113.10')->where('owned', false)->exists())->toBeTrue(); }); -test('replacing a managed dns record ignores a tampered conflict record id', function () { - ['application' => $application, 'zone' => $zone] = prepareManagedDnsApplication(); +test('dns provider state cannot be changed from the browser', function (string $property, mixed $value) { + ['application' => $application] = prepareManagedDnsApplication(); - Http::fake([ - 'https://api.cloudflare.com/client/v4/zones/zone-1/dns_records?*' => Http::response([ - 'success' => true, - 'result' => [['id' => 'record-1', 'type' => 'A', 'name' => 'app.example.com', 'content' => '198.51.100.50']], - ]), - 'https://api.cloudflare.com/client/v4/zones/zone-1/dns_records/*' => Http::response([ - 'success' => true, 'result' => ['id' => 'record-other'], - ]), - ]); + Livewire::test(Domains::class, ['application' => $application->fresh()])->set($property, $value); +})->with([ + 'server ip' => ['serverIp', '6.6.6.6'], + 'configured server ip' => ['serverIpConfigured', '6.6.6.6'], + 'conflicts' => ['dnsProviderConflicts', ['app.example.com|1' => ['record_id' => 'record-other', 'current' => 'x', 'proposed' => '6.6.6.6']]], + 'proposals' => ['dnsProviderProposals', [['hostname' => 'www.victim.com', 'zone_id' => 1]]], +])->throws(CannotUpdateLockedPropertyException::class); + +test('dns records use the zones of the resource team, not the session team', function () { + ['application' => $application] = prepareManagedDnsApplication(); + $otherTeam = Team::factory()->create(); + $otherTeam->members()->attach(auth()->id(), ['role' => 'member']); + $otherToken = IntegrationToken::factory()->for($otherTeam)->create(['provider' => 'cloudflare', 'token' => 'other-secret']); + $otherZone = DnsProviderZone::factory()->for($otherToken)->create(['provider_zone_id' => 'zone-other', 'name' => 'victim.com']); + Http::fake(); + + $component = Livewire::test(Domains::class, ['application' => $application->fresh()]); + // The user switches to the other team in another browser tab. + session(['currentTeam' => $otherTeam]); + + $component->call('createManagedDnsRecord', 'www.victim.com', $otherZone->id) + ->assertDispatched('error', 'No connected DNS provider or public server IP is available for this domain.'); + Http::assertNothingSent(); +}); + +test('dns records point only at an address of the resource server in a zone of the hostname', function (string $hostname, ?string $content) { + ['application' => $application, 'zone' => $zone] = prepareManagedDnsApplication(); + Http::fake(); Livewire::test(Domains::class, ['application' => $application->fresh()]) - ->set('dnsProviderConflicts', [ - 'app.example.com|'.$zone->id => [ - 'record_id' => 'record-other', - 'current' => '198.51.100.50', - 'proposed' => '198.51.100.1', - ], - ]) - ->call('replaceManagedDnsRecord', 'app.example.com', $zone->id) - ->assertDispatched('error', 'The DNS conflict is no longer available. Check the record again.') - ->assertSet('dnsProviderConflicts', []); - - Http::assertNotSent(fn ($request) => in_array($request->method(), ['PUT', 'PATCH'], true)); - expect(ManagedDnsRecord::query()->where('name', 'app.example.com')->exists())->toBeFalse(); -}); + ->call('createManagedDnsRecord', $hostname, $zone->id, $content) + ->assertDispatched('error', 'No connected DNS provider or public server IP is available for this domain.'); + Http::assertNothingSent(); +})->with([ + 'another ip' => ['app.example.com', '6.6.6.6'], + 'hostname outside the zone' => ['www.other.org', null], +]); /** * @return array{application: Application, zone: DnsProviderZone}