diff --git a/app/Livewire/Admin/EditExportTarget.php b/app/Livewire/Admin/EditExportTarget.php index 2233709..57195b5 100644 --- a/app/Livewire/Admin/EditExportTarget.php +++ b/app/Livewire/Admin/EditExportTarget.php @@ -3,7 +3,7 @@ namespace App\Livewire\Admin; use App\Models\ExportTarget; -use App\Services\Secrets\SecretVault; +use App\Services\Secrets\SecretCipher; use LivewireUI\Modal\ModalComponent; /** @@ -56,8 +56,7 @@ class EditExportTarget extends ModalComponent $this->port = $target->port ?: 22; $this->username = (string) $target->username; $this->active = (bool) $target->active; - $this->hasPassword = $target->secret_key !== null - && app(SecretVault::class)->has($target->secret_key); + $this->hasPassword = trim((string) $target->secret_key) !== ''; } public function save() @@ -102,10 +101,15 @@ class EditExportTarget extends ModalComponent $target->save(); + // Encrypted onto the row rather than into the vault. The vault keeps a + // registry of fixed, documented credentials and refuses anything else, + // which is right for it — a per-destination password is not one of + // those, and forcing it in would make the registry meaningless. Same + // cipher, same key, so it is no more readable in a dump than the rest. if ($this->driver === ExportTarget::SFTP && $this->password !== '') { - $key = 'export_target.'.$target->uuid; - app(SecretVault::class)->put($key, $this->password, auth('operator')->user()); - $target->forceFill(['secret_key' => $key])->save(); + $target->forceFill([ + 'secret_key' => app(SecretCipher::class)->encrypt($this->password), + ])->save(); } $this->dispatch('notify', message: __('finance.targets_saved')); diff --git a/app/Services/Billing/InvoiceArchive.php b/app/Services/Billing/InvoiceArchive.php index 01e70fb..b8fbd01 100644 --- a/app/Services/Billing/InvoiceArchive.php +++ b/app/Services/Billing/InvoiceArchive.php @@ -4,7 +4,7 @@ namespace App\Services\Billing; use App\Models\ExportTarget; use App\Models\Invoice; -use App\Services\Secrets\SecretVault; +use App\Services\Secrets\SecretCipher; use Illuminate\Filesystem\FilesystemAdapter; use Illuminate\Support\Facades\Storage; use RuntimeException; @@ -21,9 +21,9 @@ use RuntimeException; * thing stays testable against a temporary directory. * * 'sftp' is for something reachable over the network with credentials — a - * Hetzner Storage Box being the obvious one. The password lives in the secret - * vault and only its KEY is stored here: a credential in a settings table is a - * credential in every database dump. + * Hetzner Storage Box being the obvious one. The password is encrypted on the + * row with the same cipher the credential vault uses: a credential in a + * settings table in the clear is a credential in every database dump. * * Neither is allowed to create its own root. See ensureRoot(). */ @@ -31,7 +31,7 @@ final class InvoiceArchive { public function __construct( private readonly InvoiceRenderer $renderer, - private readonly SecretVault $vault, + private readonly SecretCipher $cipher, ) {} /** @@ -117,10 +117,19 @@ final class InvoiceArchive private function disk(ExportTarget $target): FilesystemAdapter { if ($target->driver === ExportTarget::SFTP) { - $password = $target->secret_key ? $this->vault->get($target->secret_key) : null; + $stored = trim((string) $target->secret_key); - if ($password === null || $password === '') { - throw new RuntimeException("No usable credential for {$target->name}."); + if ($stored === '') { + throw new RuntimeException("No credential stored for {$target->name}."); + } + + $password = $this->cipher->decrypt($stored); + + if ($password === '') { + // A rotated SECRETS_KEY makes every stored credential + // unreadable. Saying so beats an authentication failure against + // a host that is perfectly fine. + throw new RuntimeException("The credential for {$target->name} cannot be decrypted — has SECRETS_KEY changed?"); } return Storage::build([ diff --git a/tests/Feature/Billing/ExportTargetFormTest.php b/tests/Feature/Billing/ExportTargetFormTest.php new file mode 100644 index 0000000..10adc91 --- /dev/null +++ b/tests/Feature/Billing/ExportTargetFormTest.php @@ -0,0 +1,43 @@ +test(EditExportTarget::class); + + // A mounted directory has no host: the mount is made on the server with + // mount(8), and the application only names the directory it writes into. + $c->assertDontSee(__('finance.target.host')) + ->assertSee(__('finance.target.path')); + + $c->set('driver', 'sftp') + ->assertSee(__('finance.target.host')) + ->assertSee(__('finance.target.port')) + ->assertSee(__('finance.target.username')) + ->assertSee(__('finance.target.password')); +}); + +it('stores an SFTP destination with its host and keeps the password out of the row', function () { + Livewire::actingAs(operator('Owner'), 'operator') + ->test(EditExportTarget::class) + ->set('name', 'Storage Box') + ->set('driver', 'sftp') + ->set('host', 'u123456.your-storagebox.de') + ->set('port', 23) + ->set('username', 'u123456') + ->set('password', 'geheim') + ->set('path', '/rechnungen') + ->call('save') + ->assertHasNoErrors(); + + $target = App\Models\ExportTarget::query()->where('name', 'Storage Box')->firstOrFail(); + + expect($target->host)->toBe('u123456.your-storagebox.de') + ->and($target->port)->toBe(23) + ->and($target->username)->toBe('u123456') + // Stored encrypted, never in the clear — a credential in a settings + // table in plain text is a credential in every database dump. + ->and($target->secret_key)->not->toBe('geheim') + ->and($target->secret_key)->not->toBeEmpty() + ->and(app(App\Services\Secrets\SecretCipher::class)->decrypt($target->secret_key))->toBe('geheim'); +});