fix: respect customized OIDC subject claims in azd pipeline config#7705
fix: respect customized OIDC subject claims in azd pipeline config#7705
Conversation
When GitHub organizations customize their OIDC subject claim format (e.g. using repository_owner_id and repository_id instead of the default repo:owner/name format), azd pipeline config now queries the GitHub OIDC customization API to determine the actual subject format before creating federated identity credentials. Changes: - Add GetOIDCSubjectConfig() and GetRepoInfo() methods to github.Cli - Add BuildOIDCSubject() helper to construct correct subject strings - Update credentialOptions() to query OIDC config and build subjects - Add user confirmation prompt with option for manual subject override - Graceful fallback to default format when OIDC API is unavailable Based on initial work in PR #7551 by @charris-msft, with fixes for: - Correct repo vs org fallback (only on 404) - No type assertions to concrete struct - Single repo info fetch (no duplicate API calls) - Error on unknown/empty claim keys - Full unit test coverage Fixes #7374 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates azd pipeline config’s GitHub federated credential generation to respect GitHub Actions OIDC subject-claim customizations (including ID-based subject formats) by querying GitHub’s OIDC customization endpoints and constructing subjects accordingly.
Changes:
- Add GitHub OIDC customization support (
GetOIDCSubjectConfig,GetRepoInfo,BuildOIDCSubject) and unit tests for these helpers. - Update GitHub pipeline credential creation to use detected OIDC subjects, with an interactive confirmation/override prompt (and auto-use in
--no-promptmode). - Add/adjust pipeline tests to cover OIDC-aware credential option behavior and mocking of the OIDC API.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/pkg/tools/github/oidc.go | Implements OIDC customization discovery + subject construction helpers. |
| cli/azd/pkg/tools/github/oidc_test.go | Adds unit tests for OIDC helpers and API fallback logic. |
| cli/azd/pkg/pipeline/github_provider.go | Uses detected OIDC subjects when creating federated credentials; adds optional prompt/override flow. |
| cli/azd/pkg/pipeline/github_provider_test.go | Adds tests validating default/custom/fallback OIDC subject behavior. |
| cli/azd/pkg/pipeline/pipeline_helpers_test.go | Updates existing credentialOptions tests to mock OIDC API behavior. |
| cli/azd/pkg/pipeline/pipeline_coverage3_test.go | Updates branch-special-chars test setup to account for new OIDC API calls. |
- Remove unused params from promptForSubjects - Sort branch names for deterministic display/prompt ordering - Use int64 for RepoInfo IDs to avoid overflow - Add nil-check for repoInfo in BuildOIDCSubject - Fix misleading comment about skip option - Use t.Context() instead of context.Background() in tests - Fix import ordering in pipeline_helpers_test.go Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Tighten isGitHubNotFoundError to match only HTTP 404 signals - Use MessageUxItem with ux.WarningMessage for OIDC fallback warning - Use t.Context() instead of context.Background() in test helpers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix import ordering in oidc_test.go (stdlib → external → azd internal) - Only call GetRepoInfo when ID-based claim keys are present Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Validate custom subject inputs: trim whitespace, reject empty values Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
left a comment
There was a problem hiding this comment.
Solid work. The repo-to-org-to-default fallback chain is well layered, BuildOIDCSubject correctly constructs the {key}:{value}:...:{context} format matching GitHub's token output, and the explicit error on unsupported claim keys is the right incremental approach. All prior review comments are addressed.
A few optional notes for future consideration:
-
Test gap: org-level non-404 error -
TestGetOIDCSubjectConfigcovers repo 404 falling back to org (success) and repo non-404 error (hard fail), but doesn't cover repo 404 then org returning a non-404 error (e.g. 403). That path exists inGetOIDCSubjectConfigand would be good to verify. -
Test gap:
GetRepoInfoerror path - Only the happy path is tested. A test for API failure or malformed JSON would round out the coverage. -
detectOIDCConfigfallback in--no-promptmode - Any OIDC API error (including 403 permission denied) falls back to default subjects with a warning. In CI (--no-prompt), this could silently create credentials that don't match the org's actual OIDC tokens. Consider either failing hard on non-recoverable errors in--no-promptmode, or logging at a higher severity so it's harder to miss in CI output.
wbreza
left a comment
There was a problem hiding this comment.
Code Review — respect customized OIDC subject claims
Well-designed OIDC detection. The repo→org→default fallback chain is correct, BuildOIDCSubject is a clean pure function, unknown claim keys are properly rejected, and backward compat is preserved. Solid improvement over #7551.
🟡 Medium
1. Silent fallback in --no-prompt on 403 creates mismatched credentials
📁 github_provider.go — detectOIDCConfig
When GetOIDCSubjectConfig fails with 403 (org has OIDC customization but user lacks API permission), the code silently falls back to default subjects with a warning. In --no-prompt/CI mode, this creates credentials that won't match the org's actual OIDC tokens — causing AADSTS700213 later with no clear cause. This is the same class of bug the PR is fixing.
Trade-off: Failing hard would break CI that works by accident (default format matches). But silently creating wrong credentials is strictly worse — the failure surfaces much later as a confusing AADSTS error. Suggest: fail hard on 403/401 in --no-prompt mode, fall back on transient errors (timeouts, 5xx). In interactive mode, current behavior is fine since the user sees the warning.
2. Missing test: repo 404 → org 403 (jongio note #1)
📁 oidc_test.go — TestGetOIDCSubjectConfig
Test matrix covers repo→org fallback but not the case where repo returns 404 and org returns 403. Trivial to add, completes the coverage matrix.
3. Missing test: GetRepoInfo error/malformed JSON (jongio note #2)
📁 oidc_test.go — TestGetRepoInfo
Only happy path tested. Since GetRepoInfo uses --jq, malformed or partial responses from the GitHub CLI are worth covering.
🔵 Low
4. Pre-existing context.Background() in Test_gitHub_provider_getRepoDetails — PR already fixed this pattern elsewhere in the same file.
5. repository_owner case in BuildOIDCSubject assumes repoSlug contains / — works because caller always constructs it, but a comment noting the assumption would help.
✅ What looks good
- Clean repo→org→default fallback with proper 404 vs non-404 distinction
BuildOIDCSubjectis pure, testable, handles all known claim key types- Errors on unknown claim keys with actionable message suggesting azd update
- Errors on empty claim keys with
use_default=false— prevents overly broad subjects - User confirmation prompt with override option — good UX
- 16 new tests covering the core paths
- Supersedes #7551 with all improvements listed in the PR description
Nice work, Victor. Finding #1 is the main one — the irony of the fallback silently re-creating the mismatch that this PR fixes. 👍
- Add test: repo 404 → org 403 error path in GetOIDCSubjectConfig - Add tests: GetRepoInfo API error and malformed JSON paths - Add code comment noting repoSlug format assumption Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Thanks for the approval and the thorough notes, @jongio! Here's how I'm addressing each:
|
|
Thanks for the detailed review, @wbreza! Addressing each point: 🟡 #1 — Silent fallback in
|
|
Thanks for tackling this — the overall direction makes sense, but after pulling the GitHub OIDC subject-claim docs I think there are a couple of semantic mismatches with GitHub’s documented behavior that will still produce incorrect federated credential subjects in some cases. The biggest issue is repo-vs-org resolution. The docs say an organization OIDC template does not affect a repository until that repository explicitly opts in via the repo-level API with The second issue is how the subject string is reconstructed. GitHub documents Concretely, the mismatch scenarios I’d use as repros/integration tests are:
If it helps, I’d suggest covering this with one small integration harness plus unit tests:
Happy to take another pass once the repo/org opt-in behavior and exact |
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
jongio
left a comment
There was a problem hiding this comment.
Updating my review after the latest push and @weikanglim's analysis. The fixes for my earlier notes look good (test gaps filled, tighter 404 matching, sorted map iteration). CI is green.
After reading the GitHub OIDC docs end-to-end, I think @weikanglim's concern about the repo-vs-org fallback is correct and worth addressing before merge.
The docs explicitly state: "When the organization template is applied, it will not affect any workflows already using OIDC unless their repository has opted in to custom organization templates. For all repositories, existing and new, the repository owner will need to use the repository-level REST API to opt in to receive this configuration by setting use_default to false."
This means a repo-level 404 signals "repo hasn't opted in" - not "check the org." A repo that hasn't opted in still gets default-format tokens from GitHub, so applying the org template would create the exact AADSTS700213 mismatch this PR fixes.
In practice, the repos you're targeting (Microsoft, Azure-Samples) have opted in, so the repo API returns their config directly and the org fallback never fires. But for repos in customized orgs that haven't opted in, this is a new failure mode the old code didn't have.
The fix should be straightforward: repo 404 -> default format, skip the org fallback entirely.
Description
Fixes #7374
Supersedes #7551 (based on initial work by @charris-msft)
When GitHub organizations customize their OIDC subject claim format (e.g. using
repository_owner_idandrepository_idinstead of the defaultrepo:owner/nameformat),azd pipeline confignow queries the GitHub OIDC customization API to determine the actual subject format before creating federated identity credentials.Problem
azd pipeline configalways created federated credentials with the default subject format:But organizations like
microsoftandAzure-Samplesuse customized OIDC subject claims, producing tokens with subjects like:This mismatch caused
AADSTS700213: No matching federated identity record founderrors.Changes
cli/azd/pkg/tools/github/oidc.go(new):OIDCSubjectConfigandRepoInfotypesGetOIDCSubjectConfig()— queries repo-level OIDC API first, falls back to org-level only on 404GetRepoInfo()— fetches numeric repo/owner IDs for ID-based claim keysBuildOIDCSubject()— pure function to construct subject strings from configcli/azd/pkg/pipeline/github_provider.go:credentialOptions()to query OIDC config and build correct subjects--no-promptmode, auto-detected subjects are used without promptingcli/azd/pkg/tools/github/oidc_test.go(new):BuildOIDCSubject,GetOIDCSubjectConfig, andGetRepoInfocli/azd/pkg/pipeline/github_provider_test.go:Improvements over #7551
use_default=trueCliuse_default=false— prevents dangerously broad subjects