19 Commits (fed4acf31ca28a24bd1c6eeb365225da388dc9fc)

Author	SHA1	Message	Date
nexxo	fed4acf31c	Accept terms instead of a start date, and fix the sender no server accepts ... **The test mail was rejected: "553 5.7.1 Sender address rejected: not owned by user no-reply@…".** Every purpose mailbox here has its own SMTP account, and a mail server lets an account send only from the address it owns. Mail::to() hands back a pending mail bound to the DEFAULT mailer and sendNow() then ignores the mailer the mailable asked for, so a mail addressed support@ went out over the no-reply@ login. It sends through the mailable's own mailer now, and the cloud-ready preview — the one mailable with no mailbox at all — takes the provisioning mailbox like the real notification does. Writing the test for that found the same bug in production code: **InvoiceMail and OrderConfirmationMail set their From to billing@ and never named a mailer**, so both went out over mail.default's no-reply@ login. On this installation no customer had ever received an invoice mail or an order confirmation; they rendered perfectly, queued without complaint and were refused at the door. MailSenderOwnershipTest now scans for the mismatch: a mail that takes a mailbox From must use that mailbox's mailer. **The box on the order page accepts the terms now**, not an immediate start. It used to carry the whole FAGG §16 sentence, which read like a choice between "now" and "in fourteen days" — and there is no second option. The terms are what regulate the sale, so they had to exist: resources/views/legal/terms.blade.php replaces the placeholder with fourteen sections written from what this software actually does — the delivery, the capacity queue, the full refund on withdrawal, the cancellation at period end, the deletion deadlines. The company data comes from CompanyProfile, so the page and the invoices cannot drift. No availability figure and no liability cap has been invented. No order goes through without it: the button is unusable until the box is ticked and says why, and CheckoutController still refuses server-side — the browser half refuses nothing. The request field is `terms_accepted`; the Stripe metadata key stays `immediate_start`, because a session opened before a deploy is paid after it and the webhook would find nothing under a new name. **"Wird der Account nach fünf Tagen gelöscht?"** Only an unconfirmed one. That was the whole of what we said, so the answer looked like yes. The second rule now exists and is stated: PruneDormantAccounts removes a confirmed account after a year when it never had a package — no customer record at all, which is where every order, contract and seven-year invoice hangs. A fortnight's notice goes out first, once, and `dormant_warned_at` is what permits the deletion: an account whose warning never went out is never removed. Signing in resets the clock, measured off the device rows because users has no last_login_at. Both deadlines are said in the portal settings, on the verification page, and in the terms — each reading the number off the command that enforces it. Also: the wordmark scan matched any element whose text merely BEGINS with the company name, which a paragraph of terms does. It looks for the lockup form now (no whitespace after the tag), which is what it was always about. The seven checkout tests in the parallel session's files were posting the old field name and now post the new one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-30 15:22:35 +02:00
nexxo	0e25fe88d4	Keep a register of what was sent, and answer the customer from here ... Three complaints, one shape: the console knew things and could not act on them, so the work happened somewhere else and left no trace. The header said "4 Hinweis(e)" and led nowhere. Whoever read it had to know that the list was a card further down the same page, and then go looking for the host or the failed run by hand. The count is a link now, every notice carries the page that shows the thing it is about, and the plural is a word rather than a bracket. Nothing recorded what this installation had sent. "Hat der Kunde die Zugangsdaten je bekommen, und wann?" was answered on the mail server — a different machine, a different program, and nothing an operator can put in front of somebody who says nothing ever arrived. Every mail now writes a row: when, to whom, which mailable, and the customer it belongs to. Written from MessageSent, so a row means the transport accepted it; that is the strongest thing an application can honestly claim, and a column called "delivered" would be pretending otherwise. Verification and reset mails are left out on purpose — they go to unconfirmed addresses and anybody who can type into a form can send them, so recording them would hand the register to whoever wants to fill it. And the customer list was the end of the road. There is a page behind it now: what they bought, what they asked in their own words, what we sent them, and a box to write the next message. It goes out from the support mailbox and is recorded on the way — with its body, because an operator typed it and "what exactly did I write to them in March" is a question the mail server's log cannot answer either. Answering a request from here closes it, which is the reason to answer from here at all: answering from a mail client leaves it open for ever, because nothing tells it otherwise. Found on the way, by a test that sent a real mail rather than asserting it was queued: resources/views/mail/reset-password.blade.php was missing its closing </x-mail.layout>. The component's contents were never terminated, the layout's own @if ran off the end of the file, and the mail raised a Blade syntax error instead of rendering. Password reset has therefore never delivered a link since it was built. Closed, plus a test that renders it and one that counts opening against closing tags across every mail view. Reading INCOMING mail in the console is not in this: it needs the mailbox polled over IMAP, which is a credential and a dependency. Requests filed through the portal appear here in full; a customer who writes by mail instead still has to be read in the mail client. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-29 21:02:36 +02:00
nexxo	3a4324fb6f	Give people a way back in, and put the URL in English ... ── The hole ──────────────────────────────────────────────────────────────── There was no password reset. Fortify's feature was commented out, so there was no link on the sign-in form, no page and no route — a customer who forgot their password was locked out of their own cloud until somebody opened a shell. On a product whose selling point is that you can ring somebody, that is a support call a week and an embarrassing one. Two pages of ours under Fortify's route names (R1/R2), because with views off Fortify registers only the POST endpoints. The mail is a Mailable in this product's design rather than the framework's MailMessage: somebody who has just been locked out is exactly the person a phishing mail is aimed at, and a message that looks nothing like the rest of our post is one they cannot check — ours carries the footer that names our domains. Three decisions worth stating, each with a test: The answer is identical whether the address is known or not. "No account with that address" turns the form into a way of finding out who is a customer. The reset kills every other session. Whoever knew the old password may still be signed in somewhere, and a reset that leaves them there has fixed nothing. It does NOT sign the visitor in. The link arrived by email, and a mailbox somebody else can read would otherwise be a session somebody else gets. ── /sicherheit → /security ───────────────────────────────────────────────── R13: paths are English. Mine was not. The old path stays as a permanent redirect because it has already gone out in mail footers. ── One wordmark ──────────────────────────────────────────────────────────── "Sometimes it says Cloud and sometimes it does not" — correct. It was on the footer, the placeholder and the maintenance screen, and absent from the header, the sidebar and the sign-in plate. The product is called CluPilot Cloud; that is the name on the invoices and in every mail, so it is now the name everywhere. The console sidebar takes the small size so the lockup, the ADMIN badge and the close button still share one line (R18). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-29 16:50:29 +02:00
nexxo	92bcdcd186	Send the invoice with the invoice attached ... The PDF is rendered when the mail goes out rather than fetched from anywhere, because nothing stores it — and rendering from the frozen snapshot means the attachment is the document as issued no matter when the queue gets to it or how often it retries. fromData rather than fromPath: there is no path. Three attempts at testing it, and the first two were wrong in ways worth recording. Attachment::$data does not exist — the closure lives in a protected $resolver, and reaching for it was me testing the framework's internals rather than my own code. And a byte comparison of the PDF cannot work at all: TCPDF embeds a creation time and a document id, so two renders of one invoice differ. Split into the two things that can actually go wrong. The mail carries an attachment with the right filename and type — that is metadata, and it is asserted as metadata. And the renderer produces a real PDF rather than an empty one, asserted where the renderer is, because an attachment closure that returns nothing still makes a perfectly valid mail with a nought-byte file on it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-29 02:13:41 +02:00
nexxo	b1ab0f7f31	Put every mail in one design, and confirm an order when the money arrives ... Four templates, one layout. Two of them — the maintenance announcement and its cancellation — still wore Laravel's markdown component, which brings its own logo, its own button and its own footer: the product was sending mail that looked like two different companies, one of which was the framework. New: an order confirmation, sent when the payment lands rather than when provisioning finishes. Those are minutes apart at best and can be much longer, and somebody who has just been charged and heard nothing assumes the worst about both the charge and the product. It deliberately promises no time for the cloud itself — provisioning takes what it takes, CloudReady announces the end, and a promised minute that slips is worse than no promise. Queued after the transaction, never inside it, and never allowed to fail the webhook: Stripe retries anything that is not a 2xx, and a retry would re-enter provisioning over a mail server having a bad minute. The finished-cloud mail now names what was actually built — address, plan, storage, location — because somebody who ordered a fortnight ago is checking it against what they bought, and that is a comparison rather than a sentence. Still no credential in it. Two tests earn their place. One holds every template on the shared layout, so a new one written in a hurry cannot copy-paste its way back to the framework's. The other proves the finished-cloud mail carries no password — asserted against the data the mail is given and the HTML it renders, not against the template's source, because the first version of that test tripped over the word in a comment and would have passed a template that leaked the value through a variable. It tested the prose, not the mail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-29 00:14:04 +02:00
nexxo	5b8c28595a	Require a confirmed address before an account can use anything ... Registration opened an account that could sign in immediately on an address nobody had proved was theirs — and anybody can type a stranger's into a registration form. Both halves were commented out: the Fortify feature and MustVerifyEmail on the model. `verified` sits on the whole portal group rather than on the pages that spend money. An unconfirmed address is not a billing problem to be caught at checkout; it is an account that may not belong to the person holding it, and the servers, the users and the backups behind it are worth as much to whoever typed the address as to whoever owns it. The mail is a Mailable in this product's design rather than Laravel's notification, which would arrive with the framework's logo, button and footer as the first thing anybody ever receives from CluPilot. It also has to leave through the SYSTEM mailbox like every other account mail, which the notification path does not do on its own. The link is signed and expires, and the address is part of what is signed — so a link issued to a mistyped address stops working the moment the address is corrected, rather than confirming an address nobody can read. Tests cover the tampered link, the expired one, the corrected address, and one account trying to confirm another. The waiting page is ours (Fortify has views off) and offers the only two things that move somebody forward: send it again, or sign out and register with the address they meant. The resend is throttled, because otherwise it is a button that makes the server send mail to an arbitrary address as fast as it can be clicked. Somebody already confirmed is redirected away from it — the `verified` middleware only guards the other direction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-28 23:43:20 +02:00
nexxo	2609393e3a	Recognise the devices an account signs in from, and warn about a new one ... Sessions lived in Redis, where they cannot be enumerated — one key per session, no index by user — so "where am I signed in" and "end that one" were not answerable at all. The database driver keeps a row per session and makes ending one a delete. It costs every currently signed-in person one forced sign-in, once, which is why it is happening now rather than later. The warning is only worth anything if it is rare, so the two obvious signals are deliberately not used. Not the IP: it changes on every train, every café, every mobile handover, and behind a VPN it is not theirs to begin with. Not the user-agent: Chrome ships every four weeks and the string changes each time, which would be twelve warnings a year per customer, describing nothing. A device is a long-lived signed cookie holding a random token, and nothing else is consulted. The honest cost — cleared cookies, a private window, a second browser — is written into the mail itself, because a reader who thinks the warning is wrong stops reading the next one. Three things the tests found or hold: The first device on an account is new and must not warn. Replying to somebody creating an account by telling them they signed in from a new device is noise on the one message that has to stay worth reading. token_hash was globally unique, and the shared-browser test failed on it. Two people on one machine share one cookie and must each own a device row against it; the obvious repair — issue a fresh token for the second — would orphan the first person's row and warn them about their own desk. Unique is now per identity. Recording a device may never block a sign-in. A safeguard that locks the owner out when it breaks has stopped being one, so the listener logs and swallows, and a test drops the tables to prove it. Sessions are linked by a uuid carried inside the session payload rather than by the session id, which Laravel rotates on every sign-in and every regenerate(). Laravel's own sessions.user_id cannot carry the link either: it is filled from the default guard, while operators and customers are two guards and two tables (R21) — operator 1 and customer 1 would each be offered the other's sessions. Also lands the shared mail layout the rest of the templates will use: tables and inline styles for Outlook, no image and no web font because half of all clients block remote content, and the button in --accent-active rather than the brand orange, which is 2.9:1 against white. Not tagged: the session list is not built yet, and without a tag no server can install this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-28 23:28:34 +02:00
nexxo	254674f456	Bound the mail test-send and real send to a timeout instead of hanging ... A blocked outbound port (587 filtered at a firewall, as on the live server) left both MailboxTester and MailboxTransport blocking until PHP's default_socket_timeout (60s) or the reverse proxy cut the request — a 504 with no error the operator ever saw. Mail::build() honours a 'timeout' config key by calling SocketStream::setTimeout() (confirmed by reading MailManager::configureSmtpTransport() and by an end-to-end run against a genuinely unroutable host), so MailboxTester now passes 'timeout' => 10 through it. MailboxTransport builds its EsmtpTransport directly rather than through Mail::build(), so it sets the same call on the transport's stream, at 30s — a queue worker can afford to wait longer than an operator watching a browser, and a timed-out send is retried rather than lost. The test button also now shows a visible "sending" state while the request is in flight, matching the wire:loading span pattern already used on admin/host-create's save button — previously only wire:loading.attr="disabled" fired, leaving a greyed-out button with no other sign anything was happening. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-28 16:36:14 +02:00
nexxo	ebe29f8a5d	Stop demanding a cipher, a real port, or a decrypted password a mailbox never uses ... MailboxTester required a usable SECRETS_KEY unconditionally, refusing an unauthenticated relay's test-send on a fresh install with no key yet, even though real sending never touches the cipher for that mailbox. Gated the check on authenticates, same as the password-presence check beside it. Comparison pass across MailboxTester and MailboxTransport turned up two more of the same shape. MailboxTransport's delegate-cache fingerprint decrypted $box->password unconditionally, even when unauthenticated and never going to use it — a mailbox that once authenticated, stored a password, and later had authenticates unchecked would crash real sending the moment SECRETS_KEY became unusable. And MailboxTester had no guard against a stored port <1; Symfony silently reinterprets that as port 25 rather than refusing it, so the test button could reach whatever happens to listen there and report success for a configuration MailboxTransport refuses outright. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-28 06:21:28 +02:00
nexxo	5d306fce31	Let a mailbox send without a password when it never needed one ... An SMTP relay with a host and a from address but no username or password - a trusted local or private-network relay - always used to be seeded as a placeholder, because the takeover required a username to treat an account as real, and Mailbox::isConfigured() always required a stored password. Laravel's own SMTP mailer has supported unauthenticated relays all along. Add mailboxes.authenticates (default true, so every existing row keeps its current behaviour) to tell "does not authenticate" apart from "authenticates, but nobody has typed the password yet" - the same empty password column used to mean both. The takeover migration now seeds the real account off the host/address alone and marks it authenticates=false only when neither a username nor a password resolved; isConfigured(), MailboxTransport and MailboxTester all read it before requiring or sending a password; the edit modal gets a checkbox for it with the password field's hint updated to match, in both languages.	2026-07-28 05:54:21 +02:00
nexxo	c55a5d2b49	Make ssl, tls, and none mean what they say on the wire ... EsmtpTransport's autoTls (default true) and requireTls (default false) are independent of the implicit-TLS constructor argument: 'tls' left these at their defaults, so a server that omitted STARTTLS sent mailbox credentials in the clear while reporting success, and 'none' would silently upgrade the moment a server offered STARTTLS. Both MailboxTransport and MailboxTester already duplicated the "ssl, or port 465 ⇒ implicit" half of this decision independently; MailTlsPolicy is now the one place both read, for both dimensions. MailboxTester still builds through Mail::build() rather than a hand-built transport: MailManager::createSmtpTransport() forwards the whole config array to EsmtpTransportFactory as DSN options, which already reads auto_tls/require_tls itself, so the same policy reaches this path through keys the config array already supports.	2026-07-28 04:35:03 +02:00
nexxo	c899d41946	Pin CloudReady's null-mailbox guard and share it with the trait ... A mutation the reviewer ran (if ($box !== null) -> if (true)) left every test green: CloudReady hand-rolled the same null-mailbox guard as SendsFromMailbox, but nothing exercised its null branch. Added the missing test, then extracted the decision itself (null mailbox -> no sender fields, no_reply -> no Reply-To) into one shared method so an Envelope-shaped and a MailMessage-shaped consumer can no longer disagree.	2026-07-27 23:59:05 +02:00
nexxo	64a33c675e	Put a real sender on every mail, and a reply address where one helps	2026-07-27 23:40:34 +02:00
nexxo	961bf62032	Make the DSN agree with send() when a mailbox has no password ... Re-review of the previous fix round found the "cannot disagree" part of finding 4 was still open. resolution() had unified the log/array/null/ mailbox decision, but inside the mailbox branch describe() returned $box?->address unconditionally while delegate() additionally required isConfigured() before using it. A mailbox with active=true, a real address, and no password produced 'mailbox://support/support@clupilot.com' — reading as entirely healthy — while send() threw. That is exactly the harm the original finding named: the log says fine, the mail fails. resolution() now decides "unconfigured" too, not just "which non-delivering mode": it calls isConfigured() itself, once, and returns that verdict alongside the box. describe() and delegate() both branch on the SAME result again, the same way they already did for log/array/ null — a mailbox with no password now reads as 'support@clupilot.com [unconfigured]' rather than a plain address, and the address stays visible so an operator can tell WHICH mailbox needs fixing. Also renamed a test the re-review flagged as promising more than it checked: "registers a mailer per purpose without touching the database at boot" only ever verified the config array's shape — the DB-touching claim is a separate, already-passing test just below it (defines every purpose mailer as a plain literal...). Renamed to "registers a mailer for every purpose". No behaviour change. Added a test creating a mailbox with no password and asserting the DSN both names the address AND carries the marker, plus that send() still throws — proving agreement, not just a matching label painted on separately. Mutated describe()'s unconfigured branch to drop the marker: the test failed exactly on the missing 'unconfigured' string (DSN read as a plain, healthy-looking address again). Reverted, reran, passes. Full suite: 667 passed. vendor/bin/pint --test clean on both files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-27 23:17:42 +02:00
nexxo	a39670f920	Close the safety-net gaps the review found in MailboxTransport ... The reviewer continued the mutation sweep from Task 4's own commit and found three more unpinned lines in delegate(), plus two real behaviour gaps, all inside the same 40-line method: - The fingerprint refresh (the headline feature: a worker's mailer must pick up a corrected password without a restart) had no test — mutating its check to "delegate === null" left the suite green. - The implicit-TLS third argument to EsmtpTransport had no test either — mutating it to a hardcoded true left the suite green, because the only assertion touching it was instanceof EsmtpTransport, which the hanging variant also satisfies. - isLogging() only ever checked for 'log', but phpunit.xml sets MAIL_MAILER=array for the whole suite. Task 5 wires real mailables to these mailers next; the first feature test that sends one without Mail::fake() would have opened a real SMTP connection. - describe() and delegate() each ran their own copy of that guard — which is exactly how deleting it from ONLY delegate() passed the whole suite in the first place: __toString() kept reporting "log" from its own untouched copy. - An unconfigured host/port (Settings::get('mail.host', '') and a stored null port casting to 0) built a transport pointed at smtp://:587 or silently downgraded to plaintext port 25, instead of refusing the way a missing mailbox already does. Fixes: replaced isLogging() with resolution(), the one place that now decides "log, array, null-default, or a real mailbox" — describe() and delegate() both branch on ITS result, so a guard broken in one cannot look fine in the other. Added an explicit RuntimeException for a blank host or non-positive port, guarded the same way as the missing-mailbox case just above it. Tests: added coverage for the fingerprint rebuild (reuses the same delegate until the password actually changes, proven by object identity), the exact DSN string for both the STARTTLS (587) and implicit-TLS (465) cases, 'array' and unset-default both landing on a non-delivering transport, and the blank host/bad port throwing. Mutated each of these four in turn on the final code and confirmed: fingerprint check removed -> the reuse test fails (same object handed back after the password changed); TLS argument hardcoded true -> the 587 DSN test fails ('smtps://...' where 'smtp://...:587' was expected); NON_DELIVERING narrowed back to ['log'] -> both the array and unset-default tests fail (a real EsmtpTransport where a safe transport was expected); host/port guards removed -> both throw expectations fail. Reverted each, reran, all twelve tests pass again. Replaced the two tests that reached into delegate() via Closure::bind to check "log or SMTP" with one behavioural test: point Settings at a closed loopback port (127.0.0.1:1, so refusal is instant and entirely local — no DNS, no dependency on this environment's network egress policy) and assert Log::shouldReceive('debug')->once() while sending for real through Mail::mailer('cp_support')->raw(...). Mutated NON_DELIVERING to drop 'log' specifically and confirmed this test alone catches it: a real connection attempt to the closed port throws TransportException (Connection refused) in under 300ms. Reverted, reran, passes again. Also: the two tests that only checked the config array's shape and a snapshot of the resolved address promised more than they verified — CLAUDE.md R19 names a test that recomputes the implementation as checking nothing. Added a source-level test that the five cp_* config entries are parenthesis-free literals (same technique DisplayTimezoneTest/IconLayoutTest use: a property of the file, not of one run) as the actual "no query at boot" proof, and renamed the send-time test to what it verifies now that the fingerprint test above covers the live-refresh claim properly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-27 23:02:18 +02:00
nexxo	095d8e694a	Resolve the sending mailbox when the mail goes out, not at boot ... Five purpose-named mailers (cp_maintenance, cp_provisioning, cp_support, cp_billing, cp_system) land in config/mail.php as plain static entries — no query runs to build them. Mail::extend('mailbox', ...) in AppServiceProvider registers a transport that looks up its mailbox through MailboxResolver only once a mailer is actually resolved, which for queued mail is inside the worker at send time, never at boot. Mutation-tested the brief's own three tests by deleting the MAIL_MAILER guard from MailboxTransport::delegate(): all three stayed green. They only exercise __toString()/describe(), a code path separate from the one that actually sends. Added two tests that reach into delegate() itself (a bound closure, since it's private) and assert the transport it actually builds: LogTransport while MAIL_MAILER=log, EsmtpTransport once it isn't. Re-ran the same mutation against the strengthened suite: it now fails (EsmtpTransport where LogTransport was expected), then passes again once reverted. Verified two things against the installed versions before writing this: LogTransport takes a Psr\Log\LoggerInterface and Log::channel(null) resolves to the default channel rather than throwing, and Mail::extend('mailbox', ...) is read by MailManager::createSymfonyTransport() via $config['transport'] before it would fall back to createSmtpTransport() — both matched the brief exactly, no changes needed there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-07-27 22:28:37 +02:00
nexxo	493b81aadf	fix(admin): confirm maintenance delivery via MessageSent (retryable until sent); guard resend/cancel on derived state; fix flaky non-unique host wg_ip ... Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-25 16:14:01 +02:00
nexxo	52fa7a34a2	feat(admin): email affected customers when maintenance is cancelled (ledger-guarded) ... Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-25 16:07:21 +02:00
nexxo	eff8c08258	feat(admin): maintenance windows — schedule once, assign many hosts, notify ... - maintenance_windows + host pivot + notification ledger; derived state (never stored); affected-customer + banner queries live off instances - admin /maintenance: create draft/publish, multi-host select, impact counts, cancel; capability-gated (maintenance.manage) - publish emails affected customers once (queued Mailable, ledger-idempotent) - customer portal maintenance banner (upcoming <=72h + active) merged per window Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-25 15:53:50 +02:00