[PM-40694] fix: Derive Stripe tax ID type server-side

[amorask-bitwarden]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-40694

📔 Objective

UK business customers were being charged 20% VAT they didn't owe.

The web client resolves a tax ID's Stripe type from the billing country alone, returning the first candidate flagged as impacting tax calculation. GB has two, and the Northern Ireland eu_vat entry is listed first — so every UK VAT ID reached Stripe typed eu_vat. Stripe stores it without complaint but will not apply reverse charge.

Four server write sites forwarded the client-supplied type verbatim. Each now derives it from (country, value) via ITaxService.GetStripeTaxCode:

• UpdateBillingAddressCommand
• UpgradePremiumToOrganizationCommand
• ProviderBillingService.SetupCustomer
• PreviewOrganizationTaxCommand

That method already existed and already distinguishes GB from XI. Before this PR its only non-test caller was OrganizationBillingService on the org-signup path — which is why creating an organization stored the correct type while editing the billing address afterwards overwrote it.

The second commit adds an unprefixed ^[0-9]{9}$ → gb_vat rule for GB. A UK VAT number is nine digits and the GB prefix is routinely omitted; without this, the fix misses exactly the customers the ticket is about.

Design decisions

Fallback rather than throw. When derivation returns null the client-supplied type is kept and a warning logged. TaxService covers 83 countries; the client renders a tax ID field for 112. Throwing would break tax ID saves that work today. OrganizationBillingService does throw here — reconciling the two is deliberate follow-up, not part of this PR.

No feature flag. The fallback makes the null path byte-identical to previous behaviour, so there is no scenario where flag-off is safer than flag-on. Revert is four files with no schema or data dependency.

Derivation sits after the OneOf match in PreviewOrganizationTaxCommand, so it also corrects previews for customers whose stored type is already wrong, ahead of data remediation.

Verified against Stripe (test account)

• gb_vat with an unprefixed value is accepted and verifies against HMRC, so the type is corrected without rewriting the stored value.
• eu_vat with an unprefixed value is rejected (tax_id_invalid). Customers in that state were never receiving reverse charge, so deriving a country-specific type for them is not a regression.
• On a live 13% Ontario registration: ca_gst_hst with a full …RT0001 number zeroes the tax; ca_bn does not. A bare Business Number correctly grants no relief, and ca_gst_hst with a bare value is rejected by Stripe.

Known caveat

Bare nine digits cannot distinguish mainland UK from Northern Ireland. An NI customer who omits XI now resolves to gb_vat instead of eu_vat. NI postcodes all start with BT, which is the disambiguator if this needs addressing.

Follow-up, deliberately out of scope

• TaxService is 36 tax ID types behind Stripe's current table, and Nigeria's and Argentina's documented formats have drifted from ours. Adding missing types is low risk; changing the two live formats needs confirmation of which forms are actually in circulation.
• The client still sends the wrong type and still shows a misleading format hint after a failed verification. That is what closes PM-27010 — this PR fixes its blocking symptom (a bare Canadian BN now saves) but not the hint.
• Three ♻️ Refactor findings from review, deferred: the derive-fallback block is inlined at four sites, the warning carries no subscriber identifier, and GetStripeTaxCode is still declared non-nullable. All are best done alongside the fallback-policy reconciliation, which touches all five sites anyway.
• Stripe data remediation: existing eu_vat records with GB-prefixed values need delete-and-recreate as gb_vat (Stripe has no update endpoint). Must run after this ships, or the customer's next address save re-corrupts the record.

Testing

298 unit tests passing across the five affected suites. The GB regression test sends eu_vat with a GB-prefixed value and asserts Stripe receives gb_vat, so it fails on main.

QA note: read the customer.tax_id.created event payload, not the Dashboard Details panel — Stripe renders a GB-prefixed eu_vat as "GB VAT", which masks the defect entirely.

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This bugfix routes four Stripe tax-ID write sites (UpdateBillingAddressCommand, UpgradePremiumToOrganizationCommand, ProviderBillingService.SetupCustomer, PreviewOrganizationTaxCommand) through ITaxService.GetStripeTaxCode, so the Stripe type is derived from (country, value) instead of trusted from the client, and adds an unprefixed nine-digit gb_vat rule to TaxService. I verified regex ordering and cross-country reclassification risk, the Spanish NIF secondary-VAT gating, the delete-then-recreate path in UpdateBillingAddressCommand, DI wiring, logging hygiene, and constructor updates across all test suites. No findings at or above the reporting threshold.

Code Review Details

No findings at or above the reporting threshold.

Verification notes from this review:

• No silent reclassification of currently-working tax IDs. GetStripeTaxCode returns the first country-filtered regex match in declaration order. I walked every multi-entry country (AU, BR, BG, CA, CH, DE, ES, GB, HR, HU, JP, LI, MY, NO, RO, RU, SG, SI, UZ) checking whether two anchored patterns in the same country can match one value. GB is the only intentional overlap; every other pair is disjoint by length, prefix, or suffix. So the only behaviour change for non-GB countries is null → a concrete type, never one valid type → a different one.
• The new GB rule is a strict widening. ^[0-9]{9}$ is declared after ^XI[0-9]{9}$ and ^GB[0-9]{9}$, so no previously-resolving GB value changes outcome. The bare-digit Northern Ireland ambiguity is called out in the PR description.
• UpdateBillingAddressCommand self-heals. Existing tax IDs are unconditionally deleted and recreated on every business address save, so a stored GB-prefixed eu_vat is corrected on the customer's next save rather than being skipped as unchanged.
• Spanish NIF pairing is correct. All four sites now gate the secondary eu_vat creation on the derived code. I confirmed a stored ESA12345678 value matches neither the es_cif nor the ES eu_vat expression, so the preview path falls back rather than producing a double-prefixed ESESA… value.
• Logging contains no sensitive data. The warning emits only country and the client-supplied type code; the tax ID value is never logged, and each of the four fallback tests explicitly asserts its absence.
• DI and construction are complete. ITaxService is registered by AddBillingOperations(), invoked in every startup that also calls AddCommercialCoreServices() (Api, Billing, Admin). All three manual command instantiations in tests were updated; no other construction sites exist.
• Deferred refactors not re-raised. The duplicated derive-fallback block, the warning lacking a subscriber identifier, and the non-nullable GetStripeTaxCode return type are already documented in the PR description as follow-up alongside the fallback-policy reconciliation with OrganizationBillingService.

The dependency ordering in the PR description is worth preserving through merge: the Stripe data remediation for existing GB-prefixed eu_vat records must run after this ships, or the next address save re-corrupts them.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.89%. Comparing base (b4d4c55) to head (483dd41).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #8122       +/-   ##
===========================================
+ Coverage   14.99%   62.89%   +47.90%     
===========================================
  Files        1409     2301      +892     
  Lines       61097   100256    +39159     
  Branches     4864     9027     +4163     
===========================================
+ Hits         9161    63061    +53900     
+ Misses      51780    35013    -16767     
- Partials      156     2182     +2026     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.