Change default token cache storage to disk (#1264)

* Change default token cache storage to disk

* Fix

* Fix

* Clean up both storages

pkg/cmd/clean.go

@@ -9,15 +9,11 @@ import (
 )
 
 type cleanOptions struct {
-	tokenCacheOptions tokenCacheOptions
+	TokenCacheDir string
 }
 
 func (o *cleanOptions) addFlags(f *pflag.FlagSet) {
-	o.tokenCacheOptions.addFlags(f)
-}
-
-func (o *cleanOptions) expandHomedir() {
-	o.tokenCacheOptions.expandHomedir()
+	f.StringVar(&o.TokenCacheDir, "token-cache-dir", getDefaultTokenCacheDir(), "Path to a directory of the token cache")
 }
 
 type Clean struct {
@@ -31,18 +27,13 @@ func (cmd *Clean) New() *cobra.Command {
 		Short: "Delete the token cache",
 		Long: `Delete the token cache.
 
-This deletes both the OS keyring and the directory by default.
-If you encounter an error of keyring, try --token-cache-storage=disk.
+This deletes the token cache directory from both the file system and the keyring.
 `,
 		Args: cobra.NoArgs,
 		RunE: func(c *cobra.Command, _ []string) error {
-			o.expandHomedir()
-			tokenCacheConfig, err := o.tokenCacheOptions.tokenCacheConfig()
-			if err != nil {
-				return fmt.Errorf("clean: %w", err)
-			}
+			o.TokenCacheDir = expandHomedir(o.TokenCacheDir)
 			in := clean.Input{
-				TokenCacheConfig: tokenCacheConfig,
+				TokenCacheDir: o.TokenCacheDir,
 			}
 			if err := cmd.Clean.Do(c.Context(), in); err != nil {
 				return fmt.Errorf("clean: %w", err)

pkg/cmd/cmd_test.go

@@ -118,7 +118,6 @@ func TestCmd_Run(t *testing.T) {
 					},
 					TokenCacheConfig: tokencache.Config{
 						Directory: filepath.Join(userHomeDir, ".kube/cache/oidc-login"),
-						Storage:   tokencache.StorageAuto,
 					},
 					GrantOptionSet: defaultGrantOptionSet,
 				},
@@ -131,7 +130,7 @@ func TestCmd_Run(t *testing.T) {
 					"--oidc-client-secret", "YOUR_CLIENT_SECRET",
 					"--oidc-extra-scope", "email",
 					"--oidc-extra-scope", "profile",
-					"--token-cache-storage", "disk",
+					"--token-cache-storage", "keyring",
 					"-v1",
 				},
 				in: credentialplugin.Input{
@@ -143,7 +142,7 @@ func TestCmd_Run(t *testing.T) {
 					},
 					TokenCacheConfig: tokencache.Config{
 						Directory: filepath.Join(userHomeDir, ".kube/cache/oidc-login"),
-						Storage:   tokencache.StorageDisk,
+						Storage:   tokencache.StorageKeyring,
 					},
 					GrantOptionSet: defaultGrantOptionSet,
 				},
@@ -163,7 +162,6 @@ func TestCmd_Run(t *testing.T) {
 					},
 					TokenCacheConfig: tokencache.Config{
 						Directory: filepath.Join(userHomeDir, ".kube/cache/oidc-login"),
-						Storage:   tokencache.StorageAuto,
 					},
 					GrantOptionSet: defaultGrantOptionSet,
 				},
@@ -185,7 +183,6 @@ func TestCmd_Run(t *testing.T) {
 					},
 					TokenCacheConfig: tokencache.Config{
 						Directory: filepath.Join(userHomeDir, ".kube/oidc-cache"),
-						Storage:   tokencache.StorageAuto,
 					},
 					GrantOptionSet: authentication.GrantOptionSet{
 						AuthCodeBrowserOption: &authcode.BrowserOption{

pkg/cmd/tokencache.go

@@ -18,7 +18,7 @@ func getDefaultTokenCacheDir() string {
 	return filepath.Join("~", ".kube", "cache", "oidc-login")
 }
 
-var allTokenCacheStorage = strings.Join([]string{"auto", "keyring", "disk"}, "|")
+var allTokenCacheStorage = strings.Join([]string{"disk", "keyring"}, "|")
 
 type tokenCacheOptions struct {
 	TokenCacheDir     string
@@ -27,7 +27,7 @@ type tokenCacheOptions struct {
 
 func (o *tokenCacheOptions) addFlags(f *pflag.FlagSet) {
 	f.StringVar(&o.TokenCacheDir, "token-cache-dir", getDefaultTokenCacheDir(), "Path to a directory of the token cache")
-	f.StringVar(&o.TokenCacheStorage, "token-cache-storage", "auto", fmt.Sprintf("Storage for the token cache. One of (%s)", allTokenCacheStorage))
+	f.StringVar(&o.TokenCacheStorage, "token-cache-storage", "disk", fmt.Sprintf("Storage for the token cache. One of (%s)", allTokenCacheStorage))
 }
 
 func (o *tokenCacheOptions) expandHomedir() {
@@ -39,12 +39,10 @@ func (o *tokenCacheOptions) tokenCacheConfig() (tokencache.Config, error) {
 		Directory: o.TokenCacheDir,
 	}
 	switch o.TokenCacheStorage {
-	case "auto":
-		config.Storage = tokencache.StorageAuto
-	case "keyring":
-		config.Storage = tokencache.StorageKeyring
 	case "disk":
 		config.Storage = tokencache.StorageDisk
+	case "keyring":
+		config.Storage = tokencache.StorageKeyring
 	default:
 		return tokencache.Config{}, fmt.Errorf("token-cache-storage must be one of (%s)", allTokenCacheStorage)
 	}

pkg/di/wire_gen.go

@@ -97,9 +97,7 @@ func NewCmdForHeadless(clockInterface clock.Interface, stdin stdio.Stdin, stdout
 		Standalone: standaloneStandalone,
 		Logger:     loggerInterface,
 	}
-	repositoryRepository := &repository.Repository{
-		Logger: loggerInterface,
-	}
+	repositoryRepository := &repository.Repository{}
 	reader3 := &reader2.Reader{}
 	writer3 := &writer2.Writer{
 		Stdout: stdout,

pkg/tokencache/repository/repository.go

@@ -5,7 +5,6 @@ import (
 	"encoding/gob"
 	"encoding/hex"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -13,7 +12,6 @@ import (
 
 	"github.com/gofrs/flock"
 	"github.com/google/wire"
-	"github.com/int128/kubelogin/pkg/infrastructure/logger"
 	"github.com/int128/kubelogin/pkg/oidc"
 	"github.com/int128/kubelogin/pkg/tokencache"
 	"github.com/zalando/go-keyring"
@@ -39,9 +37,7 @@ type entity struct {
 
 // Repository provides access to the token cache on the local filesystem.
 // Filename of a token cache is sha256 digest of the issuer, zero-character and client ID.
-type Repository struct {
-	Logger logger.Interface
-}
+type Repository struct{}
 
 // keyringService is used to namespace the keyring access.
 // Some implementations may also display this string when prompting the user
@@ -57,16 +53,6 @@ func (r *Repository) FindByKey(config tokencache.Config, key tokencache.Key) (*o
 		return nil, fmt.Errorf("could not compute the key: %w", err)
 	}
 	switch config.Storage {
-	case tokencache.StorageAuto:
-		t, err := readFromKeyring(checksum)
-		if errors.Is(err, keyring.ErrUnsupportedPlatform) ||
-			errors.Is(err, keyring.ErrNotFound) {
-			return readFromFile(config, checksum)
-		}
-		if err != nil {
-			return nil, err
-		}
-		return t, nil
 	case tokencache.StorageDisk:
 		return readFromFile(config, checksum)
 	case tokencache.StorageKeyring:
@@ -120,17 +106,6 @@ func (r *Repository) Save(config tokencache.Config, key tokencache.Key, tokenSet
 		return fmt.Errorf("could not compute the key: %w", err)
 	}
 	switch config.Storage {
-	case tokencache.StorageAuto:
-		if err := writeToKeyring(checksum, tokenSet); err != nil {
-			if errors.Is(err, keyring.ErrUnsupportedPlatform) {
-				return writeToFile(config, checksum, tokenSet)
-			}
-			if errors.Is(err, keyring.ErrSetDataTooBig) {
-				return writeToFile(config, checksum, tokenSet)
-			}
-			return err
-		}
-		return nil
 	case tokencache.StorageDisk:
 		return writeToFile(config, checksum, tokenSet)
 	case tokencache.StorageKeyring:
@@ -188,36 +163,20 @@ func (r *Repository) Lock(config tokencache.Config, key tokencache.Key) (io.Clos
 }
 
 func (r *Repository) DeleteAll(config tokencache.Config) error {
-	return errors.Join(
-		func() error {
-			if err := os.RemoveAll(config.Directory); err != nil {
-				return fmt.Errorf("remove the directory %s: %w", config.Directory, err)
-			}
-			r.Logger.Printf("Deleted the token cache at %s", config.Directory)
-			return nil
-		}(),
-		func() error {
-			switch config.Storage {
-			case tokencache.StorageAuto:
-				if err := keyring.DeleteAll(keyringService); err != nil {
-					if errors.Is(err, keyring.ErrUnsupportedPlatform) {
-						return nil
-					}
-					return fmt.Errorf("keyring delete: %w", err)
-				}
-				r.Logger.Printf("Deleted the token cache in the keyring")
-				return nil
-			case tokencache.StorageKeyring:
-				if err := keyring.DeleteAll(keyringService); err != nil {
-					return fmt.Errorf("keyring delete: %w", err)
-				}
-				r.Logger.Printf("Deleted the token cache in the keyring")
-				return nil
-			default:
-				return nil
-			}
-		}(),
-	)
+	switch config.Storage {
+	case tokencache.StorageDisk:
+		if err := os.RemoveAll(config.Directory); err != nil {
+			return fmt.Errorf("remove the directory %s: %w", config.Directory, err)
+		}
+		return nil
+	case tokencache.StorageKeyring:
+		if err := keyring.DeleteAll(keyringService); err != nil {
+			return fmt.Errorf("keyring delete: %w", err)
+		}
+		return nil
+	default:
+		return fmt.Errorf("unknown storage mode: %v", config.Storage)
+	}
 }
 
 func encodeKey(tokenSet oidc.TokenSet) ([]byte, error) {

pkg/tokencache/types.go

@@ -25,10 +25,8 @@ type Config struct {
 type Storage byte
 
 const (
-	// StorageAuto will prefer keyring when available, and fallback to disk when not.
-	StorageAuto Storage = iota
 	// StorageDisk will only store cached keys on disk.
-	StorageDisk
+	StorageDisk Storage = iota
 	// StorageDisk will only store cached keys in the OS keyring.
 	StorageKeyring
 )

pkg/usecases/clean/clean.go

@@ -21,7 +21,7 @@ type Interface interface {
 
 // Input represents an input of the Clean use-case.
 type Input struct {
-	TokenCacheConfig tokencache.Config
+	TokenCacheDir string
 }
 
 type Clean struct {
@@ -31,8 +31,17 @@ type Clean struct {
 
 func (u *Clean) Do(ctx context.Context, in Input) error {
 	u.Logger.V(1).Infof("Deleting the token cache")
-	if err := u.TokenCacheRepository.DeleteAll(in.TokenCacheConfig); err != nil {
-		return fmt.Errorf("delete the token cache: %w", err)
+
+	if err := u.TokenCacheRepository.DeleteAll(tokencache.Config{Directory: in.TokenCacheDir, Storage: tokencache.StorageDisk}); err != nil {
+		return fmt.Errorf("delete the token cache from %s: %w", in.TokenCacheDir, err)
+	}
+	u.Logger.Printf("Deleted the token cache from %s", in.TokenCacheDir)
+
+	if err := u.TokenCacheRepository.DeleteAll(tokencache.Config{Directory: in.TokenCacheDir, Storage: tokencache.StorageKeyring}); err != nil {
+		// Do not return an error because the keyring may not be available.
+		u.Logger.Printf("Could not delete the token cache from the keyring: %s", err)
+	} else {
+		u.Logger.Printf("Deleted the token cache from the keyring")
 	}
 	return nil
 }