Add opt-in Key Vault RBAC authorization for CAF/landing zone compliance

[Michael Flanakin (flanakin)]

Summary

Adds an enableRbacAuthorization parameter to the FinOps hub Bicep templates that switches the remote-hub Key Vault from the legacy access-policy model to Azure RBAC. This is required by Cloud Adoption Framework (CAF) / Enterprise-Scale landing zone Azure Policy guardrails (e.g., "Enforce recommended guardrails for Azure Key Vault"), which flag vaults that use access policies instead of RBAC.

Fixes #1067

What changed

• src/templates/finops-hub/modules/fx/hub-app.bicep — the Key Vault resource now sets enableRbacAuthorization: app.hub.options.keyVaultEnableRbacAuthorization instead of a hardcoded false. When RBAC is enabled:
  • accessPolicies is forced to an empty array (Azure rejects a non-empty accessPolicies array when enableRbacAuthorization: true).
  • A new keyVaultRoleAssignment resource grants the Data Factory managed identity the built-in Key Vault Secrets User role (4633458b-17de-408a-b874-0445c86b69e6) on the vault — the RBAC equivalent of the secrets: ['get'] access policy it previously relied on. This is the only identity that reads secrets from this vault (verified below), so it's the only equivalent role assignment needed.
• src/templates/finops-hub/modules/fx/hub-types.bicep — added keyVaultEnableRbacAuthorization to HubProperties.options and threaded it through newHubInternal/newHub.
• src/templates/finops-hub/modules/hub.bicep and main.bicep — added an enableRbacAuthorization bool = false parameter, following the exact pattern already used for enablePurgeProtection.
• createUiDefinition.json — added a matching "Enable Key Vault RBAC authorization" checkbox next to the existing purge protection checkbox, gated to the remote-hub analytics engine path (the only path that deploys this vault).
• docs-mslearn/toolkit/changelog.md — added an entry under Unreleased.

Purge protection (enablePurgeProtection / enableSoftDelete) was already fully implemented on this branch (not something I needed to add) — enableSoftDelete: true is hardcoded, and enablePurgeProtection is already wired end-to-end as an opt-in parameter defaulting to false, following the exact convention I followed for RBAC here.

Investigation: what currently depends on access policies

I traced every consumer of the Key Vault (Microsoft.KeyVault/vaults in src/templates/finops-hub/modules/fx/hub-app.bicep:471, only deployed when the RemoteHub app requests the 'KeyVault' feature — modules/Microsoft.FinOpsHubs/RemoteHub/app.bicep:52):

1. Data Factory's AzureKeyVault linked service (hub-app.bicep:201, linkedService_keyVault) — ADF authenticates as its own system-assigned managed identity at runtime to read the remoteHubStorage connection secret via AzureKeyVaultSecret. This is the only data-plane secret reader, and it's exactly what the pre-existing keyVaultAccessPolicies var (secrets: ['get']) granted. Now granted via RBAC (Key Vault Secrets User) when enableRbacAuthorization: true.
2. The keyVault_secret module (fx/hub-vault.bicep, invoked from RemoteHub/app.bicep:59) writes the storage key as a nested ARM Microsoft.KeyVault/vaults/secrets resource. This is a management-plane (ARM) write authorized by the deploying principal's Azure RBAC role on the resource group (e.g., Contributor), not by the vault's own access-policy/RBAC-authorization setting — so it is unaffected either way.
3. No other hub app (Core, Exports, Analytics, AzureResourceGraph, etc.) references this Key Vault at all.

So the only identity needing a new role assignment is Data Factory's managed identity, which I added.

PR #1349 (closed, not merged)

Read gh pr diff 1349 before starting. It only added enablePurgeProtection (12 lines across main.bicep/hub.bicep/keyVault.bicep) against the pre-reorganization template layout — those exact file paths (modules/keyVault.bicep) no longer exist; the Key Vault resource now lives in modules/fx/hub-app.bicep as part of the namespace-based fx/Microsoft.FinOpsHubs restructuring. It didn't touch RBAC at all, and its purge-protection change has since been superseded by a more complete implementation already on dev/v15-prep. I did not build on it — nothing to build on for RBAC, and the purge-protection piece it targeted was already done more thoroughly elsewhere.

Design decision: opt-in, not forced-on

Both enableRbacAuthorization and enablePurgeProtection are irreversible per Azure's API contract:

• Once enablePurgeProtection: true is set on a vault, it cannot be reverted.
• Switching enableRbacAuthorization from false to true on an existing vault immediately stops honoring access policies; any identity not covered by an equivalent RBAC role assignment loses access with no rollback path (short of recreating the vault).

Because the FinOps hub template supports redeploying over existing hub instances (upgrade scenario), forcing either property on unconditionally would silently break already-deployed hubs that don't opt in, with no way back. I followed the existing precedent set by enablePurgeProtection (already opt-in, defaulting to false, surfaced identically in main.bicep/hub.bicep/createUiDefinition.json) and applied the same pattern to enableRbacAuthorization. Organizations that need CAF/Enterprise-Scale compliance can set both parameters to true explicitly; organizations upgrading an existing hub are not force-migrated into a breaking, irreversible change they didn't ask for.

I believe this is the safer default, but it does mean the compliance gap in #1067 isn't closed by default — only when explicitly enabled. If maintainers prefer defaulting new deployments to true while keeping upgrades safe, that would need a way to distinguish first-deploy from redeploy, which Bicep can't do natively (no reliable "does this resource already exist" check without a existing lookup that fails hard on first deploy). Flagging this as an open discussion point for reviewers.

Verification

• bicep build src/templates/finops-hub/main.bicep --stdout — clean, no errors (2 pre-existing unrelated warnings in Recommendations/app.bicep, confirmed present before this change via git stash).
• bicep build on hub.bicep, hub-app.bicep, and hub-types.bicep individually — all clean.
• pwsh -Command "./src/scripts/Test-PowerShell.ps1 -Lint" — 3418/3418 passed, including ms.date frontmatter checks (already current at 08/12/2026, no update needed).
• Did not run a live az deployment ... what-if — no Azure credentials available in this environment. Everything else that could be verified statically was.

Open questions for reviewers

1. Should new (first-time) deployments default enableRbacAuthorization/enablePurgeProtection to true while upgrades stay opt-in? Flagged above as not straightforwardly expressible in Bicep.
2. Confirm Key Vault Secrets User is the right role scope (read-only get/list on secrets) rather than Key Vault Secrets Officer — I matched the existing access policy's secrets: ['get'] scope exactly, so Secrets User (read-only) seemed correct, but worth a second look given ADF also needs to enumerate the linked service's secret at authoring/refresh time.

Test plan

• Lint tests (PowerShell / bicep build)
• PS -WhatIf / az validate (no Azure credentials in this environment)
• Manually deployed + verified
• Unit tests
• Integration tests

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Michael Flanakin (flanakin)]

Is this name too generic? This isn't clear it's for Key Vault only.

[Michael Flanakin (flanakin)]

🤖 [AI][Claude] ✅ Implemented

Yes, it was too generic. Renamed enableRbacAuthorization → enableKeyVaultRbacAuthorization in main.bicep and hub.bicep (parameter declaration + the newHub() call site), and the matching createUiDefinition.json element name/output binding, for consistency with the already-correctly-scoped keyVaultEnableRbacAuthorization used internally in hub-types.bicep/hub-app.bicep. Left the literal enableRbacAuthorization: ... property in hub-app.bicep untouched — that one is the actual ARM schema property name on Microsoft.KeyVault/vaults, not our parameter, so it has to stay as-is. Didn't touch enablePurgeProtection: purge protection is an unambiguous, Key-Vault-only Azure term, so it doesn't have the same generic-RBAC ambiguity.

[Roland Krummenacher (RolandKrummenacher)]

What happens on existing deployments? Will it end up with both, rbac and access policies, as we are not removing access policies via bicep I guess.

[Michael Flanakin (flanakin)]

🤖 [AI][Claude] 💬 Needs discussion

Traced this precisely: no, it will not end up with both. accessPolicies is set as a direct properties.accessPolicies array on the Microsoft.KeyVault/vaults resource itself (hub-app.bicep), not via the separate additive Microsoft.KeyVault/vaults/accessPolicies child resource type. Since the keyVault resource is redeclared on every deployment where the RemoteHub/KeyVault feature is enabled (not skipped on redeploy), Incremental mode's "don't touch what's absent from the template" guarantee doesn't apply here — it only protects other resources not mentioned in the template, not properties on a resource that is declared. Whatever array the template sends fully replaces the live value.

The keyVaultAccessPolicies variable already reflects this: it evaluates to [] whenever keyVaultEnableRbacAuthorization is true (see the comment already there: "RBAC-authorized vaults must have an empty accessPolicies array; Azure rejects a non-empty array otherwise"). So flipping the flag to true on a redeploy of an existing hub cleanly replaces the old access policy with an empty array — the vault ends up RBAC-only, not dual-auth. The new keyVaultRoleAssignment resource (also gated on the same flag) grants the equivalent Key Vault Secrets User role to Data Factory's identity in the same deployment, so access isn't dropped in the process — it's a clean cutover in one apply.

No code change needed; the existing-deployment behavior is already correct and non-destructive by construction (Azure itself enforces the empty-array requirement, which is what forces the replace). The real risk here isn't dual-auth, it's the one already called out in the PR description and param docs: if any identity other than Data Factory's happened to rely on an access-policy grant on this vault, it would lose access with no automatic RBAC equivalent — but the PR's own investigation confirmed Data Factory's managed identity is the only data-plane secret reader, so that gap doesn't apply today.

[Roland Krummenacher (RolandKrummenacher)]

Review — PR #2249

Approving the approach. The consumer trace is the part that matters most here and it holds up: the ADF AzureKeyVault linked service is the only data-plane secret reader, the keyVault_secret module write is management-plane and genuinely unaffected by the vault's authorization model, and no other hub app touches this vault. Granting exactly one Key Vault Secrets User assignment is the right scope — not a broader role, not a second assignment for the deploying principal.

Opt-in defaulting to false is also right, and for the reason stated: flipping enableRbacAuthorization on an existing vault silently stops honoring access policies with no rollback short of recreating the vault, and this template is explicitly redeploy-over-existing. Forcing it on would break deployed hubs at upgrade time. Following the enablePurgeProtection precedent keeps that consistent.

Two notes, neither blocking.

newHub() positional-argument fragility

keyVaultEnableRbacAuthorization is inserted mid-signature in both newHubInternal and newHub, and both are called positionally. There's exactly one caller today (modules/hub.bicep:190), so this is safe as written — I checked. But the signature is now 15+ positional bools and strings, and the next insertion is one where a mis-ordered pair of same-typed args compiles clean and misconfigures a hub silently. Not this PR's job to fix, but worth an issue: these should take a single object parameter, or at minimum new options should append rather than insert.

RBAC propagation vs. access-policy immediacy

Access policy grants take effect essentially immediately; RBAC role assignments can take a few minutes to propagate. ADF reads the secret at pipeline runtime rather than at deployment time, so in the normal case the assignment has long since propagated by the time anything needs it. The edge case is a pipeline triggered immediately after a first deployment with the flag on — that could see a transient 403 that looks like a misconfiguration rather than a timing artifact.

Not worth adding a dependsOn dance over, but if the deployment surfaces a "hub is ready" message anywhere, a sentence in the docs noting that RBAC-authorized vaults may need a few minutes before the first ingestion run succeeds would save someone a support round-trip.

Small thing

The enablePurgeProtection tooltip gained "This cannot be disabled once enabled" — good addition. The enableRbacAuthorization tooltip doesn't carry the equivalent warning, and its irreversibility is arguably the sharper one of the two (purge protection costs you a 90-day wait; a botched RBAC switch costs you vault access). Consider mirroring the wording from the main.bicep parameter description, which does say it.