diff --git a/go.mod b/go.mod index 52b59de..306e960 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,6 @@ module github.com/chatwoot/cli -go 1.25.5 - -toolchain go1.25.10 +go 1.26.4 require ( github.com/alecthomas/kong v1.15.0 diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index 5c10a6b..4026b95 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -63,6 +63,13 @@ func (c *AuthLoginCmd) Run(app *App) error { if err != nil { return fmt.Errorf("authentication failed: %w", err) } + + // The token is valid, but that doesn't mean it can access the account ID the + // user typed. Verify membership now so a typo/wrong account fails here with a + // clear message instead of as a cryptic 404 on the first account-scoped call. + if err := verifyAccountAccess(profile, cfg.AccountID); err != nil { + return err + } cfg.UserID = profile.ID if err := config.SaveAPIKey(cfg, apiKey); err != nil { @@ -82,6 +89,37 @@ func loginSuccessMessage(name, email string) string { return fmt.Sprintf("Logged in as %s (%s)\n", output.SanitizeText(name), output.SanitizeText(email)) } +// verifyAccountAccess fails login when the entered account ID is not one the +// authenticated user belongs to. The profile payload's accounts array is the +// source of truth. If the instance returns no accounts (older Chatwoot, or a +// token type that omits them), the check is skipped rather than block login. +func verifyAccountAccess(profile *sdk.ProfileResponse, accountID int) error { + if len(profile.Accounts) == 0 { + return nil + } + for _, acc := range profile.Accounts { + if acc.ID == accountID { + return nil + } + } + return fmt.Errorf("account %d is not accessible with this API key; %s", accountID, accessibleAccountsHint(profile.Accounts)) +} + +func accessibleAccountsHint(accounts []sdk.ProfileAccount) string { + parts := make([]string, 0, len(accounts)) + for _, acc := range accounts { + if name := output.SanitizeText(acc.Name); name != "" { + parts = append(parts, fmt.Sprintf("%d (%s)", acc.ID, name)) + } else { + parts = append(parts, strconv.Itoa(acc.ID)) + } + } + if len(parts) == 1 { + return "this key can access account " + parts[0] + } + return "this key can access accounts: " + strings.Join(parts, ", ") +} + func readAPIKey(reader *bufio.Reader) (string, error) { fmt.Print("API Key: ") diff --git a/internal/cmd/auth_test.go b/internal/cmd/auth_test.go index 572ec6d..645e439 100644 --- a/internal/cmd/auth_test.go +++ b/internal/cmd/auth_test.go @@ -3,13 +3,16 @@ package cmd import ( "bytes" "errors" + "io" "net/http" "net/http/httptest" + "os" "strings" "testing" "github.com/chatwoot/cli/internal/config" "github.com/chatwoot/cli/internal/output" + "github.com/chatwoot/cli/internal/sdk" "github.com/zalando/go-keyring" ) @@ -125,6 +128,33 @@ func TestLoginSuccessMessageStripsTerminalControls(t *testing.T) { } } +func TestVerifyAccountAccess(t *testing.T) { + accounts := []sdk.ProfileAccount{ + {ID: 7, Name: "Acme", Role: "administrator"}, + {ID: 9, Name: "Beta", Role: "agent"}, + } + + if err := verifyAccountAccess(&sdk.ProfileResponse{Accounts: accounts}, 9); err != nil { + t.Fatalf("expected access to a member account, got error: %v", err) + } + + err := verifyAccountAccess(&sdk.ProfileResponse{Accounts: accounts}, 42) + if err == nil { + t.Fatal("expected error for non-member account, got nil") + } + // The message should name the accessible accounts so the user can correct the ID. + for _, want := range []string{"42", "7 (Acme)", "9 (Beta)"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("error %q missing %q", err.Error(), want) + } + } + + // No accounts in payload (older instances) → skip rather than block login. + if err := verifyAccountAccess(&sdk.ProfileResponse{}, 42); err != nil { + t.Fatalf("expected skip when no accounts present, got error: %v", err) + } +} + func TestMeAndWhoamiAliasAuthStatus(t *testing.T) { profile := `{ "id": 7, @@ -167,22 +197,23 @@ func TestAuthStatusNotLoggedIn(t *testing.T) { func TestAuthLogoutRemovesKeyringTokenWithoutConfig(t *testing.T) { keyring.MockInit() - if err := keyring.DeleteAll("chatwoot-cli"); err != nil { - t.Fatalf("keyring.DeleteAll: %v", err) - } t.Setenv("HOME", t.TempDir()) t.Setenv(config.APIKeyEnv, "") - if err := keyring.Set("chatwoot-cli", "api-key", "stale-token"); err != nil { - t.Fatalf("keyring.Set: %v", err) + // Seed the token through the production path so it lands under whichever + // keyring service the active build profile uses (prod vs dev), without + // writing config.yaml — this exercises logout with no config present. + seed := &config.Config{BaseURL: "https://app.chatwoot.com", AccountID: 1} + if err := config.SaveAPIKey(seed, "stale-token"); err != nil { + t.Fatalf("SaveAPIKey: %v", err) } if err := (&AuthLogoutCmd{}).Run(&App{}); err != nil { t.Fatalf("Run: %v", err) } - if _, err := keyring.Get("chatwoot-cli", "api-key"); !errors.Is(err, keyring.ErrNotFound) { - t.Fatalf("expected logout to delete stale keyring token, err = %v", err) + if _, _, err := config.ResolveAPIKey(seed); !errors.Is(err, config.ErrAPIKeyNotFound) { + t.Fatalf("expected logout to delete the keyring token, err = %v", err) } } @@ -244,3 +275,108 @@ func TestAuthStatusDoesNotCacheUserIDFromEnvironmentToken(t *testing.T) { t.Fatalf("expected env-token auth status to preserve cached UserID=42, got %d", post.UserID) } } + +// TestAuthLoginVerifiesAccountAccess drives the full `auth login` flow (stdin → +// profile fetch → membership check → persist) to cover the wiring, not just the +// verifyAccountAccess helper. +func TestAuthLoginVerifiesAccountAccess(t *testing.T) { + profileBody := `{"id":5,"name":"Eve","email":"eve@example.com","availability_status":"online","role":"agent",` + + `"accounts":[{"id":7,"name":"Acme","role":"administrator"},{"id":9,"name":"Beta","role":"agent"}]}` + + t.Run("rejects an account the token cannot access", func(t *testing.T) { + server := loginProfileServer(t, profileBody) + defer server.Close() + isolateAuthEnv(t) + + err := runLogin(t, server.URL+"\ntoken\n42\n") + if err == nil { + t.Fatal("expected login to fail for an inaccessible account") + } + for _, want := range []string{"42", "Acme", "Beta"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("error %q should name entered + accessible accounts", err.Error()) + } + } + // Nothing must be persisted when login is rejected. + if cfg, _ := config.Load(); cfg != nil { + t.Fatalf("config was saved despite a rejected login: %#v", cfg) + } + }) + + t.Run("accepts a member account and persists config + key", func(t *testing.T) { + server := loginProfileServer(t, profileBody) + defer server.Close() + isolateAuthEnv(t) + + if err := runLogin(t, server.URL+"\ntoken\n7\n"); err != nil { + t.Fatalf("login: %v", err) + } + + cfg, err := config.Load() + if err != nil || cfg == nil { + t.Fatalf("config not saved: cfg=%#v err=%v", cfg, err) + } + if cfg.AccountID != 7 || cfg.UserID != 5 { + t.Fatalf("saved cfg = %#v, want AccountID 7, UserID 5", cfg) + } + apiKey, source, err := config.ResolveAPIKey(cfg) + if err != nil || apiKey != "token" || source != config.CredentialSourceKeyring { + t.Fatalf("ResolveAPIKey = (%q, %v, %v), want token/keyring", apiKey, source, err) + } + }) +} + +// isolateAuthEnv gives a test its own HOME + mocked keyring and clears the +// CHATWOOT_API_KEY override so credential resolution exercises the keyring path. +func isolateAuthEnv(t *testing.T) { + t.Helper() + keyring.MockInit() + if err := keyring.DeleteAll("chatwoot-cli"); err != nil { + t.Fatalf("keyring.DeleteAll: %v", err) + } + t.Setenv("HOME", t.TempDir()) + t.Setenv(config.APIKeyEnv, "") +} + +func loginProfileServer(t *testing.T, body string) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/profile" { + http.Error(w, "not found", http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(body)) + })) +} + +// runLogin feeds scripted answers to the interactive login prompts via os.Stdin +// and silences the prompt/banner output, returning the command's error. +func runLogin(t *testing.T, stdin string) error { + t.Helper() + + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe: %v", err) + } + if _, err := io.WriteString(w, stdin); err != nil { + t.Fatalf("write stdin: %v", err) + } + _ = w.Close() + + devnull, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0) + if err != nil { + t.Fatalf("open devnull: %v", err) + } + + oldStdin, oldStdout := os.Stdin, os.Stdout + os.Stdin, os.Stdout = r, devnull + defer func() { + os.Stdin, os.Stdout = oldStdin, oldStdout + _ = r.Close() + _ = devnull.Close() + }() + + printer := output.NewPrinter("text", false, false) + return (&AuthLoginCmd{}).Run(&App{Printer: printer}) +} diff --git a/internal/cmd/config.go b/internal/cmd/config.go index 1a65762..2151398 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -39,11 +39,15 @@ func (c *ConfigViewCmd) Run(app *App) error { credential := credentialStatus(cfg) - app.Printer.PrintDetail([]output.KeyValue{ + detail := []output.KeyValue{ {Key: "Base URL", Value: cfg.BaseURL}, {Key: "Account ID", Value: fmt.Sprintf("%d", cfg.AccountID)}, {Key: "Credential", Value: credential}, - }) + } + if config.IsDev { + detail = append(detail, output.KeyValue{Key: "Profile", Value: "dev"}) + } + app.Printer.PrintDetail(detail) return nil } diff --git a/internal/config/CLAUDE.md b/internal/config/CLAUDE.md index 2d97edd..f72105f 100644 --- a/internal/config/CLAUDE.md +++ b/internal/config/CLAUDE.md @@ -18,6 +18,25 @@ Credential resolution and OS keyring storage. Provides: - `SaveAPIKey()` — write validated login token to keyring - `DeleteAPIKey()` — remove saved keyring token on logout +## Build Profiles (dev vs prod) + +`configFileName` and `keyringService` are selected at build time via the `dev` +build tag (`profile_prod.go` for `//go:build !dev`, `profile_dev.go` for +`//go:build dev`): + +| | config file | keyring service | `config.IsDev` | +|---|---|---|---| +| prod (default `go build`, releases) | `~/.chatwoot/config.yaml` | `chatwoot-cli` | `false` | +| dev (`go build -tags dev`, `mise run dev`) | `~/.chatwoot/config.dev.yaml` | `chatwoot-cli-dev` | `true` | + +A dev build keeps its own credentials, so iterating on the CLI never reads or +clobbers the production login. The keyring **service** (not just the entry name) +is namespaced per profile, so `auth logout` — which does +`keyring.DeleteAll(keyringService)` to clear stale entries — only wipes the +active build's tokens and never the other profile's. Release builds (goreleaser +passes no tags) exclude `profile_dev.go` entirely — the dev path is compiled +out. `config view` shows a `Profile: dev` line on dev builds. + ## Config Schema ```yaml diff --git a/internal/config/config.go b/internal/config/config.go index e9eb7d4..6c7533d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -34,7 +34,7 @@ func ConfigPath() (string, error) { if err != nil { return "", err } - return filepath.Join(dir, "config.yaml"), nil + return filepath.Join(dir, configFileName), nil } func Load() (*Config, error) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 0486d2c..6960d22 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -2,7 +2,6 @@ package config import ( "os" - "path/filepath" "strings" "testing" ) @@ -93,7 +92,12 @@ func TestLegacyAPIKeyIsIgnoredAndRemovedOnSave(t *testing.T) { t.Fatalf("MkdirAll() error = %v", err) } - path := filepath.Join(dir, "config.yaml") + // Derive the path from ConfigPath() rather than hardcoding "config.yaml" so + // this test is correct under any build profile (e.g. dev → config.dev.yaml). + path, err := ConfigPath() + if err != nil { + t.Fatalf("ConfigPath() error = %v", err) + } legacy := "base_url: https://app.chatwoot.com\napi_key: plaintext-token\naccount_id: 123\n" if err := os.WriteFile(path, []byte(legacy), 0600); err != nil { t.Fatalf("WriteFile() error = %v", err) diff --git a/internal/config/credentials.go b/internal/config/credentials.go index eb9cd0c..0c4b176 100644 --- a/internal/config/credentials.go +++ b/internal/config/credentials.go @@ -12,10 +12,15 @@ import ( const ( APIKeyEnv = "CHATWOOT_API_KEY" - keyringService = "chatwoot-cli" apiKeyKeyringEntry = "api-key" ) +// keyringService is build-profile specific ("chatwoot-cli" for prod, +// "chatwoot-cli-dev" for dev). Namespacing the whole service per profile keeps +// logout's DeleteAll(keyringService) scoped to the active build, so a dev logout +// can't erase a prod login's token and vice versa. See profile_prod.go / +// profile_dev.go. + type CredentialSource string const ( diff --git a/internal/config/credentials_test.go b/internal/config/credentials_test.go index 31bb463..82b76fd 100644 --- a/internal/config/credentials_test.go +++ b/internal/config/credentials_test.go @@ -193,3 +193,40 @@ func TestDeleteAPIKeyRemovesAllServiceEntries(t *testing.T) { } } } + +// TestDeleteAPIKeyLeavesOtherProfileServiceIntact guards the dev/prod isolation +// guarantee: keyringService is namespaced per build profile, so logging out of +// one build must not delete the other build's saved token. DeleteAPIKey wipes +// only keyringService, never another service's entries. +func TestDeleteAPIKeyLeavesOtherProfileServiceIntact(t *testing.T) { + initMockKeyring(t) + cfg := &Config{BaseURL: "https://app.chatwoot.com", AccountID: 130} + + // Stand in for the other build profile's keyring namespace (prod's + // "chatwoot-cli" vs dev's "chatwoot-cli-dev"); the exact name doesn't matter, + // only that it differs from the active keyringService. + const otherProfileService = "chatwoot-cli-other-profile" + if err := keyring.Set(otherProfileService, apiKeyKeyringEntry, "other-profile-token"); err != nil { + t.Fatalf("seed other-profile service: %v", err) + } + if err := SaveAPIKey(cfg, "this-profile-token"); err != nil { + t.Fatalf("SaveAPIKey() error = %v", err) + } + + if err := DeleteAPIKey(cfg); err != nil { + t.Fatalf("DeleteAPIKey() error = %v", err) + } + + // Active profile's token is gone... + if _, _, err := ResolveAPIKey(cfg); !errors.Is(err, ErrAPIKeyNotFound) { + t.Fatalf("active profile token survived logout, err = %v", err) + } + // ...but the other profile's token must be untouched. + got, err := keyring.Get(otherProfileService, apiKeyKeyringEntry) + if err != nil { + t.Fatalf("other profile token erased by logout: %v", err) + } + if got != "other-profile-token" { + t.Fatalf("other profile token = %q, want other-profile-token", got) + } +} diff --git a/internal/config/profile_dev.go b/internal/config/profile_dev.go new file mode 100644 index 0000000..7b36b08 --- /dev/null +++ b/internal/config/profile_dev.go @@ -0,0 +1,18 @@ +//go:build dev + +package config + +// Dev build profile, compiled in only with `go build -tags dev` (which is what +// `mise run dev` uses). Dev builds read a separate config file and store their +// token under a separate keyring service, so iterating on the CLI never reads, +// overwrites, or — on logout — deletes the production credentials managed by an +// installed binary. +// +// None of this is compiled into release builds; see profile_prod.go. +const ( + configFileName = "config.dev.yaml" + keyringService = "chatwoot-cli-dev" + + // IsDev reports whether this binary was built with `-tags dev`. + IsDev = true +) diff --git a/internal/config/profile_dev_test.go b/internal/config/profile_dev_test.go new file mode 100644 index 0000000..6dd40ff --- /dev/null +++ b/internal/config/profile_dev_test.go @@ -0,0 +1,32 @@ +//go:build dev + +package config + +import ( + "strings" + "testing" +) + +// Guards the dev build profile. Run with `go test -tags dev ./internal/config/`. +// Dev builds must read a separate config file and keyring entry so development +// never touches production credentials. +func TestDevBuildProfile(t *testing.T) { + if !IsDev { + t.Fatal("IsDev = false in a dev build") + } + if configFileName != "config.dev.yaml" { + t.Fatalf("configFileName = %q, want config.dev.yaml", configFileName) + } + if keyringService != "chatwoot-cli-dev" { + t.Fatalf("keyringService = %q, want chatwoot-cli-dev", keyringService) + } + + t.Setenv("HOME", t.TempDir()) + path, err := ConfigPath() + if err != nil { + t.Fatalf("ConfigPath() error = %v", err) + } + if !strings.HasSuffix(path, "/config.dev.yaml") { + t.Fatalf("ConfigPath() = %q, want suffix /config.dev.yaml", path) + } +} diff --git a/internal/config/profile_prod.go b/internal/config/profile_prod.go new file mode 100644 index 0000000..3049304 --- /dev/null +++ b/internal/config/profile_prod.go @@ -0,0 +1,17 @@ +//go:build !dev + +package config + +// Production build profile. This is the default for `go build` and every +// released binary (goreleaser passes no build tags), so credentials live in the +// canonical config file and keyring service. +// +// The dev counterpart lives in profile_dev.go behind `//go:build dev` and is +// never compiled into release builds — building for prod disables that path. +const ( + configFileName = "config.yaml" + keyringService = "chatwoot-cli" + + // IsDev reports whether this binary was built with `-tags dev`. + IsDev = false +) diff --git a/internal/config/profile_prod_test.go b/internal/config/profile_prod_test.go new file mode 100644 index 0000000..00b4d2d --- /dev/null +++ b/internal/config/profile_prod_test.go @@ -0,0 +1,31 @@ +//go:build !dev + +package config + +import ( + "strings" + "testing" +) + +// Guards the production build profile. Runs under the default (untagged) build; +// the dev counterpart is profile_dev_test.go behind //go:build dev. +func TestProdBuildProfile(t *testing.T) { + if IsDev { + t.Fatal("IsDev = true in a non-dev build") + } + if configFileName != "config.yaml" { + t.Fatalf("configFileName = %q, want config.yaml", configFileName) + } + if keyringService != "chatwoot-cli" { + t.Fatalf("keyringService = %q, want chatwoot-cli", keyringService) + } + + t.Setenv("HOME", t.TempDir()) + path, err := ConfigPath() + if err != nil { + t.Fatalf("ConfigPath() error = %v", err) + } + if !strings.HasSuffix(path, "/config.yaml") { + t.Fatalf("ConfigPath() = %q, want suffix /config.yaml", path) + } +} diff --git a/internal/sdk/profile.go b/internal/sdk/profile.go index e92e710..0c6fb87 100644 --- a/internal/sdk/profile.go +++ b/internal/sdk/profile.go @@ -14,6 +14,18 @@ type ProfileResponse struct { AvatarURL string `json:"avatar_url"` AccountID int `json:"account_id"` UISettings map[string]interface{} `json:"ui_settings"` + // Accounts lists every account the authenticated user belongs to. Used at + // login to verify the user has access to the account ID they enter. + Accounts []ProfileAccount `json:"accounts"` +} + +// ProfileAccount is one membership entry from the profile payload's accounts +// array. The endpoint returns more fields (permissions, availability, +// custom_role, ...); only the ones the CLI uses are parsed. +type ProfileAccount struct { + ID int `json:"id"` + Name string `json:"name"` + Role string `json:"role"` } // Get fetches the current user's profile. Uses a non-account-scoped endpoint. diff --git a/mise.toml b/mise.toml index eca2d14..70c8745 100644 --- a/mise.toml +++ b/mise.toml @@ -8,8 +8,8 @@ description = "Build the CLI" run = "mkdir -p ./bin && go build -o ./bin/chatwoot ./cmd/chatwoot/" [tasks.dev] -description = "Auto-build on Go file changes" -run = "mkdir -p ./bin && watchexec -e go --restart --debounce 300 -- go build -o ./bin/chatwoot ./cmd/chatwoot/" +description = "Auto-build on Go file changes (dev profile: separate config + keyring)" +run = "mkdir -p ./bin && watchexec -e go --restart --debounce 300 -- go build -tags dev -o ./bin/chatwoot ./cmd/chatwoot/" [tasks.lint] description = "Run golangci-lint across the module"