Skip to content

Migrate pub handler to OIDCRegistry#89

Open
kbukum1 wants to merge 1 commit intomainfrom
kamil/oidc-migrate-pub
Open

Migrate pub handler to OIDCRegistry#89
kbukum1 wants to merge 1 commit intomainfrom
kamil/oidc-migrate-pub

Conversation

@kbukum1
Copy link
Copy Markdown
Contributor

@kbukum1 kbukum1 commented Apr 3, 2026

What

Migrate the Pub repository handler from manual OIDC credential map + mutex to the shared OIDCRegistry type introduced in #78.

Why

Part of the phased migration to fix OIDC credential collisions when multiple registries share a host (#87).

Pub already used the full URL as the OIDC key, so this is a pure structural refactor with no behavior change. OIDC registration is guarded with url != "" to preserve the original URL-scoped behavior. Single-file diff, no test changes.

Behavior changes

  • Credential selection is now deterministic. The old code iterated over a Go map (map[string]*OIDCCredential), so with multiple OIDC credentials on the same host, which one matched was nondeterministic. OIDCRegistry.TryAuth uses longest path-prefix matching, ensuring the most specific credential always wins. This is the core fix for OIDC credential collision when multiple registries share a host #87.

  • Host matching uses strings.ToLower instead of IDNA normalization. The old TryAuthOIDCRequestWithPrefix used helpers.AreHostnamesEqual (IDNA ToASCII), while OIDCRegistry.TryAuth uses lowercase comparison. This is acceptable because all real OIDC registries (Azure DevOps, JFrog, AWS CodeArtifact, Cloudsmith) use ASCII hostnames — no package registry uses internationalized domain names.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Migrates the Pub repository handler’s OIDC handling from a per-handler map + RWMutex approach to the shared oidc.OIDCRegistry to avoid credential collisions and centralize matching logic.

Changes:

  • Replaces oidcCredentials map + sync.RWMutex with an *oidc.OIDCRegistry in PubRepositoryHandler.
  • Registers Pub OIDC credentials via OIDCRegistry.Register(...), guarded by url != "" to keep URL-scoped behavior.
  • Switches request-time auth from oidc.TryAuthOIDCRequestWithPrefix(...) to oidcRegistry.TryAuth(...).

Copy link
Copy Markdown

Copilot AI left a comment

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 1 out of 1 changed files in this pull request and generated no new comments.

@kbukum1 kbukum1 marked this pull request as ready for review April 3, 2026 04:54
@kbukum1 kbukum1 force-pushed the kamil/oidc-migrate-pub branch from 9dfeb71 to 21c9979 Compare April 3, 2026 05:20
@kbukum1 kbukum1 requested a review from Copilot April 3, 2026 05:33
Copy link
Copy Markdown

Copilot AI left a comment

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 1 out of 1 changed files in this pull request and generated 1 comment.

Base automatically changed from kamil/oidc-registry-generalized to main April 4, 2026 00:12
Replace manual OIDC credential map and mutex with the shared
OIDCRegistry type. Pub already used the full URL as the OIDC key,
so this is a pure structural refactor with no behavior change.
OIDC registration is guarded with url != "" to preserve the
original URL-scoped behavior.
@kbukum1 kbukum1 force-pushed the kamil/oidc-migrate-pub branch from 21c9979 to 25ab327 Compare April 4, 2026 00:20
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.

2 participants