Skip to content

refactor(credentials): drop legacy cert-based Azure WIF path#31

Merged
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
refactor/drop-azure-wif-legacy-pem
Apr 24, 2026
Merged

refactor(credentials): drop legacy cert-based Azure WIF path#31
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
refactor/drop-azure-wif-legacy-pem

Conversation

@cristim
Copy link
Copy Markdown
Member

@cristim cristim commented Apr 24, 2026

Summary

Drops the legacy cert-based Azure Workload Identity Federation path entirely. Azure WIF now has exactly one implementation: KMS-signed client assertions via CUDly's OIDC issuer — secret-free, nothing stored per-account. The cert path (stored RSA private key + self-signed certificate + x5t thumbprint) was scaffolding from before the federation redesign; the IaC templates already emit federation-only guidance, so removing the path aligns the whole stack.

Issue #14 reported "Azure WIF account submits without a required private key"; the correct fix isn't to require the key — it's to remove the obsolete field entirely so the reporter's scenario is impossible.

Changes

Frontend

  • Delete the RSA private-key textarea + cert-path instructions from the Azure account modal.
  • Drop the WIF branch from the credential save path.
  • Remove 'azure_wif_private_key' from the TS credential_type union.

Backend — credential resolver

  • credentials/resolver.go: delete buildAzureWIFCredential, parsePEMBlob, processPEMBlock, parseRSAKeyBlock, CredTypeAzureWIF. Drop crypto/rsa, crypto/sha1, crypto/x509, encoding/base64, encoding/pem, golang-jwt/v5 imports.
  • credentials/azure_federated.go: resolveAzureWIFCredential becomes a straight delegation to BuildAzureFederatedCredential; fails fast with an operator-facing error when the OIDC signer isn't wired.

Backend — API

  • api/handler_accounts.go: remove azure_wif_private_key from validCredentialTypes and error strings. Drop the legacy fallback in azureFederatedCredResult. credTypeForAccount returns "" for Azure WIF; checkCredentialPresence short-circuits empty credType with an actionable operator message (covers the degraded "signer not wired" case).
  • api/validation.go: drop the azure_wif_private_key payload schema and switch arm.
  • api/handler_federation.go: rewrite the Azure README generator — federation-only guidance matches the bundled IaC templates.
  • api/handler_registrations.go: refresh accountHasCredentialFreePath comment.

Tests (net -345 lines)

  • credentials/resolver_extra_test.go: delete all TestBuildAzureWIFCredential_*, TestParsePEMBlob_*, and the cert-generation helpers. Reframe the Azure-WIF resolver tests as "requires wired OIDC signer".
  • 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.

Risk

  • Accounts with stored PEMs fail auth after merge. User confirmed none exist in any environment.
  • Third-party automation posting azure_wif_private_key credentials breaks. This is an internal-only surface; only CUDly's own frontend uses it.

Closes #14.

Test plan

  • go test ./... — all Go packages green
  • go build ./... — clean
  • npx jest — 1234 tests pass (35 suites)
  • npx tsc --noEmit — clean
  • Pre-commit hooks all pass (gofmt, vet, gocyclo, gosec, trivy, Go tests, frontend build + tests)
  • Post-merge verification: AWS/Azure/GCP deploy CI green; probe /api/info on the deployed AWS Lambda URL; open the Azure add-account modal in a browser and confirm the RSA Private Key textarea is gone.

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.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

Warning

Rate limit exceeded

@cristim has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 25 minutes and 27 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 25 minutes and 27 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e52d2ac1-ef94-41f4-88f5-24e36ac57882

📥 Commits

Reviewing files that changed from the base of the PR and between f760d4d and 4bc6793.

📒 Files selected for processing (13)
  • frontend/src/api/accounts.ts
  • frontend/src/index.html
  • frontend/src/settings.ts
  • go.mod
  • internal/api/coverage_gaps_test.go
  • internal/api/handler_accounts.go
  • internal/api/handler_federation.go
  • internal/api/handler_registrations.go
  • internal/api/validation.go
  • internal/api/validation_test.go
  • internal/credentials/azure_federated.go
  • internal/credentials/resolver.go
  • internal/credentials/resolver_extra_test.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/drop-azure-wif-legacy-pem

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Apr 24, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim cristim merged commit 559aed9 into feat/multicloud-web-frontend Apr 24, 2026
3 checks passed
@cristim cristim deleted the refactor/drop-azure-wif-legacy-pem branch April 24, 2026 14:58
@cristim cristim added type/chore Maintenance / non-user-visible severity/low Minor harm urgency/this-quarter Within the quarter impact/internal Team-internal only effort/m Days priority/p3 Polish / idea / may never ship triaged Item has been triaged labels Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/internal Team-internal only priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/chore Maintenance / non-user-visible urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant