Clear every mailbox's verification when the shared server config changes
saveServer() wrote host/port/encryption without touching any mailbox, so the console kept showing every mailbox as verified against a server nothing had tested since. Mirrors EditMailbox::save()'s existing guard on a single mailbox's own identity, just at server scope: only clears when a field actually changed, so re-saving unedited values leaves a real verification alone. Both places now delegate the actual clear to new Mailbox:: invalidateVerification()/invalidateAllVerifications() methods rather than touching last_verified_at directly, sharing the mutation. The "did this change" comparison stays separate in each caller - one diffs a loaded model's attributes, the other diffs persisted settings against incoming scalars, and forcing them through one function would add machinery no actual duplication justifies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>feat/mailboxes
parent
ebe29f8a5d
commit
729f57755a
|
|
@ -86,10 +86,33 @@ class Mail extends Component
|
|||
'encryption' => ['required', 'in:tls,ssl,none'],
|
||||
]);
|
||||
|
||||
// Codex R15#6, P2: last_verified_at on EVERY mailbox proves a test
|
||||
// against THIS server config specifically — the shared-server mirror
|
||||
// of EditMailbox::save()'s guard on a single mailbox's own address/
|
||||
// username/authenticates. Compared against the STORED values, not
|
||||
// just "did the operator touch the field": re-opening the card and
|
||||
// saving without editing anything must not wipe a legitimate
|
||||
// verification. Read before Settings::set() overwrites them below,
|
||||
// and deliberately not merged with EditMailbox's own comparison — that
|
||||
// one diffs a single loaded model's in-memory attributes, this one
|
||||
// diffs persisted Settings against incoming scalars for a config that
|
||||
// has no single row to load at all; forcing one comparison function
|
||||
// over both shapes would add an abstraction with nothing genuinely
|
||||
// shared to justify it. What IS shared is the actual clear itself:
|
||||
// both this method and EditMailbox::save() delegate to a Mailbox::
|
||||
// invalidate*() method rather than touching the column directly.
|
||||
$serverChanged = $this->host !== (string) Settings::get('mail.host', '')
|
||||
|| (int) $this->port !== (int) Settings::get('mail.port', 587)
|
||||
|| $this->encryption !== (string) Settings::get('mail.encryption', 'tls');
|
||||
|
||||
Settings::set('mail.host', $this->host);
|
||||
Settings::set('mail.port', (int) $this->port);
|
||||
Settings::set('mail.encryption', $this->encryption);
|
||||
|
||||
if ($serverChanged) {
|
||||
Mailbox::invalidateAllVerifications();
|
||||
}
|
||||
|
||||
$this->dispatch('notify', message: __('mail_settings.server_saved'));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -121,16 +121,20 @@ class EditMailbox extends ModalComponent
|
|||
// authenticates itself is the fourth: whether that identity gets
|
||||
// sent at all. Leaving last_verified_at standing after any of the
|
||||
// three would keep showing a successful verification for a
|
||||
// connection nothing has actually tested since.
|
||||
// connection nothing has actually tested since. Mailbox::
|
||||
// invalidateVerification() is the same clear Admin\Mail::saveServer()
|
||||
// uses for the server card, in its single-row shape — see that
|
||||
// method's own comment for why the two do NOT also share the "did
|
||||
// this actually change" comparison above this block.
|
||||
if ($box->address !== $oldAddress || $box->username !== $oldUsername || $box->authenticates !== $oldAuthenticates) {
|
||||
$box->last_verified_at = null;
|
||||
$box->invalidateVerification();
|
||||
}
|
||||
|
||||
// Empty means "leave it alone", not "delete it" — otherwise every edit
|
||||
// of an address would silently drop the password.
|
||||
if ($this->password !== '') {
|
||||
$box->password = $this->password;
|
||||
$box->last_verified_at = null;
|
||||
$box->invalidateVerification();
|
||||
}
|
||||
|
||||
$box->save();
|
||||
|
|
|
|||
|
|
@ -62,6 +62,32 @@ class Mailbox extends Model
|
|||
: $this->address;
|
||||
}
|
||||
|
||||
/**
|
||||
* last_verified_at proves a successful test against the config THIS
|
||||
* mailbox held at the time — an address, username or authenticates
|
||||
* change (or a new password) invalidates only this row. Sets the
|
||||
* attribute without saving: EditMailbox::save() calls this alongside
|
||||
* other attribute changes so the whole edit lands in one write, not two.
|
||||
*/
|
||||
public function invalidateVerification(): void
|
||||
{
|
||||
$this->last_verified_at = null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The mirror of invalidateVerification() for the server card
|
||||
* (Admin\Mail::saveServer()): host, port and encryption are shared by
|
||||
* every mailbox at once, not a per-row concept, so a change there
|
||||
* invalidates all of them in one write rather than each row deciding
|
||||
* independently — Codex R15#6, P2. Scoped to rows that still have
|
||||
* something to clear, so saving unrelated server settings does not bump
|
||||
* every mailbox's updated_at for nothing.
|
||||
*/
|
||||
public static function invalidateAllVerifications(): int
|
||||
{
|
||||
return static::query()->whereNotNull('last_verified_at')->update(['last_verified_at' => null]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Enough to send with: an address, and a password ONLY when this mailbox
|
||||
* actually authenticates.
|
||||
|
|
|
|||
|
|
@ -263,6 +263,115 @@ it('rejects an empty mail server host', function () {
|
|||
->assertHasErrors('host');
|
||||
});
|
||||
|
||||
// --- Codex R15#6, P2: last_verified_at proves a mailbox was tested against a
|
||||
// SPECIFIC server config. Changing host, port or encryption here silently
|
||||
// invalidates that proof for every mailbox at once — the shared-server mirror
|
||||
// of EditMailbox::save()'s guard on a single mailbox's own address/username/
|
||||
// authenticates. Each test below pins host/port/encryption to a KNOWN old
|
||||
// value via Settings::set() before mounting, rather than trusting whatever
|
||||
// RefreshDatabase's baseline seed happens to contain — the point is to control
|
||||
// exactly which one field moves.
|
||||
|
||||
it("clears every mailbox's last_verified_at when the server host changes", function () {
|
||||
Settings::set('mail.host', 'old-relay.example.com');
|
||||
Settings::set('mail.port', 587);
|
||||
Settings::set('mail.encryption', 'tls');
|
||||
|
||||
$a = Mailbox::factory()->create(['last_verified_at' => now()]);
|
||||
$b = Mailbox::factory()->create(['last_verified_at' => now()]);
|
||||
|
||||
Livewire::actingAs(User::factory()->operator('Owner')->create())
|
||||
->test(MailPage::class)
|
||||
->set('confirmablePassword', 'password')
|
||||
->call('confirmPassword')
|
||||
->set('host', 'new-relay.example.com')
|
||||
->call('saveServer')
|
||||
->assertHasNoErrors();
|
||||
|
||||
expect($a->fresh()->last_verified_at)->toBeNull()
|
||||
->and($b->fresh()->last_verified_at)->toBeNull();
|
||||
});
|
||||
|
||||
it("clears every mailbox's last_verified_at when the server port changes", function () {
|
||||
Settings::set('mail.host', 'relay.example.com');
|
||||
Settings::set('mail.port', 587);
|
||||
Settings::set('mail.encryption', 'tls');
|
||||
|
||||
$box = Mailbox::factory()->create(['last_verified_at' => now()]);
|
||||
|
||||
Livewire::actingAs(User::factory()->operator('Owner')->create())
|
||||
->test(MailPage::class)
|
||||
->set('confirmablePassword', 'password')
|
||||
->call('confirmPassword')
|
||||
->set('port', 2525)
|
||||
->call('saveServer')
|
||||
->assertHasNoErrors();
|
||||
|
||||
expect($box->fresh()->last_verified_at)->toBeNull();
|
||||
});
|
||||
|
||||
it("clears every mailbox's last_verified_at when the server encryption changes", function () {
|
||||
Settings::set('mail.host', 'relay.example.com');
|
||||
Settings::set('mail.port', 587);
|
||||
Settings::set('mail.encryption', 'tls');
|
||||
|
||||
$box = Mailbox::factory()->create(['last_verified_at' => now()]);
|
||||
|
||||
Livewire::actingAs(User::factory()->operator('Owner')->create())
|
||||
->test(MailPage::class)
|
||||
->set('confirmablePassword', 'password')
|
||||
->call('confirmPassword')
|
||||
->set('encryption', 'ssl')
|
||||
->call('saveServer')
|
||||
->assertHasNoErrors();
|
||||
|
||||
expect($box->fresh()->last_verified_at)->toBeNull();
|
||||
});
|
||||
|
||||
it('keeps last_verified_at when the server card is saved with no actual change', function () {
|
||||
// The control case for the three tests above: an operator who opens the
|
||||
// card and saves without editing anything has changed nothing, and must
|
||||
// not wipe a legitimate verification. Sets every field to the SAME value
|
||||
// mount() already read, rather than never calling set() at all — this is
|
||||
// what "re-saving identical values" actually looks like through the form.
|
||||
Settings::set('mail.host', 'relay.example.com');
|
||||
Settings::set('mail.port', 587);
|
||||
Settings::set('mail.encryption', 'tls');
|
||||
|
||||
$box = Mailbox::factory()->create(['last_verified_at' => now()]);
|
||||
|
||||
Livewire::actingAs(User::factory()->operator('Owner')->create())
|
||||
->test(MailPage::class)
|
||||
->set('confirmablePassword', 'password')
|
||||
->call('confirmPassword')
|
||||
->set('host', 'relay.example.com')
|
||||
->set('port', 587)
|
||||
->set('encryption', 'tls')
|
||||
->call('saveServer')
|
||||
->assertHasNoErrors();
|
||||
|
||||
expect($box->fresh()->last_verified_at)->not->toBeNull();
|
||||
});
|
||||
|
||||
it('shows "not yet verified" instead of a blank cell once the server change clears it', function () {
|
||||
// The console-list-facing half of the finding: a cleared timestamp must
|
||||
// read as an honest, explicit "not yet verified", not a blank cell that
|
||||
// could be mistaken for a rendering error.
|
||||
Settings::set('mail.host', 'relay.example.com');
|
||||
Settings::set('mail.port', 587);
|
||||
Settings::set('mail.encryption', 'tls');
|
||||
|
||||
Mailbox::factory()->create(['key' => 'support', 'last_verified_at' => now()]);
|
||||
|
||||
Livewire::actingAs(User::factory()->operator('Owner')->create())
|
||||
->test(MailPage::class)
|
||||
->set('confirmablePassword', 'password')
|
||||
->call('confirmPassword')
|
||||
->set('host', 'new-relay.example.com')
|
||||
->call('saveServer')
|
||||
->assertSee(__('mail_settings.never_verified'));
|
||||
});
|
||||
|
||||
it('refuses to change the mail server without a recently confirmed password, capability notwithstanding', function () {
|
||||
// saveServer() writes mail.host — the platform's outbound relay for every
|
||||
// purpose mailbox at once. An Owner has mail.manage for the whole test;
|
||||
|
|
|
|||
Loading…
Reference in New Issue