diff --git a/app/Console/Commands/PublishProcessingAgreement.php b/app/Console/Commands/PublishProcessingAgreement.php index 12a6cd4..1e69d9f 100644 --- a/app/Console/Commands/PublishProcessingAgreement.php +++ b/app/Console/Commands/PublishProcessingAgreement.php @@ -41,8 +41,19 @@ class PublishProcessingAgreement extends Command $agreementPath = 'dpa/av-vertrag-'.$this->slug($version).'.pdf'; $measuresPath = 'dpa/tom-'.$this->slug($version).'.pdf'; - Storage::disk('local')->put($agreementPath, $renderer->agreement($version)); - Storage::disk('local')->put($measuresPath, $renderer->measures($version)); + // Visibility "public" is about the FILE MODE, not about the web: this + // disk's root is outside the document root and nothing here becomes + // reachable by URL. What it changes is 0755/0644 instead of 0700/0600 — + // and that is the difference between a document the web process can read + // and one it cannot. + // + // It matters because this command is normally run as root (an artisan + // call in a container is root; PHP-FPM is www-data). The first run left + // a directory only root could enter, the route's exists() check answered + // false, and every link on the page 404'd with nothing in any log to say + // why. + Storage::disk('local')->put($agreementPath, $renderer->agreement($version), 'public'); + Storage::disk('local')->put($measuresPath, $renderer->measures($version), 'public'); $row = DpaVersion::query()->create([ 'version' => $version, @@ -55,6 +66,12 @@ class PublishProcessingAgreement extends Command 'published_at' => $this->option('draft') ? null : now(), ]); + foreach ([$agreementPath, $measuresPath] as $path) { + if (Storage::disk('local')->getVisibility($path) !== 'public') { + $this->warn("{$path} is not readable by the web process. Run this as the user PHP runs as, or fix the mode — the page will 404 otherwise."); + } + } + $this->info(($this->option('draft') ? 'Drafted' : 'Published').' version '.$row->version.'.'); $this->line(' '.$agreementPath.' ('.number_format(strlen(Storage::disk('local')->get($agreementPath)) / 1024, 0).' KB)'); $this->line(' '.$measuresPath.' ('.number_format(strlen(Storage::disk('local')->get($measuresPath)) / 1024, 0).' KB)'); diff --git a/app/Livewire/Admin/ProcessingAgreements.php b/app/Livewire/Admin/ProcessingAgreements.php index a3fb285..6a35497 100644 --- a/app/Livewire/Admin/ProcessingAgreements.php +++ b/app/Livewire/Admin/ProcessingAgreements.php @@ -62,13 +62,21 @@ class ProcessingAgreements extends Component 'measures' => 'nullable|file|mimes:pdf|max:10240', ]); - // The private disk. An agreement is not a public asset, and a guessable - // URL to one would be a list of who our customers are. + // The private DISK. An agreement is not a public asset, and a guessable + // URL to one would be a list of who our customers are — the root of this + // disk is outside the document root and nothing here is reachable except + // through the two routes that check who is asking. + // + // The visibility below is the file MODE, not the web: 0755/0644 rather + // than 0700/0600, so a document written by one process (an artisan run + // as root) stays readable by the other (PHP-FPM as www-data). Getting + // that wrong produced a directory nothing could enter and a page whose + // every link 404'd silently. $version = DpaVersion::query()->create([ 'version' => $data['version'], 'note' => $data['note'] ?: null, - 'agreement_path' => $this->agreement->store('dpa', 'local'), - 'measures_path' => $this->measures?->store('dpa', 'local'), + 'agreement_path' => $this->agreement->store('dpa', 'local', 'public'), + 'measures_path' => $this->measures?->store('dpa', 'local', 'public'), 'created_by' => Auth::guard('operator')->id(), ]); diff --git a/resources/views/livewire/admin/processing-agreements.blade.php b/resources/views/livewire/admin/processing-agreements.blade.php index 26d86cd..ce81e8a 100644 --- a/resources/views/livewire/admin/processing-agreements.blade.php +++ b/resources/views/livewire/admin/processing-agreements.blade.php @@ -129,9 +129,13 @@ {{ __('dpa_admin.file_measures') }} @endif + {{-- A download, so it says so and looks like + a control rather than a third link in + a row of two. --}} - {{ __('dpa_admin.download') }} + download + class="inline-flex items-center gap-1 rounded border border-line-strong bg-surface px-2 py-0.5 text-xs font-semibold text-body hover:bg-surface-hover"> + {{ __('dpa_admin.download') }} diff --git a/resources/views/livewire/settings.blade.php b/resources/views/livewire/settings.blade.php index 6ae2ea9..48b924c 100644 --- a/resources/views/livewire/settings.blade.php +++ b/resources/views/livewire/settings.blade.php @@ -379,7 +379,7 @@ {{-- Keep, not just read: the file carries the version in its name, so "which fassung did I agree to" is answerable from a downloads folder months later. --}} - + {{ __('dpa.download') }} {{ __('dpa.version', ['version' => $dpa->version]) }} diff --git a/tests/Feature/ProcessingAgreementTest.php b/tests/Feature/ProcessingAgreementTest.php index c102d98..d01c630 100644 --- a/tests/Feature/ProcessingAgreementTest.php +++ b/tests/Feature/ProcessingAgreementTest.php @@ -287,3 +287,32 @@ it('hands the file over under a name carrying the version', function () { ->assertOk() ->assertDownload('CluPilot-TOM-2.5.pdf'); }); + +it('leaves the documents readable by the process that has to serve them', function () { + // The failure this prevents had no error message anywhere: an artisan run in + // a container is root, PHP-FPM is www-data, and Laravel's local disk creates + // a private directory as 0700. The web process could not enter it, the + // route's exists() check answered false, and every link on the page 404'd + // while the file sat there perfectly intact. + // + // "public" here is the file MODE and nothing to do with the web: this disk's + // root is outside the document root either way. + Storage::fake('local'); + + $this->artisan('clupilot:publish-dpa', ['version' => '6.1'])->assertSuccessful(); + + $version = DpaVersion::query()->where('version', '6.1')->sole(); + + expect(Storage::disk('local')->getVisibility($version->agreement_path))->toBe('public') + ->and(Storage::disk('local')->getVisibility($version->measures_path))->toBe('public'); +}); + +it('serves the console download too, not only the portal one', function () { + publishedDpa('3.3'); + $version = DpaVersion::query()->where('version', '3.3')->sole(); + + $this->actingAs(operator('Owner'), 'operator') + ->get(route('admin.dpa.file', ['uuid' => $version->uuid, 'which' => 'agreement', 'download' => 1])) + ->assertOk() + ->assertDownload('CluPilot-AV-Vertrag-3.3.pdf'); +});