Change default token cache storage to disk (#1264)

* Change default token cache storage to disk

* Fix

* Fix

* Clean up both storages

Author: int128

README.md

@@ -83,16 +83,16 @@ If the refresh token has expired, it will perform re-authentication.
 
 ### Token cache
 
-If the OS keyring is available, kubelogin stores the token cache to the OS keyring.
-Otherwise, kubelogin stores the token cache to the file system.
+Kubelogin stores the token cache to the file system by default.
+For enhanced security, it is recommended to store it to the keyring.
 See the [token cache](docs/usage.md#token-cache) for details.
 
 You can log out by deleting the token cache.
 
 ```console
 % kubectl oidc-login clean
 Deleted the token cache at /home/user/.kube/cache/oidc-login
-Deleted the token cache in the keyring
+Deleted the token cache from the keyring
 ```
 
 Kubelogin will ask you to log in via the browser again.

docs/setup.md

@@ -203,7 +203,7 @@ Add `oidc` user to the kubeconfig.
 
 ```sh
 kubectl config set-credentials oidc \
-  --exec-interactive-mode=Never
+  --exec-interactive-mode=Never \
   --exec-api-version=client.authentication.k8s.io/v1 \
   --exec-command=kubectl \
   --exec-arg=oidc-login \
@@ -214,6 +214,9 @@ kubectl config set-credentials oidc \
 
 If your provider requires a client secret, add `--oidc-client-secret=YOUR_CLIENT_SECRET`.
 
+For security, it is recommended to add `--token-cache-storage=keyring` to store the token cache to the keyring instead of the file system.
+If you encounter an error, see the [token cache](usage.md#token-cache) for details.
+
 ## 6. Verify cluster access
 
 Make sure you can access the Kubernetes cluster.

docs/usage.md

@@ -14,7 +14,7 @@ Flags:
       --oidc-use-access-token                           Instead of using the id_token, use the access_token to authenticate to Kubernetes
       --force-refresh                                   If set, refresh the ID token regardless of its expiration time
       --token-cache-dir string                          Path to a directory of the token cache (default "~/.kube/cache/oidc-login")
-      --token-cache-storage string                      Storage for the token cache. One of (auto|keyring|disk) (default "auto")
+      --token-cache-storage string                      Storage for the token cache. One of (disk|keyring) (default "disk")
       --certificate-authority stringArray               Path to a cert file for the certificate authority
       --certificate-authority-data stringArray          Base64 encoded cert for the certificate authority
       --insecure-skip-tls-verify                        [SECURITY RISK] If set, the server's certificate will not be checked for validity
@@ -105,16 +105,21 @@ See also [net/http#ProxyFromEnvironment](https://golang.org/pkg/net/http/#ProxyF
 
 ### Token cache
 
-Kubelogin stores the token cache to the OS keyring if available.
-It depends on [zalando/go-keyring](https://github.com/zalando/go-keyring) for the keyring storage.
+Kubelogin stores the token cache to the file system by default.
 
-If you encounter a problem, try `--token-cache-storage` to set the storage.
+You can store the token cache to the OS keyring for enhanced security.
+It depends on [zalando/go-keyring](https://github.com/zalando/go-keyring).
 
 ```yaml
-# Force to use the OS keyring
 - --token-cache-storage=keyring
-# Force to use the file system
-- --token-cache-storage=disk
+```
+
+You can delete the token cache by the clean command.
+
+```console
+% kubectl oidc-login clean
+Deleted the token cache at /home/user/.kube/cache/oidc-login
+Deleted the token cache from the keyring
 ```
 
 ### Home directory expansion

integration_test/clean_test.go

@@ -20,7 +20,6 @@ func TestClean(t *testing.T) {
 		"kubelogin",
 		"clean",
 		"--token-cache-dir", tokenCacheDir,
-		"--token-cache-storage", "disk",
 	}, "HEAD")
 	if exitCode != 0 {
 		t.Errorf("exit status wants 0 but %d", exitCode)

integration_test/credetial_plugin_test.go

@@ -443,7 +443,6 @@ func runGetToken(t *testing.T, ctx context.Context, cfg getTokenConfig) {
 		"kubelogin",
 		"get-token",
 		"--token-cache-dir", cfg.tokenCacheDir,
-		"--token-cache-storage", "disk",
 		"--oidc-issuer-url", cfg.issuerURL,
 		"--oidc-client-id", "kubernetes",
 		"--listen-address", "127.0.0.1:0",

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

@@ -5,15 +5,13 @@ import (
 	"encoding/gob"
 	"encoding/hex"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"os"
 	"path/filepath"
 
 	"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/repository/repository.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/tokencache/types.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
 }

pkg/usecases/clean/clean.go

@@ -29,6 +29,7 @@ test: build
 		--exec-arg=--oidc-client-id=YOUR_CLIENT_ID \
 		--exec-arg=--oidc-client-secret=YOUR_CLIENT_SECRET \
 		--exec-arg=--oidc-extra-scope=email \
+		--exec-arg=--token-cache-storage=keyring \
 		--exec-arg=--certificate-authority=$(CERT_DIR)/ca.crt \
 		--exec-arg=--browser-command=$(BIN_DIR)/chromelogin