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.

[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.

[Roland Krummenacher (RolandKrummenacher)]

Re-review — PR #2249

One new commit since my 2026-08-18 review (3d9b06ab, the rename), plus the two replies. Approach and implementation still hold up; one gap worth closing before merge.

Resolved

Rename enableRbacAuthorization → enableKeyVaultRbacAuthorization — complete. Grepped the branch: the only surviving enableRbacAuthorization is the ARM schema property on Microsoft.KeyVault/vaults in hub-app.bicep, correctly left alone. Parameter declaration, newHub() call site, createUiDefinition element name and output binding, and the changelog entry all agree.

Dual-auth question — the answer is correct, and I verified the mechanism rather than taking it on faith. accessPolicies is a direct property on the vault resource, not the additive Microsoft.KeyVault/vaults/accessPolicies child type, and the vault is redeclared on every deploy. Incremental mode's "leave absent things alone" guarantee covers resources, not properties of a resource that is declared — so the [] array genuinely replaces the live policy. Clean cutover, no dual-auth state. Resolving that thread.

One gap: the PowerShell deployment path

Deploy-FinOpsHub exposes purge protection but not this. src/powershell/Public/Deploy-FinOpsHub.ps1 declares -EnablePurgeProtection as a switch (line 146), documents it (line 34), and forwards it under a >= '13.0' version gate (line 322). There is no equivalent for enableKeyVaultRbacAuthorization, so anyone deploying through the PowerShell module can't enable RBAC at all — portal and raw Bicep only.

The PR follows "the exact pattern already used for enablePurgeProtection," and that pattern spans four surfaces: main.bicep, hub.bicep, createUiDefinition.json, and the cmdlet. Three of four are done. The fix is small, with a template right beside it:

[Parameter()]
[switch]
$EnableKeyVaultRbacAuthorization,

plus a >= '15.0' gate block (matching how enableNatGateway is gated — branch is 15.0.0-dev.0) and a .PARAMETER help entry. I'd treat this as blocking-ish: the module is the scripted-deployment path for exactly the CAF/landing-zone customers this feature targets. Commented inline on the param declaration since the file itself isn't in the diff.

Still open from last review

Details inline: the tooltip irreversibility warning (actionable), RBAC propagation timing (cosmetic), and newHub() positional fragility (separate issue, not this PR's job).

Housekeeping

PR shows CONFLICTING against flanakin/v15-prep. I ran the merge locally — the only conflict is changelog.md. Details inline.

Verdict: sound work and a clean rename. Add the Deploy-FinOpsHub switch and the tooltip sentence, merge the base in, and this is ready to go.

[Roland Krummenacher (RolandKrummenacher)]

This is where the missing surface shows up. enablePurgeProtection (three lines up) is reachable three ways: this template, the portal UI, and Deploy-FinOpsHub -EnablePurgeProtection. enableKeyVaultRbacAuthorization is reachable only the first two ways.

src/powershell/Public/Deploy-FinOpsHub.ps1 needs the matching switch — it isn't in this PR's diff, so I can't anchor a comment there directly:

• line ~34 — .PARAMETER EnableKeyVaultRbacAuthorization help entry, mirroring the EnablePurgeProtection one.
• line ~146 — the switch declaration next to $EnablePurgeProtection.
• line ~322 — forwarding. enablePurgeProtection sits in the >= '13.0' gate; this one wants a new >= '15.0' block, matching how enableNatGateway is gated:

if ($Version -eq 'latest' -or [version]$Version -ge '15.0')
{
    $parameterSplat.TemplateParameterObject.Add('enableKeyVaultRbacAuthorization', $EnableKeyVaultRbacAuthorization.IsPresent)
}

Worth doing here rather than as a follow-up: CAF/Enterprise-Scale shops are disproportionately the ones deploying via script rather than the portal blade, so the audience for this feature is the audience most likely to hit the gap.

[Roland Krummenacher (RolandKrummenacher)]

Still missing the irreversibility warning I flagged last round. The enablePurgeProtection tooltip directly above gained "This cannot be disabled once enabled" in this same PR — this one didn't, and its failure mode is the sharper of the two. Purge protection costs you a 90-day wait; a botched RBAC switch costs you vault access with no rollback short of recreating the vault.

The main.bicep parameter description already has usable wording. Something like appending:

Switching an existing vault from access policies to RBAC takes effect immediately and cannot be rolled back without recreating the vault.

Portal users clicking this checkbox on a redeploy are exactly the people who need that sentence, and they never see the Bicep description.

[Roland Krummenacher (RolandKrummenacher)]

Non-blocking, carried over from last review. Access policy grants take effect essentially immediately; RBAC role assignments can take a few minutes to propagate.

In the normal case this is a non-issue — ADF reads the secret at pipeline runtime, long after the assignment has settled. The edge case is a pipeline triggered right after a first deployment with the flag on: a transient 403 that reads like a misconfiguration rather than a timing artifact.

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

The gating itself is right, for the record — usesKeyVault && usesDataFactory && ...keyVaultEnableRbacAuthorization matches the vault's own condition, and the guid(keyVault.id, roleId, dataFactory.id) name is properly deterministic across redeploys.

[Roland Krummenacher (RolandKrummenacher)]

Repeating from last review since it's unaddressed, and explicitly not asking this PR to fix it — logging it so it doesn't evaporate.

keyVaultEnableRbacAuthorization is inserted mid-signature in both newHubInternal and newHub, and both are called positionally. There is exactly one caller today (modules/hub.bicep:193), so this insertion is safe — I checked it. But the signature is now 15+ positional bools and strings, and a mis-ordered pair of same-typed args would compile clean and silently misconfigure a hub.

Worth a standalone issue: these should take a single object parameter, or at minimum new options should append rather than insert. Happy to open it if nobody objects.

[Roland Krummenacher (RolandKrummenacher)]

This entry is the merge conflict against flanakin/v15-prep. I ran the merge locally — it's the only one, and it's the routine pattern: this line plus the neighboring Unreleased entries, and ms.date at the top of the file.

Per the repo's conflict guidance: git merge origin/flanakin/v15-prep, keep entries from both sides (they're additive and independent), and set ms.date to today's date rather than either side's value.