From 2277dfe4cb7edcbeec2fc74ade23e1dbae544e34 Mon Sep 17 00:00:00 2001 From: nexxo Date: Wed, 29 Jul 2026 22:42:29 +0200 Subject: [PATCH] Ask the EU register whether a VAT number is real vat_id_verified_at was declared on the customer and read by TaxTreatment, and set by no code anywhere. hasVerifiedVatId() therefore never returned true, so reverse charge could not trigger for anybody - and every EU business customer was invoiced Austrian VAT they cannot reclaim in their own country, which is not a pass-through for them but a real cost until the invoice is corrected. The check has THREE outcomes, and the third is why VatIdCheck exists. VIES is a gateway onto twenty-seven national registers and any of them is regularly down. Reading "I could not ask" as "not registered" would clear a verification that has stood for months, move that customer onto the domestic rate, and change what their next invoice says - because a register was asleep. Reading it as "registered" would put reverse charge on an unchecked number, which is our liability. So on UNAVAILABLE nothing at all is written. Asked at the moment the number is entered, because that is when somebody is looking at the field and can fix a typo. The save has already happened by then and the check cannot undo it: a register that is down must not cost a customer their address change. Shape and membership are refused before a request is spent, and the fake refuses on the same rule as the real verifier. A fake that answers where VIES would never have been asked is not a simplification but a different product, and free text normalises into a well-formed number more easily than it looks - "not a number" becomes NOTANUMBER, which the shape rule alone reads as a Norwegian one. clupilot:verify-vat-ids re-asks about the numbers reverse charge rests on, since a registration can be withdrawn. Deliberately not scheduled: it queries a public service with a concurrency limit on behalf of somebody else's tax position, and the cadence is the owner's to set. Co-Authored-By: Claude Opus 5 --- app/Actions/VerifyCustomerVatId.php | 84 ++++++++++ app/Console/Commands/VerifyVatIds.php | 104 ++++++++++++ app/Livewire/Settings.php | 35 +++- app/Providers/AppServiceProvider.php | 7 +- app/Services/Tax/FakeVatIdVerifier.php | 60 +++++++ app/Services/Tax/VatIdCheck.php | 77 +++++++++ app/Services/Tax/VatIdShape.php | 54 ++++++ app/Services/Tax/VatIdVerifier.php | 17 ++ app/Services/Tax/ViesVatIdVerifier.php | 109 ++++++++++++ lang/de/settings.php | 8 + lang/en/settings.php | 8 + .../Feature/Billing/VatIdVerificationTest.php | 156 ++++++++++++++++++ 12 files changed, 716 insertions(+), 3 deletions(-) create mode 100644 app/Actions/VerifyCustomerVatId.php create mode 100644 app/Console/Commands/VerifyVatIds.php create mode 100644 app/Services/Tax/FakeVatIdVerifier.php create mode 100644 app/Services/Tax/VatIdCheck.php create mode 100644 app/Services/Tax/VatIdShape.php create mode 100644 app/Services/Tax/VatIdVerifier.php create mode 100644 app/Services/Tax/ViesVatIdVerifier.php create mode 100644 tests/Feature/Billing/VatIdVerificationTest.php diff --git a/app/Actions/VerifyCustomerVatId.php b/app/Actions/VerifyCustomerVatId.php new file mode 100644 index 0000000..453c559 --- /dev/null +++ b/app/Actions/VerifyCustomerVatId.php @@ -0,0 +1,84 @@ +normalisedVatId(); + + if ($vatId === '') { + // Removing the number removes the verification with it. Otherwise the + // stamp would sit on the record with nothing to compare against, and + // the next number typed in would look pre-verified for one save. + $this->clear($customer); + + return null; + } + + $check = $this->verifier->check($vatId); + + // The whole reason VatIdCheck has three outcomes. An outage tells us + // nothing, so nothing is written: a verification that has been standing + // for months must not be cleared — and a customer's invoices changed — + // because a national register was down for the night. + if ($check->isUnavailable()) { + return $check; + } + + if ($check->isInvalid()) { + $this->clear($customer); + + return $check; + } + + // Bound to the value that was checked, not to a flag. Customer:: + // hasVerifiedVatId() compares the two, so editing the number afterwards + // cannot inherit this confirmation whichever code path does the editing. + $customer->forceFill([ + 'vat_id_verified_at' => now(), + 'vat_id_verified_value' => $vatId, + ])->save(); + + return $check; + } + + private function clear(Customer $customer): void + { + if ($customer->vat_id_verified_at === null && $customer->vat_id_verified_value === null) { + return; + } + + $customer->forceFill([ + 'vat_id_verified_at' => null, + 'vat_id_verified_value' => null, + ])->save(); + } +} diff --git a/app/Console/Commands/VerifyVatIds.php b/app/Console/Commands/VerifyVatIds.php new file mode 100644 index 0000000..3baaba6 --- /dev/null +++ b/app/Console/Commands/VerifyVatIds.php @@ -0,0 +1,104 @@ +whereNotNull('vat_id') + ->where('vat_id', '!=', '') + ->when($this->option('customer'), fn ($q, $uuid) => $q->where('uuid', $uuid)) + // Longest unchecked first, so a short run covers the ones we know + // least about rather than the ones checked yesterday. A plain + // ascending sort does that on its own — never-checked rows are NULL + // and both engines sort those first — which is why there is no raw + // SQL here: MariaDB and SQLite would need different expressions for + // it, and a query that only runs on one of them makes the test + // database a different product from the live one. + ->orderBy('vat_id_verified_at') + ->limit((int) $this->option('limit')) + ->get(); + + if ($customers->isEmpty()) { + $this->info('No VAT numbers on record.'); + + return self::SUCCESS; + } + + $counts = ['verified' => 0, 'rejected' => 0, 'unchecked' => 0]; + + foreach ($customers as $customer) { + if ($this->option('dry-run')) { + $this->line(sprintf('would ask: %s (%s)', $customer->normalisedVatId(), $customer->name)); + + continue; + } + + $check = $verify($customer); + + match (true) { + $check === null => null, + $check->isValid() => $counts['verified']++, + $check->isInvalid() => $counts['rejected']++, + default => $counts['unchecked']++, + }; + + if ($check?->isInvalid()) { + // Named individually, because this one needs a person: an invoice + // already issued at 0 % against a number the register now rejects + // has to be looked at, and a count in a summary hides that. + $this->warn(sprintf( + 'not registered: %s (%s) — reverse charge no longer applies', + $customer->normalisedVatId(), + $customer->name, + )); + } + } + + if ($this->option('dry-run')) { + $this->info($customers->count().' number(s) would be asked about.'); + + return self::SUCCESS; + } + + $this->info(sprintf( + '%d confirmed, %d rejected, %d could not be checked (register unavailable — nothing changed for those).', + $counts['verified'], + $counts['rejected'], + $counts['unchecked'], + )); + + return self::SUCCESS; + } +} diff --git a/app/Livewire/Settings.php b/app/Livewire/Settings.php index 13209b6..336216f 100644 --- a/app/Livewire/Settings.php +++ b/app/Livewire/Settings.php @@ -2,6 +2,7 @@ namespace App\Livewire; +use App\Actions\VerifyCustomerVatId; use App\Actions\WithdrawContract; use App\Livewire\Concerns\ChangesOwnPassword; use App\Livewire\Concerns\ConfirmsPassword; @@ -206,7 +207,39 @@ class Settings extends Component 'billing_address' => $data['billingAddress'] ?: null, ]); - $this->dispatch('notify', message: __('settings.profile_saved')); + $this->dispatch('notify', message: $this->verifyVatId($c)); + } + + /** + * Check the number against the register, and say what came back. + * + * Here, at the moment it is entered, because that is the only moment somebody + * is looking at the field and can correct a typo. Reverse charge is only + * lawful against a number that is actually registered, so an unchecked number + * is charged the domestic rate — which means an EU business that mistyped + * theirs pays VAT it cannot reclaim at home until somebody notices. + * + * The save has already happened by the time this runs and cannot be undone by + * it. A register that is down must not cost the customer their address change, + * and a number the register rejects is still the number they typed — theirs to + * correct, not ours to discard. + */ + private function verifyVatId(Customer $customer): string + { + $check = app(VerifyCustomerVatId::class)($customer); + + if ($check === null) { + return __('settings.profile_saved'); + } + + if ($check->isValid()) { + return __('settings.vat_verified'); + } + + // Told apart on purpose. "We could not check right now" is our problem and + // resolves itself on the next sweep; "the register does not know this + // number" is something only the customer can fix. + return __($check->isUnavailable() ? 'settings.vat_unchecked' : 'settings.vat_unknown'); } public function saveBranding(): void diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 1c25740..27c012d 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -7,8 +7,6 @@ use App\Http\Middleware\EnsureCustomerActive; use App\Http\Middleware\RestrictAdminHost; use App\Http\Middleware\RestrictConsoleNetwork; use App\Listeners\RecordSentMail; -use App\Services\Mail\ImapMailbox; -use App\Services\Mail\InboundMailbox; use App\Listeners\RecordSignInDevice; use App\Mail\MaintenanceCancelledMail; use App\Mail\Transport\MailboxTransport; @@ -20,6 +18,8 @@ use App\Services\Dns\FileHostDnsDirectory; use App\Services\Dns\HetznerDnsClient; use App\Services\Dns\HostDnsDirectory; use App\Services\Dns\HttpHetznerDnsClient; +use App\Services\Mail\ImapMailbox; +use App\Services\Mail\InboundMailbox; use App\Services\Maintenance\MaintenanceNotifier; use App\Services\Monitoring\HttpMonitoringClient; use App\Services\Monitoring\MonitoringClient; @@ -29,6 +29,8 @@ use App\Services\Ssh\PhpseclibRemoteShell; use App\Services\Ssh\RemoteShell; use App\Services\Stripe\HttpStripeClient; use App\Services\Stripe\StripeClient; +use App\Services\Tax\VatIdVerifier; +use App\Services\Tax\ViesVatIdVerifier; use App\Services\Traefik\SshTraefikWriter; use App\Services\Traefik\TraefikWriter; use App\Services\Wireguard\LocalWireguardHub; @@ -64,6 +66,7 @@ class AppServiceProvider extends ServiceProvider $this->app->bind(TraefikWriter::class, SshTraefikWriter::class); $this->app->bind(MonitoringClient::class, HttpMonitoringClient::class); $this->app->bind(StripeClient::class, HttpStripeClient::class); + $this->app->bind(VatIdVerifier::class, ViesVatIdVerifier::class); // The support mailbox, read over IMAP. A test puts a FakeMailbox here: // the socket half cannot be exercised without a mail server, and a test // that needs one is a test nobody runs. What IS exercised is the diff --git a/app/Services/Tax/FakeVatIdVerifier.php b/app/Services/Tax/FakeVatIdVerifier.php new file mode 100644 index 0000000..94a9e6d --- /dev/null +++ b/app/Services/Tax/FakeVatIdVerifier.php @@ -0,0 +1,60 @@ + */ + private array $answers = []; + + /** @var array every number that was asked about, in order */ + public array $asked = []; + + public function registered(string $vatId, ?string $name = null): self + { + $this->answers[Customer::normaliseVatId($vatId)] = VatIdCheck::valid($name); + + return $this; + } + + public function notRegistered(string $vatId): self + { + $this->answers[Customer::normaliseVatId($vatId)] = VatIdCheck::invalid(); + + return $this; + } + + /** The register is down — the case that must never clear a verification. */ + public function down(string $vatId): self + { + $this->answers[Customer::normaliseVatId($vatId)] = VatIdCheck::unavailable('test'); + + return $this; + } + + public function check(string $vatId): VatIdCheck + { + $normalised = Customer::normaliseVatId($vatId); + + // Refused before it is recorded as asked, exactly as the real verifier + // refuses before it spends a request. Without this the fake would answer + // where VIES would never have been consulted, and a test could prove the + // opposite of what the product does. + if (! VatIdShape::verifiable($normalised)) { + return VatIdCheck::invalid(); + } + + $this->asked[] = $normalised; + + return $this->answers[$normalised] ?? VatIdCheck::unavailable('not armed in this test'); + } +} diff --git a/app/Services/Tax/VatIdCheck.php b/app/Services/Tax/VatIdCheck.php new file mode 100644 index 0000000..60e0b7e --- /dev/null +++ b/app/Services/Tax/VatIdCheck.php @@ -0,0 +1,77 @@ +outcome === self::VALID; + } + + public function isInvalid(): bool + { + return $this->outcome === self::INVALID; + } + + /** + * Nothing was learnt. + * + * Callers must leave whatever they already knew alone when this is true — + * see the class comment for why that is not merely cautious. + */ + public function isUnavailable(): bool + { + return $this->outcome === self::UNAVAILABLE; + } +} diff --git a/app/Services/Tax/VatIdShape.php b/app/Services/Tax/VatIdShape.php new file mode 100644 index 0000000..786ed85 --- /dev/null +++ b/app/Services/Tax/VatIdShape.php @@ -0,0 +1,54 @@ +timeout(8) + ->post($this->endpoint(), [ + 'countryCode' => $parts[1], + 'vatNumber' => $parts[2], + ]); + } catch (Throwable $e) { + return VatIdCheck::unavailable($e->getMessage()); + } + + if (! $response->successful()) { + return VatIdCheck::unavailable('http_'.$response->status()); + } + + $error = (string) ($response->json('userError') ?? ''); + + if (in_array($error, self::GATEWAY_ERRORS, true)) { + Log::info('VIES could not answer for a VAT number', ['reason' => $error]); + + return VatIdCheck::unavailable($error); + } + + if ($response->json('valid') !== true) { + return VatIdCheck::invalid(); + } + + // The register returns "---" for a company that has asked not to have its + // details published. That is a registered number with no name attached, + // not an empty answer, so it is stored as nothing rather than as dashes. + return VatIdCheck::valid( + $this->orNull($response->json('name')), + $this->orNull($response->json('address')), + ); + } + + private function orNull(mixed $value): ?string + { + $value = trim((string) $value); + + return $value === '' || trim($value, '- ') === '' ? null : $value; + } + + private function endpoint(): string + { + return rtrim((string) config( + 'services.vies.endpoint', + 'https://ec.europa.eu/taxation_customs/vies/rest-api/check-vat-number', + ), '/'); + } +} diff --git a/lang/de/settings.php b/lang/de/settings.php index 34e0f37..aeb6bc3 100644 --- a/lang/de/settings.php +++ b/lang/de/settings.php @@ -13,6 +13,14 @@ return [ 'billing_address' => 'Rechnungsadresse', 'profile_saved' => 'Firmendaten gespeichert.', + // Drei Meldungen statt einer, weil die drei Fälle für den Kunden etwas + // Verschiedenes bedeuten: bestätigt, von uns gerade nicht prüfbar, oder dem + // Register unbekannt. Nur der letzte ist etwas, das er selbst richtigstellen + // kann — siehe App\Services\Tax\VatIdCheck. + 'vat_verified' => 'Firmendaten gespeichert. Ihre UID ist im EU-Register bestätigt.', + 'vat_unchecked' => 'Firmendaten gespeichert. Das EU-Register ist gerade nicht erreichbar — Ihre UID wird erneut geprüft.', + 'vat_unknown' => 'Firmendaten gespeichert. Das EU-Register kennt diese UID nicht — bitte prüfen Sie die Nummer. Bis dahin wird Umsatzsteuer verrechnet.', + 'branding_title' => 'Branding', 'branding_sub' => 'Logo und Farben werden bei der Bereitstellung auf Ihre Cloud angewendet. Ohne Angabe nutzen wir die CluPilot-Standardwerte.', 'brand_display_name' => 'Anzeigename', diff --git a/lang/en/settings.php b/lang/en/settings.php index f1dab31..4427fd1 100644 --- a/lang/en/settings.php +++ b/lang/en/settings.php @@ -13,6 +13,14 @@ return [ 'billing_address' => 'Billing address', 'profile_saved' => 'Company details saved.', + // Three messages rather than one, because the three cases mean different + // things to the customer: confirmed, not checkable by us right now, or + // unknown to the register. Only the last is something they can put right — + // see App\Services\Tax\VatIdCheck. + 'vat_verified' => 'Company details saved. Your VAT number is confirmed in the EU register.', + 'vat_unchecked' => 'Company details saved. The EU register cannot be reached right now — your VAT number will be checked again.', + 'vat_unknown' => 'Company details saved. The EU register does not know this VAT number — please check it. Until then VAT is charged.', + 'branding_title' => 'Branding', 'branding_sub' => 'Your logo and colors are applied to your cloud during provisioning. If left empty we use the CluPilot defaults.', 'brand_display_name' => 'Display name', diff --git a/tests/Feature/Billing/VatIdVerificationTest.php b/tests/Feature/Billing/VatIdVerificationTest.php new file mode 100644 index 0000000..b1224a1 --- /dev/null +++ b/tests/Feature/Billing/VatIdVerificationTest.php @@ -0,0 +1,156 @@ +instance(VatIdVerifier::class, $fake); + + return $fake; +} + +function businessWithVatId(string $vatId): Customer +{ + return Customer::factory()->create([ + 'customer_type' => Customer::TYPE_BUSINESS, + 'vat_id' => $vatId, + ]); +} + +it('records a number the register knows, and reverse charge then applies', function () { + armRegister()->registered('NL123456789B01', 'Berger B.V.'); + $customer = businessWithVatId('NL123456789B01'); + + app(VerifyCustomerVatId::class)($customer); + + expect($customer->fresh()->hasVerifiedVatId())->toBeTrue() + // Proven through the tax rule, not through the column: the column is only + // interesting because of what TaxTreatment does with it. + ->and(TaxTreatment::for($customer->fresh())->reverseCharge)->toBeTrue() + ->and(TaxTreatment::for($customer->fresh())->rate)->toBe(0.0); +}); + +it('refuses a number the register does not know, and keeps charging VAT', function () { + armRegister()->notRegistered('NL999999999B99'); + $customer = businessWithVatId('NL999999999B99'); + + $check = app(VerifyCustomerVatId::class)($customer); + + expect($check->isInvalid())->toBeTrue() + ->and($customer->fresh()->hasVerifiedVatId())->toBeFalse() + ->and(TaxTreatment::for($customer->fresh())->reverseCharge)->toBeFalse(); +}); + +it('changes nothing at all when the register is unreachable', function () { + // The one that matters. An outage must not clear a verification that has been + // standing for months and quietly move that customer onto the domestic rate — + // their next invoice would say something different because a national register + // was asleep. + $customer = businessWithVatId('NL123456789B01'); + armRegister()->registered('NL123456789B01'); + app(VerifyCustomerVatId::class)($customer); + + $stampedAt = $customer->fresh()->vat_id_verified_at; + + armRegister()->down('NL123456789B01'); + $check = app(VerifyCustomerVatId::class)($customer->fresh()); + + expect($check->isUnavailable())->toBeTrue() + ->and($customer->fresh()->hasVerifiedVatId())->toBeTrue() + ->and($customer->fresh()->vat_id_verified_at->equalTo($stampedAt))->toBeTrue() + ->and(TaxTreatment::for($customer->fresh())->reverseCharge)->toBeTrue(); +}); + +it('withdraws reverse charge once the register stops knowing the number', function () { + // A registration can be revoked. From that moment we are zero-rating against + // a number that does not exist, which is our liability, not the customer's. + $customer = businessWithVatId('NL123456789B01'); + armRegister()->registered('NL123456789B01'); + app(VerifyCustomerVatId::class)($customer); + + armRegister()->notRegistered('NL123456789B01'); + app(VerifyCustomerVatId::class)($customer->fresh()); + + expect($customer->fresh()->hasVerifiedVatId())->toBeFalse() + ->and(TaxTreatment::for($customer->fresh())->reverseCharge)->toBeFalse(); +}); + +it('drops the verification when the number is removed', function () { + $customer = businessWithVatId('NL123456789B01'); + armRegister()->registered('NL123456789B01'); + app(VerifyCustomerVatId::class)($customer); + + $customer->update(['vat_id' => null]); + $check = app(VerifyCustomerVatId::class)($customer->fresh()); + + expect($check)->toBeNull() + ->and($customer->fresh()->vat_id_verified_at)->toBeNull() + ->and($customer->fresh()->vat_id_verified_value)->toBeNull(); +}); + +it('never asks the register about a number no member state could hold', function () { + $fake = armRegister(); + $customer = businessWithVatId('not-a-number'); + + $check = app(VerifyCustomerVatId::class)($customer); + + expect($check->isInvalid())->toBeTrue() + // Not one request spent on something that cannot be a VAT number. + ->and($fake->asked)->toBeEmpty(); +}); + +it('still charges domestic VAT to a business in our own country', function () { + // Reverse charge is a cross-border rule. An Austrian business pays Austrian + // VAT and reclaims it as input tax like any other Austrian buyer. + armRegister()->registered('ATU12345678'); + $customer = businessWithVatId('ATU12345678'); + + app(VerifyCustomerVatId::class)($customer); + + expect($customer->fresh()->hasVerifiedVatId())->toBeTrue() + ->and(TaxTreatment::for($customer->fresh())->reverseCharge)->toBeFalse() + ->and(TaxTreatment::for($customer->fresh())->rate)->toBeGreaterThan(0.0); +}); + +it('still charges VAT to somebody who has recorded themselves as a private person', function () { + armRegister()->registered('NL123456789B01'); + $customer = Customer::factory()->create([ + 'customer_type' => Customer::TYPE_CONSUMER, + 'vat_id' => 'NL123456789B01', + ]); + + app(VerifyCustomerVatId::class)($customer); + + // The number is real — it is just not theirs to zero the tax with. + expect($customer->fresh()->hasVerifiedVatId())->toBeTrue() + ->and(TaxTreatment::for($customer->fresh())->reverseCharge)->toBeFalse(); +}); + +it('asks about every number on record and touches nothing in a dry run', function () { + armRegister()->registered('NL123456789B01'); + $customer = businessWithVatId('NL123456789B01'); + + $this->artisan('clupilot:verify-vat-ids --dry-run')->assertSuccessful(); + expect($customer->fresh()->hasVerifiedVatId())->toBeFalse(); + + $this->artisan('clupilot:verify-vat-ids')->assertSuccessful(); + expect($customer->fresh()->hasVerifiedVatId())->toBeTrue(); +});