Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions api/http_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,24 @@ type tokenGetter interface {
}

type HTTPClientOptions struct {
AppVersion string
CacheTTL time.Duration
Config tokenGetter
EnableCache bool
Log io.Writer
LogColorize bool
LogVerboseHTTP bool
AppVersion string
CacheTTL time.Duration
Config tokenGetter
EnableCache bool
Log io.Writer
LogColorize bool
LogVerboseHTTP bool
SkipDefaultHeaders bool
}

func NewHTTPClient(opts HTTPClientOptions) (*http.Client, error) {
// Provide invalid host, and token values so gh.HTTPClient will not automatically resolve them.
// The real host and token are inserted at request time.
clientOpts := ghAPI.ClientOptions{
Host: "none",
AuthToken: "none",
LogIgnoreEnv: true,
Host: "none",
AuthToken: "none",
LogIgnoreEnv: true,
SkipDefaultHeaders: opts.SkipDefaultHeaders,
}

debugEnabled, debugValue := utils.IsDebugEnabled()
Expand Down
88 changes: 59 additions & 29 deletions api/http_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,16 @@ import (

func TestNewHTTPClient(t *testing.T) {
type args struct {
config tokenGetter
appVersion string
logVerboseHTTP bool
config tokenGetter
appVersion string
logVerboseHTTP bool
skipDefaultHeaders bool
}
tests := []struct {
name string
args args
host string
wantHeader map[string]string
wantHeader map[string][]string
wantStderr string
}{
{
Expand All @@ -37,10 +38,10 @@ func TestNewHTTPClient(t *testing.T) {
logVerboseHTTP: false,
},
host: "github.com",
wantHeader: map[string]string{
"authorization": "token MYTOKEN",
"user-agent": "GitHub CLI v1.2.3",
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
wantHeader: map[string][]string{
"authorization": {"token MYTOKEN"},
"user-agent": {"GitHub CLI v1.2.3"},
"accept": {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
},
wantStderr: "",
},
Expand All @@ -51,10 +52,10 @@ func TestNewHTTPClient(t *testing.T) {
appVersion: "v1.2.3",
},
host: "example.com",
wantHeader: map[string]string{
"authorization": "token GHETOKEN",
"user-agent": "GitHub CLI v1.2.3",
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
wantHeader: map[string][]string{
"authorization": {"token GHETOKEN"},
"user-agent": {"GitHub CLI v1.2.3"},
"accept": {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
},
wantStderr: "",
},
Expand All @@ -66,10 +67,10 @@ func TestNewHTTPClient(t *testing.T) {
logVerboseHTTP: false,
},
host: "github.com",
wantHeader: map[string]string{
"authorization": "",
"user-agent": "GitHub CLI v1.2.3",
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
wantHeader: map[string][]string{
"authorization": nil, // should not be set
"user-agent": {"GitHub CLI v1.2.3"},
"accept": {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
},
wantStderr: "",
},
Expand All @@ -81,10 +82,10 @@ func TestNewHTTPClient(t *testing.T) {
logVerboseHTTP: false,
},
host: "example.com",
wantHeader: map[string]string{
"authorization": "",
"user-agent": "GitHub CLI v1.2.3",
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
wantHeader: map[string][]string{
"authorization": nil, // should not be set
"user-agent": {"GitHub CLI v1.2.3"},
"accept": {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
},
wantStderr: "",
},
Expand All @@ -96,10 +97,10 @@ func TestNewHTTPClient(t *testing.T) {
logVerboseHTTP: true,
},
host: "github.com",
wantHeader: map[string]string{
"authorization": "token MYTOKEN",
"user-agent": "GitHub CLI v1.2.3",
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
wantHeader: map[string][]string{
"authorization": {"token MYTOKEN"},
"user-agent": {"GitHub CLI v1.2.3"},
"accept": {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
},
wantStderr: heredoc.Doc(`
* Request at <time>
Expand All @@ -115,6 +116,34 @@ func TestNewHTTPClient(t *testing.T) {
< HTTP/1.1 204 No Content
< Date: <time>

* Request took <duration>
`),
},
{
name: "respect skip default headers option",
args: args{
appVersion: "v1.2.3",
logVerboseHTTP: true,
skipDefaultHeaders: true,
},
host: "github.com",
wantHeader: map[string][]string{
"accept": nil,
"authorization": nil,
"content-type": nil,
"user-agent": {"GitHub CLI v1.2.3"},
},
wantStderr: heredoc.Doc(`
* Request at <time>
* Request to http://<host>:<port>
> GET / HTTP/1.1
> Host: github.com
> Time-Zone: <timezone>
> User-Agent: GitHub CLI v1.2.3

< HTTP/1.1 204 No Content
< Date: <time>

* Request took <duration>
`),
},
Expand All @@ -131,10 +160,11 @@ func TestNewHTTPClient(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, stderr := iostreams.Test()
client, err := NewHTTPClient(HTTPClientOptions{
AppVersion: tt.args.appVersion,
Config: tt.args.config,
Log: ios.ErrOut,
LogVerboseHTTP: tt.args.logVerboseHTTP,
AppVersion: tt.args.appVersion,
Config: tt.args.config,
Log: ios.ErrOut,
LogVerboseHTTP: tt.args.logVerboseHTTP,
SkipDefaultHeaders: tt.args.skipDefaultHeaders,
})
require.NoError(t, err)

Expand All @@ -148,7 +178,7 @@ func TestNewHTTPClient(t *testing.T) {
require.NoError(t, err)

for name, value := range tt.wantHeader {
assert.Equal(t, value, gotReq.Header.Get(name), name)
assert.Equal(t, value, gotReq.Header.Values(name), name)
}

assert.Equal(t, 204, res.StatusCode)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ require (
github.com/gorilla/websocket v1.5.3
github.com/hashicorp/go-multierror v1.1.1
github.com/hashicorp/go-version v1.7.0
github.com/henvic/httpretty v0.1.4
github.com/hinshun/vt10x v0.0.0-20220119200601-820417d04eec
github.com/in-toto/attestation v1.1.2
github.com/joho/godotenv v1.5.1
Expand Down Expand Up @@ -148,6 +147,7 @@ require (
github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
github.com/henvic/httpretty v0.1.4 // indirect
github.com/huandu/xstrings v1.5.0 // indirect
github.com/in-toto/in-toto-golang v0.9.0 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
Expand Down
43 changes: 4 additions & 39 deletions internal/authflow/flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,13 @@ import (
"io"
"net/http"
"net/url"
"regexp"
"strings"

"github.com/atotto/clipboard"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/cli/cli/v2/utils"
"github.com/cli/oauth"
"github.com/henvic/httpretty"

ghauth "github.com/cli/go-gh/v2/pkg/auth"
)
Expand All @@ -26,21 +22,15 @@ var (
oauthClientID = "178c6fc778ccc68e1d6a"
// This value is safe to be embedded in version control
oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b"

jsonTypeRE = regexp.MustCompile(`[/+]json($|;)`)
)

func AuthFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, isInteractive bool, b browser.Browser, isCopyToClipboard bool) (string, string, error) {
// AuthFlow initiates an OAuth device or web application flow to acquire a
// token. The provided HTTP client should be a plain client that does not set
// auth or other headers.
func AuthFlow(httpClient *http.Client, oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, isInteractive bool, b browser.Browser, isCopyToClipboard bool) (string, string, error) {
w := IO.ErrOut
cs := IO.ColorScheme()

httpClient := &http.Client{}
debugEnabled, debugValue := utils.IsDebugEnabled()
if debugEnabled {
logTraffic := strings.Contains(debugValue, "api")
httpClient.Transport = verboseLog(IO.ErrOut, logTraffic, IO.ColorEnabled())(httpClient.Transport)
}

minimumScopes := []string{"repo", "read:org", "gist"}
scopes := append(minimumScopes, additionalScopes...)

Expand Down Expand Up @@ -150,28 +140,3 @@ func waitForEnter(r io.Reader) error {
scanner.Scan()
return scanner.Err()
}

func verboseLog(out io.Writer, logTraffic bool, colorize bool) func(http.RoundTripper) http.RoundTripper {
logger := &httpretty.Logger{
Time: true,
TLS: false,
Colors: colorize,
RequestHeader: logTraffic,
RequestBody: logTraffic,
ResponseHeader: logTraffic,
ResponseBody: logTraffic,
Formatters: []httpretty.Formatter{&httpretty.JSONFormatter{}},
MaxResponseBody: 10000,
}
logger.SetOutput(out)
logger.SetBodyFilter(func(h http.Header) (skip bool, err error) {
return !inspectableMIMEType(h.Get("Content-Type")), nil
})
return logger.RoundTripper
}

func inspectableMIMEType(t string) bool {
return strings.HasPrefix(t, "text/") ||
strings.HasPrefix(t, "application/x-www-form-urlencoded") ||
jsonTypeRE.MatchString(t)
}
52 changes: 30 additions & 22 deletions pkg/cmd/auth/login/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ import (
)

type LoginOptions struct {
IO *iostreams.IOStreams
Config func() (gh.Config, error)
HttpClient func() (*http.Client, error)
GitClient *git.Client
Prompter shared.Prompt
Browser browser.Browser
IO *iostreams.IOStreams
Config func() (gh.Config, error)
HttpClient func() (*http.Client, error)
PlainHttpClient func() (*http.Client, error)
GitClient *git.Client
Prompter shared.Prompt
Browser browser.Browser

MainExecutable string

Expand All @@ -43,12 +44,13 @@ type LoginOptions struct {

func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Command {
opts := &LoginOptions{
IO: f.IOStreams,
Config: f.Config,
HttpClient: f.HttpClient,
GitClient: f.GitClient,
Prompter: f.Prompter,
Browser: f.Browser,
IO: f.IOStreams,
Config: f.Config,
HttpClient: f.HttpClient,
PlainHttpClient: f.PlainHttpClient,
GitClient: f.GitClient,
Prompter: f.Prompter,
Browser: f.Browser,
}

var tokenStdin bool
Expand Down Expand Up @@ -190,6 +192,11 @@ func loginRun(opts *LoginOptions) error {
return cmdutil.SilentError
}

plainHTTPClient, err := opts.PlainHttpClient()
if err != nil {
return err
}

httpClient, err := opts.HttpClient()
if err != nil {
return err
Expand All @@ -210,16 +217,17 @@ func loginRun(opts *LoginOptions) error {
}

return shared.Login(&shared.LoginOptions{
IO: opts.IO,
Config: authCfg,
HTTPClient: httpClient,
Hostname: hostname,
Interactive: opts.Interactive,
Web: opts.Web,
Scopes: opts.Scopes,
GitProtocol: opts.GitProtocol,
Prompter: opts.Prompter,
Browser: opts.Browser,
IO: opts.IO,
Config: authCfg,
HTTPClient: httpClient,
PlainHTTPClient: plainHTTPClient,
Hostname: hostname,
Interactive: opts.Interactive,
Web: opts.Web,
Scopes: opts.Scopes,
GitProtocol: opts.GitProtocol,
Prompter: opts.Prompter,
Browser: opts.Browser,
CredentialFlow: &shared.GitCredentialFlow{
Prompter: opts.Prompter,
HelperConfig: &gitcredentials.HelperConfig{
Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/auth/login/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,9 @@ func Test_loginRun_nontty(t *testing.T) {
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
tt.opts.PlainHttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
if tt.httpStubs != nil {
tt.httpStubs(reg)
}
Expand Down Expand Up @@ -775,6 +778,9 @@ func Test_loginRun_Survey(t *testing.T) {
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
tt.opts.PlainHttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
if tt.httpStubs != nil {
tt.httpStubs(reg)
} else {
Expand Down
Loading