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 {