feat(provx/azure): provision Azure trust with a managed identity - #6
Merged
Conversation
Replace the Entra app-registration design with an ordinary ARM resource: a user-assigned managed identity plus a federated identity credential and subscription-scoped role assignments. App registrations need Entra directory privileges a subscription owner does not hold by default and many tenants disable outright; a managed identity needs only subscription privileges. This step converges the target resource group: create it tagged as ours when missing, adopt it unmodified when present, and refuse (rather than silently fail to converge) when an existing group's location does not match, since a resource group's location is immutable. Drop the msgraph/kiota/std-uritemplate dependency subtree, now fully unused.
…edential ensureIdentity creates the deterministically-named user-assigned identity tagged as ours, or adopts an existing one unmodified. ensureFederatedCredential is strict rather than reconciling: a federated credential grants near-owner access to whoever can present a matching token, so an identity is only adopted when it carries exactly one federated credential and that credential's issuer, subject and audience already match ours exactly. Any other shape - a mismatch, or a second credential alongside a matching one - is refused with IdentityNotOursError naming the offending credential, rather than patched or ignored. Depends on the sibling oidcx/azure package for its Audience constant, so provx now requires the root oox module (via a local replace).
…gnments ensureRoleAssignments grants Contributor and User Access Administrator to the identity's service principal, retrying while ARM cannot yet see a freshly created principal (replication lag) via an injectable clock so tests drive the timeout without waiting on it for real, and treating RoleAssignmentExists as success regardless of which name the equivalent grant was created under. Introduces Operation and Classify in errors.go: the same ARM error code means different things on different calls (AuthorizationFailed on a role assignment specifically needs Owner or User Access Administrator, not just "some permission"), so Classify takes the operation as well as the error.
…assify ensureResourceGroup, ensureIdentity and ensureFederatedCredential previously returned the raw SDK error on an unexpected ARM failure. Route them through Classify so a subscription that has never registered Microsoft.ManagedIdentity or Microsoft.Resources gets a ProviderNotRegisteredError naming the provider, AuthorizationFailed on these calls reads as PermissionDeniedError (never as RoleAssignmentForbiddenError, which is reserved for the role assignment call specifically), and an unrecognised failure carries its HTTP status without exposing the raw SDK error as something a caller could errors.Is/As back to.
Create converges the resource group, identity, federated credential and role assignments in order and returns the coordinates a later connect flow registers (azureTenantId, azureClientId - explicitly the identity's client id, never its principal id). Delete revalidates the identity's federated credential before removing anything: resolving by the deterministic name alone and deleting whatever occupies it would destroy a foreign identity sharing that name, the mirror image of the adoption defect ensureFederated- Credential already guards against. Delete never touches the resource group, which may hold other installations' identities. VerifySubscription confirms the ambient credential can reach the target subscription and echoes back the pinned tenant, classifying any failure the same way every other operation does. Extracts solelyOurs out of ensureFederatedCredential so Delete's revalidation (verifyFederatedCredential) shares the exact adoption rule instead of re-implementing it, with one deliberate difference: zero credentials is a fresh identity to ensureFederatedCredential, but a refusal to Delete, since by the time Delete runs this installation's own Create should have left exactly one.
Importing oidcx/azure across the provx/oox module boundary pulled the root module's go 1.26.4 floor and an unresolvable local replace directive into provx/go.mod - both break formae, which consumes provx standalone under its own pinned go 1.26.0 toolchain. provx/gcp already established the pattern for this: don't share code across that edge, duplicate the pinned literal and prove agreement with a golden test in each side's own suite. Do the same here: declare tokenAudience locally, drop the oidcx import and the oox replace/go-version bump entirely, and add a golden test asserting the federated credential is created with the literal audience string. Also switch VerifySubscription from the wrong ARM package: armsubscription (the Microsoft.Subscription RP, for aliases/tenant policy) doesn't expose a tenant id on its Subscription model, but armsubscriptions (Microsoft. Resources, the actual Subscriptions.Get) does. VerifySubscription now reads the subscription's real tenant from ARM - deriving it when none was pinned at construction, cross-checking it when one was, and reporting a disagreement as *TenantMismatchError instead of silently trusting the caller's value.
New now accepts an empty azTenantID: derivation from the subscription is the normal path (VerifySubscription reads the tenant straight from ARM), but an external or guest account can need an explicit tenant in order to authenticate at all, before the subscription is even reachable - so the caller supplies one only when it has one, rather than always. Create now calls VerifySubscription itself and uses its result for Result.TenantID, instead of echoing the constructor's (possibly empty) azTenantID field. A Create that cannot verify a tenant fails outright rather than returning a Result with an empty TenantID.
Credential hardcoded nil for azidentity.NewClientAssertionCredential's options, so nothing downstream could inject a transport. A regression test in the Azure plugin needs exactly that to cover live-context propagation through the assertion callback without reaching login.microsoftonline.com from a unit test. Credential now takes *azidentity.ClientAssertionCredentialOptions and passes it straight through; production callers pass nil. There are no other callers in this repo to update. Adds a test proving the seam works fully offline: a fake policy.Transporter plus DisableInstanceDiscovery. Empirically, DisableInstanceDiscovery only suppresses the global instance-discovery call - MSAL Go still fetches the tenant-specific OIDC metadata document before the token request - so the fake answers both requests it actually makes, not just the token endpoint. This is a root oox module change riding on the same branch as the provx/azure work; it does not affect provx's own go.mod (provx keeps zero dependency on the root module, per the prior fix round).
…a role assignment deleteRoleAssignments relied on the server-side $filter=principalId alone to scope which assignments it deletes. If that filter is ever dropped, mistyped, or not honoured by ARM, this is the one path in the package that would delete every Contributor and User Access Administrator assignment at subscription scope for every principal, not just this installation's. Add the client-side re-check so the filter is a narrowing optimization, not the safety mechanism. Route deleteRoleAssignments' own errors through Classify (matches the resource-group and identity paths already fixed in 705ce59): a Delete that hits AuthorizationFailed on teardown - the normal-day Contributor failure - now surfaces RoleAssignmentForbiddenError with the remedy instead of an opaque SDK error. Also, following review: delete PermissionDeniedError's unused Action/Scope fields (never populated, so the branch reading them was unreachable) rather than populate them for their own sake; collapse opResourceGroup and opSubscription, which were the same Operation value under two names; remove Classify's RoleAssignmentExists case, which no caller reaches since ensureRoleAssignment's retry loop already intercepts that code before Classify ever sees it; and add a test driving RoleAssignmentUpdateNotPermitted through the public role-assignment operation, which previously had none.
…stant The offline credential test asserted the assertion callback's audience against the Audience constant itself, which passes for any value the constant takes - editing oidcx/azure.Audience would break federation in production with a green suite. provx/azure duplicates this exact string across the module boundary and pins the literal on its side (see the golden test next to tokenAudience in provx/azure/identity_test.go); this test now does the same, so a drift between the two sides fails loudly instead of silently, matching the gcp precedent (oox/gcpname and provx/gcp both pin their shared literal, not a shared constant).
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
2 participants
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
Provisions the Azure side of a hosted formae installation's federated access to a customer subscription, using a user-assigned managed identity rather than an Entra app registration.
Builds on the
feat: azurecommit in this branch (PR #4's work), retargeting it from Microsoft Graph to ARM.Why a managed identity rather than an app registration
An app registration is a directory object, so creating one needs Entra directory privileges. A subscription owner does not hold those by default, and many tenants disable app registration outright — so that path can fail for a customer who has done nothing wrong and cannot fix it themselves. A managed identity is an ordinary ARM resource, needing exactly the subscription privileges the person running
formae connect azurealready has.Consequences, all favourable: Microsoft Graph leaves the dependency graph entirely (
msgraph-sdk-goplus ~11 indirect deps), the identity is visible in the subscription and deletes like any resource, and the formae Azure plugin already models both object types.It also makes per-installation isolation structural. Each installation gets its own identity, its own federated credential and its own role assignments, so a second installation connecting the same subscription cannot rewrite the first's trust — a class of bug that had to be defended against explicitly on the GCP path.
Proven, not assumed
Against a live subscription: a managed identity, a federated identity credential naming our issuer, and two subscription-scoped role assignments were created; a real formae-issued token was then exchanged through
oidcx/azureand made a live ARM call. Repointing the credential's subject and replaying the same token returns401 invalid_client, so the trust is real rather than incidental.What is in here
provx/azurerewritten ontoarmmsi/armauthorization/armresources, split intoazure.go,identity.go,roles.go,errors.go,seams.go.Createreturns the registration coordinates;Deleteis scoped to one installation and revalidates ownership before deleting anything.deleteRoleAssignmentsre-checks the principal client-side rather than trusting the server-side filter. It is the one path here that can destroy another principal's grants.ErrorCode, never on a bare HTTP status. An unregisteredMicrosoft.ManagedIdentityprovider is named and explained, not registered on the operator's behalf.oidcx/azure.Credentialgains an options parameter (root module) so callers can inject a transport. This is what lets the Azure plugin test live-context propagation without reachinglogin.microsoftonline.com.Deliberate duplication
provx/azuredeclares the token audience locally instead of importingoidcx/azure.Audience.oidcxis in the root module (go 1.26.4);provxis a separate module (go 1.26.0) consumed byformae, which pinsgo 1.26.0. Importing across that edge forced agobump and a localreplaceintoprovx/go.mod, either of which breaks formae's build.provx/gcpalready solved this the same way forgcpname, and says so in its comments. The contract is pinned by the literal string asserted on both sides.Verification
go -C provx test ./...,go test ./oidcx/...,go -C gcpname test ./...all green. Ago 1.26.0module was compiled againstprovx/azureunderGOTOOLCHAIN=go1.26.0to confirm the consumer constraint holds. The offline claim for the newoidcx/azuretest was checked by running it inside a network namespace with no connectivity.