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

Merge pull request #7278 from cli/auth-refresh-storage

Browse source 
extend secure storage default to auth refresh
This commit is contained in:
Nate Smith 2023-04-04 15:35:57 -07:00 • committed by GitHub
commit 633ca03b53
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 52 additions and 51 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -28,8 +28,8 @@ type RefreshOptions struct {
Scopes []string
AuthFlow func(*config.AuthConfig, *iostreams.IOStreams, string, []string, bool, bool) error
Interactive bool
SecureStorage bool
Interactive bool
InsecureStorage bool
}
func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.Command {
@ -90,7 +90,12 @@ 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")
// 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().BoolVarP(&opts.InsecureStorage, "insecure-storage", "", false, "Save authentication credentials in plain text instead of credential store")
return cmd
}
@ -139,7 +144,7 @@ func refreshRun(opts *RefreshOptions) error {
}
var additionalScopes []string
if oldToken, source := authCfg.Token(hostname); oldToken != "" {
if oldToken, _ := authCfg.Token(hostname); oldToken != "" {
if oldScopes, err := shared.GetScopes(opts.HttpClient, hostname, oldToken); err == nil {
for _, s := range strings.Split(oldScopes, ",") {
s = strings.TrimSpace(s)
@ -148,13 +153,6 @@ func refreshRun(opts *RefreshOptions) error {
}
}
}
// If previous token was stored in secure storage assume
// user wants to continue storing it there even if not
// explicitly stated with the secure storage flag.
if source == "keyring" {
opts.SecureStorage = true
}
}
credentialFlow := &shared.GitCredentialFlow{
@ -170,7 +168,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, opts.SecureStorage); err != nil {
if err := opts.AuthFlow(authCfg, opts.IO, hostname, append(opts.Scopes, additionalScopes...), opts.Interactive, !opts.InsecureStorage); err != nil {
return err
}

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

@ -86,11 +86,17 @@ func Test_NewCmdRefresh(t *testing.T) {
},
},
{
name: "secure storage",
name: "secure storage",
tty: true,
cli: "--secure-storage",
wants: RefreshOptions{},
},
{
name: "insecure storage",
tty: true,
cli: "--secure-storage",
cli: "--insecure-storage",
wants: RefreshOptions{
SecureStorage: true,
InsecureStorage: true,
},
},
}
@ -178,8 +184,9 @@ func Test_refreshRun(t *testing.T) {
Hostname: "obed.morton",
},
wantAuthArgs: authArgs{
hostname: "obed.morton",
scopes: nil,
hostname: "obed.morton",
scopes: nil,
secureStorage: true,
},
},
{
@ -191,8 +198,9 @@ func Test_refreshRun(t *testing.T) {
Hostname: "",
},
wantAuthArgs: authArgs{
hostname: "github.com",
scopes: nil,
hostname: "github.com",
scopes: nil,
secureStorage: true,
},
},
{
@ -210,8 +218,9 @@ func Test_refreshRun(t *testing.T) {
}
},
wantAuthArgs: authArgs{
hostname: "github.com",
scopes: nil,
hostname: "github.com",
scopes: nil,
secureStorage: true,
},
},
{
@ -223,12 +232,13 @@ func Test_refreshRun(t *testing.T) {
Scopes: []string{"repo:invite", "public_key:read"},
},
wantAuthArgs: authArgs{
hostname: "github.com",
scopes: []string{"repo:invite", "public_key:read"},
hostname: "github.com",
scopes: []string{"repo:invite", "public_key:read"},
secureStorage: true,
},
},
{
name: "scopes provided",
name: "more scopes provided",
cfgHosts: []string{
"github.com",
},
@ -237,37 +247,16 @@ func Test_refreshRun(t *testing.T) {
Scopes: []string{"repo:invite", "public_key:read"},
},
wantAuthArgs: authArgs{
hostname: "github.com",
scopes: []string{"repo:invite", "public_key:read", "delete_repo", "codespace"},
},
},
{
name: "explicit secure storage",
cfgHosts: []string{
"obed.morton",
},
opts: &RefreshOptions{
Hostname: "obed.morton",
SecureStorage: true,
},
wantAuthArgs: authArgs{
hostname: "obed.morton",
scopes: nil,
hostname: "github.com",
scopes: []string{"repo:invite", "public_key:read", "delete_repo", "codespace"},
secureStorage: true,
},
},
{
name: "implicit secure storage",
config: func() config.Config {
cfg := config.NewFromString("")
authCfg := cfg.Authentication()
authCfg.SetHosts([]string{"obed.morton"})
authCfg.SetToken("abc123", "keyring")
cfg.AuthenticationFunc = func() *config.AuthConfig {
return authCfg
}
return cfg
}(),
name: "secure storage",
cfgHosts: []string{
"obed.morton",
},
opts: &RefreshOptions{
Hostname: "obed.morton",
},
@ -277,6 +266,20 @@ func Test_refreshRun(t *testing.T) {
secureStorage: true,
},
},
{
name: "insecure storage",
cfgHosts: []string{
"obed.morton",
},
opts: &RefreshOptions{
Hostname: "obed.morton",
InsecureStorage: true,
},
wantAuthArgs: authArgs{
hostname: "obed.morton",
scopes: nil,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {