Merge pull request #7276 from cli/default-secure-storage

secure storage by default
This commit is contained in:
Nate Smith 2023-04-04 11:32:47 -07:00 committed by GitHub
commit 69573b39b1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 64 additions and 43 deletions

View file

@ -30,12 +30,12 @@ type LoginOptions struct {
Interactive bool
Hostname string
Scopes []string
Token string
Web bool
GitProtocol string
SecureStorage bool
Hostname string
Scopes []string
Token string
Web bool
GitProtocol string
InsecureStorage bool
}
func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Command {
@ -124,7 +124,13 @@ 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")
// secure storage became the default on 2023/4/04; this flag is left as a no-op for backwards compatibility
var secureStorage bool
cmd.Flags().BoolVar(&secureStorage, "secure-storage", false, "Save authentication credentials in secure credential store")
_ = cmd.Flags().MarkHidden("secure-storage")
cmd.Flags().BoolVar(&opts.InsecureStorage, "insecure-storage", false, "Save authentication credentials in plain text instead of credential store")
return cmd
}
@ -136,11 +142,6 @@ 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
@ -166,7 +167,7 @@ func loginRun(opts *LoginOptions) error {
return fmt.Errorf("error validating token: %w", err)
}
// 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)
return authCfg.Login(hostname, "x-access-token", opts.Token, opts.GitProtocol, !opts.InsecureStorage)
}
existingToken, _ := authCfg.Token(hostname)
@ -195,7 +196,7 @@ func loginRun(opts *LoginOptions) error {
Prompter: opts.Prompter,
GitClient: opts.GitClient,
Browser: opts.Browser,
SecureStorage: opts.SecureStorage,
SecureStorage: !opts.InsecureStorage,
})
}

View file

@ -178,16 +178,31 @@ func Test_NewCmdLogin(t *testing.T) {
stdinTTY: true,
cli: "--secure-storage",
wants: LoginOptions{
Interactive: true,
SecureStorage: true,
Interactive: true,
},
},
{
name: "nontty secure-storage",
cli: "--secure-storage",
wants: LoginOptions{
Hostname: "github.com",
SecureStorage: true,
Hostname: "github.com",
},
},
{
name: "tty insecure-storage",
stdinTTY: true,
cli: "--insecure-storage",
wants: LoginOptions{
Interactive: true,
InsecureStorage: true,
},
},
{
name: "nontty insecure-storage",
cli: "--insecure-storage",
wants: LoginOptions{
Hostname: "github.com",
InsecureStorage: true,
},
},
}
@ -251,10 +266,11 @@ func Test_loginRun_nontty(t *testing.T) {
wantSecureToken string
}{
{
name: "with token",
name: "insecure with token",
opts: &LoginOptions{
Hostname: "github.com",
Token: "abc123",
Hostname: "github.com",
Token: "abc123",
InsecureStorage: true,
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org"))
@ -262,11 +278,12 @@ func Test_loginRun_nontty(t *testing.T) {
wantHosts: "github.com:\n oauth_token: abc123\n user: x-access-token\n",
},
{
name: "with token and https git-protocol",
name: "insecure with token and https git-protocol",
opts: &LoginOptions{
Hostname: "github.com",
Token: "abc123",
GitProtocol: "https",
Hostname: "github.com",
Token: "abc123",
GitProtocol: "https",
InsecureStorage: true,
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org"))
@ -276,8 +293,9 @@ func Test_loginRun_nontty(t *testing.T) {
{
name: "with token and non-default host",
opts: &LoginOptions{
Hostname: "albert.wesker",
Token: "abc123",
Hostname: "albert.wesker",
Token: "abc123",
InsecureStorage: true,
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.ScopesResponder("repo,read:org"))
@ -309,8 +327,9 @@ func Test_loginRun_nontty(t *testing.T) {
{
name: "has admin scope",
opts: &LoginOptions{
Hostname: "github.com",
Token: "abc456",
Hostname: "github.com",
Token: "abc456",
InsecureStorage: true,
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,admin:org"))
@ -358,16 +377,14 @@ func Test_loginRun_nontty(t *testing.T) {
{
name: "with token and secure storage",
opts: &LoginOptions{
Hostname: "github.com",
Token: "abc123",
SecureStorage: true,
Hostname: "github.com",
Token: "abc123",
},
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",
},
}
@ -463,8 +480,9 @@ func Test_loginRun_Survey(t *testing.T) {
{
name: "hostname set",
opts: &LoginOptions{
Hostname: "rebecca.chambers",
Interactive: true,
Hostname: "rebecca.chambers",
Interactive: true,
InsecureStorage: true,
},
wantHosts: heredoc.Doc(`
rebecca.chambers:
@ -504,7 +522,8 @@ func Test_loginRun_Survey(t *testing.T) {
git_protocol: https
`),
opts: &LoginOptions{
Interactive: true,
Interactive: true,
InsecureStorage: true,
},
prompterStubs: func(pm *prompter.PrompterMock) {
pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) {
@ -543,7 +562,8 @@ func Test_loginRun_Survey(t *testing.T) {
git_protocol: https
`),
opts: &LoginOptions{
Interactive: true,
Interactive: true,
InsecureStorage: true,
},
prompterStubs: func(pm *prompter.PrompterMock) {
pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) {
@ -573,7 +593,8 @@ func Test_loginRun_Survey(t *testing.T) {
git_protocol: ssh
`),
opts: &LoginOptions{
Interactive: true,
Interactive: true,
InsecureStorage: true,
},
prompterStubs: func(pm *prompter.PrompterMock) {
pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) {
@ -593,9 +614,8 @@ func Test_loginRun_Survey(t *testing.T) {
{
name: "secure storage",
opts: &LoginOptions{
Hostname: "github.com",
Interactive: true,
SecureStorage: true,
Hostname: "github.com",
Interactive: true,
},
prompterStubs: func(pm *prompter.PrompterMock) {
pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) {
@ -617,7 +637,7 @@ func Test_loginRun_Survey(t *testing.T) {
user: jillv
git_protocol: https
`),
wantErrOut: regexp.MustCompile("! Using secure storage could break installed extensions"),
wantErrOut: regexp.MustCompile("Logged in as jillv"),
wantSecureToken: "def456",
},
}

View file

@ -38,7 +38,7 @@ 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")
_ = cmd.Flags().MarkHidden("secure-storage")
return cmd
}