feat(auth): add TokenFetcher interface and token abstraction#1181
Open
TerryHowe wants to merge 5 commits into
Open
feat(auth): add TokenFetcher interface and token abstraction#1181TerryHowe wants to merge 5 commits into
TerryHowe wants to merge 5 commits into
Conversation
TerryHowe
requested review from
Wwwsylvia,
sabre1041 and
shizhMSFT
as code owners
May 23, 2026 15:52
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1181 +/- ##
==========================================
+ Coverage 83.09% 83.19% +0.09%
==========================================
Files 82 83 +1
Lines 5834 5926 +92
==========================================
+ Hits 4848 4930 +82
- Misses 605 610 +5
- Partials 381 386 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced May 23, 2026
TerryHowe
force-pushed
the
feat/auth-token-fetcher
branch
from
May 23, 2026 15:59
4573581 to
c71120f
Compare
TerryHowe
force-pushed
the
feat/auth-token-fetcher
branch
from
May 25, 2026 22:08
c71120f to
16542eb
Compare
Extract bearer-token acquisition from auth.Client into a pluggable
TokenFetcher interface:
FetchToken(ctx, TokenParams, Credential) (string, error)
Three implementations land alongside the interface:
- DistributionTokenFetcher: GET against the distribution-spec token
endpoint with optional basic auth.
- OAuth2TokenFetcher: POST OAuth2 password/refresh_token grant.
- CompositeTokenFetcher: selects between the above based on
credential shape and a LegacyMode toggle,
mirroring the existing in-client logic.
A new optional Client.TokenFetcher field lets callers inject a custom
strategy (e.g. ECR/GCR-specific token exchange) without forking the
client. When nil, the built-in fallback in fetchBearerToken preserves
the existing behavior, so this is a purely additive change to the
public API. ForceAttemptOAuth2 is unchanged.
Test coverage in token_test.go is independent of the client; two new
client_test.go cases (TestClient_Do_Bearer_CustomTokenFetcher and
_Error) exercise the wiring end-to-end.
Signed-off-by: Terry Howe <terrylhowe@gmail.com>
TerryHowe
force-pushed
the
feat/auth-token-fetcher
branch
from
June 22, 2026 14:35
16542eb to
38f90d6
Compare
gjenkins8
reviewed
Jun 24, 2026
gjenkins8
approved these changes
Jun 24, 2026
gjenkins8
left a comment
There was a problem hiding this comment.
LGTM
Perhaps a common HTTP client that does the work of DistributionTokenFetcher.send and OAuth2TokenFetcher.send could be introduced (and would establish common baseline for HTTP request sending)
Both DistributionTokenFetcher and OAuth2TokenFetcher carried byte-for-byte identical send methods and near-identical token-response parsing. Pull the header/client send logic into sendRequest and the JSON decode into decodeTokenResponse so each fetcher delegates instead of duplicating. The unified decoder also accepts a 'token' field as a fallback for the OAuth2 fetcher (previously access_token only); this is within spec and strictly more lenient. Addresses review feedback from @gjenkins8 on oras-project#1181. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Terry Howe <terrylhowe@gmail.com>
gjenkins8
approved these changes
Jun 30, 2026
gjenkins8
left a comment
There was a problem hiding this comment.
(Re) LGTM
In particular, the TokenFetcher interface w/ TokenParams representing the concrete registry details enables evolving how auth tokens are retrieved over time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part 2 of 3 splitting #1141 into reviewable chunks.
Extract bearer-token acquisition from
auth.Clientinto a pluggableTokenFetcherinterface:Three concrete implementations land in the same file:
DistributionTokenFetcherOAuth2TokenFetcherCompositeTokenFetcherLegacyModetoggle — mirrors the existing in-client logicA new optional
Client.TokenFetcherfield lets callers inject a custom strategy (e.g. ECR/GCR-specific token exchange) without forking the client. When nil, the built-in fallback infetchBearerTokenpreserves the existing behavior, so this is a purely additive change to the public API.ForceAttemptOAuth2is unchanged.Why split it out
Today the bearer-token strategy is hardcoded inside
Client.fetchBearerToken. Callers who need a custom strategy have to fork or wrap the whole client. WithTokenFetcher, they swap in one field.The composite implementation is also easier to test in isolation than the inline logic — see
token_test.go(805 lines) which exercises the strategies without spinning up an HTTP client at all.This will replace https://github.com/oras-project/oras-go/blob/main/registry/remote/auth/client.go#L266:L400
Test plan
go test -mod=mod ./registry/remote/auth/...— passes (existing tests + new TokenFetcher coverage)token_test.gocovers all three fetcher implementations independentlyclient_test.goaddsTestClient_Do_Bearer_CustomTokenFetcherand_Errorto verify wiringFollow-ups in the split
feat(auth)!: flip OAuth2 default and rename ForceAttemptOAuth2 to legacyMode— separate breaking change, independent of this PRfeat(auth): add ForceBasicAuth to override Bearer challenges— independentReplaces #1141 together with PRs A and C.