From 4bc679334419ddb80d76827e3aab8d85fed92b2e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 24 Apr 2026 16:56:35 +0200 Subject: [PATCH] refactor(credentials): drop legacy cert-based Azure WIF path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Azure Workload Identity Federation now has exactly one implementation: KMS-signed client assertions via CUDly's OIDC issuer. The legacy cert-based path (stored RSA private key + self-signed certificate, x5t thumbprint in the JWT header) is gone. The modern IaC templates at `internal/iacfiles/templates/azure-wif*` were already federation-only ("no certificate, no private key, no client secret is created or stored"). The legacy path lived on in the backend, UI, and some operator docs as orphaned scaffolding. Removing it aligns the whole stack and eliminates a secret-material surface. Frontend: - Delete the RSA private-key textarea + instructions from the Azure account modal. Tenant ID, Client ID, and Subscription ID are all that's asked for in WIF mode now. - Drop the WIF branch from the credential save path and the private-key reset on modal open. - Remove 'azure_wif_private_key' from the TS credential_type union so the compiler catches stale callers. - Simplify updateAzureAuthModeFields — only client_secret mode has a field block to show/hide now. Backend: - credentials/resolver.go: delete buildAzureWIFCredential, parsePEMBlob, processPEMBlock, parseRSAKeyBlock, and CredTypeAzureWIF. Drop the crypto/rsa, crypto/sha1, crypto/x509, encoding/base64, encoding/pem, and golang-jwt/v5 imports. - credentials/azure_federated.go: resolveAzureWIFCredential becomes a straight delegation to BuildAzureFederatedCredential. Fail fast with an operator-facing error when opts.Signer/issuer are missing instead of trying a legacy fallback. - api/handler_accounts.go: remove 'azure_wif_private_key' from validCredentialTypes and the error message. azureFederatedCredResult loses the "legacy PEM stored → fall back" branch. credTypeForAccount returns "" for Azure WIF (no stored credential to check); the new checkCredentialPresence short-circuit surfaces an actionable error for operators who hit the degraded-deployment state (Azure WIF account + no wired OIDC signer). - api/validation.go: drop the azure_wif_private_key payload schema and switch arm. - api/handler_federation.go: rewrite the Azure README generator to match the federation-only templates it bundles. - api/handler_registrations.go: refresh the accountHasCredentialFreePath doc comment — no more "legacy PEM" caveats. Tests: - credentials/resolver_extra_test.go: delete all TestBuildAzureWIFCredential_* cases, TestParsePEMBlob_* cases, and the generateTestKey*/generateECKey* helpers. Reframe TestResolveAzureTokenCredential_WIF_* as a "requires wired OIDC signer" assertion. - api/coverage_gaps_test.go: TestCredTypeForAccount expects "" for Azure WIF. - api/validation_test.go: drop the two azure_wif_private_key cases. DB: no migration. User confirmed no accounts rely on the legacy path; any stray rows become dead data and are ignored by the runtime. Closes #14. --- frontend/src/api/accounts.ts | 2 +- frontend/src/index.html | 7 - frontend/src/settings.ts | 25 +-- go.mod | 2 +- internal/api/coverage_gaps_test.go | 16 +- internal/api/handler_accounts.go | 36 ++-- internal/api/handler_federation.go | 6 +- internal/api/handler_registrations.go | 15 +- internal/api/validation.go | 7 +- internal/api/validation_test.go | 7 - internal/credentials/azure_federated.go | 52 ++--- internal/credentials/resolver.go | 147 +-------------- internal/credentials/resolver_extra_test.go | 199 ++------------------ 13 files changed, 88 insertions(+), 433 deletions(-) diff --git a/frontend/src/api/accounts.ts b/frontend/src/api/accounts.ts index d67ce21f..bee2bfdc 100644 --- a/frontend/src/api/accounts.ts +++ b/frontend/src/api/accounts.ts @@ -65,7 +65,7 @@ export interface AccountListFilters { } export interface AccountCredentialsRequest { - credential_type: 'aws_access_keys' | 'azure_client_secret' | 'azure_wif_private_key' | 'gcp_service_account' | 'gcp_workload_identity_config'; + credential_type: 'aws_access_keys' | 'azure_client_secret' | 'gcp_service_account' | 'gcp_workload_identity_config'; payload: Record; } diff --git a/frontend/src/index.html b/frontend/src/index.html index 3cb38f23..1beb2b2e 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -1047,13 +1047,6 @@

Azure Authentication

- - diff --git a/frontend/src/settings.ts b/frontend/src/settings.ts index ec44e932..8ed684e7 100644 --- a/frontend/src/settings.ts +++ b/frontend/src/settings.ts @@ -580,7 +580,6 @@ export function openAccountModal(provider: AccountProvider, account?: api.CloudA setInputValue('account-azure-tenant-id', account?.azure_tenant_id ?? ''); setInputValue('account-azure-client-id', account?.azure_client_id ?? ''); setInputValue('account-azure-client-secret', ''); - setInputValue('account-azure-wif-private-key', ''); updateAzureAuthModeFields(azureAuthMode); } else if (provider === 'gcp') { const gcpMode = account?.gcp_auth_mode ?? 'workload_identity_federation'; @@ -645,13 +644,15 @@ function updateAwsAuthModeFields(mode: string, bastionId?: string): void { } /** - * Show/hide Azure auth mode sub-fields + * Show/hide Azure auth mode sub-fields. Only client_secret mode has an + * input block; workload_identity_federation is credential-free (CUDly's + * OIDC issuer + the App Registration's federated-identity-credential + * handle authentication) and managed_identity is ambient, so neither + * needs a visible field block here. */ function updateAzureAuthModeFields(mode: string): void { const secretFields = document.getElementById('account-azure-secret-fields'); - const wifFields = document.getElementById('account-azure-wif-fields'); secretFields?.classList.toggle('hidden', mode !== 'client_secret'); - wifFields?.classList.toggle('hidden', mode !== 'workload_identity_federation'); } /** @@ -753,17 +754,11 @@ async function saveAccountCredentialsIfFilled(accountId: string, provider: Accou } } else if (provider === 'azure') { const azureMode = byId('account-azure-auth-mode')?.value ?? 'client_secret'; - if (azureMode === 'managed_identity') { - // No credential to store - } else if (azureMode === 'workload_identity_federation') { - const pem = byId('account-azure-wif-private-key')?.value.trim(); - if (pem) { - await api.saveAccountCredentials(accountId, { - credential_type: 'azure_wif_private_key', - payload: { private_key_pem: pem } - }); - } - } else { + // managed_identity is ambient; workload_identity_federation is + // secret-free (CUDly's OIDC issuer + the Azure App Registration's + // federated-identity-credential handle authentication, no material + // stored in CUDly). Only client_secret accepts a stored credential. + if (azureMode === 'client_secret') { const secret = byId('account-azure-client-secret')?.value ?? ''; if (secret) { await api.saveAccountCredentials(accountId, { diff --git a/go.mod b/go.mod index 6da7ad65..e246261d 100644 --- a/go.mod +++ b/go.mod @@ -51,7 +51,7 @@ require ( github.com/felixge/httpsnoop v1.0.4 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect - github.com/golang-jwt/jwt/v5 v5.3.1 + github.com/golang-jwt/jwt/v5 v5.3.1 // indirect github.com/google/s2a-go v0.1.9 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.14 // indirect github.com/googleapis/gax-go/v2 v2.21.0 // indirect diff --git a/internal/api/coverage_gaps_test.go b/internal/api/coverage_gaps_test.go index f61455fe..aa33d952 100644 --- a/internal/api/coverage_gaps_test.go +++ b/internal/api/coverage_gaps_test.go @@ -524,18 +524,22 @@ func TestAmbientCredResult(t *testing.T) { func TestCredTypeForAccount(t *testing.T) { tests := []struct { + name string acct *config.CloudAccount expected string }{ - {&config.CloudAccount{Provider: "aws"}, "aws_access_keys"}, - {&config.CloudAccount{Provider: "azure", AzureAuthMode: "service_principal"}, "azure_client_secret"}, - {&config.CloudAccount{Provider: "azure", AzureAuthMode: "workload_identity_federation"}, "azure_wif_private_key"}, - {&config.CloudAccount{Provider: "gcp", GCPAuthMode: "service_account"}, "gcp_service_account"}, - {&config.CloudAccount{Provider: "gcp", GCPAuthMode: "workload_identity_federation"}, "gcp_workload_identity_config"}, + {"aws", &config.CloudAccount{Provider: "aws"}, "aws_access_keys"}, + {"azure_service_principal", &config.CloudAccount{Provider: "azure", AzureAuthMode: "service_principal"}, "azure_client_secret"}, + // Azure WIF is secret-free (KMS-signed assertion, no stored + // credential), so credTypeForAccount returns "" — the caller + // handles the empty string as "no credential to check". + {"azure_wif", &config.CloudAccount{Provider: "azure", AzureAuthMode: "workload_identity_federation"}, ""}, + {"gcp_service_account", &config.CloudAccount{Provider: "gcp", GCPAuthMode: "service_account"}, "gcp_service_account"}, + {"gcp_wif", &config.CloudAccount{Provider: "gcp", GCPAuthMode: "workload_identity_federation"}, "gcp_workload_identity_config"}, } for _, tt := range tests { - t.Run(tt.expected, func(t *testing.T) { + t.Run(tt.name, func(t *testing.T) { assert.Equal(t, tt.expected, credTypeForAccount(tt.acct)) }) } diff --git a/internal/api/handler_accounts.go b/internal/api/handler_accounts.go index 6df45e9a..200c93d2 100644 --- a/internal/api/handler_accounts.go +++ b/internal/api/handler_accounts.go @@ -76,7 +76,6 @@ type AccountServiceOverrideRequest struct { var validCredentialTypes = map[string]bool{ "aws_access_keys": true, "azure_client_secret": true, - "azure_wif_private_key": true, "gcp_service_account": true, "gcp_workload_identity_config": true, } @@ -454,7 +453,7 @@ func (h *Handler) parseAndValidateCredentialsRequest(ctx context.Context, httpRe return nil, nil, NewClientError(400, "invalid request body") } if !validCredentialTypes[req.CredentialType] { - return nil, nil, NewClientError(400, "credential_type must be one of: aws_access_keys, azure_client_secret, azure_wif_private_key, gcp_service_account, gcp_workload_identity_config") + return nil, nil, NewClientError(400, "credential_type must be one of: aws_access_keys, azure_client_secret, gcp_service_account, gcp_workload_identity_config") } if err := validateCredentialPayload(req.CredentialType, req.Payload); err != nil { return nil, nil, err @@ -694,11 +693,11 @@ type tokenResult struct { // azureFederatedCredResult exercises the secret-free Azure federated // credential path end-to-end for accounts in workload_identity_federation -// mode whose CUDly deployment has an OIDC signer + issuer configured and -// no legacy PEM stored. It mints a client_assertion JWT via the KMS -// signer, exchanges it at Azure AD's token endpoint, and reports the -// result. Returns (result, true) when this path applies; (_, false) -// means the caller should fall back to the presence check. +// mode whose CUDly deployment has an OIDC signer + issuer configured. +// It mints a client_assertion JWT via the KMS signer, exchanges it at +// Azure AD's token endpoint, and reports the result. Returns +// (result, true) when this path applies; (_, false) means the caller +// should fall back to the presence check (client_secret mode only). func (h *Handler) azureFederatedCredResult(ctx context.Context, acct *config.CloudAccount) (AccountTestResult, bool) { if acct.Provider != "azure" || acct.AzureAuthMode != "workload_identity_federation" { return AccountTestResult{}, false @@ -710,13 +709,6 @@ func (h *Handler) azureFederatedCredResult(ctx context.Context, acct *config.Clo if issuer == "" { return AccountTestResult{}, false } - // If a legacy PEM is stored, let the presence-check path handle it - // so cert-based accounts keep reporting the same shape. - if h.credStore != nil { - if has, _ := h.credStore.HasCredential(ctx, acct.ID, credentials.CredTypeAzureWIF); has { - return AccountTestResult{}, false - } - } cred, err := credentials.BuildAzureFederatedCredential(h.signer, issuer, acct.AzureTenantID, acct.AzureClientID) if err != nil { @@ -744,6 +736,15 @@ func (h *Handler) azureFederatedCredResult(ctx context.Context, acct *config.Clo func (h *Handler) checkCredentialPresence(ctx context.Context, acct *config.CloudAccount) (AccountTestResult, error) { if h.credStore != nil { credType := credTypeForAccount(acct) + // Empty credType means this auth mode isn't backed by a stored + // credential — e.g. Azure workload_identity_federation on a + // deployment where the OIDC signer wasn't wired (so + // azureFederatedCredResult returned ok=false and we fell + // through to here). Report an operator-facing message instead + // of querying for "no credential stored". + if credType == "" { + return AccountTestResult{OK: false, Message: "this account's auth mode is not backed by a stored credential — check the deployment's OIDC issuer wiring"}, nil + } has, err := h.credStore.HasCredential(ctx, acct.ID, credType) if err != nil { return AccountTestResult{}, fmt.Errorf("accounts: check credential: %w", err) @@ -766,12 +767,15 @@ func (h *Handler) checkCredentialPresence(ctx context.Context, acct *config.Clou } // credTypeForAccount returns the expected credential_type for an account -// based on its provider and auth mode. +// based on its provider and auth mode. Returns "" for auth modes that +// aren't backed by a stored credential (e.g. Azure +// workload_identity_federation, which uses the deployment's OIDC signer +// at request time and stores nothing per-account). func credTypeForAccount(acct *config.CloudAccount) string { switch acct.Provider { case "azure": if acct.AzureAuthMode == "workload_identity_federation" { - return "azure_wif_private_key" + return "" } return "azure_client_secret" case "gcp": diff --git a/internal/api/handler_federation.go b/internal/api/handler_federation.go index 48a43bda..81ad8c9a 100644 --- a/internal/api/handler_federation.go +++ b/internal/api/handler_federation.go @@ -647,11 +647,9 @@ func buildBundleReadme(data federationIaCData, target, source string) string { sb.WriteString(" cd cloudformation && bash deploy-cfn.sh --region \n\n") sb.WriteString("After apply, set aws_auth_mode=workload_identity_federation and aws_role_arn in CUDly.\n") case target == "azure": - sb.WriteString("Contents:\n terraform/ - Azure App Registration + cert WIF Terraform module\n") + sb.WriteString("Contents:\n terraform/ - Azure App Registration + federated-identity-credential Terraform module\n") sb.WriteString(" terraform/*.auto.tfvars - Pre-filled variable values (auto-loaded by Terraform)\n\n") - sb.WriteString("Prerequisites:\n 1. Generate an RSA key and self-signed certificate (see tfvars comments).\n") - sb.WriteString(" 2. Paste the certificate PEM into the tfvars file.\n") - sb.WriteString(" 3. Store the private key PEM in CUDly as azure_wif_private_key.\n\n") + sb.WriteString("This uses true Workload Identity Federation — no certificate, no private key,\nno client secret is created or stored. CUDly's OIDC issuer signs a short-lived\nJWT on each Azure API call and Azure AD verifies it against the federated\ncredential provisioned here.\n\n") sb.WriteString("Deploy (Terraform):\n") sb.WriteString(" cd terraform && terraform init && terraform apply\n\n") sb.WriteString("After apply, set azure_auth_mode=workload_identity_federation in CUDly.\n") diff --git a/internal/api/handler_registrations.go b/internal/api/handler_registrations.go index c2de748f..100d8b6e 100644 --- a/internal/api/handler_registrations.go +++ b/internal/api/handler_registrations.go @@ -312,12 +312,7 @@ func (h *Handler) approveRegistration(ctx context.Context, httpReq *events.Lambd // ambient instance credentials (role assumption, managed identity, // Application Default Credentials) or via CUDly's KMS-backed OIDC // federated path. These accounts should auto-enable on approval -// because there's no follow-up "upload the PEM/JSON" step. -// -// Cert-based legacy Azure WIF is NOT included here — it needs a -// stored azure_wif_private_key blob that the operator uploads -// separately after approval, so those accounts keep the old -// opt-in-via-manual-PUT behaviour. +// because there's no follow-up "upload the JSON" step. func accountHasCredentialFreePath(acct *config.CloudAccount) bool { switch acct.Provider { case "aws": @@ -325,10 +320,10 @@ func accountHasCredentialFreePath(acct *config.CloudAccount) bool { // workload_identity_federation mints its own token file. return acct.AWSAuthMode == "role_arn" || acct.AWSAuthMode == "workload_identity_federation" case "azure": - // managed_identity: ambient. workload_identity_federation: - // federated via the KMS-backed path when no PEM is stored — - // the /test handler falls back to the cert path when one is - // present, so auto-enabling the account either way is fine. + // managed_identity is ambient; workload_identity_federation + // uses the KMS-backed OIDC signer to mint a client assertion + // on every call — nothing is stored in CUDly per-account, so + // auto-enable is always safe for either mode. return acct.AzureAuthMode == "managed_identity" || acct.AzureAuthMode == "workload_identity_federation" case "gcp": // application_default: ambient (Cloud Run / GKE). WIF: diff --git a/internal/api/validation.go b/internal/api/validation.go index 695e3aba..daa27419 100644 --- a/internal/api/validation.go +++ b/internal/api/validation.go @@ -71,9 +71,8 @@ type credentialPayloadSchema struct { } var credentialPayloadSchemas = map[string]credentialPayloadSchema{ - "aws_access_keys": {required: []string{"access_key_id", "secret_access_key"}}, - "azure_client_secret": {required: []string{"client_secret"}}, - "azure_wif_private_key": {required: []string{"private_key_pem"}}, + "aws_access_keys": {required: []string{"access_key_id", "secret_access_key"}}, + "azure_client_secret": {required: []string{"client_secret"}}, } // gcpServiceAccountKeys are the fields a Google service-account JSON file is @@ -110,7 +109,7 @@ func validateCredentialPayload(credentialType string, payload map[string]interfa } switch credentialType { - case "aws_access_keys", "azure_client_secret", "azure_wif_private_key": + case "aws_access_keys", "azure_client_secret": schema := credentialPayloadSchemas[credentialType] return validateFlatPayload(credentialType, payload, schema.required, schema.optional) case "gcp_service_account": diff --git a/internal/api/validation_test.go b/internal/api/validation_test.go index 721864de..418ad9f8 100644 --- a/internal/api/validation_test.go +++ b/internal/api/validation_test.go @@ -232,13 +232,6 @@ func TestValidateCredentialPayload(t *testing.T) { {"azure secret unknown key", "azure_client_secret", map[string]interface{}{"some_other": "abc"}, "unknown key \"some_other\""}, - // azure_wif_private_key - {"azure wif happy", "azure_wif_private_key", - map[string]interface{}{"private_key_pem": "-----BEGIN..."}, ""}, - {"azure wif unknown key", "azure_wif_private_key", - map[string]interface{}{"private_key_pem": "x", "thumbprint": "abc"}, - "unknown key \"thumbprint\""}, - // gcp_service_account {"gcp svc happy", "gcp_service_account", map[string]interface{}{ diff --git a/internal/credentials/azure_federated.go b/internal/credentials/azure_federated.go index f7b0a322..5cc98537 100644 --- a/internal/credentials/azure_federated.go +++ b/internal/credentials/azure_federated.go @@ -39,58 +39,32 @@ type AzureResolveOptions struct { } // resolveAzureWIFCredential handles the workload_identity_federation -// auth mode. Extracted from ResolveAzureTokenCredentialWithOpts to keep -// the top-level switch simple. -// -// Routing: -// - opts.Signer + issuer URL available, no stored PEM → federated -// path (BuildAzureFederatedCredential), secret-free. -// - opts.Signer set but a stored PEM exists → legacy cert path, for -// backward compatibility with accounts registered before the -// redesign. -// - opts.Signer not set → legacy cert path, requiring a stored PEM. +// auth mode. The only supported path is the secret-free federated one: +// CUDly's deployment-wide OIDC signer (KMS-backed) mints a short-lived +// client-assertion JWT that Azure AD validates against the App +// Registration's federated-identity-credential binding. No secret +// material is ever stored in CUDly. // // The issuer URL comes from opts.IssuerURL if set, otherwise from the // package-level oidc.IssuerURL() cache populated by the first inbound // HTTP request — see internal/oidc/issuer_cache.go. +// +// `store` is accepted for signature parity with the other resolver +// helpers; it is unused here because no credential is ever loaded. func resolveAzureWIFCredential( - ctx context.Context, + _ context.Context, account *config.CloudAccount, - store CredentialStore, + _ CredentialStore, opts AzureResolveOptions, ) (azcore.TokenCredential, error) { - raw, _ := loadOptionalWIFKey(ctx, store, account.ID) - issuerURL := opts.IssuerURL if issuerURL == "" { issuerURL = oidc.IssuerURL() } - - // Secret-free federated path — opt-in via signer+issuerURL, only - // when the account has no legacy PEM stored. - if opts.Signer != nil && issuerURL != "" && len(raw) == 0 { - return BuildAzureFederatedCredential(opts.Signer, issuerURL, account.AzureTenantID, account.AzureClientID) - } - - // Legacy cert-based path. - if store == nil { - return nil, fmt.Errorf("credentials: credential store required for azure wif account %s", account.ID) - } - if len(raw) == 0 { - return nil, fmt.Errorf("credentials: no wif key stored for account %s", account.ID) - } - return buildAzureWIFCredential(account, raw) -} - -// loadOptionalWIFKey attempts to read the stored Azure WIF PEM blob for -// an account. Returns (nil, nil) when the store is absent, when the -// blob is unset, or when the load fails — callers use the absence of -// a blob to route, not the error. -func loadOptionalWIFKey(ctx context.Context, store CredentialStore, accountID string) ([]byte, error) { - if store == nil { - return nil, nil + if opts.Signer == nil || issuerURL == "" { + return nil, fmt.Errorf("credentials: azure workload_identity_federation requires a wired OIDC signer and issuer URL (account %s)", account.ID) } - return store.LoadRaw(ctx, accountID, CredTypeAzureWIF) + return BuildAzureFederatedCredential(opts.Signer, issuerURL, account.AzureTenantID, account.AzureClientID) } // BuildAzureFederatedCredential returns an azcore.TokenCredential that diff --git a/internal/credentials/resolver.go b/internal/credentials/resolver.go index b9b7e637..91f049b6 100644 --- a/internal/credentials/resolver.go +++ b/internal/credentials/resolver.go @@ -2,22 +2,14 @@ package credentials import ( "context" - "crypto/rsa" - "crypto/sha1" //nolint:gosec // SHA-1 required by RFC 7517 x5t thumbprint spec - "crypto/x509" - "encoding/base64" "encoding/json" - "encoding/pem" "fmt" "os" "path/filepath" "strings" - "time" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" - "github.com/golang-jwt/jwt/v5" - "github.com/google/uuid" "golang.org/x/oauth2" "golang.org/x/oauth2/google" @@ -33,7 +25,6 @@ import ( const ( CredTypeAWSAccessKeys = "aws_access_keys" CredTypeAzureClientSecret = "azure_client_secret" - CredTypeAzureWIF = "azure_wif_private_key" CredTypeGCPServiceAccount = "gcp_service_account" CredTypeGCPWIFConfig = "gcp_workload_identity_config" ) @@ -291,11 +282,11 @@ func ResolveGCPCredentials(ctx context.Context, account *config.CloudAccount, st return raw, nil } -// ResolveAzureTokenCredential returns an azcore.TokenCredential for the -// account using the legacy (cert-based or client-secret) path. Callers -// that have access to an OIDC Signer should use -// ResolveAzureTokenCredentialWithOpts instead so the secret-free -// federated path is preferred. +// ResolveAzureTokenCredential is a convenience wrapper that passes +// zero-valued options to ResolveAzureTokenCredentialWithOpts. Since +// the legacy cert-based WIF path has been removed, this entry point +// can only resolve client_secret and managed_identity accounts — WIF +// accounts require opts.Signer + IssuerURL via the With-opts variant. func ResolveAzureTokenCredential( ctx context.Context, account *config.CloudAccount, @@ -305,15 +296,14 @@ func ResolveAzureTokenCredential( } // ResolveAzureTokenCredentialWithOpts is like ResolveAzureTokenCredential -// but accepts per-deployment options. When AzureResolveOptions.Signer -// and .IssuerURL are both set, accounts in workload_identity_federation -// mode with no stored PEM are routed through BuildAzureFederatedCredential -// (the secret-free path). Existing cert-based accounts keep working. +// but accepts per-deployment options. For workload_identity_federation +// accounts, AzureResolveOptions.Signer and .IssuerURL must both be set +// so BuildAzureFederatedCredential can mint the KMS-signed client +// assertion Azure AD expects — there is no fallback path. // // Routes by AzureAuthMode: // - managed_identity → ManagedIdentityCredential (no stored cred needed) -// - workload_identity_federation → federated (if no stored PEM and -// opts.Signer is set) or legacy cert client-assertion. +// - workload_identity_federation → federated credential via opts.Signer // - client_secret (default) → loads stored secret and returns ClientSecretCredential func ResolveAzureTokenCredentialWithOpts( ctx context.Context, @@ -341,123 +331,6 @@ func ResolveAzureTokenCredentialWithOpts( } } -// parseRSAKeyBlock parses a single PEM block into an RSA private key. -// Supports both PKCS1 ("RSA PRIVATE KEY") and PKCS8 ("PRIVATE KEY") formats. -func parseRSAKeyBlock(accountID string, block *pem.Block) (*rsa.PrivateKey, error) { - if block.Type == "RSA PRIVATE KEY" { - k, err := x509.ParsePKCS1PrivateKey(block.Bytes) - if err != nil { - return nil, fmt.Errorf("credentials: parse PKCS1 rsa key for account %s: %w", accountID, err) - } - return k, nil - } - k, err := x509.ParsePKCS8PrivateKey(block.Bytes) - if err != nil { - return nil, fmt.Errorf("credentials: parse PKCS8 key for account %s: %w", accountID, err) - } - rk, ok := k.(*rsa.PrivateKey) - if !ok { - return nil, fmt.Errorf("credentials: expected RSA key for account %s", accountID) - } - return rk, nil -} - -// parsePEMBlob extracts an RSA private key and X.509 certificate from a concatenated -// PEM blob. Both blocks must be present — the certificate is required to compute the -// x5t thumbprint for Azure AD client assertions. -func parsePEMBlob(accountID string, pemBlob []byte) (*rsa.PrivateKey, *x509.Certificate, error) { - var rsaKey *rsa.PrivateKey - var cert *x509.Certificate - rest := pemBlob - for { - var block *pem.Block - block, rest = pem.Decode(rest) - if block == nil { - break - } - var err error - rsaKey, cert, err = processPEMBlock(accountID, block, rsaKey, cert) - if err != nil { - return nil, nil, err - } - } - if rsaKey == nil { - return nil, nil, fmt.Errorf("credentials: no private key found in PEM blob for account %s", accountID) - } - if cert == nil { - return nil, nil, fmt.Errorf("credentials: no certificate found in PEM blob for account %s — store key+cert together", accountID) - } - return rsaKey, cert, nil -} - -func processPEMBlock(accountID string, block *pem.Block, existingKey *rsa.PrivateKey, existingCert *x509.Certificate) (*rsa.PrivateKey, *x509.Certificate, error) { - switch block.Type { - case "RSA PRIVATE KEY", "PRIVATE KEY": - if existingKey != nil { - return nil, nil, fmt.Errorf("credentials: multiple private key blocks in PEM blob for account %s", accountID) - } - k, err := parseRSAKeyBlock(accountID, block) - if err != nil { - return nil, nil, err - } - return k, existingCert, nil - case "CERTIFICATE": - if existingCert != nil { - return nil, nil, fmt.Errorf("credentials: multiple certificate blocks in PEM blob for account %s", accountID) - } - c, err := x509.ParseCertificate(block.Bytes) - if err != nil { - return nil, nil, fmt.Errorf("credentials: parse certificate for account %s: %w", accountID, err) - } - return existingKey, c, nil - default: - return existingKey, existingCert, nil - } -} - -// buildAzureWIFCredential creates a client-assertion credential using a stored PEM blob -// that must contain both a PRIVATE KEY and a CERTIFICATE block (concatenated). -// The certificate is registered with the Azure AD App Registration; the x5t thumbprint -// (SHA-1 of the certificate's DER encoding) must be present in the JWT header per -// RFC 7517 / AAD documentation, otherwise Azure AD returns AADSTS700027. -// -// Store format: key PEM block followed by certificate PEM block, e.g.: -// -// -----BEGIN RSA PRIVATE KEY----- -// ... -// -----END RSA PRIVATE KEY----- -// -----BEGIN CERTIFICATE----- -// ... -// -----END CERTIFICATE----- -func buildAzureWIFCredential(account *config.CloudAccount, pemBlob []byte) (azcore.TokenCredential, error) { - rsaKey, cert, err := parsePEMBlob(account.ID, pemBlob) - if err != nil { - return nil, err - } - - // Compute x5t: base64url(SHA-1(cert.Raw)) as required by AAD client assertion. - thumbprint := sha1.Sum(cert.Raw) // #nosec G401 -- SHA-1 mandated by RFC 7517 x5t, not used for security - x5t := base64.RawURLEncoding.EncodeToString(thumbprint[:]) - - tenantID := account.AzureTenantID - clientID := account.AzureClientID - assertionFunc := func(_ context.Context) (string, error) { - now := time.Now() - token := jwt.NewWithClaims(jwt.SigningMethodRS256, jwt.MapClaims{ - "aud": fmt.Sprintf("https://login.microsoftonline.com/%s/oauth2/v2.0/token", tenantID), - "iss": clientID, - "sub": clientID, - "jti": uuid.NewString(), - "nbf": now.Unix(), - "iat": now.Unix(), - "exp": now.Add(5 * time.Minute).Unix(), - }) - token.Header["x5t"] = x5t - return token.SignedString(rsaKey) - } - return azidentity.NewClientAssertionCredential(tenantID, clientID, assertionFunc, nil) -} - // ResolveGCPTokenSource is the legacy entry point — calls // ResolveGCPTokenSourceWithOpts with zero-valued options so existing // cert-based / stored-JSON accounts keep working. diff --git a/internal/credentials/resolver_extra_test.go b/internal/credentials/resolver_extra_test.go index c807b858..c381601d 100644 --- a/internal/credentials/resolver_extra_test.go +++ b/internal/credentials/resolver_extra_test.go @@ -2,18 +2,13 @@ package credentials import ( "context" - "crypto/ecdsa" - "crypto/elliptic" "crypto/rand" "crypto/rsa" "crypto/x509" - "crypto/x509/pkix" "encoding/json" "encoding/pem" - "math/big" "os" "testing" - "time" "github.com/LeanerCloud/CUDly/internal/config" "github.com/stretchr/testify/assert" @@ -189,172 +184,33 @@ func TestResolveAzureTokenCredential_ClientSecret_ValidCred(t *testing.T) { } func TestResolveAzureTokenCredential_WIF_NilStore(t *testing.T) { + // Azure WIF no longer loads anything from the credential store, so a + // nil store is no longer an error by itself. What still errors is + // the absence of an OIDC signer (zero-opts caller) — same code path + // as TestResolveAzureTokenCredential_WIF_NoSigner, just demonstrated + // with store=nil. account := &config.CloudAccount{ ID: "acct1", AzureAuthMode: "workload_identity_federation", } _, err := ResolveAzureTokenCredential(context.Background(), account, nil) assert.Error(t, err) - assert.Contains(t, err.Error(), "credential store required") + assert.Contains(t, err.Error(), "requires a wired OIDC signer") } -func TestResolveAzureTokenCredential_WIF_NoStoredKey(t *testing.T) { - store := newMockStore() // no WIF key stored - account := &config.CloudAccount{ - ID: "acct1", - AzureAuthMode: "workload_identity_federation", - } - _, err := ResolveAzureTokenCredential(context.Background(), account, store) - assert.Error(t, err) - assert.Contains(t, err.Error(), "no wif key stored") -} - -func TestResolveAzureTokenCredential_WIF_WithValidKey(t *testing.T) { +func TestResolveAzureTokenCredential_WIF_NoSigner(t *testing.T) { + // With the legacy cert path removed, Azure WIF mode can only be + // resolved via the KMS-backed OIDC signer. A zero-opts caller + // (ResolveAzureTokenCredential) now errors cleanly instead of + // falling back to a cert-based assertion. store := newMockStore() - pemKey := generateTestKeyAndCertPEM(t) // must contain key+cert for x5t - store.data["acct1/azure_wif_private_key"] = pemKey - account := &config.CloudAccount{ ID: "acct1", AzureAuthMode: "workload_identity_federation", - AzureTenantID: "00000000-0000-0000-0000-000000000001", - AzureClientID: "00000000-0000-0000-0000-000000000002", - } - cred, err := ResolveAzureTokenCredential(context.Background(), account, store) - require.NoError(t, err) - assert.NotNil(t, cred) -} - -// --------------------------------------------------------------------------- -// buildAzureWIFCredential -// --------------------------------------------------------------------------- - -func generateTestRSAKeyPEM(t *testing.T) []byte { - t.Helper() - key, err := rsa.GenerateKey(rand.Reader, 2048) - require.NoError(t, err) - der := x509.MarshalPKCS1PrivateKey(key) - return pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: der}) -} - -func generateTestRSAKeyPKCS8PEM(t *testing.T) []byte { - t.Helper() - key, err := rsa.GenerateKey(rand.Reader, 2048) - require.NoError(t, err) - der, err := x509.MarshalPKCS8PrivateKey(key) - require.NoError(t, err) - return pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: der}) -} - -// generateTestKeyAndCertPEM returns a concatenated PEM blob with a PKCS1 RSA private key -// followed by a self-signed certificate — the format expected by buildAzureWIFCredential. -func generateTestKeyAndCertPEM(t *testing.T) []byte { - t.Helper() - key, err := rsa.GenerateKey(rand.Reader, 2048) - require.NoError(t, err) - - tmpl := &x509.Certificate{ - SerialNumber: big.NewInt(1), - Subject: pkix.Name{CommonName: "CUDly-WIF-test"}, - NotBefore: time.Now().Add(-time.Minute), - NotAfter: time.Now().Add(time.Hour), - } - certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) - require.NoError(t, err) - - keyPEM := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}) - certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) - return append(keyPEM, certPEM...) -} - -// generateTestKeyAndCertPKCS8PEM is the same as generateTestKeyAndCertPEM but uses PKCS8 key format. -func generateTestKeyAndCertPKCS8PEM(t *testing.T) []byte { - t.Helper() - key, err := rsa.GenerateKey(rand.Reader, 2048) - require.NoError(t, err) - - tmpl := &x509.Certificate{ - SerialNumber: big.NewInt(1), - Subject: pkix.Name{CommonName: "CUDly-WIF-test"}, - NotBefore: time.Now().Add(-time.Minute), - NotAfter: time.Now().Add(time.Hour), } - certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) - require.NoError(t, err) - - keyDER, err := x509.MarshalPKCS8PrivateKey(key) - require.NoError(t, err) - keyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyDER}) - certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) - return append(keyPEM, certPEM...) -} - -func generateECKeyPKCS8PEM(t *testing.T) []byte { - t.Helper() - ecKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - require.NoError(t, err) - der, err := x509.MarshalPKCS8PrivateKey(ecKey) - require.NoError(t, err) - return pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: der}) -} - -func TestBuildAzureWIFCredential_ValidPKCS1Key(t *testing.T) { - pemBlob := generateTestKeyAndCertPEM(t) // key+cert concatenated - account := &config.CloudAccount{ - ID: "acct1", - AzureTenantID: "00000000-0000-0000-0000-000000000001", - AzureClientID: "00000000-0000-0000-0000-000000000002", - } - cred, err := buildAzureWIFCredential(account, pemBlob) - require.NoError(t, err) - assert.NotNil(t, cred) -} - -func TestBuildAzureWIFCredential_ValidPKCS8Key(t *testing.T) { - pemBlob := generateTestKeyAndCertPKCS8PEM(t) // PKCS8 key+cert concatenated - account := &config.CloudAccount{ - ID: "acct1", - AzureTenantID: "00000000-0000-0000-0000-000000000001", - AzureClientID: "00000000-0000-0000-0000-000000000002", - } - cred, err := buildAzureWIFCredential(account, pemBlob) - require.NoError(t, err) - assert.NotNil(t, cred) -} - -func TestBuildAzureWIFCredential_InvalidPEM(t *testing.T) { - account := &config.CloudAccount{ID: "acct1"} - _, err := buildAzureWIFCredential(account, []byte("not a pem")) - assert.Error(t, err) - assert.Contains(t, err.Error(), "no private key found in PEM blob") -} - -func TestBuildAzureWIFCredential_NoCertificate(t *testing.T) { - // Key-only PEM — missing certificate block; should fail with informative error. - pemKey := generateTestRSAKeyPEM(t) - account := &config.CloudAccount{ID: "acct1"} - _, err := buildAzureWIFCredential(account, pemKey) - assert.Error(t, err) - assert.Contains(t, err.Error(), "no certificate found in PEM blob") -} - -func TestBuildAzureWIFCredential_NonRSAKey(t *testing.T) { - // An EC key in PKCS8 format: x509.ParsePKCS8PrivateKey succeeds but - // the type assertion to *rsa.PrivateKey will fail. - ecPriv := generateECKeyPKCS8PEM(t) - account := &config.CloudAccount{ID: "acct1"} - _, err := buildAzureWIFCredential(account, ecPriv) - assert.Error(t, err) - assert.Contains(t, err.Error(), "expected RSA key") -} - -func TestBuildAzureWIFCredential_InvalidDER(t *testing.T) { - // PEM block is present but DER content is garbage. - garbage := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: []byte("garbage")}) - account := &config.CloudAccount{ID: "acct1"} - _, err := buildAzureWIFCredential(account, garbage) + _, err := ResolveAzureTokenCredential(context.Background(), account, store) assert.Error(t, err) - assert.Contains(t, err.Error(), "parse PKCS1 rsa key") + assert.Contains(t, err.Error(), "requires a wired OIDC signer") } // --------------------------------------------------------------------------- @@ -600,35 +456,6 @@ func TestResolveAccessKeyProvider_NilStore(t *testing.T) { assert.Contains(t, err.Error(), "credential store required") } -func TestParsePEMBlob_DuplicateKeys(t *testing.T) { - key1, _ := rsa.GenerateKey(rand.Reader, 2048) - key2, _ := rsa.GenerateKey(rand.Reader, 2048) - blob := append( - pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key1)}), - pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key2)})..., - ) - _, _, err := parsePEMBlob("test-acct", blob) - require.Error(t, err) - assert.Contains(t, err.Error(), "multiple private key") -} - -func TestParsePEMBlob_DuplicateCerts(t *testing.T) { - key, _ := rsa.GenerateKey(rand.Reader, 2048) - tmpl := &x509.Certificate{ - SerialNumber: big.NewInt(1), - NotBefore: time.Now(), - NotAfter: time.Now().Add(24 * time.Hour), - } - certDER, _ := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) - certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) - keyPEM := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}) - blob := append(keyPEM, certPEM...) - blob = append(blob, certPEM...) // second cert - _, _, err := parsePEMBlob("test-acct", blob) - require.Error(t, err) - assert.Contains(t, err.Error(), "multiple certificate") -} - // minimalRSAPEM returns a minimal RSA private key PEM for use in JSON // (newlines replaced with \n literal so it fits in a JSON string). func minimalRSAPEM() string {