Add opt-in Key Vault RBAC authorization for CAF/landing zone compliance - #2249
Open
Michael Flanakin (flanakin) wants to merge 1 commit into
Open
Add opt-in Key Vault RBAC authorization for CAF/landing zone compliance#2249Michael Flanakin (flanakin) wants to merge 1 commit into
Michael Flanakin (flanakin) wants to merge 1 commit into
Conversation
Adds an `enableRbacAuthorization` parameter (default false) that switches the remote hub Key Vault from access policies to Azure RBAC, satisfying CAF / Enterprise-Scale landing zone guardrails that require RBAC-authorized key vaults. When enabled, the Data Factory managed identity is granted an equivalent Key Vault Secrets User role assignment so secret access continues to work instead of silently breaking. Defaults to false to avoid an irreversible auth-model change on redeploys of existing hubs, matching the existing opt-in `enablePurgeProtection` parameter, which cannot be disabled once enabled either. Fixes #1067
Michael Flanakin (flanakin)
requested review from
Brett Wilson (MSBrett) and
Roland Krummenacher (RolandKrummenacher)
as code owners
August 12, 2026 17:33
| storageSku, | ||
| keyVaultSku, | ||
| enablePurgeProtection, | ||
| enableRbacAuthorization, |
Collaborator
Author
There was a problem hiding this comment.
Is this name too generic? This isn't clear it's for Key Vault only.
| { | ||
| "name": "enableRbacAuthorization", | ||
| "type": "Microsoft.Common.CheckBox", | ||
| "label": "Enable Key Vault RBAC authorization", |
Collaborator
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an
enableRbacAuthorizationparameter 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 setsenableRbacAuthorization: app.hub.options.keyVaultEnableRbacAuthorizationinstead of a hardcodedfalse. When RBAC is enabled:accessPoliciesis forced to an empty array (Azure rejects a non-emptyaccessPoliciesarray whenenableRbacAuthorization: true).keyVaultRoleAssignmentresource 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 thesecrets: ['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— addedkeyVaultEnableRbacAuthorizationtoHubProperties.optionsand threaded it throughnewHubInternal/newHub.src/templates/finops-hub/modules/hub.bicepandmain.bicep— added anenableRbacAuthorization bool = falseparameter, following the exact pattern already used forenablePurgeProtection.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: trueis hardcoded, andenablePurgeProtectionis already wired end-to-end as an opt-in parameter defaulting tofalse, 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/vaultsinsrc/templates/finops-hub/modules/fx/hub-app.bicep:471, only deployed when theRemoteHubapp requests the'KeyVault'feature —modules/Microsoft.FinOpsHubs/RemoteHub/app.bicep:52):AzureKeyVaultlinked service (hub-app.bicep:201,linkedService_keyVault) — ADF authenticates as its own system-assigned managed identity at runtime to read theremoteHubStorageconnection secret viaAzureKeyVaultSecret. This is the only data-plane secret reader, and it's exactly what the pre-existingkeyVaultAccessPoliciesvar (secrets: ['get']) granted. Now granted via RBAC (Key Vault Secrets User) whenenableRbacAuthorization: true.keyVault_secretmodule (fx/hub-vault.bicep, invoked fromRemoteHub/app.bicep:59) writes the storage key as a nested ARMMicrosoft.KeyVault/vaults/secretsresource. 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.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 1349before starting. It only addedenablePurgeProtection(12 lines acrossmain.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 inmodules/fx/hub-app.bicepas part of the namespace-basedfx/Microsoft.FinOpsHubsrestructuring. It didn't touch RBAC at all, and its purge-protection change has since been superseded by a more complete implementation already ondev/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
enableRbacAuthorizationandenablePurgeProtectionare irreversible per Azure's API contract:enablePurgeProtection: trueis set on a vault, it cannot be reverted.enableRbacAuthorizationfromfalsetotrueon 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 tofalse, surfaced identically inmain.bicep/hub.bicep/createUiDefinition.json) and applied the same pattern toenableRbacAuthorization. Organizations that need CAF/Enterprise-Scale compliance can set both parameters totrueexplicitly; 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
truewhile 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 aexistinglookup 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 inRecommendations/app.bicep, confirmed present before this change viagit stash).bicep buildonhub.bicep,hub-app.bicep, andhub-types.bicepindividually — all clean.pwsh -Command "./src/scripts/Test-PowerShell.ps1 -Lint"— 3418/3418 passed, includingms.datefrontmatter checks (already current at08/12/2026, no update needed).az deployment ... what-if— no Azure credentials available in this environment. Everything else that could be verified statically was.Open questions for reviewers
enableRbacAuthorization/enablePurgeProtectiontotruewhile upgrades stay opt-in? Flagged above as not straightforwardly expressible in Bicep.Key Vault Secrets Useris the right role scope (read-onlyget/liston secrets) rather thanKey Vault Secrets Officer— I matched the existing access policy'ssecrets: ['get']scope exactly, soSecrets 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