From ea643b5e7324c785c9cd718f752c7654f71a3b1d Mon Sep 17 00:00:00 2001 From: nexxo Date: Wed, 29 Jul 2026 14:47:51 +0200 Subject: [PATCH] Prove a custom domain before serving it, and keep proving it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things. ── The update screen, still opening twice ─────────────────────────────────── Reported again on 1.3.9, and the cause was not the one fixed in 1.3.8. The agent consumes the request file BEFORE it resolves the release — deliberately, because update.sh may kill the shell and a request left in place would loop — and writes `state: running` only once it has decided to go ahead. In between, the request is gone and the status does not say running yet, so the endpoint honestly answers "nothing is running". The watcher read that as "the run has finished" and reloaded the page: overlay on the click, gone a poll later, 503 after it. The overlay now closes only once the server has BOTH confirmed a run and then stopped reporting it. Before the confirmation, silence means the agent has not got there yet. Bounded at twenty polls so a request the agent refuses does not leave the console covered forever. ── Custom domains, proven and re-proven ───────────────────────────────────── `custom_domain` was a free-text field and everything downstream believed it: the proxy served it, the certificate was issued for it, Nextcloud trusted it. Anyone who pointed any hostname at the platform got somebody else's files under their own name. The proof is a TXT record at _clupilot-challenge. holding a token only this instance has. Nothing is served until it has been read. Every reader now goes through Instance::address(), which is the one place the decision is made — `custom_domain ?: subdomain` was the hole, written out four times. It is re-read every night at 03:40, because a token checked once can be taken straight back out and a domain that later lapses keeps resolving here. Three consecutive misses before a live domain is withdrawn: one failed lookup is a nameserver having a bad minute, and withdrawing takes a working Nextcloud off its own address. The domain and its token stay on the row so the customer can put the record back rather than start over. Changing the domain mints a NEW token. Reusing it would let somebody who once verified example.com claim any other domain later without touching its DNS — the old record is still sitting there and only the value is compared. The domain is changeable at any time, and removable. Fixing it once set was considered and rejected: adding or moving an address is a proxy entry, a certificate and one line in trusted_domains. Co-Authored-By: Claude Opus 5 --- VERSION | 2 +- app/Console/Commands/VerifyCustomDomains.php | 79 +++++++ app/Livewire/Admin/Instances.php | 4 +- app/Livewire/Cloud.php | 9 +- app/Livewire/CustomDomain.php | 158 ++++++++++++++ app/Livewire/Dashboard.php | 9 +- app/Models/Instance.php | 32 +++ .../Jobs/IssueInstanceAdminAccess.php | 5 +- .../Steps/Customer/ConfigureNextcloud.php | 6 +- app/Services/Domains/DomainVerifier.php | 107 ++++++++++ app/Support/Navigation.php | 1 + ...0_add_domain_verification_to_instances.php | 57 +++++ lang/de/dashboard.php | 1 + lang/de/domain.php | 41 ++++ lang/en/dashboard.php | 1 + lang/en/domain.php | 40 ++++ resources/js/app.js | 38 +++- .../views/livewire/custom-domain.blade.php | 92 ++++++++ routes/console.php | 11 + routes/web.php | 1 + tests/Feature/Admin/UpdateButtonTest.php | 33 +++ tests/Feature/CustomDomainTest.php | 196 ++++++++++++++++++ 22 files changed, 911 insertions(+), 12 deletions(-) create mode 100644 app/Console/Commands/VerifyCustomDomains.php create mode 100644 app/Livewire/CustomDomain.php create mode 100644 app/Services/Domains/DomainVerifier.php create mode 100644 database/migrations/2026_07_29_220000_add_domain_verification_to_instances.php create mode 100644 lang/de/domain.php create mode 100644 lang/en/domain.php create mode 100644 resources/views/livewire/custom-domain.blade.php create mode 100644 tests/Feature/CustomDomainTest.php diff --git a/VERSION b/VERSION index 0c00f61..17e63e7 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.3.10 +1.3.11 diff --git a/app/Console/Commands/VerifyCustomDomains.php b/app/Console/Commands/VerifyCustomDomains.php new file mode 100644 index 0000000..1346b20 --- /dev/null +++ b/app/Console/Commands/VerifyCustomDomains.php @@ -0,0 +1,79 @@ +whereNotNull('custom_domain')->whereNotNull('domain_token'); + + if ($uuid = $this->option('instance')) { + $query->where('uuid', $uuid); + } + + $checked = 0; + $withdrawn = 0; + + foreach ($query->cursor() as $instance) { + $checked++; + $present = $verifier->proofPresent($instance); + + if ($present) { + $instance->forceFill([ + 'domain_verified_at' => $instance->domain_verified_at ?? now(), + 'domain_checked_at' => now(), + 'domain_error' => null, + 'domain_failures' => 0, + ])->save(); + + continue; + } + + $failures = $instance->domain_failures + 1; + + // Withdrawal clears verified_at, which is the single flag every + // consumer reads. Nothing else is deleted: the domain and its token + // stay on the row, so the customer sees what is wrong and can put + // the record back rather than starting over. + $withdraw = $instance->domain_verified_at !== null + && $failures >= DomainVerifier::FAILURES_BEFORE_WITHDRAWAL; + + $instance->forceFill([ + 'domain_checked_at' => now(), + 'domain_error' => 'missing_txt', + 'domain_failures' => $failures, + 'domain_verified_at' => $withdraw ? null : $instance->domain_verified_at, + ])->save(); + + if ($withdraw) { + $withdrawn++; + $this->warn("Withdrew {$instance->custom_domain}: proof missing on {$failures} consecutive checks."); + } + } + + $this->info("Checked {$checked} domain(s), withdrew {$withdrawn}."); + + return self::SUCCESS; + } +} diff --git a/app/Livewire/Admin/Instances.php b/app/Livewire/Admin/Instances.php index 79cf01e..2f7fe6e 100644 --- a/app/Livewire/Admin/Instances.php +++ b/app/Livewire/Admin/Instances.php @@ -32,7 +32,9 @@ class Instances extends Component return view('livewire.admin.instances', [ 'instances' => $instances, 'rows' => $instances->getCollection()->map(fn (Instance $i) => [ - 'address' => $i->custom_domain ?: $i->subdomain, + // The operator list shows what is actually served — an unverified + // custom domain is a plan, not an address. + 'address' => $i->domainIsVerified() ? $i->custom_domain : $i->subdomain, 'customer' => $i->customer?->name ?? '—', 'host' => $i->host?->name ?? '—', 'vmid' => $i->vmid ?? '—', diff --git a/app/Livewire/Cloud.php b/app/Livewire/Cloud.php index 9c13579..9eb8c52 100644 --- a/app/Livewire/Cloud.php +++ b/app/Livewire/Cloud.php @@ -39,8 +39,13 @@ class Cloud extends Component $growth = array_map(fn ($v) => (int) round($v / $curveMax * $quota), $growth); } $used = $growth[count($growth) - 1]; - $domain = $shown?->custom_domain - ?: ($shown?->subdomain ? $shown->subdomain.'.'.config('provisioning.dns.zone', 'clupilot.com') : 'cloud.example.com'); + // address() rather than `custom_domain ?: subdomain`: a domain whose + // DNS proof has not been read is not this instance's address, and + // printing it here would tell a customer to visit a hostname that does + // not resolve to them. + $domain = $shown?->subdomain + ? $shown->address(\App\Support\ProvisioningSettings::dnsZone()) + : 'cloud.example.com'; return view('livewire.cloud', [ 'instance' => [ diff --git a/app/Livewire/CustomDomain.php b/app/Livewire/CustomDomain.php new file mode 100644 index 0000000..c494745 --- /dev/null +++ b/app/Livewire/CustomDomain.php @@ -0,0 +1,158 @@ +domain = (string) ($this->instance()?->custom_domain ?? ''); + } + + private function instance(): ?Instance + { + // The shared resolver, not `auth()->user()->customer`: accounts created + // before the explicit link exists are matched by address, and this page + // must not be the one place that forgets that. + return $this->customer()?->instances() + ->whereIn('status', ['active', 'provisioning', 'cancellation_scheduled']) + ->latest('id') + ->first(); + } + + /** + * Save the domain and mint a fresh token. + * + * A NEW token every time the domain changes. Reusing the old one would let + * a customer who once verified example.com claim any other domain later + * without touching its DNS — the record for the first domain would still be + * sitting there, and the value is all that is compared. + */ + public function save(DomainVerifier $verifier): void + { + $this->validate(); + + $instance = $this->instance(); + + if ($instance === null) { + return; + } + + $domain = Str::lower(trim($this->domain)); + $domain = preg_replace('#^https?://#', '', $domain); + $domain = rtrim((string) $domain, '/.'); + + // A hostname, not a URL and not a path. Loose on purpose — the DNS + // lookup is the real test, and a pattern strict enough to be a + // specification would reject somebody's perfectly valid domain. + if ($domain !== '' && ! preg_match('/^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]*[a-z0-9])?)+$/', $domain)) { + $this->addError('domain', __('domain.invalid')); + + return; + } + + // Clearing the field takes the domain away entirely, including its + // verification — the instance falls back to its platform address. + if ($domain === '') { + $instance->forceFill([ + 'custom_domain' => null, 'domain_token' => null, + 'domain_verified_at' => null, 'domain_checked_at' => null, + 'domain_error' => null, 'domain_failures' => 0, + ])->save(); + + $this->domain = ''; + $this->dispatch('notify', message: __('domain.removed')); + + return; + } + + if ($domain !== $instance->custom_domain) { + $instance->forceFill([ + 'custom_domain' => $domain, + 'domain_token' => $verifier->newToken(), + 'domain_verified_at' => null, + 'domain_checked_at' => null, + 'domain_error' => null, + 'domain_failures' => 0, + ])->save(); + } + + $this->domain = $domain; + $this->dispatch('notify', message: __('domain.saved')); + } + + /** + * Look now, rather than waiting for tonight. + * + * The nightly run is what keeps a domain honest; this button is only so + * that somebody who has just created the record does not have to wait until + * the morning to find out whether they typed it correctly. + */ + public function checkNow(DomainVerifier $verifier): void + { + $instance = $this->instance(); + + if ($instance === null || blank($instance->custom_domain)) { + return; + } + + $present = $verifier->proofPresent($instance); + + $instance->forceFill([ + 'domain_checked_at' => now(), + 'domain_verified_at' => $present ? ($instance->domain_verified_at ?? now()) : null, + 'domain_error' => $present ? null : 'missing_txt', + 'domain_failures' => $present ? 0 : $instance->domain_failures + 1, + ])->save(); + + $this->justChecked = true; + $this->dispatch('notify', message: __($present ? 'domain.verified' : 'domain.not_found')); + } + + public function render() + { + $instance = $this->instance(); + $verifier = app(DomainVerifier::class); + + return view('livewire.custom-domain', [ + 'instance' => $instance, + 'platformAddress' => $instance?->subdomain + ? $instance->subdomain.'.'.ProvisioningSettings::dnsZone() + : null, + 'recordName' => $instance && filled($instance->custom_domain) + ? $verifier->recordName($instance->custom_domain) + : null, + 'recordValue' => $instance && filled($instance->domain_token) + ? $verifier->recordValue($instance) + : null, + ]); + } +} diff --git a/app/Livewire/Dashboard.php b/app/Livewire/Dashboard.php index 7dec72a..f86b67c 100644 --- a/app/Livewire/Dashboard.php +++ b/app/Livewire/Dashboard.php @@ -195,10 +195,11 @@ class Dashboard extends Component return null; } - return $instance->custom_domain - ?: ($instance->subdomain - ? $instance->subdomain.'.'.config('provisioning.dns.zone', 'clupilot.com') - : null); + // See Instance::address(): the customer's own domain counts only once + // its DNS proof has been read. + return $instance->subdomain + ? $instance->address(\App\Support\ProvisioningSettings::dnsZone()) + : null; } /** diff --git a/app/Models/Instance.php b/app/Models/Instance.php index 032035c..1f4c1ac 100644 --- a/app/Models/Instance.php +++ b/app/Models/Instance.php @@ -18,6 +18,7 @@ class Instance extends Model 'customer_id', 'order_id', 'host_id', 'vmid', 'guest_ip', 'plan', 'quota_gb', 'traffic_addons', 'disk_gb', 'ram_mb', 'cores', 'subdomain', 'custom_domain', 'nc_admin_ref', 'admin_password', 'credentials_acknowledged_at', 'route_written', 'cert_ok', 'status', 'cancel_requested_at', 'service_ends_at', + 'domain_token', 'domain_verified_at', 'domain_checked_at', 'domain_error', 'domain_failures', ]; protected $hidden = ['nc_admin_ref', 'admin_password']; @@ -32,6 +33,9 @@ class Instance extends Model // nc_admin_ref and Host::api_token_ref, for the same reason. 'admin_password' => 'encrypted', 'credentials_acknowledged_at' => 'datetime', + 'domain_verified_at' => 'datetime', + 'domain_checked_at' => 'datetime', + 'domain_failures' => 'integer', 'route_written' => 'boolean', 'cert_ok' => 'boolean', 'vmid' => 'integer', @@ -62,6 +66,34 @@ class Instance extends Model $query->where(fn ($q) => $q->where('status', '!=', 'failed')->orWhereNotNull('vmid')); } + /** + * Is the customer's own domain proven to be theirs? + * + * The one flag. A domain that has never been verified — or that lost its + * DNS proof and was withdrawn — is not this instance's address, whatever is + * typed in the column. + */ + public function domainIsVerified(): bool + { + return filled($this->custom_domain) && $this->domain_verified_at !== null; + } + + /** + * The address this instance is actually served at. + * + * Every caller goes through here rather than reading `custom_domain ?: + * subdomain` for itself. That expression was the hole: it served, certified + * and trusted whatever hostname was in the column, so anybody who pointed + * any domain at the proxy got somebody else's Nextcloud under their own + * name — with a certificate to match. + */ + public function address(string $zone): string + { + return $this->domainIsVerified() + ? (string) $this->custom_domain + : $this->subdomain.'.'.$zone; + } + public function customer(): BelongsTo { return $this->belongsTo(Customer::class); diff --git a/app/Provisioning/Jobs/IssueInstanceAdminAccess.php b/app/Provisioning/Jobs/IssueInstanceAdminAccess.php index edb91f5..89fec2f 100644 --- a/app/Provisioning/Jobs/IssueInstanceAdminAccess.php +++ b/app/Provisioning/Jobs/IssueInstanceAdminAccess.php @@ -101,7 +101,10 @@ class IssueInstanceAdminAccess implements ShouldQueue ]); ConfigHandoff::put(json_encode([ - 'url' => 'https://'.($instance->custom_domain ?: $instance->subdomain.'.'.ProvisioningSettings::dnsZone()), + // Through address(), which serves the customer's own domain only + // once its DNS proof has been read. A link to an unverified + // domain would not resolve to this instance anyway. + 'url' => 'https://'.$instance->address(ProvisioningSettings::dnsZone()), 'username' => $username, 'password' => $password, ]), $this->handoffToken); diff --git a/app/Provisioning/Steps/Customer/ConfigureNextcloud.php b/app/Provisioning/Steps/Customer/ConfigureNextcloud.php index 65edae7..bda02ae 100644 --- a/app/Provisioning/Steps/Customer/ConfigureNextcloud.php +++ b/app/Provisioning/Steps/Customer/ConfigureNextcloud.php @@ -31,7 +31,11 @@ class ConfigureNextcloud extends CustomerStep // Idempotent occ calls (setting the same values is a no-op). $this->guest($pve, $run, $occ.'config:system:set trusted_domains 1 --value='.escapeshellarg($fqdn)); - if (filled($instance->custom_domain)) { + // Only a VERIFIED domain goes into trusted_domains. Nextcloud will + // answer for anything listed here, so an unproven hostname added at + // provisioning time would serve this customer's files to whoever + // pointed that name at the proxy. + if ($instance->domainIsVerified()) { $this->guest($pve, $run, $occ.'config:system:set trusted_domains 2 --value='.escapeshellarg($instance->custom_domain)); } $this->guest($pve, $run, $occ.'background:cron'); diff --git a/app/Services/Domains/DomainVerifier.php b/app/Services/Domains/DomainVerifier.php new file mode 100644 index 0000000..1698f87 --- /dev/null +++ b/app/Services/Domains/DomainVerifier.php @@ -0,0 +1,107 @@ +> */ + private $resolve; + + /** + * The resolver is injectable so the tests can answer without touching the + * network — a suite that depends on real DNS fails on a train. + * + * @param null|callable(string): array> $resolve + */ + public function __construct(?callable $resolve = null) + { + $this->resolve = $resolve ?? static fn (string $name) => dns_get_record($name, DNS_TXT) ?: []; + } + + /** The record name a customer has to create. */ + public function recordName(string $domain): string + { + return self::RECORD_PREFIX.'.'.ltrim($domain, '.'); + } + + /** The value it has to hold. */ + public function recordValue(Instance $instance): string + { + return self::VALUE_PREFIX.$instance->domain_token; + } + + /** + * A fresh token. Called when a domain is set, and again whenever it + * changes — a new domain must not be provable with the previous one's + * token, or a customer who once verified example.com could claim any + * domain afterwards without touching its DNS. + */ + public function newToken(): string + { + return Str::lower(Str::random(32)); + } + + /** + * Is the proof in place right now? + * + * Every TXT value at the name is considered, not just the first: a domain + * may carry several TXT records at one name (SPF, other verifications), and + * DNS returns them in no particular order. + */ + public function proofPresent(Instance $instance): bool + { + if (blank($instance->custom_domain) || blank($instance->domain_token)) { + return false; + } + + $expected = $this->recordValue($instance); + + foreach (($this->resolve)($this->recordName($instance->custom_domain)) as $record) { + // dns_get_record splits long values into `entries`; `txt` is the + // joined form. Both are checked because which one is populated + // depends on the value's length, and a token is short enough to + // land in either. + $values = array_merge([(string) ($record['txt'] ?? '')], (array) ($record['entries'] ?? [])); + + foreach ($values as $value) { + if (trim($value) === $expected) { + return true; + } + } + } + + return false; + } +} diff --git a/app/Support/Navigation.php b/app/Support/Navigation.php index c7e10dc..bf474f4 100644 --- a/app/Support/Navigation.php +++ b/app/Support/Navigation.php @@ -21,6 +21,7 @@ final class Navigation ['label' => null, 'items' => [ ['dashboard', 'gauge', 'overview', null], ['cloud', 'cloud', 'cloud', null], + ['domain', 'globe', 'domain', null], ['users', 'users', 'users', null], ['backups', 'database', 'backups', null], ]], diff --git a/database/migrations/2026_07_29_220000_add_domain_verification_to_instances.php b/database/migrations/2026_07_29_220000_add_domain_verification_to_instances.php new file mode 100644 index 0000000..6dac1fb --- /dev/null +++ b/database/migrations/2026_07_29_220000_add_domain_verification_to_instances.php @@ -0,0 +1,57 @@ +string('domain_token', 40)->nullable()->after('custom_domain'); + + // Null until the record has been seen. This is the flag the proxy, + // the certificate and Nextcloud's trusted_domains all key off: + // unverified means the domain does not exist as far as we are + // concerned, and the proxy answers nothing at all. + $table->timestamp('domain_verified_at')->nullable()->after('domain_token'); + + // The last time the record was actually read, successful or not, + // and what went wrong. Separate from verified_at so a customer can + // see "checked ten minutes ago, still missing" rather than silence. + $table->timestamp('domain_checked_at')->nullable()->after('domain_verified_at'); + $table->string('domain_error', 32)->nullable()->after('domain_checked_at'); + + // How long it has been failing. One failed lookup is a nameserver + // having a bad minute; days of them is a domain that has moved on. + $table->unsignedSmallInteger('domain_failures')->default(0)->after('domain_error'); + }); + } + + public function down(): void + { + Schema::table('instances', function (Blueprint $table) { + $table->dropColumn([ + 'domain_token', 'domain_verified_at', 'domain_checked_at', + 'domain_error', 'domain_failures', + ]); + }); + } +}; diff --git a/lang/de/dashboard.php b/lang/de/dashboard.php index 25eed8e..0150739 100644 --- a/lang/de/dashboard.php +++ b/lang/de/dashboard.php @@ -124,6 +124,7 @@ return [ 'nav' => [ 'overview' => 'Übersicht', 'cloud' => 'Meine Cloud', + 'domain' => 'Eigene Domain', 'users' => 'Benutzer', 'backups' => 'Backups', 'invoices' => 'Rechnungen', diff --git a/lang/de/domain.php b/lang/de/domain.php new file mode 100644 index 0000000..b9bcf3e --- /dev/null +++ b/lang/de/domain.php @@ -0,0 +1,41 @@ + 'Eigene Domain', + 'subtitle' => 'Ihre Cloud unter Ihrer eigenen Adresse — sobald bewiesen ist, dass die Domain Ihnen gehört.', + + 'field' => 'Domain', + 'placeholder' => 'cloud.ihre-firma.at', + 'save' => 'Speichern', + 'saved' => 'Domain gespeichert. Jetzt den Nachweis eintragen.', + 'removed' => 'Domain entfernt. Ihre Cloud ist wieder unter der Plattformadresse erreichbar.', + 'invalid' => 'Das sieht nicht nach einem Hostnamen aus. Beispiel: cloud.ihre-firma.at', + 'current_address' => 'Aktuelle Adresse', + 'platform_address' => 'Plattformadresse', + + 'step_a' => 'Schritt 1 — Domain auf uns zeigen lassen', + 'step_a_body' => 'Legen Sie bei Ihrem Domain-Anbieter einen A-Eintrag auf die IP-Adresse an, die wir Ihnen genannt haben. Das können wir für Sie nicht prüfen — hinter manchen Anbietern ist die Adresse von außen nicht sichtbar.', + + 'step_b' => 'Schritt 2 — Besitz nachweisen', + 'step_b_body' => 'Legen Sie zusätzlich diesen TXT-Eintrag an. Er beweist, dass die Domain Ihnen gehört. Ohne ihn liefern wir unter dieser Adresse nichts aus.', + 'record_name' => 'Name', + 'record_type' => 'Typ', + 'record_value' => 'Wert', + + 'check_now' => 'Jetzt prüfen', + 'verified' => 'Nachweis gefunden — die Domain ist freigeschaltet.', + 'not_found' => 'Der TXT-Eintrag ist noch nicht sichtbar. DNS-Änderungen brauchen oft ein paar Minuten.', + + 'state' => [ + 'verified' => 'Freigeschaltet', + 'pending' => 'Warten auf Nachweis', + 'none' => 'Keine eigene Domain', + ], + + 'checked_at' => 'Zuletzt geprüft: :when', + 'never_checked' => 'Noch nicht geprüft.', + + // The part that is easy to get wrong and expensive to explain afterwards. + 'keep_record' => 'Bitte lassen Sie den TXT-Eintrag dauerhaft stehen. Wir prüfen ihn jede Nacht erneut; fehlt er an drei Nächten hintereinander, schalten wir die Domain wieder ab und Ihre Cloud ist unter der Plattformadresse erreichbar.', + 'failures' => 'Nachweis fehlt seit :count Prüfung(en).', +]; diff --git a/lang/en/dashboard.php b/lang/en/dashboard.php index 092dc77..55185b4 100644 --- a/lang/en/dashboard.php +++ b/lang/en/dashboard.php @@ -124,6 +124,7 @@ return [ 'nav' => [ 'overview' => 'Overview', 'cloud' => 'My cloud', + 'domain' => 'Your domain', 'users' => 'Users', 'backups' => 'Backups', 'invoices' => 'Invoices', diff --git a/lang/en/domain.php b/lang/en/domain.php new file mode 100644 index 0000000..b1fd9b4 --- /dev/null +++ b/lang/en/domain.php @@ -0,0 +1,40 @@ + 'Your own domain', + 'subtitle' => 'Your cloud on your own address — once it is proven that the domain is yours.', + + 'field' => 'Domain', + 'placeholder' => 'cloud.your-company.com', + 'save' => 'Save', + 'saved' => 'Domain saved. Now add the proof record.', + 'removed' => 'Domain removed. Your cloud is reachable at the platform address again.', + 'invalid' => 'That does not look like a hostname. Example: cloud.your-company.com', + 'current_address' => 'Current address', + 'platform_address' => 'Platform address', + + 'step_a' => 'Step 1 — point the domain at us', + 'step_a_body' => 'Create an A record at your domain provider pointing to the IP address we gave you. We cannot check this for you — behind some providers the address is not visible from outside.', + + 'step_b' => 'Step 2 — prove ownership', + 'step_b_body' => 'Also create this TXT record. It proves the domain is yours. Without it we serve nothing at this address.', + 'record_name' => 'Name', + 'record_type' => 'Type', + 'record_value' => 'Value', + + 'check_now' => 'Check now', + 'verified' => 'Proof found — the domain is live.', + 'not_found' => 'The TXT record is not visible yet. DNS changes often take a few minutes.', + + 'state' => [ + 'verified' => 'Live', + 'pending' => 'Waiting for proof', + 'none' => 'No custom domain', + ], + + 'checked_at' => 'Last checked: :when', + 'never_checked' => 'Not checked yet.', + + 'keep_record' => 'Please leave the TXT record in place. We re-read it every night; if it is missing on three consecutive nights we withdraw the domain and your cloud is reachable at the platform address.', + 'failures' => 'Proof missing for :count check(s).', +]; diff --git a/resources/js/app.js b/resources/js/app.js index eea91e9..b499a09 100644 --- a/resources/js/app.js +++ b/resources/js/app.js @@ -196,6 +196,24 @@ document.addEventListener('alpine:init', () => { // already ended by the time we can ask again. renderedCommit: commit, wasRunning: running, + // Has the SERVER ever reported this run, as opposed to us opening the + // overlay optimistically when the button was pressed? + // + // This is the gap the whole thing turned on. The agent consumes the + // request file BEFORE it resolves the release — deliberately, because + // update.sh may kill the shell and a request left in place would loop — + // and only writes `state: running` once it has decided to go ahead. In + // between, the request is gone and the status does not say running yet, + // so the endpoint honestly answers "nothing is running". The watcher + // read that as "the run has finished" and reloaded the page: the + // overlay appeared on the click, vanished a poll later, and the 503 + // arrived after it. Reported twice, exactly that way. + // + // The run has ENDED only once the server has both confirmed it and then + // stopped reporting it. Before the confirmation, silence means the agent + // has not got there yet. + serverConfirmed: running, + unconfirmedPolls: 0, // Already translated server-side ("Schritt: …", "Läuft seit …") — the // client only ever displays these, never assembles them, so German // text stays out of JavaScript entirely. @@ -236,16 +254,32 @@ document.addEventListener('alpine:init', () => { return; } + if (state.running) { + this.serverConfirmed = true; + this.unconfirmedPolls = 0; + } + // The run ended without the build changing — a check that found // nothing, or a failure. Either way the card has something new // to say. - if (this.wasRunning && !state.running) { + if (this.wasRunning && this.serverConfirmed && !state.running) { window.location.reload(); return; } - this.wasRunning = state.running; + // ...but an overlay that waits forever is its own bug. The + // agent refuses a request with nothing to install, and says so + // by writing nothing at all — so give it a bounded number of + // polls before concluding that nobody picked it up. + if (this.wasRunning && !this.serverConfirmed && ++this.unconfirmedPolls > 20) { + window.location.reload(); + + return; + } + + // Stay open while we are waiting for the agent to confirm. + this.wasRunning = state.running || (this.wasRunning && !this.serverConfirmed); this.step = state.step ?? null; this.runningSince = state.running_since ?? null; this.log = state.log ?? null; diff --git a/resources/views/livewire/custom-domain.blade.php b/resources/views/livewire/custom-domain.blade.php new file mode 100644 index 0000000..ed74ac7 --- /dev/null +++ b/resources/views/livewire/custom-domain.blade.php @@ -0,0 +1,92 @@ +
+
+

{{ __('domain.title') }}

+

{{ __('domain.subtitle') }}

+
+ + @if ($instance === null) +

{{ __('domain.state.none') }}

 + @else + {{-- What is served right now. First, because it is the question + somebody opens this page with. --}} + +
+
+

{{ __('domain.current_address') }}

+

{{ $instance->address(\App\Support\ProvisioningSettings::dnsZone()) }}

+
+
+ @if ($instance->domainIsVerified()) + {{ __('domain.state.verified') }} + @elseif (filled($instance->custom_domain)) + {{ __('domain.state.pending') }} + @else + {{ __('domain.state.none') }} + @endif +
+
+ @if ($platformAddress && $instance->domainIsVerified()) +

+ {{ __('domain.platform_address') }}: {{ $platformAddress }} +

+ @endif + + + +
+
+ + + @error('domain')

{{ $message }}

@enderror +
+ {{ __('domain.save') }} +
+ + + @if (filled($instance->custom_domain)) + +

{{ __('domain.step_a') }}

+

{{ __('domain.step_a_body') }}

+ +

{{ __('domain.step_b') }}

+

{{ __('domain.step_b_body') }}

+ +
+ @foreach ([ + __('domain.record_name') => $recordName, + __('domain.record_type') => 'TXT', + __('domain.record_value') => $recordValue, + ] as $term => $value) +
+
{{ $term }}
+
{{ $value }}
+
+ @endforeach +
+ +
+ + {{ __('domain.check_now') }} + +

+ @if ($instance->domain_checked_at) + {{ __('domain.checked_at', ['when' => $instance->domain_checked_at->local()->isoFormat('DD.MM. HH:mm')]) }} + @if (! $instance->domainIsVerified() && $instance->domain_failures > 0) + · {{ __('domain.failures', ['count' => $instance->domain_failures]) }} + @endif + @else + {{ __('domain.never_checked') }} + @endif +

+
+ + {{-- Said plainly and where it cannot be missed. A customer who + tidies the record away six months from now takes their own + cloud off its address, and "you were told at the time" is + no use to either side. --}} + {{ __('domain.keep_record') }} + + @endif + @endif +
diff --git a/routes/console.php b/routes/console.php index 4fffc70..5cb6b99 100644 --- a/routes/console.php +++ b/routes/console.php @@ -73,3 +73,14 @@ Schedule::command('clupilot:prune-exports') Schedule::command('clupilot:sample-status') ->everyFiveMinutes() ->withoutOverlapping(); + +// Re-read the DNS proof for every custom domain. +// +// Not a one-off: a token checked once can be taken straight back out, and a +// domain that later lapses and is registered by somebody else keeps resolving +// to the same instance. Nightly, and at an hour where withdrawing a domain +// inconveniences nobody — a withdrawal takes a working Nextcloud off its own +// address, which is not a thing to do at eleven in the morning. +Schedule::command('clupilot:verify-domains') + ->dailyAt('03:40') + ->withoutOverlapping(); diff --git a/routes/web.php b/routes/web.php index 31d608c..18e8f3a 100644 --- a/routes/web.php +++ b/routes/web.php @@ -241,6 +241,7 @@ $portal = function () { Route::middleware(['auth', 'verified', 'customer.active'])->group(function () { Route::get('/dashboard', Dashboard::class)->name('dashboard'); Route::get('/cloud', Cloud::class)->name('cloud'); + Route::get('/domain', \App\Livewire\CustomDomain::class)->name('domain'); Route::get('/users', Users::class)->name('users'); Route::get('/backups', Backups::class)->name('backups'); Route::get('/invoices', Invoices::class)->name('invoices'); diff --git a/tests/Feature/Admin/UpdateButtonTest.php b/tests/Feature/Admin/UpdateButtonTest.php index 7d21105..b95c8dc 100644 --- a/tests/Feature/Admin/UpdateButtonTest.php +++ b/tests/Feature/Admin/UpdateButtonTest.php @@ -860,3 +860,36 @@ it('does not cover the console because a button was pressed over nothing', funct $component->call('requestUpdate')->assertDispatched('update-started'); $component->call('requestUpdate')->assertNotDispatched('update-started'); }); + +it('does not read the agent’s start-up gap as the end of the run', function () { + // The reported loop, in the one place it can be reproduced: the agent + // consumes the request file before it resolves the release, and writes + // `state: running` only once it has decided to go ahead. In between, the + // endpoint honestly answers "nothing is running" — and the watcher read + // that as "finished" and reloaded, so the overlay opened on the click, + // vanished a poll later, and the 503 arrived after it. + $watcher = Illuminate\Support\Facades\File::get(resource_path('js/app.js')); + + // The overlay may only close once the SERVER has confirmed a run and then + // stopped reporting it. + expect($watcher)->toContain('this.wasRunning && this.serverConfirmed && !state.running') + // And it may not wait forever: the agent refuses a request with nothing + // to install by writing nothing at all. + ->and($watcher)->toContain('this.unconfirmedPolls > 20'); +}); + +it('reproduces the gap the console used to reload on', function () { + // The state the endpoint reports between the two agent steps: the request + // has been consumed, the status still says idle. `running` is false here + // and that is correct — the fix is on the reader, not on this value. + writeStatus(['state' => 'idle', 'checked_at' => now()->toIso8601String(), 'behind' => 1]); + + app(UpdateChannel::class)->request('owner@example.com'); + expect(app(UpdateChannel::class)->state()['running'])->toBeTrue(); + + // The agent takes the request… + Illuminate\Support\Facades\File::delete(storage_path('app/deploy/update-request.json')); + + // …and has not written its status yet. + expect(app(UpdateChannel::class)->state()['running'])->toBeFalse(); +}); diff --git a/tests/Feature/CustomDomainTest.php b/tests/Feature/CustomDomainTest.php new file mode 100644 index 0000000..8331c74 --- /dev/null +++ b/tests/Feature/CustomDomainTest.php @@ -0,0 +1,196 @@ +bind(DomainVerifier::class, fn () => new DomainVerifier( + fn (string $name) => $records[$name] ?? [], + )); +} + +function customerInstance(array $attributes = []): Instance +{ + // customers.user_id, not users.customer_id — the link runs the other way. + $user = User::factory()->create(['email_verified_at' => now()]); + $customer = Customer::factory()->create(['user_id' => $user->id, 'email' => $user->email]); + + return Instance::factory()->create([ + 'customer_id' => $customer->id, + 'status' => 'active', + 'subdomain' => 'berger', + ...$attributes, + ]); +} + +it('does not serve a domain nobody has proven', function () { + // The hole, stated as a test: a hostname in the column is a request, not + // an address. + $instance = customerInstance(['custom_domain' => 'cloud.fremde-firma.at', 'domain_token' => 'abc']); + + expect($instance->domainIsVerified())->toBeFalse() + ->and($instance->address('clupilot.com'))->toBe('berger.clupilot.com'); +}); + +it('serves it once the proof is in the DNS', function () { + $instance = customerInstance(['custom_domain' => 'cloud.berger.at', 'domain_token' => 'tok123']); + + withResolver(['_clupilot-challenge.cloud.berger.at' => [['txt' => 'cp-verify=tok123']]]); + + expect(app(DomainVerifier::class)->proofPresent($instance))->toBeTrue(); + + $this->artisan('clupilot:verify-domains')->assertSuccessful(); + + $instance->refresh(); + + expect($instance->domainIsVerified())->toBeTrue() + ->and($instance->address('clupilot.com'))->toBe('cloud.berger.at'); +}); + +it('finds the proof among a domain’s other TXT records', function () { + // A name commonly carries several: SPF, other providers' verifications. + // DNS returns them in no particular order, so only checking the first + // would fail on a domain that is already in use. + $instance = customerInstance(['custom_domain' => 'cloud.berger.at', 'domain_token' => 'tok123']); + + withResolver(['_clupilot-challenge.cloud.berger.at' => [ + ['txt' => 'v=spf1 -all'], + ['txt' => 'google-site-verification=xyz'], + ['txt' => 'cp-verify=tok123'], + ]]); + + expect(app(DomainVerifier::class)->proofPresent($instance))->toBeTrue(); +}); + +it('is not satisfied by somebody else’s token', function () { + $instance = customerInstance(['custom_domain' => 'cloud.berger.at', 'domain_token' => 'mine']); + + withResolver(['_clupilot-challenge.cloud.berger.at' => [['txt' => 'cp-verify=someone-elses']]]); + + expect(app(DomainVerifier::class)->proofPresent($instance))->toBeFalse(); +}); + +it('withdraws a domain whose proof was taken away — but not on the first miss', function () { + // The reason the check repeats at all: put the token in, pass, take it + // straight back out. And the reason it does not act immediately: one failed + // lookup is a nameserver having a bad minute, and withdrawing takes a + // working Nextcloud off its own address. + $instance = customerInstance([ + 'custom_domain' => 'cloud.berger.at', + 'domain_token' => 'tok123', + 'domain_verified_at' => now()->subDays(30), + ]); + + withResolver([]); // the record is gone + + $this->artisan('clupilot:verify-domains'); + expect($instance->fresh()->domainIsVerified())->toBeTrue() + ->and($instance->fresh()->domain_failures)->toBe(1); + + $this->artisan('clupilot:verify-domains'); + expect($instance->fresh()->domainIsVerified())->toBeTrue(); + + $this->artisan('clupilot:verify-domains'); + expect($instance->fresh()->domainIsVerified())->toBeFalse() + // The domain and its token stay on the row: the customer has to see + // what is wrong and put the record back, not start over. + ->and($instance->fresh()->custom_domain)->toBe('cloud.berger.at') + ->and($instance->fresh()->domain_token)->toBe('tok123'); +}); + +it('forgives a miss once the record comes back', function () { + $instance = customerInstance([ + 'custom_domain' => 'cloud.berger.at', 'domain_token' => 'tok123', + 'domain_verified_at' => now()->subDay(), 'domain_failures' => 2, + ]); + + withResolver(['_clupilot-challenge.cloud.berger.at' => [['txt' => 'cp-verify=tok123']]]); + + $this->artisan('clupilot:verify-domains'); + + expect($instance->fresh()->domain_failures)->toBe(0) + ->and($instance->fresh()->domainIsVerified())->toBeTrue(); +}); + +it('mints a new token when the domain changes', function () { + // Otherwise a customer who once verified example.com could claim any other + // domain later without touching its DNS: the old record is still sitting + // there and only the value is compared. + $instance = customerInstance([ + 'custom_domain' => 'alt.berger.at', 'domain_token' => 'old-token', + 'domain_verified_at' => now(), + ]); + + Livewire::actingAs(User::query()->findOrFail($instance->customer->user_id)) + ->test(CustomDomain::class) + ->set('domain', 'neu.berger.at') + ->call('save') + ->assertHasNoErrors(); + + $instance->refresh(); + + expect($instance->custom_domain)->toBe('neu.berger.at') + ->and($instance->domain_token)->not->toBe('old-token') + // And it is not live until the new one is proven. + ->and($instance->domainIsVerified())->toBeFalse(); +}); + +it('lets a customer take the domain away again', function () { + // It was suggested this should be one-way. It should not: moving an address + // is a proxy entry, a certificate and a line in trusted_domains. + $instance = customerInstance([ + 'custom_domain' => 'cloud.berger.at', 'domain_token' => 'tok', 'domain_verified_at' => now(), + ]); + + Livewire::actingAs(User::query()->findOrFail($instance->customer->user_id)) + ->test(CustomDomain::class) + ->set('domain', '') + ->call('save'); + + $instance->refresh(); + + expect($instance->custom_domain)->toBeNull() + ->and($instance->domain_verified_at)->toBeNull() + ->and($instance->address('clupilot.com'))->toBe('berger.clupilot.com'); +}); + +it('takes a pasted URL and stores a hostname', function () { + $instance = customerInstance(); + + Livewire::actingAs(User::query()->findOrFail($instance->customer->user_id)) + ->test(CustomDomain::class) + ->set('domain', 'https://Cloud.Berger.at/') + ->call('save'); + + expect($instance->fresh()->custom_domain)->toBe('cloud.berger.at'); +}); + +it('refuses something that is not a hostname', function () { + $instance = customerInstance(); + + Livewire::actingAs(User::query()->findOrFail($instance->customer->user_id)) + ->test(CustomDomain::class) + ->set('domain', 'nicht mal annähernd') + ->call('save') + ->assertHasErrors('domain'); + + expect($instance->fresh()->custom_domain)->toBeNull(); +});