Skip to content

Commit d253589

Browse files
babakksbabakks
authored
Merge pull request cli#11922 from cli/babakks/fix-login-through-unix-socket
Fix `auth login` and `auth refresh` to use UNIX socket
2 parents 5eb6549 + 0ce728f commit d253589

12 files changed

Lines changed: 199 additions & 127 deletions

File tree

‎api/http_client.go‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,24 @@ type tokenGetter interface {
1717
}
1818

1919
type HTTPClientOptions struct {
20-
AppVersion string
21-
CacheTTL time.Duration
22-
Config tokenGetter
23-
EnableCache bool
24-
Log io.Writer
25-
LogColorize bool
26-
LogVerboseHTTP bool
20+
AppVersion string
21+
CacheTTL time.Duration
22+
Config tokenGetter
23+
EnableCache bool
24+
Log io.Writer
25+
LogColorize bool
26+
LogVerboseHTTP bool
27+
SkipDefaultHeaders bool
2728
}
2829

2930
func NewHTTPClient(opts HTTPClientOptions) (*http.Client, error) {
3031
// Provide invalid host, and token values so gh.HTTPClient will not automatically resolve them.
3132
// The real host and token are inserted at request time.
3233
clientOpts := ghAPI.ClientOptions{
33-
Host: "none",
34-
AuthToken: "none",
35-
LogIgnoreEnv: true,
34+
Host: "none",
35+
AuthToken: "none",
36+
LogIgnoreEnv: true,
37+
SkipDefaultHeaders: opts.SkipDefaultHeaders,
3638
}
3739

3840
debugEnabled, debugValue := utils.IsDebugEnabled()

‎api/http_client_test.go‎

Lines changed: 59 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,16 @@ import (
1818

1919
func TestNewHTTPClient(t *testing.T) {
2020
type args struct {
21-
config tokenGetter
22-
appVersion string
23-
logVerboseHTTP bool
21+
config tokenGetter
22+
appVersion string
23+
logVerboseHTTP bool
24+
skipDefaultHeaders bool
2425
}
2526
tests := []struct {
2627
name string
2728
args args
2829
host string
29-
wantHeader map[string]string
30+
wantHeader map[string][]string
3031
wantStderr string
3132
}{
3233
{
@@ -37,10 +38,10 @@ func TestNewHTTPClient(t *testing.T) {
3738
logVerboseHTTP: false,
3839
},
3940
host: "github.com",
40-
wantHeader: map[string]string{
41-
"authorization": "token MYTOKEN",
42-
"user-agent": "GitHub CLI v1.2.3",
43-
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
41+
wantHeader: map[string][]string{
42+
"authorization": {"token MYTOKEN"},
43+
"user-agent": {"GitHub CLI v1.2.3"},
44+
"accept": {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
4445
},
4546
wantStderr: "",
4647
},
@@ -51,10 +52,10 @@ func TestNewHTTPClient(t *testing.T) {
5152
appVersion: "v1.2.3",
5253
},
5354
host: "example.com",
54-
wantHeader: map[string]string{
55-
"authorization": "token GHETOKEN",
56-
"user-agent": "GitHub CLI v1.2.3",
57-
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
55+
wantHeader: map[string][]string{
56+
"authorization": {"token GHETOKEN"},
57+
"user-agent": {"GitHub CLI v1.2.3"},
58+
"accept": {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
5859
},
5960
wantStderr: "",
6061
},
@@ -66,10 +67,10 @@ func TestNewHTTPClient(t *testing.T) {
6667
logVerboseHTTP: false,
6768
},
6869
host: "github.com",
69-
wantHeader: map[string]string{
70-
"authorization": "",
71-
"user-agent": "GitHub CLI v1.2.3",
72-
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
70+
wantHeader: map[string][]string{
71+
"authorization": nil, // should not be set
72+
"user-agent": {"GitHub CLI v1.2.3"},
73+
"accept": {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
7374
},
7475
wantStderr: "",
7576
},
@@ -81,10 +82,10 @@ func TestNewHTTPClient(t *testing.T) {
8182
logVerboseHTTP: false,
8283
},
8384
host: "example.com",
84-
wantHeader: map[string]string{
85-
"authorization": "",
86-
"user-agent": "GitHub CLI v1.2.3",
87-
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
85+
wantHeader: map[string][]string{
86+
"authorization": nil, // should not be set
87+
"user-agent": {"GitHub CLI v1.2.3"},
88+
"accept": {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
8889
},
8990
wantStderr: "",
9091
},
@@ -96,10 +97,10 @@ func TestNewHTTPClient(t *testing.T) {
9697
logVerboseHTTP: true,
9798
},
9899
host: "github.com",
99-
wantHeader: map[string]string{
100-
"authorization": "token MYTOKEN",
101-
"user-agent": "GitHub CLI v1.2.3",
102-
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
100+
wantHeader: map[string][]string{
101+
"authorization": {"token MYTOKEN"},
102+
"user-agent": {"GitHub CLI v1.2.3"},
103+
"accept": {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
103104
},
104105
wantStderr: heredoc.Doc(`
105106
* Request at <time>
@@ -115,6 +116,34 @@ func TestNewHTTPClient(t *testing.T) {
115116
< HTTP/1.1 204 No Content
116117
< Date: <time>
117118
119+
* Request took <duration>
120+
`),
121+
},
122+
{
123+
name: "respect skip default headers option",
124+
args: args{
125+
appVersion: "v1.2.3",
126+
logVerboseHTTP: true,
127+
skipDefaultHeaders: true,
128+
},
129+
host: "github.com",
130+
wantHeader: map[string][]string{
131+
"accept": nil,
132+
"authorization": nil,
133+
"content-type": nil,
134+
"user-agent": {"GitHub CLI v1.2.3"},
135+
},
136+
wantStderr: heredoc.Doc(`
137+
* Request at <time>
138+
* Request to http://<host>:<port>
139+
> GET / HTTP/1.1
140+
> Host: github.com
141+
> Time-Zone: <timezone>
142+
> User-Agent: GitHub CLI v1.2.3
143+
144+
< HTTP/1.1 204 No Content
145+
< Date: <time>
146+
118147
* Request took <duration>
119148
`),
120149
},
@@ -131,10 +160,11 @@ func TestNewHTTPClient(t *testing.T) {
131160
t.Run(tt.name, func(t *testing.T) {
132161
ios, _, _, stderr := iostreams.Test()
133162
client, err := NewHTTPClient(HTTPClientOptions{
134-
AppVersion: tt.args.appVersion,
135-
Config: tt.args.config,
136-
Log: ios.ErrOut,
137-
LogVerboseHTTP: tt.args.logVerboseHTTP,
163+
AppVersion: tt.args.appVersion,
164+
Config: tt.args.config,
165+
Log: ios.ErrOut,
166+
LogVerboseHTTP: tt.args.logVerboseHTTP,
167+
SkipDefaultHeaders: tt.args.skipDefaultHeaders,
138168
})
139169
require.NoError(t, err)
140170

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

150180
for name, value := range tt.wantHeader {
151-
assert.Equal(t, value, gotReq.Header.Get(name), name)
181+
assert.Equal(t, value, gotReq.Header.Values(name), name)
152182
}
153183

154184
assert.Equal(t, 204, res.StatusCode)

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ require (
3232
github.com/gorilla/websocket v1.5.3
3333
github.com/hashicorp/go-multierror v1.1.1
3434
github.com/hashicorp/go-version v1.7.0
35-
github.com/henvic/httpretty v0.1.4
3635
github.com/hinshun/vt10x v0.0.0-20220119200601-820417d04eec
3736
github.com/in-toto/attestation v1.1.2
3837
github.com/joho/godotenv v1.5.1
@@ -148,6 +147,7 @@ require (
148147
github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 // indirect
149148
github.com/hashicorp/errwrap v1.1.0 // indirect
150149
github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
150+
github.com/henvic/httpretty v0.1.4 // indirect
151151
github.com/huandu/xstrings v1.5.0 // indirect
152152
github.com/in-toto/in-toto-golang v0.9.0 // indirect
153153
github.com/inconshreveable/mousetrap v1.1.0 // indirect

‎internal/authflow/flow.go‎

Lines changed: 4 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,13 @@ import (
66
"io"
77
"net/http"
88
"net/url"
9-
"regexp"
10-
"strings"
119

1210
"github.com/atotto/clipboard"
1311
"github.com/cli/cli/v2/api"
1412
"github.com/cli/cli/v2/internal/browser"
1513
"github.com/cli/cli/v2/internal/ghinstance"
1614
"github.com/cli/cli/v2/pkg/iostreams"
17-
"github.com/cli/cli/v2/utils"
1815
"github.com/cli/oauth"
19-
"github.com/henvic/httpretty"
2016

2117
ghauth "github.com/cli/go-gh/v2/pkg/auth"
2218
)
@@ -26,21 +22,15 @@ var (
2622
oauthClientID = "178c6fc778ccc68e1d6a"
2723
// This value is safe to be embedded in version control
2824
oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b"
29-
30-
jsonTypeRE = regexp.MustCompile(`[/+]json($|;)`)
3125
)
3226

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

37-
httpClient := &http.Client{}
38-
debugEnabled, debugValue := utils.IsDebugEnabled()
39-
if debugEnabled {
40-
logTraffic := strings.Contains(debugValue, "api")
41-
httpClient.Transport = verboseLog(IO.ErrOut, logTraffic, IO.ColorEnabled())(httpClient.Transport)
42-
}
43-
4434
minimumScopes := []string{"repo", "read:org", "gist"}
4535
scopes := append(minimumScopes, additionalScopes...)
4636

@@ -150,28 +140,3 @@ func waitForEnter(r io.Reader) error {
150140
scanner.Scan()
151141
return scanner.Err()
152142
}
153-
154-
func verboseLog(out io.Writer, logTraffic bool, colorize bool) func(http.RoundTripper) http.RoundTripper {
155-
logger := &httpretty.Logger{
156-
Time: true,
157-
TLS: false,
158-
Colors: colorize,
159-
RequestHeader: logTraffic,
160-
RequestBody: logTraffic,
161-
ResponseHeader: logTraffic,
162-
ResponseBody: logTraffic,
163-
Formatters: []httpretty.Formatter{&httpretty.JSONFormatter{}},
164-
MaxResponseBody: 10000,
165-
}
166-
logger.SetOutput(out)
167-
logger.SetBodyFilter(func(h http.Header) (skip bool, err error) {
168-
return !inspectableMIMEType(h.Get("Content-Type")), nil
169-
})
170-
return logger.RoundTripper
171-
}
172-
173-
func inspectableMIMEType(t string) bool {
174-
return strings.HasPrefix(t, "text/") ||
175-
strings.HasPrefix(t, "application/x-www-form-urlencoded") ||
176-
jsonTypeRE.MatchString(t)
177-
}

‎pkg/cmd/auth/login/login.go‎

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,13 @@ import (
2020
)
2121

2222
type LoginOptions struct {
23-
IO *iostreams.IOStreams
24-
Config func() (gh.Config, error)
25-
HttpClient func() (*http.Client, error)
26-
GitClient *git.Client
27-
Prompter shared.Prompt
28-
Browser browser.Browser
23+
IO *iostreams.IOStreams
24+
Config func() (gh.Config, error)
25+
HttpClient func() (*http.Client, error)
26+
PlainHttpClient func() (*http.Client, error)
27+
GitClient *git.Client
28+
Prompter shared.Prompt
29+
Browser browser.Browser
2930

3031
MainExecutable string
3132

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

4445
func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Command {
4546
opts := &LoginOptions{
46-
IO: f.IOStreams,
47-
Config: f.Config,
48-
HttpClient: f.HttpClient,
49-
GitClient: f.GitClient,
50-
Prompter: f.Prompter,
51-
Browser: f.Browser,
47+
IO: f.IOStreams,
48+
Config: f.Config,
49+
HttpClient: f.HttpClient,
50+
PlainHttpClient: f.PlainHttpClient,
51+
GitClient: f.GitClient,
52+
Prompter: f.Prompter,
53+
Browser: f.Browser,
5254
}
5355

5456
var tokenStdin bool
@@ -190,6 +192,11 @@ func loginRun(opts *LoginOptions) error {
190192
return cmdutil.SilentError
191193
}
192194

195+
plainHTTPClient, err := opts.PlainHttpClient()
196+
if err != nil {
197+
return err
198+
}
199+
193200
httpClient, err := opts.HttpClient()
194201
if err != nil {
195202
return err
@@ -210,16 +217,17 @@ func loginRun(opts *LoginOptions) error {
210217
}
211218

212219
return shared.Login(&shared.LoginOptions{
213-
IO: opts.IO,
214-
Config: authCfg,
215-
HTTPClient: httpClient,
216-
Hostname: hostname,
217-
Interactive: opts.Interactive,
218-
Web: opts.Web,
219-
Scopes: opts.Scopes,
220-
GitProtocol: opts.GitProtocol,
221-
Prompter: opts.Prompter,
222-
Browser: opts.Browser,
220+
IO: opts.IO,
221+
Config: authCfg,
222+
HTTPClient: httpClient,
223+
PlainHTTPClient: plainHTTPClient,
224+
Hostname: hostname,
225+
Interactive: opts.Interactive,
226+
Web: opts.Web,
227+
Scopes: opts.Scopes,
228+
GitProtocol: opts.GitProtocol,
229+
Prompter: opts.Prompter,
230+
Browser: opts.Browser,
223231
CredentialFlow: &shared.GitCredentialFlow{
224232
Prompter: opts.Prompter,
225233
HelperConfig: &gitcredentials.HelperConfig{

‎pkg/cmd/auth/login/login_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,9 @@ func Test_loginRun_nontty(t *testing.T) {
483483
tt.opts.HttpClient = func() (*http.Client, error) {
484484
return &http.Client{Transport: reg}, nil
485485
}
486+
tt.opts.PlainHttpClient = func() (*http.Client, error) {
487+
return &http.Client{Transport: reg}, nil
488+
}
486489
if tt.httpStubs != nil {
487490
tt.httpStubs(reg)
488491
}
@@ -775,6 +778,9 @@ func Test_loginRun_Survey(t *testing.T) {
775778
tt.opts.HttpClient = func() (*http.Client, error) {
776779
return &http.Client{Transport: reg}, nil
777780
}
781+
tt.opts.PlainHttpClient = func() (*http.Client, error) {
782+
return &http.Client{Transport: reg}, nil
783+
}
778784
if tt.httpStubs != nil {
779785
tt.httpStubs(reg)
780786
} else {

0 commit comments

Comments
 (0)