Skip to content

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

Open
Michael Flanakin (flanakin) wants to merge 1 commit into
flanakin/v15-prepfrom
flanakin/1067-keyvault-rbac
Open

Add opt-in Key Vault RBAC authorization for CAF/landing zone compliance#2249
Michael Flanakin (flanakin) wants to merge 1 commit into
flanakin/v15-prepfrom
flanakin/1067-keyvault-rbac

Conversation

@flanakin

Copy link
Copy Markdown
Collaborator

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

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

storageSku,
keyVaultSku,
enablePurgeProtection,
enableRbacAuthorization,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@flanakin Michael Flanakin (flanakin) added this to the v16 milestone Aug 13, 2026
{
"name": "enableRbacAuthorization",
"type": "Microsoft.Common.CheckBox",
"label": "Enable Key Vault RBAC authorization",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs: Review 👀 PR that is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update Key Vault to Support RBAC Permissions and Delete Protection

4 participants