Files
projectsend/tests/Feature/Platform/SecretsNotFlashedTest.php
T
denkfabrik-li 35d68a792b Stop a rejected settings form flashing the credential it carried
When validation fails, Laravel flashes the request's input into the
session so the form can be repopulated. Its exclusion list is
current_password, password and password_confirmation -- written for the
login and password screens, and covering none of the credentials the
system settings screens take. `dontFlash` did not appear anywhere in this
repository.

So every one of these went into the session in clear the moment its form
was rejected:

    secret          ExternalStorageSettingsController   (S3 secret access key)
    key_file        ExternalStorageSettingsController   (GCS service account JSON)
    bind_password   LdapSettingsController
    client_secret   SocialLoginSettingsController, EmailSettingsController
    secret_key      CaptchaSettingsController

Each is stored with an `encrypted` cast, and config/session.php puts
sessions in the database with `encrypt => false` -- so the rejected save
wrote in clear into the same database the cast exists to protect.

The sharpest one is key_file. serviceAccountKeyRule() exists to catch a
paste that lost its last line, which makes "the request carrying a
service account private key" and "the request that fails validation" the
same request more often than not.

dontFlash() merges rather than replaces, so the framework's three stay.

The cost is that these five come back blank after a failed save. That is
already what they do after a successful one -- every screen here treats
them as write-only, and a blank means "keep what is stored" -- so the
behaviour is now the same either way instead of only on success.

Tests: one per field, each submitting a form that fails validation while
carrying a secret, then reading the old input back the way the form
would. All five fail against the unmodified bootstrap/app.php. A sixth
pins that the framework's own three are still excluded, and each
assertion checks a neighbouring non-secret field still comes back, so
this cannot pass by flashing nothing at all.

Note for the record: this is testable in the existing harness after all.
phpunit.xml sets SESSION_DRIVER=array, but old input is written to the
session whatever the driver backs it, so getOldInput() sees exactly what
a database session would have stored.
2026-08-29 00:02:21 +02:00

119 lines
4.7 KiB
PHP

<?php
declare(strict_types=1);
use App\Models\User;
/**
* A failed save must not write the credential back out in clear.
*
* Laravel flashes the request's input into the session when validation
* fails, so the form can be repopulated. Its own exclusion list is
* current_password / password / password_confirmation -- written for the
* login and password forms, and covering none of the credentials the
* settings screens take. config/session.php stores sessions in the
* database by default and does not encrypt them, so anything flashed
* lands in clear in the same database the `encrypted` casts exist to
* protect.
*
* Each of these submits a form that fails validation *and* carries a
* secret, then reads the old input back the way the form would.
*/
beforeEach(function () {
$this->admin = User::factory()->create();
});
test('a rejected storage form does not flash the secret access key', function () {
// Fails on the missing bucket, which is required.
$this->actingAs($this->admin)->patch('/system/settings/storage', [
'active' => true,
'provider' => 's3',
'access_key' => 'AKIAEXAMPLE',
'secret' => 'super-secret-access-key',
'region' => 'us-east-1',
'use_path_style' => false,
])->assertSessionHasErrors('bucket');
expect(session()->getOldInput('secret'))->toBeNull()
// The rest of the form still comes back, or the screen would clear
// itself every time somebody mistypes one field.
->and(session()->getOldInput('region'))->toBe('us-east-1');
});
test('a rejected storage form does not flash the service account key file', function () {
// The sharpest case: serviceAccountKeyRule() is what catches a paste
// that lost its last line, so the request most likely to fail here is
// the one carrying a private key.
$key = json_encode([
'type' => 'service_account',
'project_id' => 'example',
'private_key_id' => 'abc',
'private_key' => "-----BEGIN PRIVATE KEY-----\nMIIBVAIBADAN\n-----END PRIVATE KEY-----\n",
'client_email' => 'svc@example.iam.gserviceaccount.com',
]);
$this->actingAs($this->admin)->patch('/system/settings/storage', [
'active' => true,
'provider' => 'gcs',
'key_file' => $key,
'use_path_style' => false,
])->assertSessionHasErrors('bucket');
expect(session()->getOldInput('key_file'))->toBeNull()
->and(json_encode(session()->all()))->not->toContain('BEGIN PRIVATE KEY');
});
test('a rejected ldap form does not flash the bind password', function () {
// Fails on the missing email_attribute, which is required.
$this->actingAs($this->admin)->patch('/system/settings/ldap', [
'active' => true,
'port' => 636,
'encryption' => 'ssl',
'bind_dn' => 'cn=admin,dc=example,dc=test',
'bind_password' => 'super-secret-bind-password',
'name_attribute' => 'cn',
'auto_provision' => false,
'auto_approve' => false,
])->assertSessionHasErrors('email_attribute');
expect(session()->getOldInput('bind_password'))->toBeNull()
->and(session()->getOldInput('bind_dn'))->toBe('cn=admin,dc=example,dc=test');
});
test('a rejected social login form does not flash the client secret', function () {
$this->actingAs($this->admin)->patch('/system/settings/social-login/google', [
'enabled' => true,
'client_id' => '',
'client_secret' => 'super-secret-client-secret',
])->assertSessionHasErrors();
expect(session()->getOldInput('client_secret'))->toBeNull();
});
test('a rejected captcha form does not flash the secret key', function () {
// Fails on the missing site_key, which turnstile requires.
$this->actingAs($this->admin)->patch('/system/settings/captcha', [
'provider' => 'turnstile',
'secret_key' => 'super-secret-captcha-key',
'on_login' => false,
'on_registration' => false,
'on_password_reset' => false,
'on_public_comments' => false,
])->assertSessionHasErrors('site_key');
expect(session()->getOldInput('secret_key'))->toBeNull()
->and(session()->getOldInput('provider'))->toBe('turnstile');
});
test('the framework defaults are kept, not replaced', function () {
// dontFlash() merges, so adding to it must not drop password.
$this->actingAs($this->admin)->put('/settings/password', [
'current_password' => 'wrong-password',
'password' => 'a-brand-new-password',
'password_confirmation' => 'a-brand-new-password',
])->assertSessionHasErrors('current_password');
expect(session()->getOldInput('password'))->toBeNull()
->and(session()->getOldInput('current_password'))->toBeNull();
});