diff --git a/README.md b/README.md index 2fa134a330..6b4f79cf6a 100644 --- a/README.md +++ b/README.md @@ -346,6 +346,12 @@ Personal overrides, gitignored by default: | `strategy_options.summarize.enabled` | `true`, `false` | Auto-generate AI summaries at commit time | | `telemetry` | `true`, `false` | Send anonymous usage statistics to Posthog | +### Authentication Token Storage + +By default, `entire login` stores the CLI API token in your operating system keyring. For CI, set `ENTIRE_AUTH_TOKEN` to a CLI API token. For headless machines that need persistent login, set `ENTIRE_SECRETS_PATH` to an absolute path before running `entire login`; the file is plaintext JSON written with `0600` permissions. Pass `--no-keyring` to `entire login` if you want the command to fail closed when `ENTIRE_SECRETS_PATH` is missing instead of silently falling back to the OS keyring. + +`ENTIRE_AUTH_TOKEN` takes precedence over tokens stored in `ENTIRE_SECRETS_PATH`, and the file store takes precedence over the OS keyring. `ENTIRE_AUTH_TOKEN` only applies when the API origin is the production default (`https://entire.io`); custom origins set via `ENTIRE_API_BASE_URL` must use the per-origin file or keyring stores so a stray override can't leak a prod bearer to a staging endpoint. `ENTIRE_CHECKPOINT_TOKEN` is separate and only applies to checkpoint Git fetch/push operations. + ### Agent Hook Configuration Each agent stores its hook configuration in its own directory. When you run `entire enable`, hooks are installed in the appropriate location for each selected agent: diff --git a/cmd/entire/cli/auth.go b/cmd/entire/cli/auth.go index 0a441aef07..951a7e0237 100644 --- a/cmd/entire/cli/auth.go +++ b/cmd/entire/cli/auth.go @@ -96,32 +96,51 @@ func defaultListTokens(ctx context.Context, token string) ([]api.Token, error) { } func runAuthStatus(ctx context.Context, w io.Writer, store tokenStore, list authTokenLister, baseURL string) error { - token, err := store.GetToken(baseURL) + info, err := store.GetTokenInfo(baseURL) if err != nil { - return fmt.Errorf("read keychain: %w", err) + return fmt.Errorf("read auth token: %w", err) } - if token == "" { + if info.Value == "" { fmt.Fprintf(w, "Not logged in to %s\n", baseURL) fmt.Fprintln(w, "Run 'entire login' to authenticate.") return nil } - tokens, err := list(ctx, token) + tokens, err := list(ctx, info.Value) if err != nil { if api.IsHTTPErrorStatus(err, http.StatusUnauthorized) { - fmt.Fprintf(w, "Token in keychain for %s is no longer valid.\n", baseURL) - fmt.Fprintln(w, "Run 'entire login' to re-authenticate.") + fmt.Fprintf(w, "Token for %s is no longer valid.\n", baseURL) + if info.Source == auth.TokenSourceEnv { + fmt.Fprintf(w, "Update or unset %s to re-authenticate.\n", auth.AuthTokenEnvVar) + } else { + fmt.Fprintln(w, "Run 'entire login' to re-authenticate.") + } return nil } return fmt.Errorf("validate token: %w", err) } fmt.Fprintf(w, "Logged in to %s\n", baseURL) - fmt.Fprintln(w, " Token: stored in OS keychain") + fmt.Fprintf(w, " Token: %s\n", describeTokenSource(info)) fmt.Fprintf(w, " Active tokens on this account: %d\n", len(tokens)) return nil } +func describeTokenSource(info auth.TokenInfo) string { + switch info.Source { + case auth.TokenSourceNone: + return "not configured" + case auth.TokenSourceEnv: + return "supplied by " + auth.AuthTokenEnvVar + case auth.TokenSourceFile: + return "stored in file " + info.Path + case auth.TokenSourceKeyring: + return "stored in OS keychain" + default: + return "stored" + } +} + // --- list ------------------------------------------------------------------- func newAuthListCmd() *cobra.Command { @@ -146,7 +165,7 @@ func newAuthListCmd() *cobra.Command { func runAuthList(ctx context.Context, w io.Writer, store tokenStore, list authTokenLister, baseURL string, jsonOut bool) error { token, err := store.GetToken(baseURL) if err != nil { - return fmt.Errorf("read keychain: %w", err) + return fmt.Errorf("read auth token: %w", err) } if token == "" { return fmt.Errorf("not logged in to %s; run 'entire login' first", baseURL) @@ -391,7 +410,7 @@ func newAuthRevokeCmd() *cobra.Command { cmd := &cobra.Command{ Use: "revoke [id]", Short: "Revoke an API token by id", - Long: "Revoke a specific API token. Use --current to revoke the token used by this CLI (equivalent to 'entire logout').", + Long: "Revoke a specific API token. Use --current to revoke the token used by this CLI.", Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { id := "" @@ -412,7 +431,7 @@ func newAuthRevokeCmd() *cobra.Command { api.BaseURL(), id, revokeCurrent) }, } - cmd.Flags().BoolVar(&revokeCurrent, "current", false, "Revoke the token used by this CLI and remove the local copy") + cmd.Flags().BoolVar(&revokeCurrent, "current", false, "Revoke the token used by this CLI and remove the local copy when stored locally") addInsecureHTTPAuthFlag(cmd, &insecureHTTPAuth) return cmd } @@ -431,35 +450,85 @@ func runAuthRevoke( baseURL, id string, current bool, ) error { - token, err := store.GetToken(baseURL) + info, err := store.GetTokenInfo(baseURL) if err != nil { - return fmt.Errorf("read keychain: %w", err) + return fmt.Errorf("read auth token: %w", err) } - if token == "" { + if info.Value == "" { return fmt.Errorf("not logged in to %s; run 'entire login' first", baseURL) } if current { - // Revoking our own token is just logout — reuse that path so behavior - // stays identical (best-effort revoke + local delete). - return runLogout(ctx, outW, errW, store, revokeCurrent, baseURL) + return revokeCurrentToken(ctx, outW, errW, store, revokeCurrent, baseURL, info) } - if err := revokeByID(ctx, token, id); err != nil { + if err := revokeByID(ctx, info.Value, id); err != nil { return err } // The list endpoint requires bearer auth, so a 401 here means the id we // just revoked was the same one this CLI is using — the keychain entry is // now stale and would otherwise produce confusing 401s on every command. - if _, listErr := list(ctx, token); listErr != nil && api.IsHTTPErrorStatus(listErr, http.StatusUnauthorized) { + if _, listErr := list(ctx, info.Value); listErr != nil && api.IsHTTPErrorStatus(listErr, http.StatusUnauthorized) { + if info.Source == auth.TokenSourceEnv { + fmt.Fprintf(outW, "Revoked token %s (this was supplied by %s; unset it to stop using it locally).\n", id, auth.AuthTokenEnvVar) + return nil + } if delErr := store.DeleteToken(baseURL); delErr != nil { return fmt.Errorf("revoked token %s but failed to remove local copy: %w", id, delErr) } - fmt.Fprintf(outW, "Revoked token %s (this was your local token; removed from keychain).\n", id) + fmt.Fprintf(outW, "Revoked token %s (this was your local token; removed from %s).\n", id, localTokenSourceName(info)) return nil } fmt.Fprintf(outW, "Revoked token %s.\n", id) return nil } + +func revokeCurrentToken( + ctx context.Context, + outW, errW io.Writer, + store tokenStore, + revoke revokeCurrentFunc, + baseURL string, + info auth.TokenInfo, +) error { + if info.Source == auth.TokenSourceEnv { + // Env tokens have no local copy to fall back on, so a non-401 server + // failure means the token is still active — we MUST NOT print + // "Revoked" and exit zero in that case. 401 is treated as idempotent + // success (the token was already invalid server-side). + if err := revoke(ctx, info.Value); err != nil { + if !api.IsHTTPErrorStatus(err, http.StatusUnauthorized) { + return fmt.Errorf("revoke current token: %w", err) + } + } + fmt.Fprintf(outW, "Revoked current token supplied by %s. Unset it to stop using it locally.\n", auth.AuthTokenEnvVar) + return nil + } + + if err := revoke(ctx, info.Value); err != nil && !api.IsHTTPErrorStatus(err, http.StatusUnauthorized) { + fmt.Fprintf(errW, "Warning: server-side token revocation failed: %v\n", err) + } + if err := store.DeleteToken(baseURL); err != nil { + return fmt.Errorf("remove auth token: %w", err) + } + + fmt.Fprintln(outW, "Logged out.") + return nil +} + +func localTokenSourceName(info auth.TokenInfo) string { + switch info.Source { + case auth.TokenSourceFile: + return "file" + case auth.TokenSourceKeyring: + return "keychain" + case auth.TokenSourceEnv: + return auth.AuthTokenEnvVar + case auth.TokenSourceNone: + return "local storage" + default: + return "local storage" + } +} diff --git a/cmd/entire/cli/auth/env.go b/cmd/entire/cli/auth/env.go new file mode 100644 index 0000000000..f06048920b --- /dev/null +++ b/cmd/entire/cli/auth/env.go @@ -0,0 +1,35 @@ +package auth + +import ( + "os" + "strings" +) + +const ( + // AuthTokenEnvVar provides a read-only bearer token for non-interactive CLI auth. + AuthTokenEnvVar = "ENTIRE_AUTH_TOKEN" // #nosec G101 -- this is an environment variable name, not a credential. + + // SecretsPathEnvVar points to an opt-in file-backed token store. + SecretsPathEnvVar = "ENTIRE_SECRETS_PATH" +) + +// TokenSource identifies where the active auth token came from. +type TokenSource string + +const ( + TokenSourceNone TokenSource = "" + TokenSourceEnv TokenSource = "env" + TokenSourceFile TokenSource = "file" + TokenSourceKeyring TokenSource = "keyring" +) + +// TokenInfo is a source-aware auth token lookup result. +type TokenInfo struct { + Value string + Source TokenSource + Path string +} + +func envAuthToken() string { + return strings.TrimSpace(os.Getenv(AuthTokenEnvVar)) +} diff --git a/cmd/entire/cli/auth/file_store.go b/cmd/entire/cli/auth/file_store.go new file mode 100644 index 0000000000..e7859ea7a5 --- /dev/null +++ b/cmd/entire/cli/auth/file_store.go @@ -0,0 +1,210 @@ +// Package auth — production file-backed token store (issue #1036). +// +// This file implements the ENTIRE_SECRETS_PATH headless-auth store and is +// the one deliberate exception to the keyring-only production policy +// enforced by store_invariants_test.go::TestProductionAuthStoreIsKeyringOnly. +// +// Why it exists: in headless environments (SSH, WSL, Docker, CI/CD runners +// without an unlocked Secret Service collection) the OS keyring is +// unreachable and `entire login` cannot persist a token. ENTIRE_SECRETS_PATH +// lets users opt into a plaintext JSON file store at a path they control. +// +// Why it is safe to ship in production: +// - Strictly opt-in. Nothing happens unless the user sets an absolute path +// in ENTIRE_SECRETS_PATH; ~/ expansion is honored, relative paths are +// rejected. +// - 0600 permissions enforced on every read and write. A loose-perm file +// errors out with a `chmod 600` hint instead of being trusted silently. +// - Atomic writes via temp+rename inside the same directory, with 0700 on +// the parent so a partial write can't be observed. +// - Schema-versioned JSON. Unknown versions are rejected up front. +// - Separate symbol space from store_filebackend.go (which is the +// authfilestore-gated test-only backend) — they share no code paths. +package auth + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "runtime" + "strings" +) + +const fileStoreVersion = 1 + +type fileStoreData struct { + Version int `json:"version"` + Tokens map[string]string `json:"tokens"` +} + +func configuredSecretsPath() (string, bool, error) { + raw := strings.TrimSpace(os.Getenv(SecretsPathEnvVar)) + if raw == "" { + return "", false, nil + } + + path, err := expandHome(raw) + if err != nil { + return "", false, err + } + path = filepath.Clean(path) + if !filepath.IsAbs(path) { + return "", false, fmt.Errorf("%s must be an absolute path after ~/ expansion", SecretsPathEnvVar) + } + + return path, true, nil +} + +func expandHome(path string) (string, error) { + if path == "~" { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("resolve home directory: %w", err) + } + return home, nil + } + if strings.HasPrefix(path, "~/") { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("resolve home directory: %w", err) + } + return filepath.Join(home, strings.TrimPrefix(path, "~/")), nil + } + return path, nil +} + +func readFileStore(path string) (fileStoreData, error) { + info, err := os.Stat(path) + if os.IsNotExist(err) { + return newFileStoreData(), nil + } + if err != nil { + return fileStoreData{}, fmt.Errorf("stat token file: %w", err) + } + if info.IsDir() { + return fileStoreData{}, fmt.Errorf("token file path is a directory: %s", path) + } + if runtime.GOOS != "windows" && info.Mode().Perm()&0o077 != 0 { + return fileStoreData{}, fmt.Errorf("token file %s has unsafe permissions %o; run chmod 600 %s", path, info.Mode().Perm(), path) + } + + data, err := os.ReadFile(path) //nolint:gosec // path is explicit user-provided credential path. + if err != nil { + return fileStoreData{}, fmt.Errorf("read token file: %w", err) + } + if len(strings.TrimSpace(string(data))) == 0 { + return newFileStoreData(), nil + } + + var store fileStoreData + if err := json.Unmarshal(data, &store); err != nil { + return fileStoreData{}, fmt.Errorf("parse token file: %w", err) + } + if store.Version == 0 { + store.Version = fileStoreVersion + } + if store.Version != fileStoreVersion { + return fileStoreData{}, fmt.Errorf("unsupported token file version %d", store.Version) + } + if store.Tokens == nil { + store.Tokens = make(map[string]string) + } + + return store, nil +} + +func writeFileStore(path string, store fileStoreData) error { + if store.Version == 0 { + store.Version = fileStoreVersion + } + if store.Tokens == nil { + store.Tokens = make(map[string]string) + } + + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o700); err != nil { + return fmt.Errorf("create token file directory: %w", err) + } + + temp, err := os.CreateTemp(dir, "."+filepath.Base(path)+".tmp-*") + if err != nil { + return fmt.Errorf("create temporary token file: %w", err) + } + tempName := temp.Name() + defer func() { _ = os.Remove(tempName) }() + + if err := temp.Chmod(0o600); err != nil { + _ = temp.Close() + return fmt.Errorf("set temporary token file permissions: %w", err) + } + + enc := json.NewEncoder(temp) + enc.SetIndent("", " ") + if err := enc.Encode(store); err != nil { + _ = temp.Close() + return fmt.Errorf("write token file JSON: %w", err) + } + if err := temp.Close(); err != nil { + return fmt.Errorf("close temporary token file: %w", err) + } + + if err := os.Rename(tempName, path); err != nil { + return fmt.Errorf("replace token file: %w", err) + } + if runtime.GOOS != "windows" { + if err := os.Chmod(path, 0o600); err != nil { + return fmt.Errorf("set token file permissions: %w", err) + } + } + + return nil +} + +func newFileStoreData() fileStoreData { + return fileStoreData{ + Version: fileStoreVersion, + Tokens: make(map[string]string), + } +} + +func getFileToken(path, baseURL string) (string, error) { + store, err := readFileStore(path) + if err != nil { + return "", err + } + return store.Tokens[baseURL], nil +} + +func saveFileToken(path, baseURL, token string) error { + store, err := readFileStore(path) + if err != nil { + return err + } + store.Tokens[baseURL] = token + if err := writeFileStore(path, store); err != nil { + return err + } + return nil +} + +func deleteFileToken(path, baseURL string) error { + if _, err := os.Stat(path); os.IsNotExist(err) { + return nil + } else if err != nil { + return fmt.Errorf("stat token file: %w", err) + } + + store, err := readFileStore(path) + if err != nil { + return err + } + if _, ok := store.Tokens[baseURL]; !ok { + return nil + } + delete(store.Tokens, baseURL) + if err := writeFileStore(path, store); err != nil { + return err + } + return nil +} diff --git a/cmd/entire/cli/auth/store.go b/cmd/entire/cli/auth/store.go index 729bf69eb4..224eaadde2 100644 --- a/cmd/entire/cli/auth/store.go +++ b/cmd/entire/cli/auth/store.go @@ -11,9 +11,12 @@ import ( const keyringService = "entire-cli" -// Store manages CLI authentication tokens via a pluggable backend. The -// production binary always resolves to the OS keyring. A file-backed -// backend is available only in builds tagged `authfilestore` (used by +// Store manages CLI authentication tokens across the configured auth sources. +// Reads resolve in priority order: the ENTIRE_AUTH_TOKEN environment variable +// (scoped to the production API origin), a user-configured file store at +// ENTIRE_SECRETS_PATH (the headless-environments path from issue #1036), or +// the pluggable tokenBackend (OS keyring in production; a separate +// file-backed backend gated behind the `authfilestore` build tag, used by // integration tests to avoid the OS keychain). type Store struct { service string @@ -36,7 +39,9 @@ type tokenBackend interface { var chooseBackend = func() tokenBackend { return keyringBackend{} } // NewStore returns a Store backed by the system keyring (or, in -// `authfilestore` builds, optionally a file-backed test store). +// `authfilestore` builds, optionally a file-backed test store). When +// ENTIRE_SECRETS_PATH is set, reads and writes route to that file +// instead, ahead of the backend. func NewStore() *Store { return &Store{service: keyringService, backend: chooseBackend()} } @@ -55,18 +60,78 @@ func (s *Store) SaveToken(baseURL, token string) error { if token == "" { return errors.New("refusing to save empty token") } + + if path, ok, err := configuredSecretsPath(); err != nil { + return err + } else if ok { + if err := saveFileToken(path, baseURL, token); err != nil { + return fmt.Errorf("save token to file: %w", err) + } + return nil + } + return s.backend.save(s.service, baseURL, token) } // GetToken retrieves a stored token for the given base URL. // Returns an empty string (and no error) if no token is stored. func (s *Store) GetToken(baseURL string) (string, error) { - return s.backend.get(s.service, baseURL) + info, err := s.GetTokenInfo(baseURL) + if err != nil { + return "", err + } + + return info.Value, nil +} + +// GetTokenInfo retrieves a stored token and reports where it came from. +// Returns an empty TokenInfo (and no error) if no token is stored. +func (s *Store) GetTokenInfo(baseURL string) (TokenInfo, error) { + // ENTIRE_AUTH_TOKEN is scoped to the production API origin so a stray + // ENTIRE_API_BASE_URL override can't leak a prod bearer to a staging + // or local endpoint. Custom origins must use the per-origin file or + // keyring stores, which are already keyed by baseURL. + if baseURL == api.DefaultBaseURL { + if token := envAuthToken(); token != "" { + return TokenInfo{Value: token, Source: TokenSourceEnv}, nil + } + } + + if path, ok, err := configuredSecretsPath(); err != nil { + return TokenInfo{}, err + } else if ok { + token, err := getFileToken(path, baseURL) + if err != nil { + return TokenInfo{}, fmt.Errorf("get token from file: %w", err) + } + if token == "" { + return TokenInfo{}, nil + } + return TokenInfo{Value: token, Source: TokenSourceFile, Path: path}, nil + } + + token, err := s.backend.get(s.service, baseURL) + if err != nil { + return TokenInfo{}, err + } + if token == "" { + return TokenInfo{}, nil + } + return TokenInfo{Value: token, Source: TokenSourceKeyring}, nil } // DeleteToken removes a stored token for the given base URL. // Returns no error if the token does not exist. func (s *Store) DeleteToken(baseURL string) error { + if path, ok, err := configuredSecretsPath(); err != nil { + return err + } else if ok { + if err := deleteFileToken(path, baseURL); err != nil { + return fmt.Errorf("delete token from file: %w", err) + } + return nil + } + return s.backend.delete(s.service, baseURL) } diff --git a/cmd/entire/cli/auth/store_invariants_test.go b/cmd/entire/cli/auth/store_invariants_test.go index 098a008a02..679ff2cce7 100644 --- a/cmd/entire/cli/auth/store_invariants_test.go +++ b/cmd/entire/cli/auth/store_invariants_test.go @@ -7,24 +7,33 @@ import ( "testing" ) -// TestProductionAuthStoreIsKeyringOnly enforces that the file-backed auth -// store backend stays fully gated behind //go:build authfilestore. The -// production CLI binary (no build tag) must not contain code that reads -// the test env var or persists tokens to disk — otherwise an end user -// can flip the variable in their shell and silently bypass the OS -// keyring. +// TestProductionAuthStoreIsKeyringOnly enforces that the test-only +// file-backed auth backend stays fully gated behind //go:build authfilestore. +// The production CLI binary (no build tag) must not contain code that reads +// the test env var (ENTIRE_TEST_AUTH_STORE_FILE), and arbitrary new code in +// this package must not persist tokens to disk by accident — otherwise an +// end user could silently bypass the OS keyring. +// +// One production file is the deliberate exception: file_store.go provides +// the ENTIRE_SECRETS_PATH headless-auth store from issue #1036. It is +// explicitly opt-in (user must set the env var to an absolute path), it +// enforces 0600 permissions on read and write, and it writes via temp+rename +// with a versioned JSON schema. That file is allowlisted below. // // This runs as part of the regular (untagged) test suite, so any new -// production-compiled .go file in this package that mentions the -// forbidden symbols will fail CI immediately. The remediation is to -// move the offending code into a //go:build authfilestore file -// alongside store_filebackend.go. +// production-compiled .go file in this package that mentions the forbidden +// symbols will fail CI immediately. The remediation is either to move the +// offending code into a //go:build authfilestore file (see +// store_filebackend.go) or, if the addition is another deliberate +// production exception, to add it to productionFileStoreAllowlist with a +// matching design rationale. func TestProductionAuthStoreIsKeyringOnly(t *testing.T) { t.Parallel() - // Symbols that may only appear in authfilestore-tagged files. - // Keep this list tight — these are the load-bearing markers of - // "we are persisting auth tokens to a file from production code". + // Symbols that may only appear in authfilestore-tagged files or in + // productionFileStoreAllowlist files. Keep this list tight — these + // are the load-bearing markers of "we are persisting auth tokens to + // a file from production code". forbidden := []string{ "ENTIRE_TEST_AUTH_STORE_FILE", // the test env-var hook "os.WriteFile", // token-on-disk write @@ -57,13 +66,32 @@ func TestProductionAuthStoreIsKeyringOnly(t *testing.T) { continue } + if productionFileStoreAllowlist[name] { + // ENTIRE_TEST_AUTH_STORE_FILE is still forbidden even in + // allowlisted production files — that symbol is the + // test-only hook and must never leak. Other forbidden + // symbols are permitted because the allowlisted file has + // been signed off as a deliberate production exception. + if strings.Contains(src, "ENTIRE_TEST_AUTH_STORE_FILE") { + t.Errorf( + "%s is allowlisted as a production file store but references "+ + "ENTIRE_TEST_AUTH_STORE_FILE. The test env-var hook must "+ + "never appear outside an authfilestore-tagged file.", + name, + ) + } + continue + } + for _, sym := range forbidden { if strings.Contains(src, sym) { t.Errorf( "%s references %q outside a //go:build authfilestore file. "+ "File-backed auth storage must stay gated so production "+ "builds cannot opt into it. Move this code into a tagged "+ - "file (see store_filebackend.go).", + "file (see store_filebackend.go), or — if this is a new "+ + "deliberate production exception — add it to "+ + "productionFileStoreAllowlist with a written rationale.", name, sym, ) } @@ -71,6 +99,21 @@ func TestProductionAuthStoreIsKeyringOnly(t *testing.T) { } } +// productionFileStoreAllowlist names production .go files in this package +// that are permitted to do file I/O. Each entry is a deliberate exception +// to the keyring-only policy enforced by TestProductionAuthStoreIsKeyringOnly, +// and the rationale lives on the file itself. +// +// Keep this list short. Every entry expands the auth-package's disk +// footprint and must be reviewed alongside the file's own design notes. +var productionFileStoreAllowlist = map[string]bool{ + // file_store.go — the ENTIRE_SECRETS_PATH headless-auth store from + // issue #1036. Opt-in via env var, 0600 perm enforcement, atomic + // temp+rename writes, versioned JSON schema. See the file's package + // doc for the full design. + "file_store.go": true, +} + // hasAuthFileStoreBuildTag reports whether the file's build constraint // requires the authfilestore tag. Build constraints must appear before // the package clause, so we only scan up to that point. diff --git a/cmd/entire/cli/auth/store_test.go b/cmd/entire/cli/auth/store_test.go index 8ccf122372..0b92d771d3 100644 --- a/cmd/entire/cli/auth/store_test.go +++ b/cmd/entire/cli/auth/store_test.go @@ -2,12 +2,21 @@ package auth import ( "os" + "path/filepath" + "runtime" + "strings" "testing" "github.com/entireio/cli/cmd/entire/cli/api" "github.com/zalando/go-keyring" ) +const ( + testLocalToken = "local-token" + testEnvToken = "env-token" + testWindowsOS = "windows" +) + func TestMain(m *testing.M) { keyring.MockInit() os.Exit(m.Run()) @@ -43,6 +52,72 @@ func TestStoreGetToken_NotFound(t *testing.T) { } } +func TestStoreGetTokenInfo_EnvTokenTakesPrecedence(t *testing.T) { + // Not parallel: go-keyring's mock provider uses an unprotected map. + t.Setenv(AuthTokenEnvVar, " env-token ") + + store := NewStoreWithService("test-env-precedence") + if err := store.SaveToken("https://entire.io", "keyring-token"); err != nil { + t.Fatalf("SaveToken() error = %v", err) + } + + info, err := store.GetTokenInfo("https://entire.io") + if err != nil { + t.Fatalf("GetTokenInfo() error = %v", err) + } + if info.Value != testEnvToken || info.Source != TokenSourceEnv { + t.Fatalf("GetTokenInfo() = %#v, want env token/source", info) + } +} + +func TestStoreGetTokenInfo_WhitespaceEnvTokenIgnored(t *testing.T) { + // Not parallel: go-keyring's mock provider uses an unprotected map. + t.Setenv(AuthTokenEnvVar, " ") + + store := NewStoreWithService("test-env-blank") + if err := store.SaveToken("https://entire.io", "keyring-token"); err != nil { + t.Fatalf("SaveToken() error = %v", err) + } + + info, err := store.GetTokenInfo("https://entire.io") + if err != nil { + t.Fatalf("GetTokenInfo() error = %v", err) + } + if info.Value != "keyring-token" || info.Source != TokenSourceKeyring { + t.Fatalf("GetTokenInfo() = %#v, want keyring token/source", info) + } +} + +func TestStoreGetTokenInfo_EnvTokenScopedToDefaultBaseURL(t *testing.T) { + // Not parallel: go-keyring's mock provider uses an unprotected map. + t.Setenv(AuthTokenEnvVar, testEnvToken) + + store := NewStoreWithService("test-env-scope") + if err := store.SaveToken("http://localhost:8787", "local-keyring-token"); err != nil { + t.Fatalf("SaveToken() error = %v", err) + } + + // Custom base URL must NOT receive ENTIRE_AUTH_TOKEN — falls through to + // the per-origin keyring/file store so a prod bearer can't leak to a + // staging/dev override. + info, err := store.GetTokenInfo("http://localhost:8787") + if err != nil { + t.Fatalf("GetTokenInfo(custom) error = %v", err) + } + if info.Value != "local-keyring-token" || info.Source != TokenSourceKeyring { + t.Fatalf("GetTokenInfo(custom) = %#v, want local-keyring-token/keyring", info) + } + + // Default base URL still gets ENTIRE_AUTH_TOKEN. + info, err = store.GetTokenInfo(api.DefaultBaseURL) + if err != nil { + t.Fatalf("GetTokenInfo(default) error = %v", err) + } + if info.Value != testEnvToken || info.Source != TokenSourceEnv { + t.Fatalf("GetTokenInfo(default) = %#v, want env-token/env", info) + } +} + func TestStoreSaveToken_PreservesOtherBaseURLs(t *testing.T) { // Not parallel: go-keyring's mock provider uses an unprotected map. store := NewStoreWithService("test-preserve") @@ -51,7 +126,7 @@ func TestStoreSaveToken_PreservesOtherBaseURLs(t *testing.T) { t.Fatalf("SaveToken(prod) error = %v", err) } - if err := store.SaveToken("http://localhost:8787", "local-token"); err != nil { + if err := store.SaveToken("http://localhost:8787", testLocalToken); err != nil { t.Fatalf("SaveToken(local) error = %v", err) } @@ -67,8 +142,8 @@ func TestStoreSaveToken_PreservesOtherBaseURLs(t *testing.T) { if err != nil { t.Fatalf("GetToken(local) error = %v", err) } - if local != "local-token" { - t.Fatalf("local token = %q, want %q", local, "local-token") + if local != testLocalToken { + t.Fatalf("local token = %q, want %q", local, testLocalToken) } } @@ -132,11 +207,171 @@ func TestStoreDeleteToken_NotFoundIsNoop(t *testing.T) { } } +func newFileBackendTestStore(t *testing.T, service string) (*Store, string) { + t.Helper() + + path := filepath.Join(t.TempDir(), "credentials.json") + t.Setenv(SecretsPathEnvVar, path) + + return NewStoreWithService(service), path +} + +func TestStoreFileBackend_SaveGetDelete(t *testing.T) { + // Not parallel: auth env vars are process-global. + store, path := newFileBackendTestStore(t, "test-file-backend") + if err := store.SaveToken("https://entire.io", "file-token"); err != nil { + t.Fatalf("SaveToken() error = %v", err) + } + + info, err := store.GetTokenInfo("https://entire.io") + if err != nil { + t.Fatalf("GetTokenInfo() error = %v", err) + } + if info.Value != "file-token" || info.Source != TokenSourceFile || info.Path != path { + t.Fatalf("GetTokenInfo() = %#v, want file token/source/path", info) + } + + if runtime.GOOS != testWindowsOS { + stat, statErr := os.Stat(path) + if statErr != nil { + t.Fatalf("Stat() error = %v", statErr) + } + if got := stat.Mode().Perm(); got != 0o600 { + t.Fatalf("credential file mode = %o, want 600", got) + } + } + + if err := store.DeleteToken("https://entire.io"); err != nil { + t.Fatalf("DeleteToken() error = %v", err) + } + got, err := store.GetToken("https://entire.io") + if err != nil { + t.Fatalf("GetToken() error = %v", err) + } + if got != "" { + t.Fatalf("GetToken() after delete = %q, want empty", got) + } +} + +func TestStoreFileBackend_PreservesOtherBaseURLs(t *testing.T) { + // Not parallel: auth env vars are process-global. + store, _ := newFileBackendTestStore(t, "test-file-preserve") + if err := store.SaveToken("https://entire.io", "prod-token"); err != nil { + t.Fatalf("SaveToken(prod) error = %v", err) + } + if err := store.SaveToken("http://localhost:8787", testLocalToken); err != nil { + t.Fatalf("SaveToken(local) error = %v", err) + } + if err := store.DeleteToken("https://entire.io"); err != nil { + t.Fatalf("DeleteToken(prod) error = %v", err) + } + + got, err := store.GetToken("http://localhost:8787") + if err != nil { + t.Fatalf("GetToken(local) error = %v", err) + } + if got != testLocalToken { + t.Fatalf("local token = %q, want %q", got, testLocalToken) + } +} + +func TestStoreFileBackend_EnvTokenTakesPrecedence(t *testing.T) { + // Not parallel: auth env vars are process-global. + store, _ := newFileBackendTestStore(t, "test-file-env-precedence") + t.Setenv(AuthTokenEnvVar, testEnvToken) + + if err := store.SaveToken("https://entire.io", "file-token"); err != nil { + t.Fatalf("SaveToken() error = %v", err) + } + + info, err := store.GetTokenInfo("https://entire.io") + if err != nil { + t.Fatalf("GetTokenInfo() error = %v", err) + } + if info.Value != testEnvToken || info.Source != TokenSourceEnv { + t.Fatalf("GetTokenInfo() = %#v, want env token/source", info) + } +} + +func TestStoreFileBackend_RejectsRelativePath(t *testing.T) { + // Not parallel: auth env vars are process-global. + t.Setenv(SecretsPathEnvVar, "credentials.json") + + store := NewStoreWithService("test-file-relative") + if _, err := store.GetTokenInfo("https://entire.io"); err == nil || !strings.Contains(err.Error(), "absolute") { + t.Fatalf("GetTokenInfo() err = %v, want absolute-path error", err) + } + if err := store.SaveToken("https://entire.io", "tok"); err == nil || !strings.Contains(err.Error(), "absolute") { + t.Fatalf("SaveToken() err = %v, want absolute-path error", err) + } + if err := store.DeleteToken("https://entire.io"); err == nil || !strings.Contains(err.Error(), "absolute") { + t.Fatalf("DeleteToken() err = %v, want absolute-path error", err) + } +} + +func TestStoreFileBackend_RejectsMalformedJSON(t *testing.T) { + // Not parallel: auth env vars are process-global. + store, path := newFileBackendTestStore(t, "test-file-malformed") + if err := os.WriteFile(path, []byte("{not-json"), 0o600); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + if _, err := store.GetTokenInfo("https://entire.io"); err == nil || !strings.Contains(err.Error(), "parse") { + t.Fatalf("GetTokenInfo() err = %v, want parse error", err) + } +} + +func TestStoreFileBackend_RejectsGroupReadableFile(t *testing.T) { + if runtime.GOOS == testWindowsOS { + t.Skip("permission bit checks are Unix-specific") + } + // Not parallel: auth env vars are process-global. + store, path := newFileBackendTestStore(t, "test-file-perms") + if err := os.WriteFile(path, []byte(`{"version":1,"tokens":{"https://entire.io":"tok"}}`), 0o640); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + if _, err := store.GetTokenInfo("https://entire.io"); err == nil || !strings.Contains(err.Error(), "chmod 600") { + t.Fatalf("GetTokenInfo() err = %v, want chmod 600 hint", err) + } +} + +func TestStoreFileBackend_DeleteMissingFileDoesNotCreateFile(t *testing.T) { + // Not parallel: auth env vars are process-global. + store, path := newFileBackendTestStore(t, "test-file-delete-missing") + if err := store.DeleteToken("https://entire.io"); err != nil { + t.Fatalf("DeleteToken() error = %v", err) + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("Stat() err = %v, want file to remain absent", err) + } +} + +func TestStoreFileBackend_DeleteMissingTokenPreservesExistingToken(t *testing.T) { + // Not parallel: auth env vars are process-global. + store, _ := newFileBackendTestStore(t, "test-file-delete-missing-token") + if err := store.SaveToken("https://entire.io", "file-token"); err != nil { + t.Fatalf("SaveToken() error = %v", err) + } + + if err := store.DeleteToken("https://missing.example.com"); err != nil { + t.Fatalf("DeleteToken() error = %v", err) + } + + got, err := store.GetToken("https://entire.io") + if err != nil { + t.Fatalf("GetToken() error = %v", err) + } + if got != "file-token" { + t.Fatalf("GetToken() = %q, want existing token preserved", got) + } +} + func TestLookupCurrentToken(t *testing.T) { t.Setenv(api.BaseURLEnvVar, "http://localhost:8787") store := NewStore() - if err := store.SaveToken("http://localhost:8787", "local-token"); err != nil { + if err := store.SaveToken("http://localhost:8787", testLocalToken); err != nil { t.Fatalf("SaveToken() error = %v", err) } @@ -144,7 +379,7 @@ func TestLookupCurrentToken(t *testing.T) { if err != nil { t.Fatalf("LookupCurrentToken() error = %v", err) } - if got != "local-token" { - t.Fatalf("LookupCurrentToken() = %q, want %q", got, "local-token") + if got != testLocalToken { + t.Fatalf("LookupCurrentToken() = %q, want %q", got, testLocalToken) } } diff --git a/cmd/entire/cli/auth_test.go b/cmd/entire/cli/auth_test.go index 9acd3fe848..0480185145 100644 --- a/cmd/entire/cli/auth_test.go +++ b/cmd/entire/cli/auth_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/entireio/cli/cmd/entire/cli/api" + "github.com/entireio/cli/cmd/entire/cli/auth" ) const ( @@ -70,6 +71,54 @@ func TestRunAuthStatus_LoggedIn(t *testing.T) { } } +func TestRunAuthStatus_SourceLabels(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + source auth.TokenSource + path string + want string + }{ + "env": { + source: auth.TokenSourceEnv, + want: "Token: supplied by " + auth.AuthTokenEnvVar, + }, + "file": { + source: auth.TokenSourceFile, + path: "/tmp/entire-credentials.json", + want: "Token: stored in file /tmp/entire-credentials.json", + }, + "keyring": { + source: auth.TokenSourceKeyring, + want: "Token: stored in OS keychain", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + store := newMockTokenStore() + store.tokens[testBaseURL] = testAuthTok + store.source = tt.source + store.path = tt.path + + list := func(context.Context, string) ([]api.Token, error) { + return []api.Token{{ID: "a", Name: "laptop"}}, nil + } + + var out bytes.Buffer + if err := runAuthStatus(context.Background(), &out, store, list, testBaseURL); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !strings.Contains(out.String(), tt.want) { + t.Fatalf("output = %q, want %q", out.String(), tt.want) + } + }) + } +} + func TestRunAuthStatus_TokenInvalid(t *testing.T) { t.Parallel() @@ -93,6 +142,30 @@ func TestRunAuthStatus_TokenInvalid(t *testing.T) { } } +func TestRunAuthStatus_EnvTokenInvalidSuggestsUpdatingEnv(t *testing.T) { + t.Parallel() + + store := newMockTokenStore() + store.tokens[testBaseURL] = testAuthTok + store.source = auth.TokenSourceEnv + + list := func(context.Context, string) ([]api.Token, error) { + return nil, &api.HTTPError{StatusCode: http.StatusUnauthorized, Message: "Not authenticated"} + } + + var out bytes.Buffer + if err := runAuthStatus(context.Background(), &out, store, list, testBaseURL); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !strings.Contains(out.String(), auth.AuthTokenEnvVar) { + t.Fatalf("output = %q, want env-token guidance", out.String()) + } + if strings.Contains(out.String(), "entire login") { + t.Fatalf("output = %q, should not suggest login while env token has precedence", out.String()) + } +} + func TestRunAuthStatus_ServerError(t *testing.T) { t.Parallel() @@ -408,6 +481,114 @@ func TestRunAuthRevoke_CurrentDelegatesToLogout(t *testing.T) { } } +// TestRunAuthRevoke_CurrentEnvToken covers the env-source --current path: +// - success → "Revoked" message, exit 0 +// - 401 (already invalid) → idempotent success: same message, exit 0 +// - non-401 transient error → return error; do NOT print "Revoked" because +// the bearer token is still active server-side +// +// All cases must dispatch to revokeCurrent (not revokeByID) and leave any +// lower-priority stored credentials untouched. +func TestRunAuthRevoke_CurrentEnvToken(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + revokeErr error + wantSuccessMsg bool // true = expect "Revoked" stdout + nil error + wantErrSubstr string + }{ + "success": {nil, true, ""}, + "already invalid": {&api.HTTPError{StatusCode: http.StatusUnauthorized, Message: "Not authenticated"}, true, ""}, + "transient error": {errors.New("connection refused"), false, "connection refused"}, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + store := newMockTokenStore() + store.tokens[testBaseURL] = testAuthTok + store.source = auth.TokenSourceEnv + + revokedToken := "" + revokeCurrent := func(_ context.Context, token string) error { + revokedToken = token + return tt.revokeErr + } + + revokeByIDCalled := false + revokeByID := func(context.Context, string, string) error { + revokeByIDCalled = true + return nil + } + + var out, errOut bytes.Buffer + err := runAuthRevoke(context.Background(), &out, &errOut, store, + func(context.Context, string) ([]api.Token, error) { return nil, nil }, + revokeByID, revokeCurrent, testBaseURL, "", true) + + if revokeByIDCalled { + t.Fatal("revokeByID should not be called when --current is set") + } + if revokedToken != testAuthTok { + t.Errorf("revokeCurrent token = %q, want %q", revokedToken, testAuthTok) + } + if store.deleted[testBaseURL] { + t.Fatal("env-token revoke should not delete lower-priority credentials") + } + + if tt.wantSuccessMsg { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(out.String(), auth.AuthTokenEnvVar) { + t.Fatalf("output = %q, want env-token guidance", out.String()) + } + return + } + + if err == nil { + t.Fatal("expected error for non-401 server failure") + } + if !strings.Contains(err.Error(), tt.wantErrSubstr) { + t.Fatalf("error = %v, want substring %q", err, tt.wantErrSubstr) + } + if strings.Contains(out.String(), "Revoked") { + t.Fatalf("stdout = %q, must not claim revoke success when revoke failed", out.String()) + } + }) + } +} + +func TestRunAuthRevoke_ByIDEnvSelfRevokeDoesNotDeleteLowerPriority(t *testing.T) { + t.Parallel() + + store := newMockTokenStore() + store.tokens[testBaseURL] = testAuthTok + store.source = auth.TokenSourceEnv + + revokeByID := func(context.Context, string, string) error { return nil } + revokeCurrent := func(context.Context, string) error { return nil } + + list := func(context.Context, string) ([]api.Token, error) { + return nil, &api.HTTPError{StatusCode: http.StatusUnauthorized, Message: "Not authenticated"} + } + + var out, errOut bytes.Buffer + err := runAuthRevoke(context.Background(), &out, &errOut, store, + list, revokeByID, revokeCurrent, testBaseURL, testTokenID, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if store.deleted[testBaseURL] { + t.Fatal("env-token self-revoke should not delete lower-priority credentials") + } + if !strings.Contains(out.String(), auth.AuthTokenEnvVar) { + t.Fatalf("output = %q, want env-token guidance", out.String()) + } +} + func TestRunAuthRevoke_NotLoggedInErrors(t *testing.T) { t.Parallel() diff --git a/cmd/entire/cli/integration_test/login_test.go b/cmd/entire/cli/integration_test/login_test.go index 2954365147..9e86f9dbe9 100644 --- a/cmd/entire/cli/integration_test/login_test.go +++ b/cmd/entire/cli/integration_test/login_test.go @@ -11,7 +11,9 @@ import ( "io" "net/http" "net/http/httptest" + "os" "path/filepath" + "runtime" "strings" "sync" "testing" @@ -108,6 +110,92 @@ func TestLogin_SavesTokenAfterApproval(t *testing.T) { } } +func TestLogin_SavesTokenToSecretsPathAfterApproval(t *testing.T) { + t.Parallel() + + type state struct { + sync.Mutex + approved bool + } + + serverState := &state{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.Path == "/oauth/device/code": + writeJSON(t, w, http.StatusOK, map[string]any{ + "device_code": "device-123", + "user_code": "ABCD-EFGH", + "verification_uri": serverURLWithPath(r, "/approve"), + "verification_uri_complete": serverURLWithPath(r, "/approve?code=ABCD-EFGH"), + "expires_in": 10, + "interval": 1, + }) + case r.Method == http.MethodPost && r.URL.Path == "/oauth/token": + serverState.Lock() + approved := serverState.approved + serverState.Unlock() + + if !approved { + writeJSON(t, w, http.StatusBadRequest, map[string]any{"error": "authorization_pending"}) + return + } + + writeJSON(t, w, http.StatusOK, map[string]any{"access_token": "local-token", "token_type": "Bearer", "expires_in": 3600, "scope": "cli"}) + case r.Method == http.MethodPost && r.URL.Path == "/approve": + serverState.Lock() + serverState.approved = true + serverState.Unlock() + writeJSON(t, w, http.StatusOK, map[string]any{"success": true}) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + credentialsPath := filepath.Join(t.TempDir(), "credentials.json") + proc := runLoginProcess(t, server.URL, "ENTIRE_SECRETS_PATH="+credentialsPath) + + approvalURL, _ := waitForLoginPrompt(t, proc.stdout) + approveResp, doErr := http.Post(approvalURL, "application/json", http.NoBody) + if doErr != nil { + t.Fatalf("approve request failed: %v", doErr) + } + _ = approveResp.Body.Close() + + output, waitErr := proc.wait() + if waitErr != nil { + t.Fatalf("login command failed: %v\nOutput:\n%s", waitErr, output) + } + if !strings.Contains(output, "Login complete.") { + t.Fatalf("output missing login complete message:\n%s", output) + } + + if runtime.GOOS != "windows" { + stat, statErr := os.Stat(credentialsPath) + if statErr != nil { + t.Fatalf("stat credentials file: %v", statErr) + } + if got := stat.Mode().Perm(); got != 0o600 { + t.Fatalf("credentials mode = %o, want 600", got) + } + } + + data, readErr := os.ReadFile(credentialsPath) + if readErr != nil { + t.Fatalf("read credentials file: %v", readErr) + } + var credentials struct { + Version int `json:"version"` + Tokens map[string]string `json:"tokens"` + } + if err := json.Unmarshal(data, &credentials); err != nil { + t.Fatalf("unmarshal credentials: %v", err) + } + if credentials.Tokens[server.URL] != "local-token" { + t.Fatalf("stored token for %s = %q, want local-token", server.URL, credentials.Tokens[server.URL]) + } +} + func TestLogin_ExpiredFlow(t *testing.T) { t.Parallel() @@ -191,7 +279,7 @@ type loginProcess struct { waitFn func() (string, error) } -func runLoginProcess(t *testing.T, apiBaseURL string) *loginProcess { +func runLoginProcess(t *testing.T, apiBaseURL string, extraEnv ...string) *loginProcess { t.Helper() env := NewTestEnv(t) @@ -205,6 +293,7 @@ func runLoginProcess(t *testing.T, apiBaseURL string) *loginProcess { "ENTIRE_API_BASE_URL="+apiBaseURL, "ENTIRE_TEST_AUTH_STORE_FILE="+filepath.Join(env.RepoDir, ".entire-test-auth-store.json"), ) + cmd.Env = append(cmd.Env, extraEnv...) stdoutPipe, err := cmd.StdoutPipe() if err != nil { diff --git a/cmd/entire/cli/login.go b/cmd/entire/cli/login.go index 88618c522e..d693018189 100644 --- a/cmd/entire/cli/login.go +++ b/cmd/entire/cli/login.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "runtime" + "strings" "time" "github.com/entireio/cli/cmd/entire/cli/auth" @@ -33,8 +34,13 @@ type deviceAuthClient interface { BaseURL() string } +type loginTokenStore interface { + SaveToken(baseURL, token string) error +} + func newLoginCmd() *cobra.Command { var insecureHTTPAuth bool + var noKeyring bool cmd := &cobra.Command{ Use: "login", Short: "Log in to Entire", @@ -42,14 +48,36 @@ func newLoginCmd() *cobra.Command { if err := requireSecureBaseURL(insecureHTTPAuth); err != nil { return err } + if err := requireNonKeyringStorageConfigured(noKeyring); err != nil { + return err + } return runLogin(cmd.Context(), cmd.OutOrStdout(), cmd.ErrOrStderr(), auth.NewClient(nil), openBrowser) }, } + cmd.Flags().BoolVar(&noKeyring, "no-keyring", false, "Refuse to save the token to the OS keyring; requires ENTIRE_SECRETS_PATH to be set") addInsecureHTTPAuthFlag(cmd, &insecureHTTPAuth) return cmd } +// requireNonKeyringStorageConfigured fails closed before device auth when +// --no-keyring is set without a non-keyring backend configured. The flag +// exists for headless/CI environments where falling back to the OS keyring +// silently is unsafe — the user has explicitly opted out of it. +func requireNonKeyringStorageConfigured(noKeyring bool) error { + if !noKeyring { + return nil + } + if strings.TrimSpace(os.Getenv(auth.SecretsPathEnvVar)) == "" { + return fmt.Errorf("--no-keyring requires %s to be set to an absolute file path", auth.SecretsPathEnvVar) + } + return nil +} + func runLogin(ctx context.Context, outW, errW io.Writer, client deviceAuthClient, openURL browserOpenFunc) error { + return runLoginWithStore(ctx, outW, errW, client, openURL, auth.NewStore()) +} + +func runLoginWithStore(ctx context.Context, outW, errW io.Writer, client deviceAuthClient, openURL browserOpenFunc, store loginTokenStore) error { start, err := client.StartDeviceAuth(ctx) if err != nil { return fmt.Errorf("start login: %w", err) @@ -84,12 +112,17 @@ func runLogin(ctx context.Context, outW, errW io.Writer, client deviceAuthClient return fmt.Errorf("complete login: %w", err) } - store := auth.NewStore() - if err := store.SaveToken(client.BaseURL(), token); err != nil { + if strings.TrimSpace(os.Getenv(auth.SecretsPathEnvVar)) == "" { + fmt.Fprintf(errW, "For headless environments, set %s=/path/to/credentials.json and retry `entire login`.\n", auth.SecretsPathEnvVar) + } return fmt.Errorf("save auth token: %w", err) } + if strings.TrimSpace(os.Getenv(auth.AuthTokenEnvVar)) != "" { + fmt.Fprintf(errW, "Warning: %s is set and will take precedence over the newly saved login token until it is unset.\n", auth.AuthTokenEnvVar) + } + fmt.Fprintln(outW, "Login complete.") return nil } diff --git a/cmd/entire/cli/login_test.go b/cmd/entire/cli/login_test.go index 63834fec3a..a40f19174c 100644 --- a/cmd/entire/cli/login_test.go +++ b/cmd/entire/cli/login_test.go @@ -1,6 +1,7 @@ package cli import ( + "bytes" "context" "errors" "strings" @@ -13,6 +14,8 @@ import ( // mockClient implements deviceAuthClient for unit tests. type mockClient struct { responses []pollResponse + start *auth.DeviceAuthStart + startErr error calls int } @@ -22,7 +25,13 @@ type pollResponse struct { } func (m *mockClient) StartDeviceAuth(_ context.Context) (*auth.DeviceAuthStart, error) { - return nil, errors.New("not implemented in mock") + if m.startErr != nil { + return nil, m.startErr + } + if m.start == nil { + return nil, errors.New("not implemented in mock") + } + return m.start, nil } func (m *mockClient) BaseURL() string { @@ -38,6 +47,112 @@ func (m *mockClient) PollDeviceAuth(_ context.Context, _ string) (*auth.DeviceAu return r.result, r.err } +type mockLoginTokenStore struct { + saveErr error + baseURL string + savedToken string +} + +func (m *mockLoginTokenStore) SaveToken(baseURL, token string) error { + m.baseURL = baseURL + m.savedToken = token + return m.saveErr +} + +func newSuccessfulLoginClient(token string) *mockClient { + return &mockClient{ + start: &auth.DeviceAuthStart{ + DeviceCode: "device-1", + UserCode: "ABCD-EFGH", + VerificationURI: "https://entire.io/cli/auth", + ExpiresIn: 60, + Interval: 1, + }, + responses: []pollResponse{ + {result: &auth.DeviceAuthPoll{AccessToken: token}}, + }, + } +} + +func TestRequireNonKeyringStorageConfigured(t *testing.T) { + t.Run("flag unset is a no-op", func(t *testing.T) { + t.Setenv(auth.SecretsPathEnvVar, "") + if err := requireNonKeyringStorageConfigured(false); err != nil { + t.Fatalf("err = %v, want nil when --no-keyring is unset", err) + } + }) + + t.Run("flag set without ENTIRE_SECRETS_PATH errors before device auth", func(t *testing.T) { + t.Setenv(auth.SecretsPathEnvVar, "") + err := requireNonKeyringStorageConfigured(true) + if err == nil { + t.Fatal("expected error when --no-keyring is set with no SECRETS_PATH") + } + if !strings.Contains(err.Error(), auth.SecretsPathEnvVar) { + t.Fatalf("err = %v, want mention of %s", err, auth.SecretsPathEnvVar) + } + }) + + t.Run("flag set with ENTIRE_SECRETS_PATH passes preflight", func(t *testing.T) { + t.Setenv(auth.SecretsPathEnvVar, "/tmp/credentials.json") + if err := requireNonKeyringStorageConfigured(true); err != nil { + t.Fatalf("err = %v, want nil when SECRETS_PATH is set", err) + } + }) + + t.Run("flag set with whitespace-only ENTIRE_SECRETS_PATH errors", func(t *testing.T) { + t.Setenv(auth.SecretsPathEnvVar, " ") + if err := requireNonKeyringStorageConfigured(true); err == nil { + t.Fatal("whitespace SECRETS_PATH must not satisfy --no-keyring") + } + }) +} + +func TestRunLogin_SaveFailureIncludesHeadlessHint(t *testing.T) { + client := newSuccessfulLoginClient("login-token") + store := &mockLoginTokenStore{ + saveErr: errors.New("save token to keyring: failed to unlock correct collection"), + } + + var out, errOut bytes.Buffer + err := runLoginWithStore(context.Background(), &out, &errOut, client, + func(context.Context, string) error { return nil }, + store) + if err == nil { + t.Fatal("expected save failure") + } + if !strings.Contains(err.Error(), "save auth token") { + t.Fatalf("error = %v, want save auth token context", err) + } + if !strings.Contains(errOut.String(), auth.SecretsPathEnvVar) { + t.Fatalf("stderr = %q, want %s hint", errOut.String(), auth.SecretsPathEnvVar) + } +} + +func TestRunLogin_WarnsWhenEnvTokenShadowsSavedToken(t *testing.T) { + t.Setenv(auth.AuthTokenEnvVar, "env-token") + + client := newSuccessfulLoginClient("login-token") + store := &mockLoginTokenStore{} + + var out, errOut bytes.Buffer + err := runLoginWithStore(context.Background(), &out, &errOut, client, + func(context.Context, string) error { return nil }, + store) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if store.savedToken != "login-token" { + t.Fatalf("saved token = %q, want login-token", store.savedToken) + } + if !strings.Contains(errOut.String(), auth.AuthTokenEnvVar) { + t.Fatalf("stderr = %q, want %s warning", errOut.String(), auth.AuthTokenEnvVar) + } + if !strings.Contains(out.String(), "Login complete.") { + t.Fatalf("stdout = %q, want login complete", out.String()) + } +} + func TestWaitForApproval_ImmediateSuccess(t *testing.T) { t.Parallel() diff --git a/cmd/entire/cli/logout.go b/cmd/entire/cli/logout.go index 77c8533976..585587a5a7 100644 --- a/cmd/entire/cli/logout.go +++ b/cmd/entire/cli/logout.go @@ -11,11 +11,12 @@ import ( "github.com/spf13/cobra" ) -// tokenStore abstracts keyring access so commands that read or delete the -// stored bearer token can be unit-tested without hitting the real OS keyring. -// Used by logout and the auth subcommands. +// tokenStore abstracts the auth backend (keyring, file, env) so commands that +// read or delete the stored bearer token can be unit-tested without hitting +// the real OS keyring or filesystem. type tokenStore interface { GetToken(baseURL string) (string, error) + GetTokenInfo(baseURL string) (auth.TokenInfo, error) DeleteToken(baseURL string) error } @@ -45,14 +46,20 @@ func defaultRevokeCurrentToken(ctx context.Context, token string) error { } func runLogout(ctx context.Context, outW, errW io.Writer, store tokenStore, revoke revokeCurrentFunc, baseURL string) error { - token, err := store.GetToken(baseURL) + info, err := store.GetTokenInfo(baseURL) if err != nil { - // Fall through to the local delete: we still want the keyring entry + // Fall through to the local delete: we still want the stored token // gone, even if we couldn't read it well enough to revoke server-side. fmt.Fprintf(errW, "Warning: failed to read token before revocation: %v\n", err) } - if token != "" { - if err := revoke(ctx, token); err != nil && !api.IsHTTPErrorStatus(err, http.StatusUnauthorized) { + if info.Source == auth.TokenSourceEnv { + fmt.Fprintf(outW, "Token is supplied by %s. Unset that environment variable to log out locally.\n", auth.AuthTokenEnvVar) + fmt.Fprintln(outW, "Run 'entire auth revoke --current' to invalidate this token server-side.") + return nil + } + + if info.Value != "" { + if err := revoke(ctx, info.Value); err != nil && !api.IsHTTPErrorStatus(err, http.StatusUnauthorized) { // Best-effort: a transient network error shouldn't block local // logout. A 401 means the token is already invalid server-side, // so the desired state is achieved — no warning needed. diff --git a/cmd/entire/cli/logout_test.go b/cmd/entire/cli/logout_test.go index 28b3b57466..6a995f90e3 100644 --- a/cmd/entire/cli/logout_test.go +++ b/cmd/entire/cli/logout_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/entireio/cli/cmd/entire/cli/api" + "github.com/entireio/cli/cmd/entire/cli/auth" ) const testLogoutToken = "tok123" @@ -18,6 +19,8 @@ type mockTokenStore struct { deleted map[string]bool getErr error deleteErr error + source auth.TokenSource + path string getCalls int deleteCall int } @@ -37,6 +40,22 @@ func (m *mockTokenStore) GetToken(baseURL string) (string, error) { return m.tokens[baseURL], nil } +func (m *mockTokenStore) GetTokenInfo(baseURL string) (auth.TokenInfo, error) { + m.getCalls++ + if m.getErr != nil { + return auth.TokenInfo{}, m.getErr + } + token := m.tokens[baseURL] + if token == "" { + return auth.TokenInfo{}, nil + } + source := m.source + if source == auth.TokenSourceNone { + source = auth.TokenSourceKeyring + } + return auth.TokenInfo{Value: token, Source: source, Path: m.path}, nil +} + func (m *mockTokenStore) DeleteToken(baseURL string) error { m.deleteCall++ if m.deleteErr != nil { @@ -46,6 +65,38 @@ func (m *mockTokenStore) DeleteToken(baseURL string) error { return nil } +func TestRunLogout_EnvTokenDoesNotRevokeOrDelete(t *testing.T) { + t.Parallel() + + store := newMockTokenStore() + store.tokens["https://entire.io"] = testLogoutToken + store.source = auth.TokenSourceEnv + + revokeCalled := false + revoke := func(context.Context, string) error { + revokeCalled = true + return nil + } + + var out, errOut bytes.Buffer + err := runLogout(context.Background(), &out, &errOut, store, revoke, "https://entire.io") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if revokeCalled { + t.Fatal("logout should not revoke env token server-side") + } + if store.deleted["https://entire.io"] { + t.Fatal("logout should not delete lower-priority credentials for env token") + } + if !strings.Contains(out.String(), auth.AuthTokenEnvVar) { + t.Fatalf("stdout = %q, want env-token guidance", out.String()) + } + if errOut.Len() != 0 { + t.Fatalf("stderr = %q, want empty", errOut.String()) + } +} + func TestRunLogout_RevokesServerSideThenDeletesLocally(t *testing.T) { t.Parallel()