[AUT-13970] Keep phone_provider_config through a tenant pull/push

[jdabrowski]

Jira task

AUT-13970

Important

Do not merge before ciam-client-go#75. go.mod currently pins that PR's branch commit (1f193808) so this builds and tests green today; it needs re-pinning to a master pseudo-version once #75 lands. #75 is itself gated on a Customer Success sign-off.

Release Notes Description (public)

Tenant phone provider configuration can now be managed as code. cac pull writes it to phone_provider_config.yaml and cac push sends it back. Previously it was dropped on pull and could not be pushed at all.

SMS and voice delivery — including Twilio credentials and sender ID — is configured through the tenant's phone provider configuration. It is no longer driven by the provider field on an SMS or voice MFA method; that field is deprecated and no longer selects a messaging provider.

If you use --filter, add phone_provider_config to the list. --filter names what to include, so phone provider configuration is only sent when you ask for it:

cac push --tenant --method patch \
  --filter mfa_methods --filter pools --filter schemas \
  --filter phone_provider_config

Runs without --filter include it automatically.

Implementation details (internal)

Problem. TenantStorage never handled phone_provider_config. storeTenant serializes the tenant root through smodels.Tenant to strip sub-resources that live in their own files, and that struct — the Tenant API resource — has no such field, because server-side a phone provider configuration is a separate entity, not a tenant attribute. Nothing else picked it up either, so it was dropped on pull and could never be pushed back. This is why affected repositories contain no phone provider configuration at all, and it's the client-side half of the AUT-13968 incident.

Solution. Treat it as what it is — a singular sub-resource — and give it its own file, exactly as server storage already does for claims, consent, ciba, theme_binding and friends:

• writeFile(model.PhoneProviderConfig, path/phone_provider_config) on the way out
• readFileToMap(tenant, "phone_provider_config", ...) on the way back

storeTenant and tenant.yaml are untouched.

An earlier revision of this PR instead widened the model storeTenant serializes into. That was wrong: utils.TenantRootKeys is derived from the same type, deliberately, so that root-level keys stay in sync without a hand-maintained list. Widening one without the other wrote phone_provider_config into tenant.yaml while leaving it out of the root key set, so push --filter root silently dropped it. Keeping it a sub-resource avoids the coupling entirely.

Filter behaviour now matches mfa_methods:

--filter root                  -> [name]                    (correctly excludes it)
--filter phone_provider_config -> [phone_provider_config]   (targetable)
--filter mfa_methods           -> [mfa_methods]
no filter                      -> everything

Also bumps acp-client-go to the AUT-13969 spec sync so TreePhoneProviderConfig carries mode. Without it the config would round-trip mode-less, which is the shape that caused the original wipe.

Tests.

• TestTenantStorage/phone_provider_config — asserts the config lands in phone_provider_config.yaml and tenant.yaml stays clean
• TestTenantStoragePhoneProviderConfigRoundTrip — Write → Read → model, asserting mode and credentials survive

Both falsification-checked: dropping the writeFile call fails the round-trip with "phone_provider_config did not survive the round trip", and passes again when restored.

Two golden fixtures are updated for fields the client bump adds (require_user_interaction_before_prompt, enforce_application_membership, skip_dbfp). Full suite green with no replace directive.

Note for reviewers: a cac pull after this change adds a new phone_provider_config.yaml to repositories whose tenant has one. Credentials land in that file rather than in tenant.yaml.

Out of scope, but noted: the same gap also drops tenant-level features and translations, both of which are maps and would want the pools/mfa_methods directory treatment. Left as-is deliberately.

---

Reference

Twilio Enum Drift — the breaking-change analysis behind the client bump this PR depends on, including why the twilio provider value affects every migrated tenant rather than only stale checkouts. Access-controlled; ask if you need it opened up.

[Copilot]

Pull request overview

This PR updates tenant config-as-code persistence so phone_provider_config is no longer dropped when writing tenant.yaml, enabling it to survive a pull/push round trip.

Changes:

• Introduces a tenantFile wrapper type to include phone_provider_config while still stripping sub-resources stored in separate files.
• Adds/extends tests to validate phone_provider_config serialization and end-to-end round-trip preservation.
• Bumps github.com/cloudentity/acp-client-go to a commit pseudo-version and updates golden YAML fixtures for newly surfaced fields.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
internal/cac/storage/tenant_storage.go	Writes tenant.yaml via a new wrapper type to preserve phone_provider_config.
internal/cac/storage/tenant_storage_test.go	Adds fixture updates and new tests covering phone_provider_config persistence/round-trip.
internal/cac/storage/server_storage_test.go	Updates golden YAML expectations for new fields from the client bump.
go.mod	Pins acp-client-go to a specific commit pseudo-version required by upstream work.
go.sum	Updates checksums for the new acp-client-go pseudo-version.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jdabrowski]

Agreed, and tracked — the > [!IMPORTANT] block at the top of the PR description says the same thing.

Leaving this thread open deliberately as the live reminder: it must be re-pinned to a master pseudo-version once #75 merges, and this PR should not merge before that happens.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

go.mod:7

• The explicit merge prerequisite is still unmet: this pseudo-version points to ciam-client-go#75's head commit, and that PR is currently open, draft, and blocked rather than merged to master. Re-pin this dependency to the resulting master pseudo-version after #75 lands; the current pin should not be merged.

	github.com/cloudentity/acp-client-go v0.0.0-20260820095634-1f1938087bcf

[jdabrowski]

Accurate on every point, and thanks — you reconstructed the full blast radius from the diff alone, which matches what was measured independently before this PR was opened.

Confirming your analysis: the value is not only present in stale checkouts. migrate_twilio_to_phone_providers.go:24-27 deliberately leaves the stored value alone ("a binary rollback must land on data the previous release can still deliver with; the twilio→embedded rewrite ships in a later release, together with the validator tightening"), so the API still returns twilio and a fresh cac pull writes it to disk. Every migrated tenant is affected.

The compatibility shim was considered and deliberately not taken. Reasoning:

• The failure is fail-closed and pre-flight. cmd/push.go:57-61 validates before app.Client.Write, so a blind upgrade-and-push exits non-zero having sent nothing — no tenant is mutated. cac pull is unaffected and round-trips the value verbatim.
• settings.{sms,voice}.provider no longer selects anything. It is read in exactly six places, all legacy-Twilio bookkeeping, none on the delivery path — delivery has routed solely through phone_provider_config since ciam-core #10473. embedded is the field's only remaining legal value, not a provider choice.
• Normalising twilio → embedded client-side would silently rewrite configuration on the user's behalf; restoring the enum would re-publish a value that #10473 set out to retire.

The remedy is documented instead: a one-line edit per MFA method file, inert at runtime. On --method patch the line may simply be deleted (verified against a live migrated tenant: the stored value, credentials and phone provider config are all preserved).

This is gated on a Customer Success sign-off, and this PR is blocked on #75 regardless. Leaving the thread open so reviewers see the reasoning rather than a resolved thread.