Skip to content

feat: Add token source for generic auth logic - #4637

Open
stevehipwell wants to merge 3 commits into
google:masterfrom
stevehipwell:add-token-signer
Open

stevehipwell wants to merge 3 commits into
google:masterfrom
stevehipwell:add-token-signer

Conversation

@stevehipwell

@stevehipwell stevehipwell commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

This PR updates the token focused logic added in #4564 with more generic logic that will support non-fixed token patterns (e.g. app auth) via a oauth2.TokenSource (as mentioned in #4365). This pattern allows us to be specific about the way that authentication should be configured for the client.

As part of this work I've removed the BasicAuthTransport as it's use case is no longer required due to the API no longer supporting basic auth. I've also removed the UnauthenticatedRateLimitedTransport which can be replaced with the new github.WithBasicAuth option that integrates with the client URLs and wont cause unintended consequences if the client is cloned. I've also removed reference to the github.com/bradleyfalzon/ghinstallation package which doesn't align with the usage of oauth2.TokenSource.

I also needed to remove WithHTTPClient as this pattern doesn't fit as the client needs to be mutated.

As followup for these changes we should be able to stop blocking redirects to protect the auth token (e.g. DownloadReleaseAsset should be able to return the asset without passing in an additional client).

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the client's authentication mechanism to use oauth2.TokenSource and the go-githubauth package, replacing the deprecated BasicAuthTransport and UnauthenticatedRateLimitedTransport with the new WithBasicAuth and WithTokenSource options. It also updates the documentation, examples, and tests accordingly. Feedback was provided regarding the example application, where using term.ReadPassword to read a multi-line PEM private key from standard input will fail; it is recommended to prompt for the file path and read the file instead.

Comment thread example/newfilewithappauth/main.go Outdated
@gmlewis gmlewis changed the title feat: add token source for generic auth logic feat: Add token source for generic auth logic Oct 16, 2026
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
@codecov

codecov Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.61%. Comparing base (73b97a6) to head (44f72f5).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4637      +/-   ##
==========================================
- Coverage   98.62%   98.61%   -0.01%     
==========================================
  Files         199      199              
  Lines       18700    18663      -37     
==========================================
- Hits        18442    18405      -37     
  Misses        258      258              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NeedsReview PR is awaiting a review before merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants