Make auth refresh more convenient with secure storage (#7098)

cli · Mar 7, 2023 · b74ba55 · b74ba55
1 parent 44b0218
commit b74ba55
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 5 deletions.
diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go
@@ -134,7 +134,7 @@ func refreshRun(opts *RefreshOptions) error {
 	}
 
 	var additionalScopes []string
-	if oldToken, _ := authCfg.Token(hostname); oldToken != "" {
+	if oldToken, source := 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)
@@ -143,6 +143,13 @@ 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{

diff --git a/pkg/cmd/auth/refresh/refresh_test.go b/pkg/cmd/auth/refresh/refresh_test.go
@@ -146,6 +146,7 @@ func Test_refreshRun(t *testing.T) {
 		opts          *RefreshOptions
 		prompterStubs func(*prompter.PrompterMock)
 		cfgHosts      []string
+		config        config.Config
 		oldScopes     string
 		wantErr       string
 		nontty        bool
@@ -241,7 +242,7 @@ func Test_refreshRun(t *testing.T) {
 			},
 		},
 		{
-			name: "secure storage",
+			name: "explicit secure storage",
 			cfgHosts: []string{
 				"obed.morton",
 			},
@@ -255,6 +256,27 @@ func Test_refreshRun(t *testing.T) {
 				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
+			}(),
+			opts: &RefreshOptions{
+				Hostname: "obed.morton",
+			},
+			wantAuthArgs: authArgs{
+				hostname:      "obed.morton",
+				scopes:        nil,
+				secureStorage: true,
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -267,9 +289,14 @@ func Test_refreshRun(t *testing.T) {
 				return nil
 			}
 
-			cfg := config.NewFromString("")
-			for _, hostname := range tt.cfgHosts {
-				cfg.Set(hostname, "oauth_token", "abc123")
+			var cfg config.Config
+			if tt.config != nil {
+				cfg = tt.config
+			} else {
+				cfg = config.NewFromString("")
+				for _, hostname := range tt.cfgHosts {
+					cfg.Set(hostname, "oauth_token", "abc123")
+				}
 			}
 			tt.opts.Config = func() (config.Config, error) {
 				return cfg, nil