Skip to content

fix: match FIC issuer to token iss#899

Open
cheatsheet1999 wants to merge 5 commits into
Azure:devfrom
cheatsheet1999:yuelinzhao/normalize-fic-match-token-iss
Open

fix: match FIC issuer to token iss#899
cheatsheet1999 wants to merge 5 commits into
Azure:devfrom
cheatsheet1999:yuelinzhao/normalize-fic-match-token-iss

Conversation

@cheatsheet1999

@cheatsheet1999 cheatsheet1999 commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary

SecretSync used the OIDC issuer reported by ARM when creating a federated identity credential (FIC). In some configurations the ARM issuer ends with / while the service account token iss does not. Entra requires an exact match, so secret retrieval fails with AADSTS700211 (HTTP 401).

Fix

Resolve the canonical issuer from the cluster's public OIDC discovery document ({issuer}/.well-known/openid-configuration). By the OIDC contract, the discovery issuer is the same value the token carries as iss - and the same document Entra fetches unauthenticated to validate the FIC.

  • Resolve via a plain unauthenticated HTTPS GET - no kubeconfig, cluster RBAC, or service account token. This also works from an ARM-only / CI context and in the clone flow before the target cluster exists.
  • Use the discovery issuer verbatim in the FIC; accept it only when it is exact or differs from the ARM value by exactly one trailing slash.
  • Probe the verbatim URL and, for trailing-slash ARM issuers, also the one-slash-removed URL, use whichever returns a matching issuer (the single-slash form is what succeeds against Azure's OIDC endpoint).
  • Warn and keep the ARM value when discovery cannot verify - no new hard-fail on an ARM-only command.
  • Harden the request: HTTPS-only, TLS verification, short timeout, bounded response body, and no redirect following (the URL originates from ARM data).
  • Use the resolved issuer consistently for FIC lookup, naming, and creation, and apply the same handling to the clone flow.

Some clarifications...

Q: Why read the discovery document instead of minting a service account token?
A: The discovery issuer equals the token iss by contract, so it yields the same canonical value without requiring cluster access, kubeconfig, RBAC (serviceaccounts/token), or the service account to exist. It also works from CI / ARM-only contexts and in clone (before the cluster is deployed).

Q: Why not always remove the trailing slash?
A: This is not a failure that happens every time. Some clusters include / in their issuer. Normalizing unconditionally could break working configurations, so the canonical discovery value is used verbatim.

Q: Does this change the common case?
A: No. When ARM and the discovery issuer already match, the same issuer is used with no warning.

Q: Why does the FIC lookup remain exact?
A: Entra treats issuers with and without / as different values. The resolved issuer is used for both lookup and creation.

Q: Does this update existing FICs?
A: No. Repairing already-created FICs is outside this change's scope (tracked as a follow-up).

Repro and Tests

Before:

token iss and ARM issuerUrl mismatch:

iss:
image

ARM issueUrl (and FIC):
image

Note the trailing slash after fc

Result: 401
image

After

token ISS and ARM IssuerUrl remains unchanged, but FIC is normalized
Screenshot 2026-07-15 190404
No trailing slash

Result:
image

@cheatsheet1999
cheatsheet1999 force-pushed the yuelinzhao/normalize-fic-match-token-iss branch from 89e6504 to 95a0ad9 Compare July 16, 2026 06:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes SecretSync federation failures caused by a mismatch between the OIDC issuer returned by ARM (sometimes with a trailing /) and the service account token iss (sometimes without it). It adds logic to resolve the issuer from a Kubernetes-issued service account token and uses the resolved issuer consistently for federated identity credential (FIC) lookup/creation, including in the clone flow, while falling back to the ARM issuer when the cluster can’t be queried or issuers are unrelated.

Changes:

  • Add resolve_oidc_issuer() to prefer the Kubernetes token issuer when it only differs from ARM by a trailing /.
  • Apply the resolved issuer to SecretSync enable flow and clone federation flow.
  • Add unit tests validating issuer-resolution behavior and clone integration.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
azext_edge/edge/providers/orchestration/resources/instances.py Adds Kubernetes token-based issuer resolution and uses it in SecretSync enable flow.
azext_edge/edge/providers/orchestration/clone.py Uses the resolved issuer during clone federation handling.
azext_edge/tests/edge/orchestration/resources/test_instances_unit.py Adds unit tests for issuer resolution and mocks issuer resolution for unrelated tests.
azext_edge/tests/edge/orchestration/test_clone_unit.py Mocks and asserts clone flow calls issuer resolution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread azext_edge/edge/providers/orchestration/resources/instances.py Outdated
@cheatsheet1999
cheatsheet1999 marked this pull request as ready for review July 16, 2026 19:56
@cheatsheet1999
cheatsheet1999 requested a review from digimaun as a code owner July 16, 2026 19:56
Comment thread azext_edge/edge/providers/orchestration/resources/instances.py Outdated
Comment thread azext_edge/edge/providers/orchestration/resources/instances.py Outdated
@cheatsheet1999

Copy link
Copy Markdown
Member Author

@digimaun Thanks for the detailed guidance. I’ve refactored this to use the discovery resolver we agreed on. Mapping the changes to your points:

  1. Discovery in both flows, resolve_oidc_issuer(arm_issuer=...) now runs in both SecretSync enable (instances.py) and clone (clone.py). The token-based path has been removed.
  2. Verbatim issuer - The issuer returned by discovery is used as-is. No rstrip or normalized value is stored.
  3. Exact match or one trailing slash only - oidc_issuers_match() returns true only when the values are identical or differ by exactly one trailing slash. Values with multiple trailing slashes or unrelated values are rejected.
  4. URL handling - I first probe the verbatim discovery URL. If the ARM issuer ends with a trailing slash, I also probe the URL with that slash removed, then use whichever endpoint returns a matching issuer. The single-slash form succeeds against Azure’s OIDC endpoint, while the verbatim //.well-known form returns a 404.
  5. Hardened GET - The request is restricted to HTTPS, uses verify=True, has a five-second timeout, limits the response body to 64 KiB, and sets allow_redirects=False.
  6. User awareness - An exact match is silent. A one-slash correction produces a warning that the ARM and cluster issuers differed and that the FIC was corrected. If discovery is unreachable or cannot be verified, we warn the user and retain the ARM value, so this does not introduce a new hard failure.
  7. Deferred based on your guidance - Repairing an existing FIC, which would require changing the early-return behavior, and adding an opt-in --context cluster fallback will be tracked as separate follow-up items.

Could you take another look when you have a moment?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread azext_edge/edge/providers/orchestration/resources/instances.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants