Skip to content

fix: respect customized OIDC subject claims in azd pipeline config #7551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
charris-msft wants to merge 5 commits into
base: main
Choose a base branch
from
+156 −10
Open

fix: respect customized OIDC subject claims in azd pipeline config #7551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e3fe6b1
Update flexibleserver and appservice to support managed Identity
charris-msft Mar 6, 2023
f08738e
remove azd-specific tags
charris-msft Mar 10, 2023
549fad0
parameterize new settings
charris-msft Mar 13, 2023
a71691a
formatting
charris-msft Mar 13, 2023
3a7b1c3
fix: respect customized OIDC subject claims in azd pipeline config
charris-msft Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions cli/azd/pkg/commands/pipeline/github_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ func (p *GitHubCiProvider) configureFederatedAuth(
return fmt.Errorf("failed unmarshalling azure credentials: %w", err)
}

err = applyFederatedCredentials(ctx, repoSlug, &azureCredentials, p.console, credential)
err = applyFederatedCredentials(ctx, repoSlug, &azureCredentials, p.console, credential, ghCli)
if err != nil {
return err
}
Expand Down Expand Up @@ -645,6 +645,7 @@ func applyFederatedCredentials(
azureCredentials *azcli.AzureCredentials,
console input.Console,
credential azcore.TokenCredential,
ghCli github.GitHubCli,
) error {
graphClient, err := createGraphClient(ctx, credential)
if err != nil {
Expand All @@ -670,19 +671,36 @@ func applyFederatedCredentials(
return fmt.Errorf("failed retrieving federated credentials: %w", err)
}

// Query GitHub OIDC subject claim customization for the repo/org
oidcConfig, err := ghCli.GetOIDCSubjectForRepo(ctx, repoSlug)
if err != nil {
log.Printf("Warning: failed to query OIDC subject claim config, using default format: %v", err)
oidcConfig = &github.OIDCSubjectConfig{UseDefault: true}
}

mainSubject, err := github.BuildOIDCSubject(ctx, ghCli, repoSlug, oidcConfig, "ref:refs/heads/main")
if err != nil {
return fmt.Errorf("failed to build OIDC subject for main branch: %w", err)
}

prSubject, err := github.BuildOIDCSubject(ctx, ghCli, repoSlug, oidcConfig, "pull_request")
if err != nil {
return fmt.Errorf("failed to build OIDC subject for pull requests: %w", err)
Comment on lines +686 to +688
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BuildOIDCSubject is invoked twice (for main + pull_request). When a repo uses custom claim keys, each call performs its own gh api /repos/{slug} request to fetch IDs, resulting in duplicate network/CLI calls during azd pipeline config. Consider fetching repo info once and reusing it (e.g., have GetOIDCSubjectForRepo also return the needed IDs, or add a helper that builds both subjects from a single repo lookup).

Suggested change
prSubject, err := github.BuildOIDCSubject(ctx, ghCli, repoSlug, oidcConfig, "pull_request")
if err != nil {
return fmt.Errorf("failed to build OIDC subject for pull requests: %w", err)
var prSubject string
switch {
case oidcConfig.UseDefault:
prSubject = fmt.Sprintf("repo:%s:pull_request", repoSlug)
case strings.Contains(mainSubject, ":context:ref:refs/heads/main"):
prSubject = strings.Replace(mainSubject, ":context:ref:refs/heads/main", ":context:pull_request", 1)
default:
// If the customized subject does not include the context claim, the subject
// is the same for main branch and pull request workflows.
prSubject = mainSubject

Copilot uses AI. Check for mistakes.
}

// List of desired federated credentials
federatedCredentials := []graphsdk.FederatedIdentityCredential{
{
Name: "main",
Issuer: federatedIdentityIssuer,
Subject: fmt.Sprintf("repo:%s:ref:refs/heads/main", repoSlug),
Subject: mainSubject,
Description: convert.RefOf("Created by Azure Developer CLI"),
Audiences: []string{federatedIdentityAudience},
},
{
Name: "pull_request",
Issuer: federatedIdentityIssuer,
Subject: fmt.Sprintf("repo:%s:pull_request", repoSlug),
Subject: prSubject,
Description: convert.RefOf("Created by Azure Developer CLI"),
Audiences: []string{federatedIdentityAudience},
},
Expand Down
101 changes: 101 additions & 0 deletions cli/azd/pkg/tools/github/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ type GitHubCli interface {
CreatePrivateRepository(ctx context.Context, name string) error
GetGitProtocolType(ctx context.Context) (string, error)
GitHubActionsExists(ctx context.Context, repoSlug string) (bool, error)
// GetOIDCSubjectForRepo returns the OIDC subject claim format for a repo by querying
// the GitHub OIDC customization API. If the org/repo uses a custom subject template,
// the returned OIDCSubjectConfig will have UseDefault=false and IncludeClaimKeys set.
GetOIDCSubjectForRepo(ctx context.Context, repoSlug string) (*OIDCSubjectConfig, error)
}

func NewGitHubCli(ctx context.Context, console input.Console, commandRunner exec.CommandRunner) (GitHubCli, error) {
Expand Down Expand Up @@ -311,6 +315,103 @@ func (cli *ghCli) GitHubActionsExists(ctx context.Context, repoSlug string) (boo
return true, nil
}

// OIDCSubjectConfig represents the OIDC subject claim customization for a GitHub repository or org.
type OIDCSubjectConfig struct {
UseDefault bool `json:"use_default"`
IncludeClaimKeys []string `json:"include_claim_keys"`
}

// gitHubRepoInfo holds GitHub API repo metadata needed for OIDC subject construction.
type gitHubRepoInfo struct {
ID int `json:"id"`
Owner struct {
ID int `json:"id"`
} `json:"owner"`
}

// BuildOIDCSubject constructs the correct OIDC subject claim for a federated identity credential.
// If the org/repo uses custom claim keys (e.g. repository_owner_id, repository_id), this function
// queries the GitHub API for the numeric IDs and builds the subject accordingly.
// The suffix is the trailing part of the subject, e.g. "ref:refs/heads/main" or "pull_request".
func BuildOIDCSubject(
ctx context.Context, cli GitHubCli, repoSlug string, oidcConfig *OIDCSubjectConfig, suffix string,
) (string, error) {
if oidcConfig == nil || oidcConfig.UseDefault {
return fmt.Sprintf("repo:%s:%s", repoSlug, suffix), nil
}
Comment on lines +339 to +341
Copy link
Copy Markdown
Member

@jongio jongio Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] If the API returns use_default: false with an empty include_claim_keys, the subject ends up as just the suffix (e.g., ref:refs/heads/main) with no repo identifier. That'd match any repo's token - treat as error or fall back to default.

Suggested change
if oidcConfig == nil || oidcConfig.UseDefault {
return fmt.Sprintf("repo:%s:%s", repoSlug, suffix), nil
}
if oidcConfig == nil || oidcConfig.UseDefault {
return fmt.Sprintf("repo:%s:%s", repoSlug, suffix), nil
}
if len(oidcConfig.IncludeClaimKeys) == 0 {
return "", fmt.Errorf(
"OIDC config for %s has use_default=false but no claim keys specified", repoSlug)
}


// For custom claim templates, we need the repo and owner numeric IDs
ghCliImpl, ok := cli.(*ghCli)
if !ok {
// Fallback to default if we can't access the underlying CLI
return fmt.Sprintf("repo:%s:%s", repoSlug, suffix), nil
}
Comment on lines +336 to +348
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BuildOIDCSubject accepts the GitHubCli interface but then type-asserts to *ghCli to make API calls. This undermines the interface abstraction (and will silently fall back to the default subject for any alternate implementation/mocks), making the behavior harder to test and reason about. Consider moving this logic onto *ghCli (method receiver) or extending the interface with the minimal repo-metadata method needed to build the subject.

Copilot uses AI. Check for mistakes.
runArgs := ghCliImpl.newRunArgs("api", "/repos/"+repoSlug, "--jq", "{id: .id, owner: {id: .owner.id}}")
res, err := ghCliImpl.run(ctx, runArgs)
if err != nil {
return "", fmt.Errorf("failed to get repository info for %s: %w", repoSlug, err)
}
var repoInfo gitHubRepoInfo
if err := json.Unmarshal([]byte(res.Stdout), &repoInfo); err != nil {
return "", fmt.Errorf("failed to parse repository info for %s: %w", repoSlug, err)
}

// Build subject from claim keys
// The claim keys define the parts before the context (ref/pull_request).
// Example: include_claim_keys=["repository_owner_id", "repository_id"] produces
// "repository_owner_id:123:repository_id:456:ref:refs/heads/main"
var parts []string
for _, key := range oidcConfig.IncludeClaimKeys {
switch key {
case "repository_owner_id":
parts = append(parts, fmt.Sprintf("repository_owner_id:%d", repoInfo.Owner.ID))
case "repository_id":
parts = append(parts, fmt.Sprintf("repository_id:%d", repoInfo.ID))
case "repository_owner":
owner := strings.SplitN(repoSlug, "/", 2)
parts = append(parts, fmt.Sprintf("repository_owner:%s", owner[0]))
case "repository":
parts = append(parts, fmt.Sprintf("repository:%s", repoSlug))
default:
// Unknown claim key — include it literally for forward compatibility
parts = append(parts, key)
Comment on lines +375 to +377
Copy link
Copy Markdown
Member

@jongio jongio Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Unknown claim keys get appended as bare names without values. parts = append(parts, key) produces somekey:ref:refs/heads/main instead of somekey:<value>:ref:refs/heads/main. When GitHub adds a new claim key, this silently creates a non-matching subject - the same AADSTS700213 error this PR is fixing.

Return an error so the user knows azd needs updating:

Suggested change
default:
// Unknown claim key — include it literally for forward compatibility
parts = append(parts, key)
default:
return "", fmt.Errorf("unsupported OIDC claim key %q in subject template for %s" +
" - azd may need to be updated", key, repoSlug)

}
}
parts = append(parts, suffix)
return strings.Join(parts, ":"), nil
}
Comment on lines +318 to +382
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New OIDC subject logic (GetOIDCSubjectForRepo / BuildOIDCSubject) is not covered by unit tests in this package. Since cli/azd/pkg/tools/github already has github_test.go, consider adding tests that mock gh CLI output for: (1) default config, (2) repo-level custom include_claim_keys, (3) repo-level use_default=true overriding org-level custom, and (4) repo-info lookup used to build ID-based subjects.

Copilot uses AI. Check for mistakes.

// GetOIDCSubjectForRepo queries the GitHub OIDC customization API for a repository.
// It first checks the repo-level customization, then falls back to the org-level customization.
// If no customization is found (or the API returns 404), it returns a config with UseDefault=true.
func (cli *ghCli) GetOIDCSubjectForRepo(ctx context.Context, repoSlug string) (*OIDCSubjectConfig, error) {
// Try repo-level first
runArgs := cli.newRunArgs("api", "/repos/"+repoSlug+"/actions/oidc/customization/sub")
res, err := cli.run(ctx, runArgs)
if err == nil {
var config OIDCSubjectConfig
if jsonErr := json.Unmarshal([]byte(res.Stdout), &config); jsonErr == nil && !config.UseDefault {
return &config, nil
}
}

// Fall back to org-level
parts := strings.SplitN(repoSlug, "/", 2)
if len(parts) == 2 {
orgRunArgs := cli.newRunArgs("api", "/orgs/"+parts[0]+"/actions/oidc/customization/sub")
orgRes, orgErr := cli.run(ctx, orgRunArgs)
if orgErr == nil {
var config OIDCSubjectConfig
if jsonErr := json.Unmarshal([]byte(orgRes.Stdout), &config); jsonErr == nil && !config.UseDefault {
return &config, nil
}
}
}

// Default: no customization
Comment on lines +384 to +411
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetOIDCSubjectForRepo currently falls back to org-level customization whenever the repo-level response is either an error or use_default=true. If a repo explicitly sets use_default=true to override an org-level customization, this will incorrectly apply the org template. Consider returning the repo-level config whenever it unmarshals successfully (even when UseDefault is true), and only falling back to org-level on a confirmed 404/not-found response.

Suggested change
// GetOIDCSubjectForRepo queries the GitHub OIDC customization API for a repository.
// It first checks the repo-level customization, then falls back to the org-level customization.
// If no customization is found (or the API returns 404), it returns a config with UseDefault=true.
func (cli *ghCli) GetOIDCSubjectForRepo(ctx context.Context, repoSlug string) (*OIDCSubjectConfig, error) {
// Try repo-level first
runArgs := cli.newRunArgs("api", "/repos/"+repoSlug+"/actions/oidc/customization/sub")
res, err := cli.run(ctx, runArgs)
if err == nil {
var config OIDCSubjectConfig
if jsonErr := json.Unmarshal([]byte(res.Stdout), &config); jsonErr == nil && !config.UseDefault {
return &config, nil
}
}
// Fall back to org-level
parts := strings.SplitN(repoSlug, "/", 2)
if len(parts) == 2 {
orgRunArgs := cli.newRunArgs("api", "/orgs/"+parts[0]+"/actions/oidc/customization/sub")
orgRes, orgErr := cli.run(ctx, orgRunArgs)
if orgErr == nil {
var config OIDCSubjectConfig
if jsonErr := json.Unmarshal([]byte(orgRes.Stdout), &config); jsonErr == nil && !config.UseDefault {
return &config, nil
}
}
}
// Default: no customization
func isGitHubNotFoundError(err error) bool {
if err == nil {
return false
}
errText := strings.ToLower(err.Error())
return strings.Contains(errText, "404") || strings.Contains(errText, "not found")
}
// GetOIDCSubjectForRepo queries the GitHub OIDC customization API for a repository.
// It first checks the repo-level customization, then falls back to the org-level customization.
// If no customization is found (or the API returns 404), it returns a config with UseDefault=true.
func (cli *ghCli) GetOIDCSubjectForRepo(ctx context.Context, repoSlug string) (*OIDCSubjectConfig, error) {
// Try repo-level first.
runArgs := cli.newRunArgs("api", "/repos/"+repoSlug+"/actions/oidc/customization/sub")
res, err := cli.run(ctx, runArgs)
if err == nil {
var config OIDCSubjectConfig
if jsonErr := json.Unmarshal([]byte(res.Stdout), &config); jsonErr != nil {
return nil, jsonErr
}
return &config, nil
}
if !isGitHubNotFoundError(err) {
return nil, err
}
// Fall back to org-level only when the repo-level customization is not found.
parts := strings.SplitN(repoSlug, "/", 2)
if len(parts) == 2 {
orgRunArgs := cli.newRunArgs("api", "/orgs/"+parts[0]+"/actions/oidc/customization/sub")
orgRes, orgErr := cli.run(ctx, orgRunArgs)
if orgErr == nil {
var config OIDCSubjectConfig
if jsonErr := json.Unmarshal([]byte(orgRes.Stdout), &config); jsonErr != nil {
return nil, jsonErr
}
return &config, nil
}
if !isGitHubNotFoundError(orgErr) {
return nil, orgErr
}
}
// Default: no customization.

Copilot uses AI. Check for mistakes.
return &OIDCSubjectConfig{UseDefault: true}, nil
}

func (cli *ghCli) ForceConfigureAuth(authMode AuthTokenSource) {
switch authMode {
case TokenSourceFile:
Expand Down
39 changes: 32 additions & 7 deletions templates/common/infra/bicep/core/database/postgresql/flexibleserver.bicep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,22 @@ param storage object
param administratorLogin string
@secure()
param administratorLoginPassword string
param activeDirectoryAuth string = 'Disabled'
param databaseNames array = []
param allowAzureIPsFirewall bool = false
param allowAllIPsFirewall bool = false
param allowedSingleIPs array = []

param highAvailabilityMode string = 'Disabled'

param backupRetentionDays int = 7
param geoRedundantBackup string = 'Disabled'

param maintenanceWindowCustomWindow string = 'Disabled'
param maintenanceWindowDayOfWeek int = 0
param maintenanceWindowStartHour int = 0
param maintenanceWindowStartMinute int = 0

// PostgreSQL version
param version string

Expand All @@ -27,7 +38,21 @@ resource postgresServer 'Microsoft.DBforPostgreSQL/flexibleServers@2022-12-01' =
administratorLoginPassword: administratorLoginPassword
storage: storage
highAvailability: {
mode: 'Disabled'
mode: highAvailabilityMode
}
authConfig: {
activeDirectoryAuth: activeDirectoryAuth
passwordAuth: (administratorLoginPassword == null) ? 'Disabled' : 'Enabled'
}
Comment on lines +43 to +46
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

administratorLoginPassword is declared as a non-nullable string, so the (administratorLoginPassword == null) condition will never be true. As a result, passwordAuth will always be set to 'Enabled', which likely breaks the intent of supporting AAD-only auth. Either make the parameter nullable (e.g., string? with a null default) and validate combinations, or remove the null check and hardcode the intended behavior.

Copilot uses AI. Check for mistakes.
backup: {
backupRetentionDays: backupRetentionDays
geoRedundantBackup: geoRedundantBackup
}
maintenanceWindow: {
customWindow: maintenanceWindowCustomWindow
dayOfWeek: maintenanceWindowDayOfWeek
startHour: maintenanceWindowStartHour
startMinute: maintenanceWindowStartMinute
}
}

Expand All @@ -38,24 +63,24 @@ resource postgresServer 'Microsoft.DBforPostgreSQL/flexibleServers@2022-12-01' =
resource firewall_all 'firewallRules' = if (allowAllIPsFirewall) {
name: 'allow-all-IPs'
properties: {
startIpAddress: '0.0.0.0'
endIpAddress: '255.255.255.255'
startIpAddress: '0.0.0.0'
endIpAddress: '255.255.255.255'
}
}

resource firewall_azure 'firewallRules' = if (allowAzureIPsFirewall) {
name: 'allow-all-azure-internal-IPs'
properties: {
startIpAddress: '0.0.0.0'
endIpAddress: '0.0.0.0'
startIpAddress: '0.0.0.0'
endIpAddress: '0.0.0.0'
}
}

resource firewall_single 'firewallRules' = [for ip in allowedSingleIPs: {
name: 'allow-single-${replace(ip, '.', '')}'
properties: {
startIpAddress: ip
endIpAddress: ip
startIpAddress: ip
endIpAddress: ip
}
}]

Expand Down
2 changes: 2 additions & 0 deletions templates/common/infra/bicep/core/host/appservice.bicep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ param numberOfWorkers int = -1
param scmDoBuildDuringDeployment bool = false
param use32BitWorkerProcess bool = false
param ftpsState string = 'FtpsOnly'
param httpLoggingEnabled bool = false
param healthCheckPath string = ''

resource appService 'Microsoft.Web/sites@2022-03-01' = {
Expand All @@ -46,6 +47,7 @@ resource appService 'Microsoft.Web/sites@2022-03-01' = {
linuxFxVersion: linuxFxVersion
alwaysOn: alwaysOn
ftpsState: ftpsState
httpLoggingEnabled: httpLoggingEnabled
Comment on lines 33 to +50
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description and linked issue focus on GitHub OIDC subject claims, but this PR also changes shared Bicep templates (App Service + PostgreSQL flexible server). If these template changes are intentional, they should be called out in the PR description (and ideally justified/linked to an issue); otherwise, consider splitting them into a separate PR to keep the change set scoped.

Copilot uses AI. Check for mistakes.
appCommandLine: appCommandLine
numberOfWorkers: numberOfWorkers != -1 ? numberOfWorkers : null
minimumElasticInstanceCount: minimumElasticInstanceCount != -1 ? minimumElasticInstanceCount : null
Expand Down