Skip to content

Commit 6621920

Browse files
toothbrushclaude
andcommitted
auth: surface contexts read errors; cleaner not-logged-in UX
Address two review comments: - ResolveControlPlaneTarget already failed loud on a contexts.json read/parse error; make `auth status` (resolveStatusTarget) symmetric — surface a genuine load error instead of swallowing it into the legacy fallback. A missing file still reads as "no contexts" (not an error), so this only fires on real corruption/IO failure, which the user must see before a control-plane mutation acts as a stale identity. - providerSource.BearerAuth no longer prefixes the active-context error. NewRefreshingLoginProvider already returns a tailored message naming the context, its login server, and the exact re-login command; surface it verbatim. The bare ErrNotLoggedIn sentinel (static fallback path) still gets the standard 'entire login' hint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 52317c4f2e8f
1 parent 0b6eacf commit 6621920

6 files changed

Lines changed: 78 additions & 27 deletions

File tree

cmd/entire/cli/auth.go

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,10 @@ func newAuthStatusCmd() *cobra.Command {
167167
if err := requireSecureBaseURL(insecureHTTPAuth); err != nil {
168168
return err
169169
}
170-
target := resolveStatusTarget(auth.NewContextStore(), auth.Contexts, api.AuthBaseURL())
170+
target, err := resolveStatusTarget(auth.NewContextStore(), auth.Contexts, api.AuthBaseURL())
171+
if err != nil {
172+
return err
173+
}
171174
// We send the session token to target.coreURL; enforce TLS on it
172175
// too (it may differ from AuthBaseURL when a context is active).
173176
if !insecureHTTPAuth {
@@ -220,25 +223,31 @@ type statusTarget struct {
220223
// active contexts.json context wins (so `auth use` retargets status onto that
221224
// login server); otherwise it falls back to the legacy keyring entry keyed by
222225
// the configured auth host.
223-
func resolveStatusTarget(store tokenStore, listContexts contextsProvider, fallbackBaseURL string) statusTarget {
226+
//
227+
// A genuine contexts.json read/parse error is surfaced, not swallowed — a
228+
// missing file reads as "no contexts" (no error), so an error here means the
229+
// file is corrupt or unreadable, which the user must see. This keeps status
230+
// symmetric with the control-plane commands (auth.ResolveControlPlaneTarget),
231+
// which fail the same way rather than silently degrading to a stale identity.
232+
func resolveStatusTarget(store tokenStore, listContexts contextsProvider, fallbackBaseURL string) (statusTarget, error) {
224233
all, current, err := listContexts()
225-
total := 0
226-
if err == nil {
227-
total = len(all)
228-
for _, c := range all {
229-
if c.Name != current || c.CoreURL == "" {
230-
continue
231-
}
232-
if tok, terr := auth.LoginTokenForContext(c); terr == nil && tok != "" {
233-
return statusTarget{coreURL: c.CoreURL, token: tok, activeContext: c.Name, totalContexts: total}
234-
}
234+
if err != nil {
235+
return statusTarget{}, fmt.Errorf("load contexts: %w", err)
236+
}
237+
total := len(all)
238+
for _, c := range all {
239+
if c.Name != current || c.CoreURL == "" {
240+
continue
241+
}
242+
if tok, terr := auth.LoginTokenForContext(c); terr == nil && tok != "" {
243+
return statusTarget{coreURL: c.CoreURL, token: tok, activeContext: c.Name, totalContexts: total}, nil
235244
}
236245
}
237246
tok, gerr := store.GetToken(fallbackBaseURL)
238247
if gerr != nil {
239248
tok = "" // best-effort: a keyring read failure just reads as "no token"
240249
}
241-
return statusTarget{coreURL: fallbackBaseURL, token: tok, totalContexts: total}
250+
return statusTarget{coreURL: fallbackBaseURL, token: tok, totalContexts: total}, nil
242251
}
243252

244253
// defaultFetchProfile fetches a user's profile from coreURL's GET /me with the

cmd/entire/cli/auth/control_plane_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package auth
33
import (
44
"context"
55
"fmt"
6+
"os"
67
"path/filepath"
78
"testing"
89
"time"
@@ -81,6 +82,20 @@ func TestResolveControlPlaneTarget_ActiveContextWins(t *testing.T) {
8182
}
8283
}
8384

85+
// A genuine contexts.json read/parse error must fail loud — not silently fall
86+
// back to a stale legacy identity for a control-plane mutation.
87+
func TestResolveControlPlaneTarget_CorruptContextsErrors(t *testing.T) {
88+
configDir := t.TempDir()
89+
t.Setenv("ENTIRE_CONFIG_DIR", configDir)
90+
t.Setenv(api.AuthBaseURLEnvVar, "")
91+
if err := os.WriteFile(filepath.Join(configDir, "contexts.json"), []byte("{ not valid json"), 0o600); err != nil {
92+
t.Fatalf("write corrupt contexts.json: %v", err)
93+
}
94+
if _, err := ResolveControlPlaneTarget(); err == nil {
95+
t.Fatal("want an error when contexts.json is corrupt, got nil")
96+
}
97+
}
98+
8499
// With no active context, the target falls back to the configured auth origin
85100
// — the default when ENTIRE_AUTH_BASE_URL is unset, or the env value when set
86101
// (the env var is the fallback host).

cmd/entire/cli/auth_context_test.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"encoding/base64"
66
"fmt"
7+
"os"
78
"path/filepath"
89
"strings"
910
"testing"
@@ -28,7 +29,10 @@ func TestResolveStatusTarget_PrefersActiveContext(t *testing.T) {
2829
t.Fatalf("record context: %v", err)
2930
}
3031

31-
got := resolveStatusTarget(auth.NewContextStore(), auth.Contexts, "https://fallback.example.com")
32+
got, err := resolveStatusTarget(auth.NewContextStore(), auth.Contexts, "https://fallback.example.com")
33+
if err != nil {
34+
t.Fatalf("resolveStatusTarget: %v", err)
35+
}
3236
if got.coreURL != "https://eu.auth.entire.io" {
3337
t.Errorf("coreURL = %q, want the active context's CoreURL", got.coreURL)
3438
}
@@ -40,6 +44,23 @@ func TestResolveStatusTarget_PrefersActiveContext(t *testing.T) {
4044
}
4145
}
4246

47+
// A genuine contexts.json read/parse error is surfaced by resolveStatusTarget,
48+
// symmetric with the control-plane commands — not swallowed into the legacy
49+
// fallback. (A missing file reads as "no contexts" and is not an error.)
50+
func TestResolveStatusTarget_CorruptContextsErrors(t *testing.T) {
51+
cfgDir := t.TempDir()
52+
t.Setenv("ENTIRE_CONFIG_DIR", cfgDir)
53+
restore := tokenstore.UseFileBackendForTesting(filepath.Join(t.TempDir(), "tokens.json"))
54+
t.Cleanup(restore)
55+
56+
if err := os.WriteFile(filepath.Join(cfgDir, "contexts.json"), []byte("{ not valid json"), 0o600); err != nil {
57+
t.Fatalf("write corrupt contexts.json: %v", err)
58+
}
59+
if _, err := resolveStatusTarget(auth.NewContextStore(), auth.Contexts, "https://fallback.example.com"); err == nil {
60+
t.Fatal("want an error when contexts.json is corrupt, got nil")
61+
}
62+
}
63+
4364
// makeContextJWT builds a JWT-shaped token (non-"none" alg) carrying the
4465
// given claims, which is all RecordLoginContext needs.
4566
func makeContextJWT(t *testing.T, payloadJSON string) string {

cmd/entire/cli/logout.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,10 @@ func newLogoutCmd() *cobra.Command {
7474

7575
// Revoke against the active context's core (matching what
7676
// `auth status` lists), not a static AuthBaseURL.
77-
target := resolveStatusTarget(auth.NewContextStore(), auth.Contexts, api.AuthBaseURL())
77+
target, err := resolveStatusTarget(auth.NewContextStore(), auth.Contexts, api.AuthBaseURL())
78+
if err != nil {
79+
return err
80+
}
7881
if !insecureHTTPAuth {
7982
if err := api.RequireSecureURL(target.coreURL); err != nil {
8083
return fmt.Errorf("context login server URL check: %w", err)

internal/coreapi/client.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,17 @@ type providerSource struct {
8484
func (p *providerSource) BearerAuth(ctx context.Context, _ OperationName) (BearerAuth, error) {
8585
token, err := p.provide(ctx)
8686
if err != nil {
87-
// Only suggest login when the user genuinely isn't logged in.
88-
// Other failures (STS rejection, refresh-expired, network) already
89-
// carry descriptive messages and must surface verbatim rather than
90-
// be masked by a login hint.
87+
// The static fallback path returns a bare ErrNotLoggedIn sentinel with
88+
// no helpful text, so add the standard login hint. The active-context
89+
// path (NewRefreshingLoginProvider) instead returns a tailored message
90+
// that already names the context, its login server, and the exact
91+
// re-login command — surface that verbatim rather than burying it under
92+
// a generic prefix. Other failures (STS rejection, network) are
93+
// likewise self-descriptive.
9194
if errors.Is(err, auth.ErrNotLoggedIn) {
9295
return BearerAuth{}, fmt.Errorf("not logged in — run 'entire login': %w", err)
9396
}
94-
return BearerAuth{}, fmt.Errorf("resolve control-plane token: %w", err)
97+
return BearerAuth{}, err
9598
}
9699
return BearerAuth{Token: token}, nil
97100
}

internal/coreapi/client_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -108,16 +108,16 @@ func TestProviderSource_BearerAuth(t *testing.T) {
108108
}
109109
})
110110

111-
t.Run("other errors surface without the login hint", func(t *testing.T) {
111+
t.Run("other errors surface verbatim", func(t *testing.T) {
112112
t.Parallel()
113-
sentinel := errors.New("STS rejected the exchange")
113+
// The active-context provider returns an already-tailored message; it
114+
// must reach the user unprefixed (no generic "resolve control-plane
115+
// token" wrapper burying it) and without the login hint.
116+
sentinel := errors.New(`no usable login for "ctx" (https://core.example); run ENTIRE_AUTH_BASE_URL=https://core.example entire login`)
114117
src := &providerSource{provide: func(context.Context) (string, error) { return "", sentinel }}
115118
_, err := src.BearerAuth(context.Background(), "")
116-
if err == nil || !errors.Is(err, sentinel) {
117-
t.Fatalf("error = %v, want it to wrap the sentinel", err)
118-
}
119-
if strings.Contains(err.Error(), "entire login") {
120-
t.Fatalf("non-auth error must not carry a login hint: %v", err)
119+
if err == nil || err.Error() != sentinel.Error() {
120+
t.Fatalf("error = %v, want the provider message surfaced verbatim", err)
121121
}
122122
})
123123
}

0 commit comments

Comments
 (0)