From d2347dc57d41ff1dfc2b4fe41e90e15e34f6bc70 Mon Sep 17 00:00:00 2001 From: barckcode Date: Mon, 12 Oct 2026 01:14:05 +0200 Subject: [PATCH 1/2] fix(auth): sign-out ends the session on the platform (0.1.28) `nan auth logout` and the panel's `o o` only deleted session.json. The session token stayed valid on the platform until it expired, so any copy of it kept working. Both now POST /api/auth/logout with the session (as a bearer, and as the nan_session cookie every other call already uses, so a platform that predates bearer logout still revokes it) BEFORE deleting the local file. A 401 counts as already signed out. A platform that does not answer does not keep the member signed in locally: the file still goes and the member is told the session may outlive it. The logout call is bounded at 10s. The existing panel sign-out tests now talk to a fake platform instead of the real cloud-api. Co-Authored-By: Claude Opus 5.5 --- cmd/auth.go | 9 +++ cmd/credentials.go | 4 ++ cmd/logout_test.go | 95 +++++++++++++++++++++++++++++ internal/api/logout.go | 66 ++++++++++++++++++++ internal/api/logout_test.go | 82 +++++++++++++++++++++++++ internal/tui/config_test.go | 5 +- internal/tui/signout_server_test.go | 84 +++++++++++++++++++++++++ internal/tui/tui.go | 10 ++- 8 files changed, 352 insertions(+), 3 deletions(-) create mode 100644 cmd/logout_test.go create mode 100644 internal/api/logout.go create mode 100644 internal/api/logout_test.go create mode 100644 internal/tui/signout_server_test.go diff --git a/cmd/auth.go b/cmd/auth.go index f0cf1f8..0014b13 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -201,6 +201,15 @@ func flagValue(value string) (string, error) { } func runLogout(cmd *cobra.Command, args []string) error { + // End the session on the platform first, while session.json still holds + // the token: deleting the file alone leaves any copy of the token working + // until it expires. A platform that cannot be reached does not keep the + // member signed in here; the file still goes, and they are told. + if current, err := session.Load(); err == nil && current.Token != "" { + if err := api.New(current.Token).WithBaseURL(apiBaseURL).Logout(); err != nil { + fmt.Fprintf(os.Stderr, "Could not end the session on the platform (%v); it stays valid until it expires. Use \"sign out everywhere\" in the portal to end it now.\n", err) + } + } if err := session.Delete(); err != nil { return err } diff --git a/cmd/credentials.go b/cmd/credentials.go index 4a469ad..00c23a9 100644 --- a/cmd/credentials.go +++ b/cmd/credentials.go @@ -26,6 +26,10 @@ const maxTokenLen = 512 // tests can point it at a server of their own. var runsBaseURL = api.RunsBaseURL +// apiBaseURL is where the session endpoints (logout) are sent. A variable so +// the tests can point it at a server of their own. +var apiBaseURL = api.BaseURL + // tokenFileFlag is --token-file on `nan run` and `nan runs`. var tokenFileFlag string diff --git a/cmd/logout_test.go b/cmd/logout_test.go new file mode 100644 index 0000000..ca19044 --- /dev/null +++ b/cmd/logout_test.go @@ -0,0 +1,95 @@ +package cmd + +import ( + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" +) + +// Spec (prod E2E 2026-10-12): `nan auth logout` ends the session on the +// platform BEFORE it deletes session.json, so the token stops working +// everywhere and not just on this machine. A platform that cannot be reached +// does not keep the member signed in locally: the file still goes, and the +// command says the session may outlive it. + +func logoutServer(t *testing.T, status int) (*[]string, string) { + t.Helper() + seen := new([]string) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // The file must still be on disk when the platform is asked. + if _, err := os.Stat(filepath.Join(os.Getenv("HOME"), ".config", "nan", "session.json")); err != nil { + t.Error("session.json was deleted before the platform was asked to end the session") + } + *seen = append(*seen, r.Method+" "+r.URL.Path+" "+r.Header.Get("Authorization")) + w.WriteHeader(status) + })) + t.Cleanup(srv.Close) + return seen, srv.URL +} + +func withAPIBase(t *testing.T, u string) { + t.Helper() + orig := apiBaseURL + apiBaseURL = u + t.Cleanup(func() { apiBaseURL = orig }) +} + +func sessionFileExists(home string) bool { + _, err := os.Stat(filepath.Join(home, ".config", "nan", "session.json")) + return err == nil +} + +func TestLogoutEndsTheSessionOnThePlatformFirst(t *testing.T) { + home := fakeHome(t, `{"token":"session-jwt"}`) + seen, u := logoutServer(t, http.StatusOK) + withAPIBase(t, u) + keepToolsFlag = true + t.Cleanup(func() { keepToolsFlag = false }) + + if err := runLogout(logoutCmd, nil); err != nil { + t.Fatalf("runLogout: %v", err) + } + if len(*seen) != 1 || (*seen)[0] != "POST /auth/logout Bearer session-jwt" { + t.Errorf("platform saw %v, want one POST /auth/logout with the session bearer", *seen) + } + if sessionFileExists(home) { + t.Error("session.json survived the logout") + } +} + +func TestLogoutStillSignsOutLocallyWhenThePlatformFails(t *testing.T) { + home := fakeHome(t, `{"token":"session-jwt"}`) + _, u := logoutServer(t, http.StatusBadGateway) + withAPIBase(t, u) + keepToolsFlag = true + t.Cleanup(func() { keepToolsFlag = false }) + + if err := runLogout(logoutCmd, nil); err != nil { + t.Fatalf("runLogout: %v", err) + } + if sessionFileExists(home) { + t.Error("a platform failure kept the member signed in locally") + } +} + +// A session that only carries a platform token or an API key has no platform +// session to end: nothing is sent. +func TestLogoutWithoutASessionTokenSendsNothing(t *testing.T) { + home := fakeHome(t, `{"platformToken":"`+testPAT+`"}`) + seen, u := logoutServer(t, http.StatusOK) + withAPIBase(t, u) + keepToolsFlag = true + t.Cleanup(func() { keepToolsFlag = false }) + + if err := runLogout(logoutCmd, nil); err != nil { + t.Fatalf("runLogout: %v", err) + } + if len(*seen) != 0 { + t.Errorf("platform saw %v, want nothing", *seen) + } + if sessionFileExists(home) { + t.Error("session.json survived the logout") + } +} diff --git a/internal/api/logout.go b/internal/api/logout.go new file mode 100644 index 0000000..9fb9f0e --- /dev/null +++ b/internal/api/logout.go @@ -0,0 +1,66 @@ +package api + +import ( + "context" + "fmt" + "io" + "net/http" + "time" +) + +// logoutTimeout bounds the sign-out call. Signing out must not hang on a +// platform that accepts the connection and never answers: the local sign-out +// goes ahead either way, this only decides how long it waits to say whether +// the platform got the message. +const logoutTimeout = 10 * time.Second + +// WithBaseURL points the client somewhere else, for tests. +func (c *Client) WithBaseURL(u string) *Client { + c.baseURL = u + return c +} + +// Logout ends the session on the platform (POST /auth/logout), so the token +// stops working everywhere and not only on this machine. Deleting +// session.json alone leaves any copy of the token valid until it expires. +// +// The session rides as a bearer, which is how the platform recognises a CLI +// session on logout, and also as the nan_session cookie, which is how every +// other call of this client presents it: a platform that predates bearer +// logout still revokes it. Both name the same session; it is revoked once. +// +// A 401 means the platform already refuses the session, which is the outcome +// wanted, so it is not an error. Anything else that is not 2xx is: nothing was +// revoked, and the caller should say so. +func (c *Client) Logout() error { + if c.token == "" { + return nil + } + base := c.baseURL + if base == "" { + base = BaseURL + } + ctx, cancel := context.WithTimeout(context.Background(), logoutTimeout) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, base+"/auth/logout", nil) + if err != nil { + return err + } + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Cookie", "nan_session="+c.token) + req.Header.Set("Accept", "application/json") + req.Header.Set("User-Agent", "nan-cli") + + resp, err := c.http.Do(req) + if err != nil { + return err + } + defer resp.Body.Close() + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 64<<10)) + + if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode/100 == 2 { + return nil + } + return fmt.Errorf("HTTP %d", resp.StatusCode) +} diff --git a/internal/api/logout_test.go b/internal/api/logout_test.go new file mode 100644 index 0000000..49ec59f --- /dev/null +++ b/internal/api/logout_test.go @@ -0,0 +1,82 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// Spec (prod E2E 2026-10-12): signing out of the CLI must end the session on +// the platform, not only forget it locally. A session token copied off the +// machine before `nan auth logout` otherwise keeps working until it expires. +// +// Logout POSTs /auth/logout carrying the session as a bearer (and as the +// nan_session cookie, which is how every other call of this client presents +// it, so an older platform that only reads the cookie still revokes it). + +func TestLogoutPostsTheSessionToThePlatform(t *testing.T) { + var method, path, authz, cookie string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + method, path, authz = r.Method, r.URL.Path, r.Header.Get("Authorization") + if c, err := r.Cookie("nan_session"); err == nil { + cookie = c.Value + } + _, _ = w.Write([]byte(`{"status":"logged out"}`)) + })) + defer srv.Close() + + c := New("session-jwt").WithBaseURL(srv.URL) + if err := c.Logout(); err != nil { + t.Fatalf("Logout: %v", err) + } + if method != http.MethodPost || path != "/auth/logout" { + t.Errorf("sent %s %s, want POST /auth/logout", method, path) + } + if authz != "Bearer session-jwt" { + t.Errorf("Authorization = %q, want the session as a bearer", authz) + } + if cookie != "session-jwt" { + t.Errorf("nan_session cookie = %q, want the session", cookie) + } +} + +// A session the platform already refuses is already over: nothing to report. +func TestLogoutOfAnExpiredSessionIsNotAnError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + })) + defer srv.Close() + + if err := New("stale").WithBaseURL(srv.URL).Logout(); err != nil { + t.Errorf("an already-dead session reads as a failed logout: %v", err) + } +} + +// A platform that is down did not revoke anything, and the caller has to be +// able to say so. +func TestLogoutReportsAPlatformThatDidNotAnswer(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadGateway) + })) + if err := New("t").WithBaseURL(srv.URL).Logout(); err == nil { + t.Error("a 502 reads as a revoked session") + } + srv.Close() + if err := New("t").WithBaseURL(srv.URL).Logout(); err == nil { + t.Error("an unreachable platform reads as a revoked session") + } +} + +// No session, no request: there is nothing on the platform to end. +func TestLogoutWithoutASessionSendsNothing(t *testing.T) { + calls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { calls++ })) + defer srv.Close() + + if err := New("").WithBaseURL(srv.URL).Logout(); err != nil { + t.Errorf("Logout with no session: %v", err) + } + if calls != 0 { + t.Errorf("sent %d requests with no session", calls) + } +} diff --git a/internal/tui/config_test.go b/internal/tui/config_test.go index 4f081a5..92f81c9 100644 --- a/internal/tui/config_test.go +++ b/internal/tui/config_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "net/http" "os" "os/exec" "path/filepath" @@ -2267,7 +2268,7 @@ func TestSigningOutTakesTwoPressesAndClearsEverything(t *testing.T) { if err := session.Save(sess); err != nil { t.Fatal(err) } - m := newModel(api.New("t"), sess) + m := newModel(api.New("t").WithBaseURL(newFakePlatform(t, http.StatusOK).url), sess) m.lay = newLayout(90, 30) m.cache[tabUsage] = "somebody else's numbers" @@ -2317,7 +2318,7 @@ func TestSigningOutStartsTheSetupAgain(t *testing.T) { if err := session.Save(sess); err != nil { t.Fatal(err) } - m := newModel(api.New("t"), sess) + m := newModel(api.New("t").WithBaseURL(newFakePlatform(t, http.StatusOK).url), sess) m.lay = newLayout(90, 30) press := func(m model, r rune) model { diff --git a/internal/tui/signout_server_test.go b/internal/tui/signout_server_test.go new file mode 100644 index 0000000..1c5a7ec --- /dev/null +++ b/internal/tui/signout_server_test.go @@ -0,0 +1,84 @@ +package tui + +import ( + "net/http" + "net/http/httptest" + "path/filepath" + "sync" + "testing" + + tea "github.com/charmbracelet/bubbletea" + + "github.com/nxssie/nan-cli/internal/api" + "github.com/nxssie/nan-cli/internal/session" +) + +// fakePlatform stands in for cloud-api so a sign-out in a test never reaches +// the real one, and records what it was asked. +type fakePlatform struct { + url string + mu sync.Mutex + seen []string + // sessionOnDisk records, per request, whether session.json still existed. + sessionOnDisk []bool +} + +func newFakePlatform(t *testing.T, status int) *fakePlatform { + t.Helper() + fp := &fakePlatform{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, err := session.Load() + fp.mu.Lock() + fp.seen = append(fp.seen, r.Method+" "+r.URL.Path+" "+r.Header.Get("Authorization")) + fp.sessionOnDisk = append(fp.sessionOnDisk, err == nil) + fp.mu.Unlock() + w.WriteHeader(status) + })) + t.Cleanup(srv.Close) + fp.url = srv.URL + return fp +} + +// Spec (prod E2E 2026-10-12): signing out from the panel ends the session on +// the platform before session.json goes, and a platform that does not answer +// does not keep the member signed in on this machine. +func TestSigningOutEndsTheSessionOnThePlatform(t *testing.T) { + for _, status := range []int{http.StatusOK, http.StatusBadGateway} { + t.Run(http.StatusText(status), func(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("HERMES_HOME", filepath.Join(home, "h")) + + sess := &session.Session{Token: "session-jwt", APIKey: testKey} + if err := session.Save(sess); err != nil { + t.Fatal(err) + } + fp := newFakePlatform(t, status) + m := newModel(api.New("session-jwt").WithBaseURL(fp.url), sess) + m.lay = newLayout(90, 30) + + press := func(m model, r rune) model { + updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{r}}) + return updated.(model) + } + if after := press(m, 'o'); len(fp.seen) != 0 { + t.Fatalf("one press already ended the session (%v); confirm=%v", fp.seen, after.confirmSignOut) + } + out := press(press(m, 'o'), 'o') + + if len(fp.seen) != 1 || fp.seen[0] != "POST /auth/logout Bearer session-jwt" { + t.Fatalf("platform saw %v, want one POST /auth/logout with the session bearer", fp.seen) + } + if !fp.sessionOnDisk[0] { + t.Error("session.json was deleted before the platform was asked") + } + if out.sess.Token != "" { + t.Error("the panel kept the session") + } + if _, err := session.Load(); err == nil { + t.Error("session.json is still on disk") + } + }) + } +} diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 7b3bb55..aad5293 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -635,6 +635,11 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { break } m.confirmSignOut = false + // End the session on the platform first, while session.json + // still holds it: deleting the file alone leaves any copy of the + // token working until it expires. A failure does not keep the + // member signed in here; the local sign-out goes ahead. + revokeErr := m.client.Logout() if err := session.Delete(); err != nil { m.setupMsg = "error: " + err.Error() break @@ -654,6 +659,9 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.cache = make(map[tabID]any) m.keyStatus, m.keyAsked, m.keyCheck = nil, false, "" m.configured, m.setupMsg = nil, signOutMessage(removed, failed) + if revokeErr != nil && len(failed) == 0 { + m.setupMsg = "signed out here, but the platform did not confirm it (" + revokeErr.Error() + ") · the session stays valid until it expires" + } m.err = nil return m, m.resumeSetup() @@ -3559,7 +3567,7 @@ func (m model) renderSetup(l layout) string { // ── about renderer ─────────────────────────────────────────────────────────── -const Version = "0.1.27" +const Version = "0.1.28" func renderAbout(l layout) string { var b strings.Builder From b7713b3f33f423af550c87468e4df03a7b13cd4f Mon Sep 17 00:00:00 2001 From: barckcode Date: Mon, 12 Oct 2026 01:20:26 +0200 Subject: [PATCH 2/2] fix(tui): sign-out asks the platform from a command, not from Update Review follow-up (QA blockers): - The panel no longer blocks on the logout call: the second o sends it as a tea.Cmd, shows 'signing out...', ignores further o presses, and does the local teardown when signedOutMsg arrives. Quitting before then leaves the member signed in locally, consistent with nothing being revoked. - Failure warnings are now asserted: the panel message (also appended when the tools removal failed) and the stderr line of nan auth logout, which names the real portal control and never prints the token. - Logout does not follow redirects (a 3xx is not a revocation), its timeout is testable, WithBaseURL lives in client.go and trims a trailing slash. Co-Authored-By: Claude Opus 5.5 --- cmd/auth.go | 6 +- cmd/logout_test.go | 26 ++++++ internal/api/client.go | 7 ++ internal/api/logout.go | 19 ++--- internal/api/logout_test.go | 46 +++++++++++ internal/tui/config_test.go | 8 +- internal/tui/signout_server_test.go | 119 +++++++++++++++++++++++----- internal/tui/tui.go | 107 +++++++++++++++++-------- 8 files changed, 270 insertions(+), 68 deletions(-) diff --git a/cmd/auth.go b/cmd/auth.go index 0014b13..a9f0e10 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -200,6 +200,10 @@ func flagValue(value string) (string, error) { return value, nil } +// logoutWarnings is where `nan auth logout` says the platform did not confirm +// the sign-out. A variable so the tests can read it. +var logoutWarnings io.Writer = os.Stderr + func runLogout(cmd *cobra.Command, args []string) error { // End the session on the platform first, while session.json still holds // the token: deleting the file alone leaves any copy of the token working @@ -207,7 +211,7 @@ func runLogout(cmd *cobra.Command, args []string) error { // member signed in here; the file still goes, and they are told. if current, err := session.Load(); err == nil && current.Token != "" { if err := api.New(current.Token).WithBaseURL(apiBaseURL).Logout(); err != nil { - fmt.Fprintf(os.Stderr, "Could not end the session on the platform (%v); it stays valid until it expires. Use \"sign out everywhere\" in the portal to end it now.\n", err) + fmt.Fprintf(logoutWarnings, "Could not end the session on the platform (%v); it stays valid until it expires. To end it now: cloud.nan.builders, Settings > Account > Sign out other sessions.\n", err) } } if err := session.Delete(); err != nil { diff --git a/cmd/logout_test.go b/cmd/logout_test.go index ca19044..a0daa9e 100644 --- a/cmd/logout_test.go +++ b/cmd/logout_test.go @@ -1,10 +1,12 @@ package cmd import ( + "bytes" "net/http" "net/http/httptest" "os" "path/filepath" + "strings" "testing" ) @@ -29,6 +31,16 @@ func logoutServer(t *testing.T, status int) (*[]string, string) { return seen, srv.URL } +// captureLogoutWarnings collects what runLogout tells the member on stderr. +func captureLogoutWarnings(t *testing.T) *bytes.Buffer { + t.Helper() + buf := new(bytes.Buffer) + orig := logoutWarnings + logoutWarnings = buf + t.Cleanup(func() { logoutWarnings = orig }) + return buf +} + func withAPIBase(t *testing.T, u string) { t.Helper() orig := apiBaseURL @@ -45,12 +57,16 @@ func TestLogoutEndsTheSessionOnThePlatformFirst(t *testing.T) { home := fakeHome(t, `{"token":"session-jwt"}`) seen, u := logoutServer(t, http.StatusOK) withAPIBase(t, u) + warnings := captureLogoutWarnings(t) keepToolsFlag = true t.Cleanup(func() { keepToolsFlag = false }) if err := runLogout(logoutCmd, nil); err != nil { t.Fatalf("runLogout: %v", err) } + if warn := warnings.String(); warn != "" { + t.Errorf("a confirmed logout warns: %q", warn) + } if len(*seen) != 1 || (*seen)[0] != "POST /auth/logout Bearer session-jwt" { t.Errorf("platform saw %v, want one POST /auth/logout with the session bearer", *seen) } @@ -63,12 +79,22 @@ func TestLogoutStillSignsOutLocallyWhenThePlatformFails(t *testing.T) { home := fakeHome(t, `{"token":"session-jwt"}`) _, u := logoutServer(t, http.StatusBadGateway) withAPIBase(t, u) + warnings := captureLogoutWarnings(t) keepToolsFlag = true t.Cleanup(func() { keepToolsFlag = false }) if err := runLogout(logoutCmd, nil); err != nil { t.Fatalf("runLogout: %v", err) } + warn := warnings.String() + for _, want := range []string{"Could not end the session on the platform", "HTTP 502", "stays valid until it expires", "Sign out other sessions"} { + if !strings.Contains(warn, want) { + t.Errorf("warning %q does not say %q", warn, want) + } + } + if strings.Contains(warn, "session-jwt") { + t.Error("the warning prints the session token") + } if sessionFileExists(home) { t.Error("a platform failure kept the member signed in locally") } diff --git a/internal/api/client.go b/internal/api/client.go index 65a0386..02c8a41 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "strings" "time" ) @@ -36,6 +37,12 @@ func New(token string) *Client { return &Client{token: token, http: &http.Client{Timeout: requestTimeout}, baseURL: BaseURL} } +// WithBaseURL points the client somewhere else, for tests. +func (c *Client) WithBaseURL(u string) *Client { + c.baseURL = strings.TrimRight(u, "/") + return c +} + // Token is what this client sends. The panel builds a client once, at start, // and has to build another when a member signs in from inside it - a client // still holding the token it started with sends an empty cookie and the diff --git a/internal/api/logout.go b/internal/api/logout.go index 9fb9f0e..2ec31db 100644 --- a/internal/api/logout.go +++ b/internal/api/logout.go @@ -12,13 +12,9 @@ import ( // platform that accepts the connection and never answers: the local sign-out // goes ahead either way, this only decides how long it waits to say whether // the platform got the message. -const logoutTimeout = 10 * time.Second - -// WithBaseURL points the client somewhere else, for tests. -func (c *Client) WithBaseURL(u string) *Client { - c.baseURL = u - return c -} +// +// A variable only so the tests can make a hanging platform time out quickly. +var logoutTimeout = 10 * time.Second // Logout ends the session on the platform (POST /auth/logout), so the token // stops working everywhere and not only on this machine. Deleting @@ -30,7 +26,10 @@ func (c *Client) WithBaseURL(u string) *Client { // logout still revokes it. Both name the same session; it is revoked once. // // A 401 means the platform already refuses the session, which is the outcome -// wanted, so it is not an error. Anything else that is not 2xx is: nothing was +// wanted, so it is not an error. This assumes cloud-api answers 401 here only +// for a session it rejects, which is true today: /auth/logout itself never +// 401s (it is public and answers 200), and the edge in front of it does not +// authenticate. Redirects are not followed: a 3xx did not revoke anything. Anything else that is not 2xx is: nothing was // revoked, and the caller should say so. func (c *Client) Logout() error { if c.token == "" { @@ -52,7 +51,9 @@ func (c *Client) Logout() error { req.Header.Set("Accept", "application/json") req.Header.Set("User-Agent", "nan-cli") - resp, err := c.http.Do(req) + hc := *c.http + hc.CheckRedirect = noRedirects + resp, err := hc.Do(req) if err != nil { return err } diff --git a/internal/api/logout_test.go b/internal/api/logout_test.go index 49ec59f..30d250c 100644 --- a/internal/api/logout_test.go +++ b/internal/api/logout_test.go @@ -4,6 +4,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" ) // Spec (prod E2E 2026-10-12): signing out of the CLI must end the session on @@ -80,3 +81,48 @@ func TestLogoutWithoutASessionSendsNothing(t *testing.T) { t.Errorf("sent %d requests with no session", calls) } } + +// A platform that accepts the connection and never answers does not hold the +// sign-out hostage: Logout gives up and says it was not confirmed. +func TestLogoutGivesUpOnAHangingPlatform(t *testing.T) { + release := make(chan struct{}) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + select { + case <-release: + case <-r.Context().Done(): + } + })) + defer srv.Close() + defer close(release) + + orig := logoutTimeout + logoutTimeout = 50 * time.Millisecond + defer func() { logoutTimeout = orig }() + + start := time.Now() + if err := New("t").WithBaseURL(srv.URL).Logout(); err == nil { + t.Error("a platform that never answered reads as a revoked session") + } + if took := time.Since(start); took > 5*time.Second { + t.Errorf("Logout waited %v on a hanging platform", took) + } +} + +// A redirect is not a revocation, and the session is not sent on to wherever +// it points. +func TestLogoutDoesNotFollowRedirects(t *testing.T) { + followed := false + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { followed = true })) + defer target.Close() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target.URL+"/auth/logout", http.StatusTemporaryRedirect) + })) + defer srv.Close() + + if err := New("t").WithBaseURL(srv.URL + "/").Logout(); err == nil { + t.Error("a 307 reads as a revoked session") + } + if followed { + t.Error("Logout followed the redirect") + } +} diff --git a/internal/tui/config_test.go b/internal/tui/config_test.go index 92f81c9..d2d7860 100644 --- a/internal/tui/config_test.go +++ b/internal/tui/config_test.go @@ -2292,7 +2292,7 @@ func TestSigningOutTakesTwoPressesAndClearsEverything(t *testing.T) { } // Two presses do it, and take the key and the cached answers with them. - out := press(press(m, 'o'), 'o') + out := signOut(t, m) if out.sess.Token != "" || out.sess.APIKey != "" { t.Error("signing out left the session behind") } @@ -2321,11 +2321,7 @@ func TestSigningOutStartsTheSetupAgain(t *testing.T) { m := newModel(api.New("t").WithBaseURL(newFakePlatform(t, http.StatusOK).url), sess) m.lay = newLayout(90, 30) - press := func(m model, r rune) model { - updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{r}}) - return updated.(model) - } - out := press(press(m, 'o'), 'o') + out := signOut(t, m) if out.wizard != wizardEmail { t.Errorf("after signing out the panel is at %v, want the first step", out.wizard) } diff --git a/internal/tui/signout_server_test.go b/internal/tui/signout_server_test.go index 1c5a7ec..c35242a 100644 --- a/internal/tui/signout_server_test.go +++ b/internal/tui/signout_server_test.go @@ -4,8 +4,10 @@ import ( "net/http" "net/http/httptest" "path/filepath" + "strings" "sync" "testing" + "time" tea "github.com/charmbracelet/bubbletea" @@ -39,33 +41,57 @@ func newFakePlatform(t *testing.T, status int) *fakePlatform { return fp } +func pressKey(m model, r rune) (model, tea.Cmd) { + updated, cmd := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{r}}) + return updated.(model), cmd +} + +// signOut presses `o` twice and lets the sign-out command run to completion, +// feeding its answer back the way the bubbletea runtime would. +func signOut(t *testing.T, m model) model { + t.Helper() + m, _ = pressKey(m, 'o') + m, cmd := pressKey(m, 'o') + if cmd == nil { + t.Fatal("the second press started no sign-out") + } + msg, ok := cmd().(signedOutMsg) + if !ok { + t.Fatal("the sign-out command did not answer with signedOutMsg") + } + updated, _ := m.Update(msg) + return updated.(model) +} + +func signedInPanel(t *testing.T, platformURL string) model { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("HERMES_HOME", filepath.Join(home, "h")) + sess := &session.Session{Token: "session-jwt", APIKey: testKey} + if err := session.Save(sess); err != nil { + t.Fatal(err) + } + m := newModel(api.New("session-jwt").WithBaseURL(platformURL), sess) + m.lay = newLayout(90, 30) + return m +} + // Spec (prod E2E 2026-10-12): signing out from the panel ends the session on // the platform before session.json goes, and a platform that does not answer -// does not keep the member signed in on this machine. +// does not keep the member signed in on this machine; the member is told the +// session may outlive the sign-out. func TestSigningOutEndsTheSessionOnThePlatform(t *testing.T) { for _, status := range []int{http.StatusOK, http.StatusBadGateway} { t.Run(http.StatusText(status), func(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) - t.Setenv("USERPROFILE", home) - t.Setenv("HERMES_HOME", filepath.Join(home, "h")) - - sess := &session.Session{Token: "session-jwt", APIKey: testKey} - if err := session.Save(sess); err != nil { - t.Fatal(err) - } fp := newFakePlatform(t, status) - m := newModel(api.New("session-jwt").WithBaseURL(fp.url), sess) - m.lay = newLayout(90, 30) + m := signedInPanel(t, fp.url) - press := func(m model, r rune) model { - updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{r}}) - return updated.(model) - } - if after := press(m, 'o'); len(fp.seen) != 0 { - t.Fatalf("one press already ended the session (%v); confirm=%v", fp.seen, after.confirmSignOut) + if _, cmd := pressKey(m, 'o'); cmd != nil || len(fp.seen) != 0 { + t.Fatalf("one press already started a sign-out (%v)", fp.seen) } - out := press(press(m, 'o'), 'o') + out := signOut(t, m) if len(fp.seen) != 1 || fp.seen[0] != "POST /auth/logout Bearer session-jwt" { t.Fatalf("platform saw %v, want one POST /auth/logout with the session bearer", fp.seen) @@ -73,12 +99,65 @@ func TestSigningOutEndsTheSessionOnThePlatform(t *testing.T) { if !fp.sessionOnDisk[0] { t.Error("session.json was deleted before the platform was asked") } - if out.sess.Token != "" { + if out.sess.Token != "" || out.signingOut { t.Error("the panel kept the session") } if _, err := session.Load(); err == nil { t.Error("session.json is still on disk") } + warned := strings.Contains(out.setupMsg, "did not confirm") + if status == http.StatusOK && warned { + t.Errorf("a confirmed sign-out warns: %q", out.setupMsg) + } + if status != http.StatusOK && !warned { + t.Errorf("an unconfirmed sign-out does not say so: %q", out.setupMsg) + } }) } } + +// The platform is asked from a command, not from Update: a platform that +// accepts the connection and never answers must not freeze the panel. Until +// it answers, the member stays signed in here, the panel says it is signing +// out, and another `o` does not ask again. +func TestSigningOutDoesNotFreezeThePanelOnAHangingPlatform(t *testing.T) { + release := make(chan struct{}) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-release + })) + t.Cleanup(func() { close(release); srv.Close() }) + m := signedInPanel(t, srv.URL) + + m, _ = pressKey(m, 'o') + done := make(chan struct{}) + var cmd tea.Cmd + go func() { + m, cmd = pressKey(m, 'o') + close(done) + }() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("the second press blocked on the platform") + } + if cmd == nil || !m.signingOut || !strings.Contains(m.setupMsg, "signing out") { + t.Fatalf("no sign-out in flight: cmd=%v signingOut=%v msg=%q", cmd != nil, m.signingOut, m.setupMsg) + } + if _, err := session.Load(); err != nil { + t.Error("session.json went before the platform answered") + } + m, _ = pressKey(m, 'o') + if _, again := pressKey(m, 'o'); again != nil { + t.Error("another o while signing out asks the platform again") + } +} + +// A stray answer (no sign-out in flight) changes nothing. +func TestAStraySignedOutMsgIsIgnored(t *testing.T) { + fp := newFakePlatform(t, http.StatusOK) + m := signedInPanel(t, fp.url) + updated, _ := m.Update(signedOutMsg{}) + if updated.(model).sess.Token == "" { + t.Error("a signedOutMsg nobody asked for signed the member out") + } +} diff --git a/internal/tui/tui.go b/internal/tui/tui.go index aad5293..5bb6d49 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -155,6 +155,18 @@ type firstRunMsg struct{} type linkSentMsg struct{ err error } type signedInMsg struct{ err error } +// signedOutMsg is the platform's answer to ending the session (err nil when it +// confirmed). +type signedOutMsg struct{ err error } + +// revokeSession ends the session on the platform. It never fails the +// sign-out: the error only decides what the member is told. +func revokeSession(client *api.Client) tea.Cmd { + return func() tea.Msg { + return signedOutMsg{err: client.Logout()} + } +} + // ── model ───────────────────────────────────────────────────────────────────── type model struct { @@ -184,12 +196,15 @@ type model struct { // Set by the first press of `o`, cleared by anything else: signing out is // not something to do to somebody on a stray keystroke. confirmSignOut bool - wizard wizardStep - loginStage loginStage - loginInput textinput.Model - loginEmail string - loginMsg string - loginBusy bool + // signingOut: the platform has been asked to end the session and has not + // answered yet. A third `o` does not ask again. + signingOut bool + wizard wizardStep + loginStage loginStage + loginInput textinput.Model + loginEmail string + loginMsg string + loginBusy bool } // Where a member is in the guided setup. The inner inputs still drive @@ -333,6 +348,16 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.loading = false m.err = msg.err + case signedOutMsg: + // Only the answer to a sign-out still in flight. The session may have + // expired meanwhile (sessionExpiredMsg cleared the token); the member + // still asked to sign out, so the local half still runs. + if !m.signingOut { + return m, nil + } + m.signingOut = false + return m.finishSignOut(msg.err) + case sessionExpiredMsg: if msg.token == "" || msg.token != m.sess.Token { return m, nil @@ -637,39 +662,23 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.confirmSignOut = false // End the session on the platform first, while session.json // still holds it: deleting the file alone leaves any copy of the - // token working until it expires. A failure does not keep the - // member signed in here; the local sign-out goes ahead. - revokeErr := m.client.Logout() - if err := session.Delete(); err != nil { - m.setupMsg = "error: " + err.Error() + // token working until it expires. That is a network call, so it + // runs as a command and the panel stays responsive; the local + // sign-out happens when it answers (signedOutMsg), whatever the + // answer. Quitting before then leaves the member signed in here, + // which is consistent: nothing was revoked either. + if m.signingOut { break } - // Everything the session was holding goes with it: the key lives in - // the same file, and the tabs are full of answers that were true - // for somebody else. - // - // And the key does not only live there. Every tool configured from - // this panel got a copy, where deleting session.json leaves it - // working - so a member who signs out because they are handing the - // machine over would have been signed out of everything except the - // cluster their key bills. - removed, failed := RemoveNanFromTools() - m.sess = &session.Session{} - m.client = api.New("") - m.cache = make(map[tabID]any) - m.keyStatus, m.keyAsked, m.keyCheck = nil, false, "" - m.configured, m.setupMsg = nil, signOutMessage(removed, failed) - if revokeErr != nil && len(failed) == 0 { - m.setupMsg = "signed out here, but the platform did not confirm it (" + revokeErr.Error() + ") · the session stays valid until it expires" - } - m.err = nil - return m, m.resumeSetup() + m.signingOut = true + m.setupMsg = "signing out…" + return m, revokeSession(m.client) // `s` and not `l`: l is already the vim spelling of "next tab". case "s": // Any tab, because the one a member is looking at when this is // needed is whichever they walked into and got told to sign in. - if !m.showHelp && m.sess.Token == "" { + if !m.showHelp && m.sess.Token == "" && !m.signingOut { return m, m.startLogin() } @@ -2110,6 +2119,40 @@ func RemoveNanFromTools() (removed []string, failed []string) { return removed, failed } +// finishSignOut is the local half of signing out, once the platform has +// answered (revokeErr nil when it confirmed the session is over). +func (m model) finishSignOut(revokeErr error) (tea.Model, tea.Cmd) { + if err := session.Delete(); err != nil { + m.setupMsg = "error: " + err.Error() + return m, nil + } + // Everything the session was holding goes with it: the key lives in + // the same file, and the tabs are full of answers that were true + // for somebody else. + // + // And the key does not only live there. Every tool configured from + // this panel got a copy, where deleting session.json leaves it + // working - so a member who signs out because they are handing the + // machine over would have been signed out of everything except the + // cluster their key bills. + removed, failed := RemoveNanFromTools() + m.sess = &session.Session{} + m.client = api.New("") + m.cache = make(map[tabID]any) + m.keyStatus, m.keyAsked, m.keyCheck = nil, false, "" + m.configured, m.setupMsg = nil, signOutMessage(removed, failed) + if revokeErr != nil { + m.setupMsg += " · " + revokeNotConfirmed(revokeErr) + } + m.err = nil + return m, m.resumeSetup() +} + +// revokeNotConfirmed says the session may outlive the local sign-out. +func revokeNotConfirmed(err error) string { + return "the platform did not confirm the sign-out (" + err.Error() + "): the session stays valid until it expires" +} + // What to say afterwards. Signing out and leaving the key behind in four other // files is worth a sentence either way. func signOutMessage(removed, failed []string) string {