Eigene Betreiber-Identität für die Konsole (inkl. Postfächer) #2
Loading…
Reference in New Issue
There is no content yet.
Delete Branch "feat/operator-identity"
Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?
Enthält auch die Postfach-Arbeit (Zweig sitzt darauf) — mit diesem Merge ist PR #1 überflüssig.
Was drin ist
operators-Tabelle, eigener Guard, eigene Konsolen-Anmeldung mit eigenem 2FA-Ablauf. Kundenkonto kommt dort nicht durch, Betreiber nicht ins Portal.Reply-To, Konsolenseite, echter Testversand.Nach dem Merge, Reihenfolge wichtig
SECRETS_KEYerzeugen:head -c 32 /dev/urandom | base64→ in.envMAIL_SCHEME=smtpundMAIL_MAILER=smtpDie Migration übernimmt Passwort-Hash und 2FA unverändert — dasselbe Passwort wie bisher. Sie bricht vor jeder Änderung ab und nennt alle Adressen, falls jemand Betreiber und Kunde zugleich ist.
885 Tests.
The Task 2 reviewer rewired the model to Laravel's APP_KEY-based Crypt facade and every test stayed green. Laravel's encrypter is a container singleton resolved once from app.key, so config()->set('app.key') never rebuilds it and the rotation the test performed was invisible. Replaced with a positive proof — rotating SECRETS_KEY must break decryption — plus the APP_KEY case with forgetInstance(), which is what makes that direction mean anything.The old test rotated app.key and expected the password to stay readable, but Laravel's encrypter is a container singleton resolved once — config()->set('app.key', ...) never rebuilds it, so the rotation the test performed was invisible to any code path going through it. Confirmed by mutation: rewiring Mailbox to Crypt:: encryptString/decryptString (genuinely APP_KEY-keyed, the exact regression this constraint exists to forbid) left the old test green. Replace it with two tests: a positive proof that rotating SECRETS_KEY makes a stored password unreadable (the one that discriminates), and a separate check that rotating APP_KEY does not, with forgetInstance('encrypter') so that rotation is actually visible to whatever the password accessor resolves. Re-ran the same mutation against the new test: it fails (exit 1, the SECRETS_KEY test reports the expected DecryptException was not thrown), then passes again once the mutation is reverted.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>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>Mail::to($x)->queue($mail) resolves the DEFAULT mailer, and Illuminate\Mail\Mailer::queue() (vendor Mailer.php:482) does `$view->mailer($this->name)->queue($this->queue)` — overwriting MaintenanceAnnouncementMail/MaintenanceCancelledMail's own 'cp_maintenance' with the default mailer's name ('array' in tests, 'log'/whatever MAIL_MAILER is in prod) before the job is even built. Both mails kept sending through the old .env account instead of MailboxTransport. CloudReady was never affected — MailChannel.php:66 preserves a notification's own mailer. Fix: name the mailer before queueing — Mail::mailer($mail->mailer)->to(...). $this->name on the resolved Mailer is then already $mail->mailer, so the vendor overwrite becomes a no-op instead of a silent downgrade. Confirmed by reading the vendor source, not assumed. SenderAddressTest's "names the purpose mailer..." test asserted $mail->mailer on a bare, never-queued mailable, which proves nothing about what survives queueing — the constructor sets it right regardless of what MaintenanceNotifier does with it. Replaced it with one that drives MaintenanceNotifier::deliver() under Queue::fake() and inspects the pushed SendQueuedMailable's mailable. Verified it fails against the pre-fix code first (the pushed job carried mailer = 'array'), then verified the fix makes it pass, then mutated the fix away and watched it fail again.The seed migration read config('mail.mailers.smtp.host') and its three siblings directly. An install configuring SMTP through MAIL_URL (the other form config/mail.php supports) never sets those individual keys — MailManager only substitutes the URL's components in at transport-build time, inside its own protected getConfig(). Read raw, that left the migration seeding config/mail.php's own placeholder defaults, which the UNCONFIGURED_HOST/UNCONFIGURED_PORT guards then read as genuinely unset: blank host, port 0, an unconfigured mailbox, for an install that was sending mail successfully. resolveSmtpConfig() calls the same Illuminate\Support\ConfigurationUrlParser MailManager::getConfig() itself delegates to, so this is the same resolution rather than a second copy of its merge order. Verified against the real (protected) getConfig() via reflection: the URL wins for host/port/username/password wherever it actually supplies one, and an unset MAIL_URL leaves this a no-op.env('MAIL_HOST')/env('MAIL_PORT') returning anything, or MAIL_URL supplying either, now counts as "supplied" and wins over the sentinel comparison — a relay genuinely on 127.0.0.1:2525 no longer reads as unconfigured. The sentinel stays as a fallback for when neither signal says anything (including under config:cache, where env() goes null and this collapses to today's behaviour).redirect()->to(AdminArea::home()) resolved against whatever host the leave() POST itself arrived on — the portal's, once the console has a hostname of its own — landing the operator on the portal's dashboard instead of back at the console. route('admin.overview') is domain-bound to the console and generates the correct cross-host URL, matching what this line did before this branch. The test named after this behaviour now actually asserts it.It checked Auth::guard('operator')->check() instead of the users row it was handed, so with an operator's own session still attached (shared mode carries the same cookie through both legs of impersonation) it threw the moment a customer's email already had a users row — a 500 on the first impersonation of anyone who had signed into the portal themselves. Deleting it rather than fixing the check: operators live in `operators` now, never in `users` (R21), so a users row can no longer BE an operator account for this to guard against. Also drops the dead `'is_admin' => false` write — the column reads from nowhere any more.Three faults in the same migration: - up()'s removal loop was driven by operatorUserIds() (model_has_roles WHERE model_type=User) via the $carried array — but carryAcross() repoints model_has_roles to Operator for every operator before a single users row is deleted. A run that stops partway (the customer-conflict abort, or a dropped connection on real MariaDB, which this file deliberately runs without a transaction) leaves that worklist empty on the next attempt, so `migrate` records the migration as applied while every leftover users row still grants a live portal login on the operator's own password. Now driven by `operators` joined to `users` on email: that finds every carried-but-not-yet-deleted row regardless of which run carried it. - operatorUserIds() had no ORDER BY. carryAcross() remaps four owned columns in place, keyed on the old users.id, while operators.id hands out new values in the same order the list is walked — unordered, a later operator's freshly assigned id can equal an earlier operator's still-unprocessed users.id, and the second remap catches a row the first one already rewrote. One operator ends up silently owning another operator's VPN peer. Added ->orderBy('model_id'). - down() restored model_has_roles but not model_has_permissions, so an operator holding a direct (not role-granted) permission lost it on rollback. Mirrored the same move carryAcross() does forward. Also removes seed_roles_and_permissions' now-stale User::OPERATOR_ROLES/hasAnyRole()/assignRole() bootstrap loop — all three gone from User per this branch's own work — which only a deep rollback (down() of the move migration recreates is_admin users) would ever have reached, but would fatal when it did.POST /broadcasting/auth carries only the 'web' middleware group (Laravel's own withBroadcasting() registers it that way), so PusherBroadcaster::auth() resolved retrieveUser() on the DEFAULT guard before the admin.runs channel's own Auth::guard('operator') check ever ran — found nobody, since an operator is never signed in on 'web', and threw straight from there. The guard was correct but unreachable. Fixed by naming both guards on the channel registration itself. Also adds broadcasting/auth to RestrictAdminHost::SHARED: in exclusive mode it was neither admin.* by name nor on the shared list, so it 404'd on the console host too — degrading silently to wire:poll.4s rather than breaking outright, which is why nothing screamed.ApplicationBuilder::withMiddleware() defaults redirectGuestsTo() to fn () => route('login') before bootstrap/app.php's own callback ever runs — always the portal's Fortify page, regardless of which side of the site turned the guest away. In non-exclusive (shared, this VM's own mode) that sends a guest bounced off /admin to a sign-in form whose table holds no operators: no 404 any more, but a sign-in that can never succeed. Exclusive mode happened to self-correct only because RestrictAdminHost 404s the wrong host before this ever ran. Fixed for both modes at once rather than depending on which one a given deployment runs: ask AdminArea, the one place routes/web.php's own registration and RestrictAdminHost already agree is the source of truth for "is this the console".operatorUserIds() only discovered console users through model_has_roles. A pre-existing account granted console.view straight through model_has_permissions, holding no role at all, was invisible to the worklist — meanwhile the permission's own guard is switched to `operator` a few lines above, so that account would silently lose console access while staying behind as a working portal login. Worklist is now the deduplicated, sorted union of both tables; orderBy('model_id') kept on each side since carryAcross() remaps in place and the ordering still matters regardless of which table an id came from.PublicSiteGate let anyone through on a bare Auth::guard('operator')->check(), which stays true for a disabled operator's live session — disabling someone does not sign them out, so it never stopped this gate from showing them the real site indefinitely, even though EnsureAdmin already treats them as inactive. Now requires the same thing EnsureAdmin does: an active operator holding a console role. Chosen to merely ignore an invalid operator session here rather than log it out. This gate only decides what one request sees; logging out would ripple into whatever else that session is doing (a concurrent console tab, mid-task), a bigger side effect than a visibility check exists to have. EnsureAdmin sets the precedent — it also rejects a disabled operator per request without touching their session.The setup-route exemption already went through AdminArea::routeIs(), which matches both admin.<name> and admin.via*.<name>. The logout exemption next to it used a bare $request->routeIs('admin.logout'), which matches only the canonical name — on a recovery hostname the logout route is admin.via0.logout, so the check failed there and an unenrolled operator's logout POST bounced back to enrolment instead of being let through, on exactly the host that exists because the canonical one is not working.The three sites that refuse to create or rename an operator onto a customer's email (Admin\Settings::saveAccount(), ::inviteStaff(), and clupilot:create-operator) all checked Customer::where('email', ...) as a proxy for "does this address already have a portal login". A users row with no matching customers row — an email changed on one side only, or legacy/orphaned data — passed straight through: this dev database already had one. Extracted the three copies into Customer::emailTaken(), which checks both tables directly, so the three sites cannot drift from each other again.Direkt nach
mainfast-forward gemergt statt über die Oberfläche — main steht auf9fc74ee, 885 Tests grün. Nichts geht verloren.Pull request closed