STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add ability to store tokens in encrypted storage (#7043)

Browse source
This commit is contained in:
Sam Coe 2023-02-28 11:04:53 +11:00 • committed by GitHub
parent a33e12a21d
commit df83dc2d58
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 386 additions and 118 deletions
Download patch file Download diff file Expand all files Collapse all files

6
go.mod
View file

@ -9,7 +9,7 @@ require (
github.com/cenkalti/backoff/v4 v4.2.0
github.com/charmbracelet/glamour v0.5.1-0.20220727184942-e70ff2d969da
github.com/charmbracelet/lipgloss v0.5.0
github.com/cli/go-gh v1.0.0
github.com/cli/go-gh v1.2.0
github.com/cli/oauth v1.0.1
github.com/cli/safeexec v1.0.1
github.com/cpuguy83/go-md2man/v2 v2.0.2
@ -35,6 +35,7 @@ require (
github.com/spf13/cobra v1.6.1
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.7.5
github.com/zalando/go-keyring v0.2.2
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519
golang.org/x/sync v0.1.0
golang.org/x/term v0.5.0
@ -46,13 +47,16 @@ require (
require (
github.com/alecthomas/chroma v0.10.0 // indirect
github.com/alessio/shellescape v1.4.1 // indirect
github.com/aymerick/douceur v0.2.0 // indirect
github.com/cli/browser v1.1.0 // indirect
github.com/cli/shurcooL-graphql v0.0.2 // indirect
github.com/danieljoos/wincred v1.1.2 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/dlclark/regexp2 v1.4.0 // indirect
github.com/fatih/color v1.7.0 // indirect
github.com/gdamore/encoding v1.0.0 // indirect
github.com/godbus/dbus/v5 v5.1.0 // indirect
github.com/golang/protobuf v1.5.2 // indirect
github.com/gorilla/css v1.0.0 // indirect
github.com/hashicorp/errwrap v1.0.0 // indirect

13
go.sum
View file

@ -41,6 +41,8 @@ github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2 h1:+vx7roKuyA63n
github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2/go.mod h1:HBCaDeC1lPdgDeDbhX8XFpy1jqjK0IBG8W5K+xYqA0w=
github.com/alecthomas/chroma v0.10.0 h1:7XDcGkCQopCNKjZHfYrNLraA+M7e0fMiJ/Mfikbfjek=
github.com/alecthomas/chroma v0.10.0/go.mod h1:jtJATyUxlIORhUOFNA9NZDWGAQ8wpxQQqNSB4rjA/1s=
github.com/alessio/shellescape v1.4.1 h1:V7yhSDDn8LP4lc4jS8pFkt0zCnzVJlG5JXy9BVKJUX0=
github.com/alessio/shellescape v1.4.1/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30=
github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk=
github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4=
github.com/briandowns/spinner v1.18.1 h1:yhQmQtM1zsqFsouh09Bk/jCjd50pC3EOGsh28gLVvwY=
@ -60,8 +62,8 @@ github.com/cli/browser v1.1.0 h1:xOZBfkfY9L9vMBgqb1YwRirGu6QFaQ5dP/vXt5ENSOY=
github.com/cli/browser v1.1.0/go.mod h1:HKMQAt9t12kov91Mn7RfZxyJQQgWgyS/3SZswlZ5iTI=
github.com/cli/crypto v0.0.0-20210929142629-6be313f59b03 h1:3f4uHLfWx4/WlnMPXGai03eoWAI+oGHJwr+5OXfxCr8=
github.com/cli/crypto v0.0.0-20210929142629-6be313f59b03/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
github.com/cli/go-gh v1.0.0 h1:zE1YUAUYqGXNZuICEBeOkIMJ5F50BS0ftvtoWGlsEFI=
github.com/cli/go-gh v1.0.0/go.mod h1:bqxLdCoTZ73BuiPEJx4olcO/XKhVZaFDchFagYRBweE=
github.com/cli/go-gh v1.2.0 h1:LjcdjdQtCWXVg3YTNEuwrHFY/amJzBXy5QjMxnWB/0Q=
github.com/cli/go-gh v1.2.0/go.mod h1:Jxk8X+TCO4Ui/GarwY9tByWm/8zp4jJktzVZNlTW5VM=
github.com/cli/oauth v1.0.1 h1:pXnTFl/qUegXHK531Dv0LNjW4mLx626eS42gnzfXJPA=
github.com/cli/oauth v1.0.1/go.mod h1:qd/FX8ZBD6n1sVNQO3aIdRxeu5LGw9WhKnYhIIoC2A4=
github.com/cli/safeexec v1.0.0/go.mod h1:Z/D4tTN8Vs5gXYHDCbaM1S/anmEDnJb1iW0+EJ5zx3Q=
@ -76,6 +78,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46t
github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY=
github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
github.com/danieljoos/wincred v1.1.2 h1:QLdCxFs1/Yl4zduvBdcHB8goaYk9RARS2SgLLRuAyr0=
github.com/danieljoos/wincred v1.1.2/go.mod h1:GijpziifJoIBfYh+S7BbkdUTU4LfM+QnGqR5Vl2tAx0=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@ -96,6 +100,8 @@ github.com/gdamore/tcell/v2 v2.5.4/go.mod h1:dZgRy5v4iMobMEcWNYBtREnDZAT9DYmfqIk
github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk=
github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
github.com/golang/groupcache v0.0.0-20191227052852-215e87163ea7/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
@ -265,6 +271,8 @@ github.com/yuin/goldmark v1.4.13 h1:fVcFKWvrslecOb/tg+Cc05dkeYx540o0FuFt3nUVDoE=
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
github.com/yuin/goldmark-emoji v1.0.1 h1:ctuWEyzGBwiucEqxzwe0SOYDXPAucOrE9NQC18Wa1os=
github.com/yuin/goldmark-emoji v1.0.1/go.mod h1:2w1E6FEWLcDQkoTE+7HU6QF1F6SLlNGjRIBbIZQFqkQ=
github.com/zalando/go-keyring v0.2.2 h1:f0xmpYiSrHtSNAVgwip93Cg8tuF45HJM6rHq/A5RI/4=
github.com/zalando/go-keyring v0.2.2/go.mod h1:sI3evg9Wvpw3+n4SqplGSJUMwtDeROfD4nsFz4z9PG0=
go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU=
go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8=
go.opencensus.io v0.22.2/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw=
@ -386,6 +394,7 @@ golang.org/x/sys v0.0.0-20210319071255-635bc2c9138d/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210819135213-f52c844e1c1c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210831042530-f4d43177bf5e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

33
internal/config/config.go
View file

@ -6,6 +6,7 @@ import (
ghAuth "github.com/cli/go-gh/pkg/auth"
ghConfig "github.com/cli/go-gh/pkg/config"
"github.com/zalando/go-keyring"
)
const (
@ -144,7 +145,15 @@ func (c *AuthConfig) Token(hostname string) (string, string) {
if c.tokenOverride != nil {
return c.tokenOverride(hostname)
}
return ghAuth.TokenForHost(hostname)
token, source := ghAuth.TokenFromEnvOrConfig(hostname)
if token == "" {
var err error
token, err = c.TokenFromKeyring(hostname)
if err == nil {
source = "keyring"
}
}
return token, source
}
// SetToken will override any token resolution and return the given
@ -155,6 +164,12 @@ func (c *AuthConfig) SetToken(token, source string) {
}
}
// TokenFromKeyring will retrieve the auth token for the given hostname,
// only searching in encrypted storage.
func (c *AuthConfig) TokenFromKeyring(hostname string) (string, error) {
return keyring.Get(keyringServiceName(hostname), "")
}
// User will retrieve the username for the logged in user at the given hostname.
func (c *AuthConfig) User(hostname string) (string, error) {
return c.cfg.Get([]string{hosts, hostname, "user"})
@ -193,8 +208,15 @@ func (c *AuthConfig) DefaultHost() (string, string) {
// Login will set user, git protocol, and auth token for the given hostname.
// If the encrypt option is specified it will first try to store the auth token
// in encrypted storage and will fall back to the plain text config file.
func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, encrypt bool) error {
if token != "" {
func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secureStorage bool) error {
var setErr error
if secureStorage {
if setErr = keyring.Set(keyringServiceName(hostname), "", token); setErr == nil {
// Clean up the previous oauth_token from the config file.
_ = c.cfg.Remove([]string{hosts, hostname, oauthToken})
}
}
if !secureStorage || setErr != nil {
c.cfg.Set([]string{hosts, hostname, oauthToken}, token)
}
if username != "" {
@ -213,9 +235,14 @@ func (c *AuthConfig) Logout(hostname string) error {
return nil
}
_ = c.cfg.Remove([]string{hosts, hostname})
_ = keyring.Delete(keyringServiceName(hostname), "")
return ghConfig.Write(c.cfg)
}
func keyringServiceName(hostname string) string {
return "gh:" + hostname
}
type AliasConfig struct {
cfg *ghConfig.Config
}

46
pkg/cmd/auth/login/login.go
View file

@ -30,11 +30,12 @@ type LoginOptions struct {
Interactive bool
Hostname string
Scopes []string
Token string
Web bool
GitProtocol string
Hostname string
Scopes []string
Token string
Web bool
GitProtocol string
SecureStorage bool
}
func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Command {
@ -123,6 +124,7 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
cmd.Flags().BoolVar(&tokenStdin, "with-token", false, "Read token from standard input")
cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open a browser to authenticate")
cmdutil.StringEnumFlag(cmd, &opts.GitProtocol, "git-protocol", "p", "", []string{"ssh", "https"}, "The protocol to use for git operations")
cmd.Flags().BoolVarP(&opts.SecureStorage, "secure-storage", "", false, "Save authentication credentials in secure credential store")
return cmd
}
@ -134,6 +136,11 @@ func loginRun(opts *LoginOptions) error {
}
authCfg := cfg.Authentication()
if opts.SecureStorage {
cs := opts.IO.ColorScheme()
fmt.Fprintf(opts.IO.ErrOut, "%s Using secure storage could break installed extensions\n", cs.WarningIcon())
}
hostname := opts.Hostname
if opts.Interactive && hostname == "" {
var err error
@ -158,8 +165,8 @@ func loginRun(opts *LoginOptions) error {
if err := shared.HasMinimumScopes(httpClient, hostname, opts.Token); err != nil {
return fmt.Errorf("error validating token: %w", err)
}
return authCfg.Login(hostname, "", opts.Token, opts.GitProtocol, false)
// Adding a user key ensures that a nonempty host section gets written to the config file.
return authCfg.Login(hostname, "x-access-token", opts.Token, opts.GitProtocol, opts.SecureStorage)
}
existingToken, _ := authCfg.Token(hostname)
@ -176,18 +183,19 @@ 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,
Executable: opts.MainExecutable,
GitProtocol: opts.GitProtocol,
Prompter: opts.Prompter,
GitClient: opts.GitClient,
Browser: opts.Browser,
IO: opts.IO,
Config: authCfg,
HTTPClient: httpClient,
Hostname: hostname,
Interactive: opts.Interactive,
Web: opts.Web,
Scopes: opts.Scopes,
Executable: opts.MainExecutable,
GitProtocol: opts.GitProtocol,
Prompter: opts.Prompter,
GitClient: opts.GitClient,
Browser: opts.Browser,
SecureStorage: opts.SecureStorage,
})
}

163
pkg/cmd/auth/login/login_test.go
View file

@ -17,6 +17,7 @@ import (
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/zalando/go-keyring"
)
func stubHomeDir(t *testing.T, dir string) {
@ -172,6 +173,23 @@ func Test_NewCmdLogin(t *testing.T) {
Interactive: true,
},
},
{
name: "tty secure-storage",
stdinTTY: true,
cli: "--secure-storage",
wants: LoginOptions{
Interactive: true,
SecureStorage: true,
},
},
{
name: "nontty secure-storage",
cli: "--secure-storage",
wants: LoginOptions{
Hostname: "github.com",
SecureStorage: true,
},
},
}
for _, tt := range tests {
@ -223,13 +241,14 @@ func Test_NewCmdLogin(t *testing.T) {
func Test_loginRun_nontty(t *testing.T) {
tests := []struct {
name string
opts *LoginOptions
httpStubs func(*httpmock.Registry)
cfgStubs func(*config.ConfigMock)
wantHosts string
wantErr string
wantStderr string
name string
opts *LoginOptions
httpStubs func(*httpmock.Registry)
cfgStubs func(*config.ConfigMock)
wantHosts string
wantErr string
wantStderr string
wantSecureToken string
}{
{
name: "with token",
@ -240,7 +259,7 @@ func Test_loginRun_nontty(t *testing.T) {
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org"))
},
wantHosts: "github.com:\n oauth_token: abc123\n",
wantHosts: "github.com:\n oauth_token: abc123\n user: x-access-token\n",
},
{
name: "with token and https git-protocol",
@ -252,7 +271,7 @@ func Test_loginRun_nontty(t *testing.T) {
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org"))
},
wantHosts: "github.com:\n oauth_token: abc123\n git_protocol: https\n",
wantHosts: "github.com:\n oauth_token: abc123\n user: x-access-token\n git_protocol: https\n",
},
{
name: "with token and non-default host",
@ -263,7 +282,7 @@ func Test_loginRun_nontty(t *testing.T) {
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.ScopesResponder("repo,read:org"))
},
wantHosts: "albert.wesker:\n oauth_token: abc123\n",
wantHosts: "albert.wesker:\n oauth_token: abc123\n user: x-access-token\n",
},
{
name: "missing repo scope",
@ -296,7 +315,7 @@ func Test_loginRun_nontty(t *testing.T) {
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,admin:org"))
},
wantHosts: "github.com:\n oauth_token: abc456\n",
wantHosts: "github.com:\n oauth_token: abc456\n user: x-access-token\n",
},
{
name: "github.com token from environment",
@ -336,15 +355,30 @@ func Test_loginRun_nontty(t *testing.T) {
To have GitHub CLI store credentials instead, first clear the value from the environment.
`),
},
{
name: "with token and secure storage",
opts: &LoginOptions{
Hostname: "github.com",
Token: "abc123",
SecureStorage: true,
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org"))
},
wantHosts: "github.com:\n user: x-access-token\n",
wantSecureToken: "abc123",
wantStderr: "! Using secure storage could break installed extensions\n",
},
}
for _, tt := range tests {
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdinTTY(false)
ios.SetStdoutTTY(false)
tt.opts.IO = ios
t.Run(tt.name, func(t *testing.T) {
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdinTTY(false)
ios.SetStdoutTTY(false)
tt.opts.IO = ios
keyring.MockInit()
readConfigs := config.StubWriteConfig(t)
cfg := config.NewBlankConfig()
if tt.cfgStubs != nil {
@ -355,6 +389,7 @@ func Test_loginRun_nontty(t *testing.T) {
}
reg := &httpmock.Registry{}
defer reg.Verify(t)
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
@ -375,11 +410,12 @@ func Test_loginRun_nontty(t *testing.T) {
mainBuf := bytes.Buffer{}
hostsBuf := bytes.Buffer{}
readConfigs(&mainBuf, &hostsBuf)
secureToken, _ := cfg.Authentication().TokenFromKeyring(tt.opts.Hostname)
assert.Equal(t, "", stdout.String())
assert.Equal(t, tt.wantStderr, stderr.String())
assert.Equal(t, tt.wantHosts, hostsBuf.String())
reg.Verify(t)
assert.Equal(t, tt.wantSecureToken, secureToken)
})
}
}
@ -388,14 +424,15 @@ func Test_loginRun_Survey(t *testing.T) {
stubHomeDir(t, t.TempDir())
tests := []struct {
name string
opts *LoginOptions
httpStubs func(*httpmock.Registry)
prompterStubs func(*prompter.PrompterMock)
runStubs func(*run.CommandStubber)
wantHosts string
wantErrOut *regexp.Regexp
cfgStubs func(*config.ConfigMock)
name string
opts *LoginOptions
httpStubs func(*httpmock.Registry)
prompterStubs func(*prompter.PrompterMock)
runStubs func(*run.CommandStubber)
cfgStubs func(*config.ConfigMock)
wantHosts string
wantErrOut *regexp.Regexp
wantSecureToken string
}{
{
name: "already authenticated",
@ -553,32 +590,62 @@ func Test_loginRun_Survey(t *testing.T) {
},
wantErrOut: regexp.MustCompile("Tip: you can generate a Personal Access Token here https://github.com/settings/tokens"),
},
// TODO how to test browser auth?
{
name: "secure storage",
opts: &LoginOptions{
Hostname: "github.com",
Interactive: true,
SecureStorage: true,
},
prompterStubs: func(pm *prompter.PrompterMock) {
pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) {
switch prompt {
case "What is your preferred protocol for Git operations?":
return prompter.IndexFor(opts, "HTTPS")
case "How would you like to authenticate GitHub CLI?":
return prompter.IndexFor(opts, "Paste an authentication token")
}
return -1, prompter.NoSuchPromptErr(prompt)
}
},
runStubs: func(rs *run.CommandStubber) {
rs.Register(`git config credential\.https:/`, 1, "")
rs.Register(`git config credential\.helper`, 1, "")
},
wantHosts: heredoc.Doc(`
github.com:
user: jillv
git_protocol: https
`),
wantErrOut: regexp.MustCompile("! Using secure storage could break installed extensions"),
wantSecureToken: "def456",
},
}
for _, tt := range tests {
if tt.opts == nil {
tt.opts = &LoginOptions{}
}
ios, _, _, stderr := iostreams.Test()
ios.SetStdinTTY(true)
ios.SetStderrTTY(true)
ios.SetStdoutTTY(true)
tt.opts.IO = ios
readConfigs := config.StubWriteConfig(t)
cfg := config.NewBlankConfig()
if tt.cfgStubs != nil {
tt.cfgStubs(cfg)
}
tt.opts.Config = func() (config.Config, error) {
return cfg, nil
}
t.Run(tt.name, func(t *testing.T) {
if tt.opts == nil {
tt.opts = &LoginOptions{}
}
ios, _, _, stderr := iostreams.Test()
ios.SetStdinTTY(true)
ios.SetStderrTTY(true)
ios.SetStdoutTTY(true)
tt.opts.IO = ios
keyring.MockInit()
readConfigs := config.StubWriteConfig(t)
cfg := config.NewBlankConfig()
if tt.cfgStubs != nil {
tt.cfgStubs(cfg)
}
tt.opts.Config = func() (config.Config, error) {
return cfg, nil
}
reg := &httpmock.Registry{}
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
@ -620,8 +687,10 @@ func Test_loginRun_Survey(t *testing.T) {
mainBuf := bytes.Buffer{}
hostsBuf := bytes.Buffer{}
readConfigs(&mainBuf, &hostsBuf)
secureToken, _ := cfg.Authentication().TokenFromKeyring(tt.opts.Hostname)
assert.Equal(t, tt.wantHosts, hostsBuf.String())
assert.Equal(t, tt.wantSecureToken, secureToken)
if tt.wantErrOut == nil {
assert.Equal(t, "", stderr.String())
} else {

57
pkg/cmd/auth/logout/logout_test.go
View file

@ -2,6 +2,7 @@ package logout
import (
"bytes"
"fmt"
"net/http"
"regexp"
"testing"
@ -13,6 +14,7 @@ import (
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/zalando/go-keyring"
)
func Test_NewCmdLogout(t *testing.T) {
@ -96,6 +98,7 @@ func Test_logoutRun_tty(t *testing.T) {
opts *LogoutOptions
prompterStubs func(*prompter.PrompterMock)
cfgHosts []string
secureStorage bool
wantHosts string
wantErrOut *regexp.Regexp
wantErr string
@ -133,14 +136,31 @@ func Test_logoutRun_tty(t *testing.T) {
wantHosts: "github.com:\n oauth_token: abc123\n",
wantErrOut: regexp.MustCompile(`Logged out of cheryl.mason account 'cybilb'`),
},
{
name: "secure storage",
secureStorage: true,
opts: &LogoutOptions{
Hostname: "github.com",
},
cfgHosts: []string{"github.com"},
wantHosts: "{}\n",
wantErrOut: regexp.MustCompile(`Logged out of github.com account 'cybilb'`),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
keyring.MockInit()
readConfigs := config.StubWriteConfig(t)
cfg := config.NewFromString("")
for _, hostname := range tt.cfgHosts {
cfg.Set(hostname, "oauth_token", "abc123")
if tt.secureStorage {
cfg.Set(hostname, "user", "monalisa")
_ = keyring.Set(fmt.Sprintf("gh:%s", hostname), "", "abc123")
cfg.Authentication().SetToken("abc123", "keyring")
} else {
cfg.Set(hostname, "oauth_token", "abc123")
}
}
tt.opts.Config = func() (config.Config, error) {
return cfg, nil
@ -183,8 +203,10 @@ func Test_logoutRun_tty(t *testing.T) {
mainBuf := bytes.Buffer{}
hostsBuf := bytes.Buffer{}
readConfigs(&mainBuf, &hostsBuf)
secureToken, _ := cfg.Authentication().TokenFromKeyring(tt.opts.Hostname)
assert.Equal(t, tt.wantHosts, hostsBuf.String())
assert.Equal(t, "", secureToken)
reg.Verify(t)
})
}
@ -192,12 +214,13 @@ func Test_logoutRun_tty(t *testing.T) {
func Test_logoutRun_nontty(t *testing.T) {
tests := []struct {
name string
opts *LogoutOptions
cfgHosts []string
wantHosts string
wantErr string
ghtoken string
name string
opts *LogoutOptions
cfgHosts []string
secureStorage bool
ghtoken string
wantHosts string
wantErr string
}{
{
name: "hostname, one host",
@ -222,14 +245,30 @@ func Test_logoutRun_nontty(t *testing.T) {
},
wantErr: `not logged in to any hosts`,
},
{
name: "secure storage",
secureStorage: true,
opts: &LogoutOptions{
Hostname: "harry.mason",
},
cfgHosts: []string{"harry.mason"},
wantHosts: "{}\n",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
keyring.MockInit()
readConfigs := config.StubWriteConfig(t)
cfg := config.NewFromString("")
for _, hostname := range tt.cfgHosts {
cfg.Set(hostname, "oauth_token", "abc123")
if tt.secureStorage {
cfg.Set(hostname, "user", "monalisa")
_ = keyring.Set(fmt.Sprintf("gh:%s", hostname), "", "abc123")
cfg.Authentication().SetToken("abc123", "keyring")
} else {
cfg.Set(hostname, "oauth_token", "abc123")
}
}
tt.opts.Config = func() (config.Config, error) {
return cfg, nil
@ -257,8 +296,10 @@ func Test_logoutRun_nontty(t *testing.T) {
mainBuf := bytes.Buffer{}
hostsBuf := bytes.Buffer{}
readConfigs(&mainBuf, &hostsBuf)
secureToken, _ := cfg.Authentication().TokenFromKeyring(tt.opts.Hostname)
assert.Equal(t, tt.wantHosts, hostsBuf.String())
assert.Equal(t, "", secureToken)
reg.Verify(t)
})
}

16
pkg/cmd/auth/refresh/refresh.go
View file

@ -26,21 +26,26 @@ type RefreshOptions struct {
Hostname string
Scopes []string
AuthFlow func(*config.AuthConfig, *iostreams.IOStreams, string, []string, bool) error
AuthFlow func(*config.AuthConfig, *iostreams.IOStreams, string, []string, bool, bool) error
Interactive bool
Interactive bool
SecureStorage bool
}
func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.Command {
opts := &RefreshOptions{
IO: f.IOStreams,
Config: f.Config,
AuthFlow: func(authCfg *config.AuthConfig, io *iostreams.IOStreams, hostname string, scopes []string, interactive bool) error {
AuthFlow: func(authCfg *config.AuthConfig, io *iostreams.IOStreams, hostname string, scopes []string, interactive, secureStorage bool) error {
if secureStorage {
cs := io.ColorScheme()
fmt.Fprintf(io.ErrOut, "%s Using secure storage could break installed extensions", cs.WarningIcon())
}
token, username, err := authflow.AuthFlow(hostname, io, "", scopes, interactive, f.Browser)
if err != nil {
return err
}
return authCfg.Login(hostname, username, token, "", false)
return authCfg.Login(hostname, username, token, "", secureStorage)
},
HttpClient: &http.Client{},
GitClient: f.GitClient,
@ -80,6 +85,7 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.
cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The GitHub host to use for authentication")
cmd.Flags().StringSliceVarP(&opts.Scopes, "scopes", "s", nil, "Additional authentication scopes for gh to have")
cmd.Flags().BoolVarP(&opts.SecureStorage, "secure-storage", "", false, "Save authentication credentials in secure credential store")
return cmd
}
@ -152,7 +158,7 @@ func refreshRun(opts *RefreshOptions) error {
additionalScopes = append(additionalScopes, credentialFlow.Scopes()...)
}
if err := opts.AuthFlow(authCfg, opts.IO, hostname, append(opts.Scopes, additionalScopes...), opts.Interactive); err != nil {
if err := opts.AuthFlow(authCfg, opts.IO, hostname, append(opts.Scopes, additionalScopes...), opts.Interactive, opts.SecureStorage); err != nil {
return err
}

36
pkg/cmd/auth/refresh/refresh_test.go
View file

@ -85,6 +85,14 @@ func Test_NewCmdRefresh(t *testing.T) {
Scopes: []string{"repo:invite", "read:public_key"},
},
},
{
name: "secure storage",
tty: true,
cli: "--secure-storage",
wants: RefreshOptions{
SecureStorage: true,
},
},
}
for _, tt := range tests {
@ -126,8 +134,10 @@ func Test_NewCmdRefresh(t *testing.T) {
}
type authArgs struct {
hostname string
scopes []string
hostname string
scopes []string
interactive bool
secureStorage bool
}
func Test_refreshRun(t *testing.T) {
@ -230,17 +240,33 @@ func Test_refreshRun(t *testing.T) {
scopes: []string{"repo:invite", "public_key:read", "delete_repo", "codespace"},
},
},
{
name: "secure storage",
cfgHosts: []string{
"obed.morton",
},
opts: &RefreshOptions{
Hostname: "obed.morton",
SecureStorage: true,
},
wantAuthArgs: authArgs{
hostname: "obed.morton",
scopes: nil,
secureStorage: true,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
aa := authArgs{}
tt.opts.AuthFlow = func(_ *config.AuthConfig, _ *iostreams.IOStreams, hostname string, scopes []string, interactive bool) error {
tt.opts.AuthFlow = func(_ *config.AuthConfig, _ *iostreams.IOStreams, hostname string, scopes []string, interactive, secureStorage bool) error {
aa.hostname = hostname
aa.scopes = scopes
aa.interactive = interactive
aa.secureStorage = secureStorage
return nil
}
_ = config.StubWriteConfig(t)
cfg := config.NewFromString("")
for _, hostname := range tt.cfgHosts {
cfg.Set(hostname, "oauth_token", "abc123")
@ -291,6 +317,8 @@ func Test_refreshRun(t *testing.T) {
assert.Equal(t, tt.wantAuthArgs.hostname, aa.hostname)
assert.Equal(t, tt.wantAuthArgs.scopes, aa.scopes)
assert.Equal(t, tt.wantAuthArgs.interactive, aa.interactive)
assert.Equal(t, tt.wantAuthArgs.secureStorage, aa.secureStorage)
})
}
}

27
pkg/cmd/auth/shared/login_flow.go
View file

@ -24,18 +24,19 @@ type iconfig interface {
}
type LoginOptions struct {
IO *iostreams.IOStreams
Config iconfig
HTTPClient *http.Client
GitClient *git.Client
Hostname string
Interactive bool
Web bool
Scopes []string
Executable string
GitProtocol string
Prompter Prompt
Browser browser.Browser
IO *iostreams.IOStreams
Config iconfig
HTTPClient *http.Client
GitClient *git.Client
Hostname string
Interactive bool
Web bool
Scopes []string
Executable string
GitProtocol string
Prompter Prompt
Browser browser.Browser
SecureStorage bool
sshContext ssh.Context
}
@ -186,7 +187,7 @@ func Login(opts *LoginOptions) error {
fmt.Fprintf(opts.IO.ErrOut, "%s Configured git protocol\n", cs.SuccessIcon())
}
if err := cfg.Login(hostname, username, authToken, gitProtocol, false); err != nil {
if err := cfg.Login(hostname, username, authToken, gitProtocol, opts.SecureStorage); err != nil {
return err
}

8
pkg/cmd/auth/shared/writeable.go
View file

@ -1,14 +1,12 @@
package shared
import (
"github.com/cli/cli/v2/internal/config"
)
"strings"
const (
oauthToken = "oauth_token"
"github.com/cli/cli/v2/internal/config"
)
func AuthTokenWriteable(authCfg *config.AuthConfig, hostname string) (string, bool) {
token, src := authCfg.Token(hostname)
return src, (token == "" || src == oauthToken)
return src, (token == "" || !strings.HasSuffix(src, "_TOKEN"))
}

13
pkg/cmd/auth/token/token.go
View file

@ -14,7 +14,8 @@ type TokenOptions struct {
IO *iostreams.IOStreams
Config func() (config.Config, error)
Hostname string
Hostname string
SecureStorage bool
}
func NewCmdToken(f *cmdutil.Factory, runF func(*TokenOptions) error) *cobra.Command {
@ -37,6 +38,8 @@ func NewCmdToken(f *cmdutil.Factory, runF func(*TokenOptions) error) *cobra.Comm
}
cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance authenticated with")
cmd.Flags().BoolVarP(&opts.SecureStorage, "secure-storage", "", false, "Search only secure credential store for authentication token")
_ = cmd.Flags().MarkHidden("secure-storeage")
return cmd
}
@ -51,8 +54,14 @@ func tokenRun(opts *TokenOptions) error {
if err != nil {
return err
}
authCfg := cfg.Authentication()
val, _ := cfg.AuthToken(hostname)
var val string
if opts.SecureStorage {
val, _ = authCfg.TokenFromKeyring(hostname)
} else {
val, _ = authCfg.Token(hostname)
}
if val == "" {
return fmt.Errorf("no oauth token")
}

77
pkg/cmd/auth/token/token_test.go
View file

@ -9,6 +9,7 @@ import (
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/zalando/go-keyring"
)
func TestNewCmdToken(t *testing.T) {
@ -34,6 +35,11 @@ func TestNewCmdToken(t *testing.T) {
input: "-h github.mycompany.com",
output: TokenOptions{Hostname: "github.mycompany.com"},
},
{
name: "with secure-storage",
input: "--secure-storage",
output: TokenOptions{SecureStorage: true},
},
}
for _, tt := range tests {
@ -71,11 +77,12 @@ func TestNewCmdToken(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, tt.output.Hostname, cmdOpts.Hostname)
assert.Equal(t, tt.output.SecureStorage, cmdOpts.SecureStorage)
})
}
}
func Test_tokenRun(t *testing.T) {
func TestTokenRun(t *testing.T) {
tests := []struct {
name string
opts TokenOptions
@ -121,17 +128,77 @@ func Test_tokenRun(t *testing.T) {
}
for _, tt := range tests {
ios, _, stdout, _ := iostreams.Test()
tt.opts.IO = ios
t.Run(tt.name, func(t *testing.T) {
ios, _, stdout, _ := iostreams.Test()
tt.opts.IO = ios
err := tokenRun(&tt.opts)
if tt.wantErr {
assert.Error(t, err)
assert.EqualError(t, err, tt.wantErrMsg)
return
}
assert.NoError(t, err)
assert.Equal(t, tt.wantStdout, stdout.String())
})
}
}
func TestTokenRunSecureStorage(t *testing.T) {
tests := []struct {
name string
opts TokenOptions
wantStdout string
wantErr bool
wantErrMsg string
}{
{
name: "token",
opts: TokenOptions{
Config: func() (config.Config, error) {
cfg := config.NewBlankConfig()
_ = keyring.Set("gh:github.com", "", "gho_ABCDEFG")
return cfg, nil
},
},
wantStdout: "gho_ABCDEFG\n",
},
{
name: "token by hostname",
opts: TokenOptions{
Config: func() (config.Config, error) {
cfg := config.NewBlankConfig()
_ = keyring.Set("gh:mycompany.com", "", "gho_1234567")
return cfg, nil
},
Hostname: "mycompany.com",
},
wantStdout: "gho_1234567\n",
},
{
name: "no token",
opts: TokenOptions{
Config: func() (config.Config, error) {
cfg := config.NewBlankConfig()
return cfg, nil
},
},
wantErr: true,
wantErrMsg: "no oauth token",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
keyring.MockInit()
ios, _, stdout, _ := iostreams.Test()
tt.opts.IO = ios
tt.opts.SecureStorage = true
err := tokenRun(&tt.opts)
if tt.wantErr {
assert.Error(t, err)
assert.EqualError(t, err, tt.wantErrMsg)
return
}
assert.NoError(t, err)
assert.Equal(t, tt.wantStdout, stdout.String())
})

9
pkg/liveshare/test/server.go
View file

@ -242,16 +242,17 @@ func handleRequests(ctx context.Context, server *Server, channel ssh.Channel, re
errc := make(chan error, 1)
go func() {
for req := range reqs {
if req.WantReply {
if err := req.Reply(true, nil); err != nil {
r := req
if r.WantReply {
if err := r.Reply(true, nil); err != nil {
sendError(errc, fmt.Errorf("error replying to channel request: %w", err))
return
}
}
if strings.HasPrefix(req.Type, "stream-transport") {
if strings.HasPrefix(r.Type, "stream-transport") {
go func() {
if err := forwardStream(ctx, server, req.Type, channel); err != nil {
if err := forwardStream(ctx, server, r.Type, channel); err != nil {
sendError(errc, fmt.Errorf("failed to forward stream: %w", err))
}
}()