Skip to content

Commit

Permalink
Use Authorization header for Github PAT
Browse files Browse the repository at this point in the history
PATs are oauth tokens so there's no reason to use BasicAuth here.

I read the GH docs as recommending `Authorization: Bearer` if possible.

GH understands what kind of token is being used, e.g. I see:
`Added on Aug 20, 2020 via personal access token owned by
@michaelbeaumont`
when I authenticate this way and add a DeployKey
  • Loading branch information
michaelbeaumont committed Aug 20, 2020
1 parent 92426c1 commit e49a1ae
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 46 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
## Features

- **Consistency:** Using the same Client interface and high-level structs for multiple backends.
- **Authentication:** Personal Access Tokens, OAuth2 Tokens, and unauthenticated.
- **Authentication:** Personal Access Tokens/OAuth2 Tokens, and unauthenticated.
- **Pagination:** List calls automatically return all available pages.
- **Conditional Requests:** Asks the Git provider if cached data is up-to-date before requesting, to avoid being rate limited.
- **Reconciling:** Support reconciling desired state towards actual state and drift detection.
Expand Down
30 changes: 1 addition & 29 deletions github/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ import (
const (
// DefaultDomain specifies the default domain used as the backend.
DefaultDomain = "github.com"
// patUsername is the "username" for the basic auth authentication flow
// when using a personal access token as the "password". This string could
// be arbitrary, even unset, as it is not respected server-side. For conventions'
// sake, we'll set this to "git".
patUsername = "git"
)

// ClientOption is the interface to implement for passing options to NewClient.
Expand Down Expand Up @@ -169,7 +164,6 @@ func WithPostChainTransportHook(postRoundTripperFunc gitprovider.ChainableRoundT

// WithOAuth2Token initializes a Client which authenticates with GitHub through an OAuth2 token.
// oauth2Token must not be an empty string.
// WithOAuth2Token is mutually exclusive with WithPersonalAccessToken.
func WithOAuth2Token(oauth2Token string) ClientOption {
// Don't allow an empty value
if len(oauth2Token) == 0 {
Expand All @@ -191,28 +185,6 @@ func oauth2Transport(oauth2Token string) gitprovider.ChainableRoundTripperFunc {
}
}

// WithPersonalAccessToken initializes a Client which authenticates with GitHub through a personal access token.
// patToken must not be an empty string.
// WithPersonalAccessToken is mutually exclusive with WithOAuth2Token.
func WithPersonalAccessToken(patToken string) ClientOption {
// Don't allow an empty value
if len(patToken) == 0 {
return optionError(fmt.Errorf("patToken cannot be empty: %w", gitprovider.ErrInvalidClientOptions))
}

return &clientOptions{AuthTransport: patTransport(patToken)}
}

func patTransport(patToken string) gitprovider.ChainableRoundTripperFunc {
return func(in http.RoundTripper) http.RoundTripper {
return &github.BasicAuthTransport{
Username: patUsername,
Password: patToken,
Transport: in,
}
}
}

// WithConditionalRequests instructs the client to use Conditional Requests to GitHub, asking GitHub
// whether a resource has changed (without burning your quota), and using an in-memory cached "database"
// if so. See: https://developer.github.com/v3/#conditional-requests for more information.
Expand All @@ -233,7 +205,7 @@ func makeOptions(opts ...ClientOption) (*clientOptions, error) {

// NewClient creates a new gitprovider.Client instance for GitHub API endpoints.
//
// Using WithOAuth2Token or WithPersonalAccessToken you can specify authentication
// Using WithOAuth2Token you can specify authentication
// credentials, passing no such ClientOption will allow public read access only.
//
// Basic Auth is not supported because it is deprecated by GitHub, see
Expand Down
15 changes: 0 additions & 15 deletions github/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,21 +162,6 @@ func Test_makeOptions(t *testing.T) {
opts: []ClientOption{WithOAuth2Token("")},
expectedErrs: []error{gitprovider.ErrInvalidClientOptions},
},
{
name: "WithPersonalAccessToken",
opts: []ClientOption{WithPersonalAccessToken("foo")},
want: &clientOptions{AuthTransport: patTransport("foo")},
},
{
name: "WithPersonalAccessToken, empty",
opts: []ClientOption{WithPersonalAccessToken("")},
expectedErrs: []error{gitprovider.ErrInvalidClientOptions},
},
{
name: "WithPersonalAccessToken and WithOAuth2Token, exclusive",
opts: []ClientOption{WithPersonalAccessToken("foo"), WithOAuth2Token("foo")},
expectedErrs: []error{gitprovider.ErrInvalidClientOptions},
},
{
name: "WithConditionalRequests",
opts: []ClientOption{WithConditionalRequests(true)},
Expand Down
2 changes: 1 addition & 1 deletion github/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ var _ = Describe("GitHub Provider", func() {

var err error
c, err = NewClient(
WithPersonalAccessToken(githubToken),
WithOAuth2Token(githubToken),
WithDestructiveAPICalls(true),
WithConditionalRequests(true),
WithPreChainTransportHook(customTransportFactory),
Expand Down

0 comments on commit e49a1ae

Please sign in to comment.