Skip to content

Commit 3bf6405

Browse files
authored
feat: update auth flow (#19)
* feat: allow dev auth for local testing * fix: isolate dev and prod credentials in separate keyrings The dev build profile shared keyringService="chatwoot-cli" with prod and only varied the entry name, but DeleteAPIKey calls DeleteAll(keyringService). A dev `auth logout` therefore erased the prod login's token (and vice versa), breaking the separate-credentials guarantee. Namespace the keyring service per build profile ("chatwoot-cli" for prod, "chatwoot-cli-dev" for dev) so DeleteAll stays scoped to the active build while keeping its "wipe stale entries" behavior. The entry name is shared again. Add build-profile guards (prod/dev) and a logout-isolation regression test, and derive the legacy-config test path from ConfigPath() so the package passes under -tags dev too. * feat: verify account access at login `auth login` validated the API token via /api/v1/profile but never checked that the token could access the account ID the user entered. A wrong or mistyped account was saved anyway and only surfaced later as a cryptic 404 on the first account-scoped call. Parse the accounts membership list already returned by the profile endpoint and reject login when the entered account is not among them, naming the accounts the key can access. Instances that return no accounts are left unaffected. * chore: update go toolchain * chore: update go toolchain
1 parent fe9f89f commit 3bf6405

15 files changed

Lines changed: 369 additions & 18 deletions

go.mod

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
module github.com/chatwoot/cli
22

3-
go 1.25.5
4-
5-
toolchain go1.25.10
3+
go 1.26.4
64

75
require (
86
github.com/alecthomas/kong v1.15.0

internal/cmd/auth.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,13 @@ func (c *AuthLoginCmd) Run(app *App) error {
6363
if err != nil {
6464
return fmt.Errorf("authentication failed: %w", err)
6565
}
66+
67+
// The token is valid, but that doesn't mean it can access the account ID the
68+
// user typed. Verify membership now so a typo/wrong account fails here with a
69+
// clear message instead of as a cryptic 404 on the first account-scoped call.
70+
if err := verifyAccountAccess(profile, cfg.AccountID); err != nil {
71+
return err
72+
}
6673
cfg.UserID = profile.ID
6774

6875
if err := config.SaveAPIKey(cfg, apiKey); err != nil {
@@ -82,6 +89,37 @@ func loginSuccessMessage(name, email string) string {
8289
return fmt.Sprintf("Logged in as %s (%s)\n", output.SanitizeText(name), output.SanitizeText(email))
8390
}
8491

92+
// verifyAccountAccess fails login when the entered account ID is not one the
93+
// authenticated user belongs to. The profile payload's accounts array is the
94+
// source of truth. If the instance returns no accounts (older Chatwoot, or a
95+
// token type that omits them), the check is skipped rather than block login.
96+
func verifyAccountAccess(profile *sdk.ProfileResponse, accountID int) error {
97+
if len(profile.Accounts) == 0 {
98+
return nil
99+
}
100+
for _, acc := range profile.Accounts {
101+
if acc.ID == accountID {
102+
return nil
103+
}
104+
}
105+
return fmt.Errorf("account %d is not accessible with this API key; %s", accountID, accessibleAccountsHint(profile.Accounts))
106+
}
107+
108+
func accessibleAccountsHint(accounts []sdk.ProfileAccount) string {
109+
parts := make([]string, 0, len(accounts))
110+
for _, acc := range accounts {
111+
if name := output.SanitizeText(acc.Name); name != "" {
112+
parts = append(parts, fmt.Sprintf("%d (%s)", acc.ID, name))
113+
} else {
114+
parts = append(parts, strconv.Itoa(acc.ID))
115+
}
116+
}
117+
if len(parts) == 1 {
118+
return "this key can access account " + parts[0]
119+
}
120+
return "this key can access accounts: " + strings.Join(parts, ", ")
121+
}
122+
85123
func readAPIKey(reader *bufio.Reader) (string, error) {
86124
fmt.Print("API Key: ")
87125

internal/cmd/auth_test.go

Lines changed: 143 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,16 @@ package cmd
33
import (
44
"bytes"
55
"errors"
6+
"io"
67
"net/http"
78
"net/http/httptest"
9+
"os"
810
"strings"
911
"testing"
1012

1113
"github.com/chatwoot/cli/internal/config"
1214
"github.com/chatwoot/cli/internal/output"
15+
"github.com/chatwoot/cli/internal/sdk"
1316
"github.com/zalando/go-keyring"
1417
)
1518

@@ -125,6 +128,33 @@ func TestLoginSuccessMessageStripsTerminalControls(t *testing.T) {
125128
}
126129
}
127130

131+
func TestVerifyAccountAccess(t *testing.T) {
132+
accounts := []sdk.ProfileAccount{
133+
{ID: 7, Name: "Acme", Role: "administrator"},
134+
{ID: 9, Name: "Beta", Role: "agent"},
135+
}
136+
137+
if err := verifyAccountAccess(&sdk.ProfileResponse{Accounts: accounts}, 9); err != nil {
138+
t.Fatalf("expected access to a member account, got error: %v", err)
139+
}
140+
141+
err := verifyAccountAccess(&sdk.ProfileResponse{Accounts: accounts}, 42)
142+
if err == nil {
143+
t.Fatal("expected error for non-member account, got nil")
144+
}
145+
// The message should name the accessible accounts so the user can correct the ID.
146+
for _, want := range []string{"42", "7 (Acme)", "9 (Beta)"} {
147+
if !strings.Contains(err.Error(), want) {
148+
t.Fatalf("error %q missing %q", err.Error(), want)
149+
}
150+
}
151+
152+
// No accounts in payload (older instances) → skip rather than block login.
153+
if err := verifyAccountAccess(&sdk.ProfileResponse{}, 42); err != nil {
154+
t.Fatalf("expected skip when no accounts present, got error: %v", err)
155+
}
156+
}
157+
128158
func TestMeAndWhoamiAliasAuthStatus(t *testing.T) {
129159
profile := `{
130160
"id": 7,
@@ -167,22 +197,23 @@ func TestAuthStatusNotLoggedIn(t *testing.T) {
167197

168198
func TestAuthLogoutRemovesKeyringTokenWithoutConfig(t *testing.T) {
169199
keyring.MockInit()
170-
if err := keyring.DeleteAll("chatwoot-cli"); err != nil {
171-
t.Fatalf("keyring.DeleteAll: %v", err)
172-
}
173200
t.Setenv("HOME", t.TempDir())
174201
t.Setenv(config.APIKeyEnv, "")
175202

176-
if err := keyring.Set("chatwoot-cli", "api-key", "stale-token"); err != nil {
177-
t.Fatalf("keyring.Set: %v", err)
203+
// Seed the token through the production path so it lands under whichever
204+
// keyring service the active build profile uses (prod vs dev), without
205+
// writing config.yaml — this exercises logout with no config present.
206+
seed := &config.Config{BaseURL: "https://app.chatwoot.com", AccountID: 1}
207+
if err := config.SaveAPIKey(seed, "stale-token"); err != nil {
208+
t.Fatalf("SaveAPIKey: %v", err)
178209
}
179210

180211
if err := (&AuthLogoutCmd{}).Run(&App{}); err != nil {
181212
t.Fatalf("Run: %v", err)
182213
}
183214

184-
if _, err := keyring.Get("chatwoot-cli", "api-key"); !errors.Is(err, keyring.ErrNotFound) {
185-
t.Fatalf("expected logout to delete stale keyring token, err = %v", err)
215+
if _, _, err := config.ResolveAPIKey(seed); !errors.Is(err, config.ErrAPIKeyNotFound) {
216+
t.Fatalf("expected logout to delete the keyring token, err = %v", err)
186217
}
187218
}
188219

@@ -244,3 +275,108 @@ func TestAuthStatusDoesNotCacheUserIDFromEnvironmentToken(t *testing.T) {
244275
t.Fatalf("expected env-token auth status to preserve cached UserID=42, got %d", post.UserID)
245276
}
246277
}
278+
279+
// TestAuthLoginVerifiesAccountAccess drives the full `auth login` flow (stdin →
280+
// profile fetch → membership check → persist) to cover the wiring, not just the
281+
// verifyAccountAccess helper.
282+
func TestAuthLoginVerifiesAccountAccess(t *testing.T) {
283+
profileBody := `{"id":5,"name":"Eve","email":"eve@example.com","availability_status":"online","role":"agent",` +
284+
`"accounts":[{"id":7,"name":"Acme","role":"administrator"},{"id":9,"name":"Beta","role":"agent"}]}`
285+
286+
t.Run("rejects an account the token cannot access", func(t *testing.T) {
287+
server := loginProfileServer(t, profileBody)
288+
defer server.Close()
289+
isolateAuthEnv(t)
290+
291+
err := runLogin(t, server.URL+"\ntoken\n42\n")
292+
if err == nil {
293+
t.Fatal("expected login to fail for an inaccessible account")
294+
}
295+
for _, want := range []string{"42", "Acme", "Beta"} {
296+
if !strings.Contains(err.Error(), want) {
297+
t.Fatalf("error %q should name entered + accessible accounts", err.Error())
298+
}
299+
}
300+
// Nothing must be persisted when login is rejected.
301+
if cfg, _ := config.Load(); cfg != nil {
302+
t.Fatalf("config was saved despite a rejected login: %#v", cfg)
303+
}
304+
})
305+
306+
t.Run("accepts a member account and persists config + key", func(t *testing.T) {
307+
server := loginProfileServer(t, profileBody)
308+
defer server.Close()
309+
isolateAuthEnv(t)
310+
311+
if err := runLogin(t, server.URL+"\ntoken\n7\n"); err != nil {
312+
t.Fatalf("login: %v", err)
313+
}
314+
315+
cfg, err := config.Load()
316+
if err != nil || cfg == nil {
317+
t.Fatalf("config not saved: cfg=%#v err=%v", cfg, err)
318+
}
319+
if cfg.AccountID != 7 || cfg.UserID != 5 {
320+
t.Fatalf("saved cfg = %#v, want AccountID 7, UserID 5", cfg)
321+
}
322+
apiKey, source, err := config.ResolveAPIKey(cfg)
323+
if err != nil || apiKey != "token" || source != config.CredentialSourceKeyring {
324+
t.Fatalf("ResolveAPIKey = (%q, %v, %v), want token/keyring", apiKey, source, err)
325+
}
326+
})
327+
}
328+
329+
// isolateAuthEnv gives a test its own HOME + mocked keyring and clears the
330+
// CHATWOOT_API_KEY override so credential resolution exercises the keyring path.
331+
func isolateAuthEnv(t *testing.T) {
332+
t.Helper()
333+
keyring.MockInit()
334+
if err := keyring.DeleteAll("chatwoot-cli"); err != nil {
335+
t.Fatalf("keyring.DeleteAll: %v", err)
336+
}
337+
t.Setenv("HOME", t.TempDir())
338+
t.Setenv(config.APIKeyEnv, "")
339+
}
340+
341+
func loginProfileServer(t *testing.T, body string) *httptest.Server {
342+
t.Helper()
343+
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
344+
if r.URL.Path != "/api/v1/profile" {
345+
http.Error(w, "not found", http.StatusNotFound)
346+
return
347+
}
348+
w.Header().Set("Content-Type", "application/json")
349+
_, _ = w.Write([]byte(body))
350+
}))
351+
}
352+
353+
// runLogin feeds scripted answers to the interactive login prompts via os.Stdin
354+
// and silences the prompt/banner output, returning the command's error.
355+
func runLogin(t *testing.T, stdin string) error {
356+
t.Helper()
357+
358+
r, w, err := os.Pipe()
359+
if err != nil {
360+
t.Fatalf("os.Pipe: %v", err)
361+
}
362+
if _, err := io.WriteString(w, stdin); err != nil {
363+
t.Fatalf("write stdin: %v", err)
364+
}
365+
_ = w.Close()
366+
367+
devnull, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0)
368+
if err != nil {
369+
t.Fatalf("open devnull: %v", err)
370+
}
371+
372+
oldStdin, oldStdout := os.Stdin, os.Stdout
373+
os.Stdin, os.Stdout = r, devnull
374+
defer func() {
375+
os.Stdin, os.Stdout = oldStdin, oldStdout
376+
_ = r.Close()
377+
_ = devnull.Close()
378+
}()
379+
380+
printer := output.NewPrinter("text", false, false)
381+
return (&AuthLoginCmd{}).Run(&App{Printer: printer})
382+
}

internal/cmd/config.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,15 @@ func (c *ConfigViewCmd) Run(app *App) error {
3939

4040
credential := credentialStatus(cfg)
4141

42-
app.Printer.PrintDetail([]output.KeyValue{
42+
detail := []output.KeyValue{
4343
{Key: "Base URL", Value: cfg.BaseURL},
4444
{Key: "Account ID", Value: fmt.Sprintf("%d", cfg.AccountID)},
4545
{Key: "Credential", Value: credential},
46-
})
46+
}
47+
if config.IsDev {
48+
detail = append(detail, output.KeyValue{Key: "Profile", Value: "dev"})
49+
}
50+
app.Printer.PrintDetail(detail)
4751

4852
return nil
4953
}

internal/config/CLAUDE.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,25 @@ Credential resolution and OS keyring storage. Provides:
1818
- `SaveAPIKey()` — write validated login token to keyring
1919
- `DeleteAPIKey()` — remove saved keyring token on logout
2020

21+
## Build Profiles (dev vs prod)
22+
23+
`configFileName` and `keyringService` are selected at build time via the `dev`
24+
build tag (`profile_prod.go` for `//go:build !dev`, `profile_dev.go` for
25+
`//go:build dev`):
26+
27+
| | config file | keyring service | `config.IsDev` |
28+
|---|---|---|---|
29+
| prod (default `go build`, releases) | `~/.chatwoot/config.yaml` | `chatwoot-cli` | `false` |
30+
| dev (`go build -tags dev`, `mise run dev`) | `~/.chatwoot/config.dev.yaml` | `chatwoot-cli-dev` | `true` |
31+
32+
A dev build keeps its own credentials, so iterating on the CLI never reads or
33+
clobbers the production login. The keyring **service** (not just the entry name)
34+
is namespaced per profile, so `auth logout` — which does
35+
`keyring.DeleteAll(keyringService)` to clear stale entries — only wipes the
36+
active build's tokens and never the other profile's. Release builds (goreleaser
37+
passes no tags) exclude `profile_dev.go` entirely — the dev path is compiled
38+
out. `config view` shows a `Profile: dev` line on dev builds.
39+
2140
## Config Schema
2241

2342
```yaml

internal/config/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func ConfigPath() (string, error) {
3434
if err != nil {
3535
return "", err
3636
}
37-
return filepath.Join(dir, "config.yaml"), nil
37+
return filepath.Join(dir, configFileName), nil
3838
}
3939

4040
func Load() (*Config, error) {

internal/config/config_test.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package config
22

33
import (
44
"os"
5-
"path/filepath"
65
"strings"
76
"testing"
87
)
@@ -93,7 +92,12 @@ func TestLegacyAPIKeyIsIgnoredAndRemovedOnSave(t *testing.T) {
9392
t.Fatalf("MkdirAll() error = %v", err)
9493
}
9594

96-
path := filepath.Join(dir, "config.yaml")
95+
// Derive the path from ConfigPath() rather than hardcoding "config.yaml" so
96+
// this test is correct under any build profile (e.g. dev → config.dev.yaml).
97+
path, err := ConfigPath()
98+
if err != nil {
99+
t.Fatalf("ConfigPath() error = %v", err)
100+
}
97101
legacy := "base_url: https://app.chatwoot.com\napi_key: plaintext-token\naccount_id: 123\n"
98102
if err := os.WriteFile(path, []byte(legacy), 0600); err != nil {
99103
t.Fatalf("WriteFile() error = %v", err)

internal/config/credentials.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,15 @@ import (
1212

1313
const (
1414
APIKeyEnv = "CHATWOOT_API_KEY"
15-
keyringService = "chatwoot-cli"
1615
apiKeyKeyringEntry = "api-key"
1716
)
1817

18+
// keyringService is build-profile specific ("chatwoot-cli" for prod,
19+
// "chatwoot-cli-dev" for dev). Namespacing the whole service per profile keeps
20+
// logout's DeleteAll(keyringService) scoped to the active build, so a dev logout
21+
// can't erase a prod login's token and vice versa. See profile_prod.go /
22+
// profile_dev.go.
23+
1924
type CredentialSource string
2025

2126
const (

internal/config/credentials_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,3 +193,40 @@ func TestDeleteAPIKeyRemovesAllServiceEntries(t *testing.T) {
193193
}
194194
}
195195
}
196+
197+
// TestDeleteAPIKeyLeavesOtherProfileServiceIntact guards the dev/prod isolation
198+
// guarantee: keyringService is namespaced per build profile, so logging out of
199+
// one build must not delete the other build's saved token. DeleteAPIKey wipes
200+
// only keyringService, never another service's entries.
201+
func TestDeleteAPIKeyLeavesOtherProfileServiceIntact(t *testing.T) {
202+
initMockKeyring(t)
203+
cfg := &Config{BaseURL: "https://app.chatwoot.com", AccountID: 130}
204+
205+
// Stand in for the other build profile's keyring namespace (prod's
206+
// "chatwoot-cli" vs dev's "chatwoot-cli-dev"); the exact name doesn't matter,
207+
// only that it differs from the active keyringService.
208+
const otherProfileService = "chatwoot-cli-other-profile"
209+
if err := keyring.Set(otherProfileService, apiKeyKeyringEntry, "other-profile-token"); err != nil {
210+
t.Fatalf("seed other-profile service: %v", err)
211+
}
212+
if err := SaveAPIKey(cfg, "this-profile-token"); err != nil {
213+
t.Fatalf("SaveAPIKey() error = %v", err)
214+
}
215+
216+
if err := DeleteAPIKey(cfg); err != nil {
217+
t.Fatalf("DeleteAPIKey() error = %v", err)
218+
}
219+
220+
// Active profile's token is gone...
221+
if _, _, err := ResolveAPIKey(cfg); !errors.Is(err, ErrAPIKeyNotFound) {
222+
t.Fatalf("active profile token survived logout, err = %v", err)
223+
}
224+
// ...but the other profile's token must be untouched.
225+
got, err := keyring.Get(otherProfileService, apiKeyKeyringEntry)
226+
if err != nil {
227+
t.Fatalf("other profile token erased by logout: %v", err)
228+
}
229+
if got != "other-profile-token" {
230+
t.Fatalf("other profile token = %q, want other-profile-token", got)
231+
}
232+
}

0 commit comments

Comments
 (0)