diff --git a/README.md b/README.md index dd05526..bd75c13 100644 --- a/README.md +++ b/README.md @@ -83,8 +83,8 @@ 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. @@ -92,7 +92,7 @@ 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. diff --git a/docs/setup.md b/docs/setup.md index bea0176..2b575ac 100644 --- a/docs/setup.md +++ b/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. diff --git a/docs/usage.md b/docs/usage.md index 5105db8..dd0b441 100644 --- a/docs/usage.md +++ b/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 diff --git a/integration_test/clean_test.go b/integration_test/clean_test.go index 8524ca0..44b416f 100644 --- a/integration_test/clean_test.go +++ b/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) diff --git a/integration_test/credetial_plugin_test.go b/integration_test/credetial_plugin_test.go index 7805514..81b44a9 100644 --- a/integration_test/credetial_plugin_test.go +++ b/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", diff --git a/pkg/cmd/clean.go b/pkg/cmd/clean.go index 17cc771..f5f2d04 100644 --- a/pkg/cmd/clean.go +++ b/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) diff --git a/pkg/cmd/cmd_test.go b/pkg/cmd/cmd_test.go index d6ce3a2..7724510 100644 --- a/pkg/cmd/cmd_test.go +++ b/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{ diff --git a/pkg/cmd/tokencache.go b/pkg/cmd/tokencache.go index 0d94839..29156c6 100644 --- a/pkg/cmd/tokencache.go +++ b/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) } diff --git a/pkg/di/wire_gen.go b/pkg/di/wire_gen.go index 6a5a1d4..c5654c6 100644 --- a/pkg/di/wire_gen.go +++ b/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, diff --git a/pkg/tokencache/repository/repository.go b/pkg/tokencache/repository/repository.go index f33c5aa..ea292f0 100644 --- a/pkg/tokencache/repository/repository.go +++ b/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) { diff --git a/pkg/tokencache/types.go b/pkg/tokencache/types.go index 9b7d9dc..3349afe 100644 --- a/pkg/tokencache/types.go +++ b/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 ) diff --git a/pkg/usecases/clean/clean.go b/pkg/usecases/clean/clean.go index 680e1e1..cd42830 100644 --- a/pkg/usecases/clean/clean.go +++ b/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 } diff --git a/system_test/login/Makefile b/system_test/login/Makefile index 99c7e03..9feedd7 100644 --- a/system_test/login/Makefile +++ b/system_test/login/Makefile @@ -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