From 763e95ea2e666b676f1afab79d5b86ff04086957 Mon Sep 17 00:00:00 2001 From: Antony Pegg Date: Mon, 19 Oct 2026 16:26:12 -0400 Subject: [PATCH 1/6] feat: renew browser sessions with refresh tokens Login asks for offline_access and keeps Auth0's refresh token in the OS keychain. A session near expiry renews itself, saving the new access token and the rotated refresh token before use. invalid_grant clears the refresh token and ends the session; logout revokes it. Without a refresh token or a keychain, step-1 behaviour is unchanged. --- internal/auth/auth.go | 40 +++ internal/clitest/stderr_sanitize_test.go | 4 +- internal/httplog/httplog.go | 6 +- internal/starfleet/account/cmd/auth.go | 93 ++++-- .../starfleet/account/cmd/auth_browser.go | 29 +- .../account/cmd/auth_browser_test.go | 279 +++++++++++++++++- internal/starfleet/account/cmd/doctor.go | 20 +- internal/starfleet/conn/conn.go | 115 ++++++-- internal/starfleet/conn/session_test.go | 30 +- internal/starfleet/signin/signin.go | 108 +++++-- internal/starfleet/signin/signin_test.go | 5 +- 11 files changed, 626 insertions(+), 103 deletions(-) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index d95471c..f0438d8 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -321,6 +321,46 @@ func (a *Auth) KeychainSecret() (string, error) { return a.Keychain.Get(a.KeychainAccount()) } +// sessionAccountPrefix keeps a browser session's refresh token in an +// entry of its own, apart from any client secret of the same profile. +const sessionAccountPrefix = "session-refresh-token:" + +// SessionAccount is the keychain entry for this profile's browser +// session refresh token. +func (a *Auth) SessionAccount() string { + return sessionAccountPrefix + a.KeychainAccount() +} + +// SessionRefreshToken reads the browser session's refresh token. +func (a *Auth) SessionRefreshToken() (string, error) { + if a.Keychain == nil { + return "", keychain.ErrUnavailable + } + return a.Keychain.Get(a.SessionAccount()) +} + +// SaveSessionRefreshToken stores the browser session's refresh token. +// It is never written to the config file or the token cache. +func (a *Auth) SaveSessionRefreshToken(token string) error { + if a.Keychain == nil { + return keychain.ErrUnavailable + } + return a.Keychain.Set(a.SessionAccount(), token) +} + +// ClearSessionRefreshToken removes the browser session's refresh +// token; there being none is not an error. +func (a *Auth) ClearSessionRefreshToken() error { + if a.Keychain == nil { + return nil + } + err := a.Keychain.Delete(a.SessionAccount()) + if errors.Is(err, keychain.ErrNotFound) { + return nil + } + return err +} + // SaveToken writes the token to the profile+module cache file with // 0600 permissions. It refuses a token with no Fingerprint, so no // caller, including a future one, can write an unbound cache file; diff --git a/internal/clitest/stderr_sanitize_test.go b/internal/clitest/stderr_sanitize_test.go index 224671a..9d57234 100644 --- a/internal/clitest/stderr_sanitize_test.go +++ b/internal/clitest/stderr_sanitize_test.go @@ -299,8 +299,10 @@ var stderrComposedMessages = map[string]struct { // 199 -> 200: `branch get` prints the display name. // 200 -> 211: browser sign-in's URL, tenant prompt and summary lines, // and whoami's `User:` line. +// 211 -> 215: session renewal's keychain warning, and logout's revoke +// and keychain warnings. const ( - stderrArgSites = 211 + stderrArgSites = 215 stderrComposedSites = 22 ) diff --git a/internal/httplog/httplog.go b/internal/httplog/httplog.go index 8f793ca..f12ef2a 100644 --- a/internal/httplog/httplog.go +++ b/internal/httplog/httplog.go @@ -308,9 +308,9 @@ func RedactBody(raw []byte, limit int) string { return wholeBodySummary(doc) } -// formSecretNames are the form fields of an OAuth code exchange that -// would let a reader of the log finish it. -var formSecretNames = []string{"code", "code_verifier"} +// formSecretNames are the OAuth form fields that would let a reader of +// the log finish a code exchange or use a token being revoked. +var formSecretNames = []string{"code", "code_verifier", "token"} // RedactForm is RedactBody for a form-encoded body: the value of every // secret field, JSON names included, is masked, and the field order is diff --git a/internal/starfleet/account/cmd/auth.go b/internal/starfleet/account/cmd/auth.go index 6ea43c4..2cb1312 100644 --- a/internal/starfleet/account/cmd/auth.go +++ b/internal/starfleet/account/cmd/auth.go @@ -16,6 +16,7 @@ import ( "github.com/pgEdge/pgedge-cli/internal/module" "github.com/pgEdge/pgedge-cli/internal/output" "github.com/pgEdge/pgedge-cli/internal/starfleet/conn" + "github.com/pgEdge/pgedge-cli/internal/starfleet/signin" "github.com/spf13/cobra" "golang.org/x/term" ) @@ -60,11 +61,14 @@ you, through the browser, or as an API client. With no credential flags, login opens your browser on the pgEdge sign-in page and prints the same URL on stderr, so you can open it by hand when no browser starts. Once you sign in, every command runs as -you, in your tenant, until the session expires; login prints when -that is, and you run login again after it. The profile stores the API -URL, the tenant and the fact that it is a browser sign-in; the token -goes to the token cache, never the config file. When you belong to -several tenants, login asks which to use when it can prompt, and uses +you, in your tenant. The session renews itself while in use and ends +after a period without use; run login again to start a new one. The +profile stores the API URL, the tenant and the fact that it is a +browser sign-in; the access token goes to the token cache and the +token that renews the session to the OS keychain, never the config +file. Where no keychain can be used, the session lasts until the time +login prints. When you belong to several tenants, login asks which to +use when it can prompt, and uses your account's default tenant otherwise. --tenant picks one by ID or name without asking. Browser sign-in is available only for pgEdge Starfleet APIs that have a sign-in app; for any other --api-url, @@ -184,6 +188,9 @@ Example: } return fmt.Errorf("save credentials: %w", err) } + if cp.IsBrowserSession() { + _ = store.ClearSessionRefreshToken() + } // The file Save() actually wrote, not DefaultPath(): Save // writes the --config value when one was given. @@ -268,6 +275,8 @@ type authStatusReport struct { // TokenBound means what authInfo.TokenBound (doctor.go) means. TokenBound bool `json:"token_bound"` ExpiresAt string `json:"expires_at,omitempty"` + // Renews reports a browser session that can renew its own token. + Renews bool `json:"renews"` } func newAuthStatusCmd(rt *module.Runtime, f *conn.Flags) *cobra.Command { @@ -279,15 +288,18 @@ current profile, the API URL in effect, and whether a cached token is present and still valid. -o json and -o yaml carry the same facts as a single object, so a script can check them without parsing prose. -For a browser sign-in it reports the user, the tenant and when the -session expires; for an API client, its client ID. +For a browser sign-in it reports the user, the tenant, when the +current access token expires and whether the session renews itself; +for an API client, its client ID. It exits 5 (the auth-failure code) when no usable credentials resolve, 2 if you supplied half a credential pair, and 0 whenever they do. An API client's token may be missing or expired with exit 0, since the CLI mints a new one on demand the next time it -is needed. A browser session cannot be renewed that way, so an -expired one exits 5; run 'pgedge starfleet auth login' again. Run +is needed. A browser session that renews itself does the same, so an +expired access token is exit 0 and renews on next use; one that +cannot renew itself exits 5 once it expires, and 'pgedge starfleet +auth login' starts a new session. Run status to confirm you are signed in before using resource commands. Example: @@ -383,10 +395,10 @@ Example: // sessionState is what status and doctor read from a browser session's // cache, and the failure a command would hit with it, if any. type sessionState struct { - tok *auth.CachedToken - valid, bound bool - tenant string - problem error + tok *auth.CachedToken + valid, bound, renews bool + tenant string + problem error } // readSession checks a session the way conn.Resolve would, without @@ -403,8 +415,10 @@ func readSession(rt *module.Runtime, res *conn.Resolved, s.tok = tok s.valid = time.Until(tok.ExpiresAt) > auth.RefreshWindow s.bound = tok.SessionFor(res.APIURL) + _, rerr := conn.Store(rt).SessionRefreshToken() + s.renews = rerr == nil claims, err := auth.ParseClaims(tok.AccessToken) - if !s.valid || !s.bound || err != nil { + if (!s.valid && !s.renews) || !s.bound || err != nil { s.problem = expired return s } @@ -415,9 +429,8 @@ func readSession(rt *module.Runtime, res *conn.Resolved, return s } -// sessionStatus reports a browser session. Unlike an API client, an -// expired session cannot be renewed on demand, so it exits 5; a tenant -// the token cannot reach exits 2. +// sessionStatus reports a browser session. An expired one that cannot +// renew itself exits 5; a tenant the token cannot reach exits 2. func sessionStatus( rt *module.Runtime, res *conn.Resolved, flagTenant string, ) error { @@ -429,6 +442,7 @@ func sessionStatus( TenantID: s.tenant, TokenValid: s.valid, TokenBound: s.bound, + Renews: s.renews, } if s.tok != nil { report.User = s.tok.User @@ -449,12 +463,19 @@ func sessionStatus( case !s.bound: lines = append(lines, "Token: stale (signed in for a different API URL)") + case !s.valid && s.renews: + lines = append(lines, "Token: expired, renews on next use") case !s.valid: lines = append(lines, "Token: expired") default: lines = append(lines, fmt.Sprintf( "Token: valid (expires %s)", report.ExpiresAt)) } + renews := "no" + if s.renews { + renews = "yes" + } + lines = append(lines, "Renews: "+renews) if pErr := printAuthStatus( rt, report, strings.Join(lines, "\n")+"\n"); pErr != nil { return pErr @@ -481,14 +502,19 @@ func newAuthLogoutCmd(rt *module.Runtime) *cobra.Command { Long: `logout removes the cached access token and the stored credential for the active profile: for an API client, its client ID and secret, from the OS keychain and from ~/.pgedge/cli/config.yaml; -for a browser sign-in, the session and its tenant. The API URL and the -profile itself are kept, so 'pgedge starfleet auth login' can sign in -again without re-entering the endpoint. +for a browser sign-in, the session and its tenant, after asking pgEdge +to end the session so it cannot be renewed. If that request fails, +logout warns and still removes the session from this machine. The API +URL and the profile itself are kept, so 'pgedge starfleet auth login' +can sign in again without re-entering the endpoint. Example: pgedge starfleet auth logout`, Args: cobra.NoArgs, RunE: func(_ *cobra.Command, _ []string) error { + if rt.Config.StarfleetProfile(rt.Profile).IsBrowserSession() { + endSession(rt) + } if err := conn.Logout(rt); err != nil { return fmt.Errorf("clear token: %w", err) } @@ -522,6 +548,33 @@ Example: } } +// endSession revokes the session's refresh token, best effort, and +// removes it from the keychain. +func endSession(rt *module.Runtime) { + store := conn.Store(rt) + token, err := store.SessionRefreshToken() + if err != nil { + return + } + apiURL := conn.ResolveAPIURL(rt.Config.StarfleetProfile(rt.Profile), "") + if app, base, ok := conn.SignInApp(apiURL); ok { + ctx, cancel := context.WithTimeout(context.Background(), + conn.RequestTimeout) + defer cancel() + if err := signin.Revoke(ctx, + conn.SignInHTTPClient(rt, conn.RequestTimeout), base, + app.ClientID, token); err != nil { + fmt.Fprintf(rt.Stderr, "Warning: could not end the session "+ + "with pgEdge (%s); it is removed from this machine.\n", + output.Sanitize(err.Error())) + } + } + if err := store.ClearSessionRefreshToken(); err != nil { + fmt.Fprintf(rt.Stderr, "Warning: could not remove the session "+ + "from the OS keychain: %s\n", output.Sanitize(err.Error())) + } +} + // promptCredentials asks for a client ID and secret on stdin. Call it // only when neither credential flag was supplied: a closed stdin fails // at the first read, which would break `login` in a script. diff --git a/internal/starfleet/account/cmd/auth_browser.go b/internal/starfleet/account/cmd/auth_browser.go index f3f70dc..fd4e629 100644 --- a/internal/starfleet/account/cmd/auth_browser.go +++ b/internal/starfleet/account/cmd/auth_browser.go @@ -23,10 +23,6 @@ import ( "github.com/pgEdge/pgedge-cli/internal/starfleet/signin" ) -// auth0For finds the sign-in app for an API URL. A variable so tests -// can sign in against a stub API. -var auth0For = apidefaults.Auth0For - // signInFlow builds the browser sign-in. A variable so tests can point // it at a fake Auth0 and stand in for the browser. var signInFlow = func(rt *module.Runtime, app apidefaults.Auth0App, @@ -59,7 +55,7 @@ var promptIsInteractive = func(rt *module.Runtime) bool { func browserLogin(rt *module.Runtime, f *conn.Flags) error { apiURL := conn.ResolveAPIURL( rt.Config.StarfleetProfile(rt.Profile), f.APIURL) - app, ok := auth0For(apiURL) + app, _, ok := conn.SignInApp(apiURL) if !ok { // Exit 5, not 2: the command is well formed, and the same line // works once the API has a sign-in app. @@ -103,6 +99,15 @@ func browserLogin(rt *module.Runtime, f *conn.Flags) error { return newExitError(fmt.Sprintf("save session: %v", err), ExitAuth) } + // A previous session's refresh token must not outlive this sign-in. + store := conn.Store(rt) + renews := false + if toks.RefreshToken != "" { + renews = store.SaveSessionRefreshToken(toks.RefreshToken) == nil + } + if !renews { + _ = store.ClearSessionRefreshToken() + } // One profile, one kind of credential: an API client's secret goes. // Saved before the secret goes, so a failed save keeps a working @@ -114,7 +119,7 @@ func browserLogin(rt *module.Runtime, f *conn.Flags) error { return fmt.Errorf("save profile: %w", err) } if old.ClientID != "" && old.ClientSecret == "" { - if err := keychainDelete(conn.Store(rt)); err != nil { + if err := keychainDelete(store); err != nil { fmt.Fprintf(rt.Stderr, "Warning: could not remove the "+ "previous client secret from the OS keychain: %s\n", output.Sanitize(err.Error())) @@ -123,9 +128,15 @@ func browserLogin(rt *module.Runtime, f *conn.Flags) error { fmt.Fprintf(rt.Stderr, "Signed in as %s.\n", output.Sanitize(user)) fmt.Fprintf(rt.Stderr, "Tenant: %s\n", output.Sanitize(label)) - fmt.Fprintf(rt.Stderr, "The session lasts until %s; run 'pgedge "+ - "starfleet auth login' again after that.\n", - expiresAt.Format(time.RFC3339)) + if renews { + fmt.Fprintln(rt.Stderr, "The session renews itself while in use "+ + "and ends after a period without use; run 'pgedge starfleet "+ + "auth login' to start a new one.") + } else { + fmt.Fprintf(rt.Stderr, "The session lasts until %s; run 'pgedge "+ + "starfleet auth login' again after that.\n", + expiresAt.Format(time.RFC3339)) + } fmt.Fprintf(rt.Stderr, "Profile %s saved to %s\n", output.Sanitize(rt.Profile), output.Sanitize(rt.Config.Path())) return nil diff --git a/internal/starfleet/account/cmd/auth_browser_test.go b/internal/starfleet/account/cmd/auth_browser_test.go index 5f473dc..f365daf 100644 --- a/internal/starfleet/account/cmd/auth_browser_test.go +++ b/internal/starfleet/account/cmd/auth_browser_test.go @@ -4,18 +4,22 @@ import ( "bytes" "encoding/json" "errors" + "fmt" "io" "net/http" "net/http/httptest" "net/url" "os" "path/filepath" + "slices" "strings" + "sync" "sync/atomic" "testing" "time" "github.com/pgEdge/pgedge-cli/internal/apidefaults" + "github.com/pgEdge/pgedge-cli/internal/auth" "github.com/pgEdge/pgedge-cli/internal/config" "github.com/pgEdge/pgedge-cli/internal/keychain" "github.com/pgEdge/pgedge-cli/internal/module" @@ -31,7 +35,9 @@ const tenantsBody = `[` + // signInStubs stands up a fake Auth0 and a fake API, and points the // login command's seams at them. The fake Auth0 issues an access token -// reaching tenants, with the first as its default. +// reaching tenants, with the first as its default, and, when +// issueRefresh is set, a new refresh token per answer: refresh~1, +// refresh~2 and so on. A tilde cannot occur in a JWT. type signInStubs struct { api string tenants []string @@ -40,6 +46,19 @@ type signInStubs struct { reply func(q url.Values) url.Values apiHits map[string]int headers []http.Header + + issueRefresh bool + refreshes atomic.Int32 + // refreshError, when set, is the OAuth error a refresh gets. + refreshError string + // revokeStatus, when set, is the status /oauth/revoke answers. + revokeStatus int + // atAPI, when set, runs as each API request arrives. + atAPI func(*http.Request) + + mu sync.Mutex + refreshedWith []string + revoked []string } // stubCode is the authorization code the fake browser returns. @@ -54,20 +73,48 @@ func newSignInStubs(t *testing.T, tenants ...string) *signInStubs { auth0 := httptest.NewServer(http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { _ = r.ParseForm() - s.verifier.Store(r.PostForm.Get("code_verifier")) + w.Header().Set("Content-Type", "application/json") + s.mu.Lock() + defer s.mu.Unlock() + if r.URL.Path == "/oauth/revoke" { + s.revoked = append(s.revoked, r.PostForm.Get("token")) + if s.revokeStatus != 0 { + w.WriteHeader(s.revokeStatus) + } + return + } + if r.PostForm.Get("grant_type") == "refresh_token" { + s.refreshedWith = append(s.refreshedWith, + r.PostForm.Get("refresh_token")) + if s.refreshError != "" { + w.WriteHeader(http.StatusForbidden) + _ = json.NewEncoder(w).Encode(map[string]string{ + "error": s.refreshError}) + return + } + } else { + s.verifier.Store(r.PostForm.Get("code_verifier")) + } at := testsupport.SessionToken(t, "auth0|u1", time.Now().Add(24*time.Hour), s.tenants...) it := testsupport.FakeJWT(t, map[string]any{"email": "user@example.com"}) s.issued.Store(at + " " + it) - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(map[string]any{ + body := map[string]any{ "access_token": at, "id_token": it, "expires_in": 86400, - }) + } + if s.issueRefresh { + body["refresh_token"] = fmt.Sprintf("refresh~%d", + s.refreshes.Add(1)) + } + _ = json.NewEncoder(w).Encode(body) })) t.Cleanup(auth0.Close) api := httptest.NewServer(http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { + if s.atAPI != nil { + s.atAPI(r) + } s.apiHits[r.URL.Path]++ s.headers = append(s.headers, r.Header.Clone()) w.Header().Set("Content-Type", "application/json") @@ -83,15 +130,15 @@ func newSignInStubs(t *testing.T, tenants ...string) *signInStubs { t.Cleanup(api.Close) s.api = api.URL - savedFor, savedFlow, savedTTY := auth0For, signInFlow, + savedApp, savedFlow, savedTTY := conn.SignInApp, signInFlow, promptIsInteractive t.Cleanup(func() { - auth0For, signInFlow, promptIsInteractive = savedFor, savedFlow, - savedTTY + conn.SignInApp, signInFlow, promptIsInteractive = savedApp, + savedFlow, savedTTY }) - auth0For = func(string) (apidefaults.Auth0App, bool) { + conn.SignInApp = func(string) (apidefaults.Auth0App, string, bool) { return apidefaults.Auth0App{Domain: "unused", - Audience: "aud", ClientID: "cid"}, true + Audience: "aud", ClientID: "cid"}, auth0.URL, true } promptIsInteractive = func(*module.Runtime) bool { return false } signInFlow = func(rt *module.Runtime, app apidefaults.Auth0App, @@ -131,19 +178,34 @@ func exitCode(err error) int { return -1 } +// TestBrowserLoginDebugOutputCarriesNoSecrets follows a session from +// sign-in through a renewal to logout's revoke. func TestBrowserLoginDebugOutputCarriesNoSecrets(t *testing.T) { s := newSignInStubs(t, "t-a") + s.issueRefresh = true rt, _, errb := testsupport.NewRuntime(t, "", "text") + rt.Keychain = &keychain.Fake{} rt.Debug = true if err := login(t, rt, s); err != nil { t.Fatalf("login: %v", err) } - if !strings.Contains(errb.String(), "/oauth/token") { - t.Fatalf("--debug did not log the exchange:\n%s", errb.String()) - } issued, _ := s.issued.Load().(string) + testsupport.SeedSession(t, rt, s.api, "u1", + time.Now().Add(-time.Minute), "t-a") + for _, args := range [][]string{{"auth", "whoami"}, {"auth", "logout"}} { + if err := runAccount(t, rt, &bytes.Buffer{}, args...); err != nil { + t.Fatalf("%v: %v", args, err) + } + } + for _, want := range []string{"/oauth/token", "/oauth/revoke"} { + if !strings.Contains(errb.String(), want) { + t.Fatalf("--debug did not log %s:\n%s", want, errb.String()) + } + } + renewed, _ := s.issued.Load().(string) verifier, _ := s.verifier.Load().(string) - secrets := append(strings.Fields(issued), verifier, stubCode) + secrets := append(strings.Fields(issued+" "+renewed), verifier, + stubCode, "refresh~1", "refresh~2") for _, secret := range secrets { if secret == "" || strings.Contains(errb.String(), secret) { t.Errorf("--debug output carries %q", secret) @@ -151,6 +213,56 @@ func TestBrowserLoginDebugOutputCarriesNoSecrets(t *testing.T) { } } +// TestBrowserLoginSavesASession checks that a session renews itself +// only when Auth0 issued a refresh token and the keychain holds it, and +// that the token is never written to a file. +func TestBrowserLoginSavesASession(t *testing.T) { + for _, tc := range []struct { + name string + issue, keychain bool + renews bool + }{ + {"refresh token kept", true, true, true}, + {"no refresh token issued", false, true, false}, + {"no keychain", true, false, false}, + } { + t.Run(tc.name, func(t *testing.T) { + s := newSignInStubs(t, "t-a") + s.issueRefresh = tc.issue + rt, _, errb := testsupport.NewRuntime(t, "", "text") + kc := &keychain.Fake{} + if tc.keychain { + rt.Keychain = kc + } + // A previous session's refresh token must not survive. + _ = kc.Set(conn.Store(rt).SessionAccount(), "refresh~old") + if err := login(t, rt, s); err != nil { + t.Fatalf("login: %v\n%s", err, errb.String()) + } + got, _ := kc.Get(conn.Store(rt).SessionAccount()) + if want := map[bool]string{true: "refresh~1"}[tc.renews]; tc.keychain && + got != want { + t.Errorf("keychain holds %q, want %q", got, want) + } + for _, path := range []string{rt.Config.Path(), filepath.Join( + os.Getenv("HOME"), ".pgedge", "cli", "cache", + "default-starfleet.json")} { + raw, err := os.ReadFile(path) + if err != nil || bytes.Contains(raw, []byte("refresh~")) { + t.Errorf("%s: %v, or it holds a refresh token:\n%s", + path, err, raw) + } + } + renews := strings.Contains(errb.String(), + "The session renews itself while in use") + if renews != tc.renews { + t.Errorf("stderr says renews = %v, want %v:\n%s", renews, + tc.renews, errb.String()) + } + }) + } +} + func TestBrowserLoginTenantChoice(t *testing.T) { for _, tc := range []struct { name string @@ -347,6 +459,11 @@ func TestStatusForAnExpiredSession(t *testing.T) { &config.StarfleetProfile{APIURL: api, Auth: config.AuthBrowser}) }, "Token: not cached"}, + // Not a failure: the next command renews it. + {"expired, renews", func(t *testing.T, rt *module.Runtime, + api string) { + seedRenewableSession(t, rt, api, time.Now().Add(-time.Minute)) + }, "Token: expired, renews on next use"}, } { t.Run(tc.name, func(t *testing.T) { rt, out, _ := testsupport.NewRuntime(t, "", "text") @@ -354,7 +471,11 @@ func TestStatusForAnExpiredSession(t *testing.T) { tc.seed(t, rt, api) err := runAccount(t, rt, &bytes.Buffer{}, "auth", "status", "--api-url", api) - if exitCode(err) != ExitAuth || + if strings.HasSuffix(tc.name, "renews") { + if err != nil { + t.Fatalf("err = %v, want exit 0", err) + } + } else if exitCode(err) != ExitAuth || !strings.Contains(err.Error(), "auth login") { t.Fatalf("err = %v, want exit 5", err) } @@ -406,3 +527,131 @@ func TestTenantFlagIsValidatedByStatusAndDoctor(t *testing.T) { }) } } + +// seedRenewableSession is a session expiring at expiresAt whose +// keychain holds refresh~1, and the fake keychain. +func seedRenewableSession(t *testing.T, rt *module.Runtime, api string, + expiresAt time.Time) (kc *keychain.Fake, accessToken string) { + t.Helper() + kc = &keychain.Fake{} + rt.Keychain = kc + accessToken = testsupport.SeedSession(t, rt, api, "u1", expiresAt, + "t-a") + if err := conn.Store(rt).SaveSessionRefreshToken("refresh~1"); err != nil { + t.Fatal(err) + } + return kc, accessToken +} + +// TestSessionRenewalRotatesBeforeUse checks that by the time the API +// sees a renewed access token, it and Auth0's rotated refresh token are +// both saved: the old refresh token is dead from that moment. +func TestSessionRenewalRotatesBeforeUse(t *testing.T) { + s := newSignInStubs(t, "t-a") + s.issueRefresh = true + s.refreshes.Store(1) + rt, _, _ := testsupport.NewRuntime(t, "", "text") + kc, old := seedRenewableSession(t, rt, s.api, + time.Now().Add(auth.RefreshWindow/2)) + type seen struct { + refresh string + bearer, saved string + } + var atAPI []seen + s.atAPI = func(r *http.Request) { + v, _ := kc.Get(conn.Store(rt).SessionAccount()) + tok, _ := conn.Store(rt).LoadToken() + atAPI = append(atAPI, seen{v, strings.TrimPrefix( + r.Header.Get("Authorization"), "Bearer "), tok.AccessToken}) + } + if err := runAccount(t, rt, &bytes.Buffer{}, "auth", "whoami"); err != nil { + t.Fatalf("whoami: %v", err) + } + if !slices.Equal(s.refreshedWith, []string{"refresh~1"}) { + t.Fatalf("Auth0 got refresh tokens %v, want [refresh~1]", + s.refreshedWith) + } + if len(atAPI) == 0 { + t.Fatal("the API was never called") + } + for _, got := range atAPI { + if got.refresh != "refresh~2" || got.bearer == old || + got.bearer != got.saved { + t.Errorf("at the API: keychain %q, bearer renewed %v, "+ + "bearer cached %v", got.refresh, got.bearer != old, + got.bearer == got.saved) + } + } +} + +func TestSessionRenewalRefused(t *testing.T) { + for _, tc := range []struct { + name string + auth0Down bool + want string + keepRefresh bool + }{ + {"invalid_grant", false, "has expired or is missing: run " + + "'pgedge starfleet auth login'", false}, + {"network failure", true, "could not renew", true}, + } { + t.Run(tc.name, func(t *testing.T) { + s := newSignInStubs(t, "t-a") + s.refreshError = "invalid_grant" + if tc.auth0Down { + conn.SignInApp = func(string) (apidefaults.Auth0App, + string, bool) { + return apidefaults.Auth0App{ClientID: "cid"}, + "http://127.0.0.1:1", true + } + } + rt, _, _ := testsupport.NewRuntime(t, "", "text") + kc, _ := seedRenewableSession(t, rt, s.api, + time.Now().Add(-time.Minute)) + err := runAccount(t, rt, &bytes.Buffer{}, "auth", "whoami") + if exitCode(err) != ExitAuth || + !strings.Contains(err.Error(), tc.want) { + t.Fatalf("err = %v, want exit 5 with %q", err, tc.want) + } + _, kerr := kc.Get(conn.Store(rt).SessionAccount()) + if kept := kerr == nil; kept != tc.keepRefresh { + t.Errorf("refresh token kept = %v, want %v", kept, + tc.keepRefresh) + } + }) + } +} + +// TestLogoutRevokesTheSession checks that logout revokes the refresh +// token, then removes it and the session even when the revoke fails. +func TestLogoutRevokesTheSession(t *testing.T) { + for _, status := range []int{http.StatusOK, + http.StatusInternalServerError} { + t.Run(http.StatusText(status), func(t *testing.T) { + s := newSignInStubs(t, "t-a") + s.revokeStatus = status + rt, _, errb := testsupport.NewRuntime(t, "", "text") + kc, _ := seedRenewableSession(t, rt, s.api, + time.Now().Add(time.Hour)) + if err := runAccount(t, rt, &bytes.Buffer{}, "auth", + "logout"); err != nil { + t.Fatalf("logout: %v", err) + } + if !slices.Equal(s.revoked, []string{"refresh~1"}) { + t.Errorf("revoked %v, want [refresh~1]", s.revoked) + } + if _, err := kc.Get(conn.Store(rt).SessionAccount()); err == nil { + t.Error("logout left the refresh token in the keychain") + } + if _, err := conn.Store(rt).LoadToken(); err == nil { + t.Error("logout left the session token") + } + warned := strings.Contains(errb.String(), + "could not end the session") + if warned != (status != http.StatusOK) { + t.Errorf("warning = %v for %d:\n%s", warned, status, + errb.String()) + } + }) + } +} diff --git a/internal/starfleet/account/cmd/doctor.go b/internal/starfleet/account/cmd/doctor.go index 9188074..212f8be 100644 --- a/internal/starfleet/account/cmd/doctor.go +++ b/internal/starfleet/account/cmd/doctor.go @@ -40,6 +40,8 @@ type authInfo struct { ExpiresAt string `json:"expires_at,omitempty"` // User is a browser session's email or subject. User string `json:"user,omitempty"` + // Renews reports a browser session that can renew its own token. + Renews bool `json:"renews"` } type apiInfo struct { @@ -179,9 +181,8 @@ func checkAuth(rt *module.Runtime, f *conn.Flags) authInfo { return info } -// checkSessionAuth fills the Auth row for a browser session, whose -// token cannot be re-minted: TokenValid and TokenBound are read the way -// conn.LoadSession reads them. +// checkSessionAuth fills the Auth row for a browser session the way +// conn.Resolve reads it, without renewing it. func checkSessionAuth(rt *module.Runtime, res *conn.Resolved, flagTenant string, info authInfo, ) authInfo { @@ -191,6 +192,7 @@ func checkSessionAuth(rt *module.Runtime, res *conn.Resolved, info.ExpiresAt = s.tok.ExpiresAt.Format(time.RFC3339) } info.TokenValid, info.TokenBound = s.valid, s.bound + info.Renews = s.renews if s.problem != nil { info.Authenticated = false info.Problem = s.problem.Error() @@ -369,9 +371,15 @@ func runDoctor(rt *module.Runtime, f *conn.Flags) error { switch { case session && report.Auth.Authenticated: authStatus = "ok" - authDetail = fmt.Sprintf("signed in as %s via browser "+ - "(expires %s)", output.Sanitize(report.Auth.User), - report.Auth.ExpiresAt) + state := "expires " + report.Auth.ExpiresAt + switch { + case !report.Auth.TokenValid: + state = "token expired, renews on next use" + case report.Auth.Renews: + state += ", renews itself" + } + authDetail = fmt.Sprintf("signed in as %s via browser (%s)", + output.Sanitize(report.Auth.User), state) case report.Auth.Authenticated && report.Auth.Source == auth.SourceEnv: authStatus = "ok" authDetail = fmt.Sprintf("authenticated via %s and %s "+ diff --git a/internal/starfleet/conn/conn.go b/internal/starfleet/conn/conn.go index 0b9c337..4839af2 100644 --- a/internal/starfleet/conn/conn.go +++ b/internal/starfleet/conn/conn.go @@ -27,7 +27,9 @@ import ( "github.com/pgEdge/pgedge-cli/internal/errbody" "github.com/pgEdge/pgedge-cli/internal/httplog" "github.com/pgEdge/pgedge-cli/internal/module" + "github.com/pgEdge/pgedge-cli/internal/output" accountapi "github.com/pgEdge/pgedge-cli/internal/starfleet/account/api" + "github.com/pgEdge/pgedge-cli/internal/starfleet/signin" "github.com/pgEdge/pgedge-cli/internal/useragent" "github.com/spf13/pflag" ) @@ -654,16 +656,32 @@ func SessionTenant(rt *module.Runtime, profileTenant, flagTenant string, return claims.TenantID, nil } -// LoadSession reads the profile's browser session token, usable when it -// is bound to apiURL and more than auth.RefreshWindow from expiry. -// Anything else is SessionExpiredError. -func LoadSession(rt *module.Runtime, apiURL string, -) (*auth.CachedToken, *auth.Claims, error) { +// SignInApp finds the Auth0 sign-in app for an API URL and the base URL +// of its endpoints. A variable so tests can point it at a fake Auth0. +var SignInApp = func(apiURL string) (apidefaults.Auth0App, string, bool) { + app, ok := apidefaults.Auth0For(apiURL) + return app, "https://" + app.Domain, ok +} + +// loadSession reads the profile's browser session token. One bound to +// apiURL but near expiry is renewed with the session's refresh token +// when policy allows; anything else unusable is SessionExpiredError. +func loadSession(rt *module.Runtime, apiURL string, timeout time.Duration, + policy tokenPolicy) (*auth.CachedToken, *auth.Claims, error) { tok, err := Store(rt).LoadToken() - if err != nil || !tok.SessionFor(apiURL) || - time.Until(tok.ExpiresAt) <= auth.RefreshWindow { + if err != nil || !tok.SessionFor(apiURL) { return nil, nil, SessionExpiredError(rt, apiURL) } + if time.Until(tok.ExpiresAt) <= auth.RefreshWindow { + if policy == ephemeralToken { + return nil, nil, SessionExpiredError(rt, apiURL) + } + tok, err = renewSession(context.Background(), rt, apiURL, + tok.User, timeout) + if err != nil { + return nil, nil, err + } + } claims, err := auth.ParseClaims(tok.AccessToken) if err != nil { return nil, nil, SessionExpiredError(rt, apiURL) @@ -671,6 +689,62 @@ func LoadSession(rt *module.Runtime, apiURL string, return tok, claims, nil } +// renewSession exchanges the session's refresh token for a new access +// token. Auth0 rotates the refresh token, so both are saved before the +// access token is used: the old refresh token no longer works. +func renewSession(ctx context.Context, rt *module.Runtime, apiURL, + user string, timeout time.Duration) (*auth.CachedToken, error) { + store := Store(rt) + refresh, err := store.SessionRefreshToken() + if err != nil { + return nil, SessionExpiredError(rt, apiURL) + } + app, base, ok := SignInApp(apiURL) + if !ok { + return nil, SessionExpiredError(rt, apiURL) + } + if timeout <= 0 { + timeout = RequestTimeout + } + toks, err := signin.Refresh(ctx, SignInHTTPClient(rt, timeout), base, + app.ClientID, refresh) + if signin.IsInvalidGrant(err) { + // Expired, reused or revoked: it can never work again. A + // network failure keeps it, since it may work next time. + _ = store.ClearSessionRefreshToken() + return nil, SessionExpiredError(rt, apiURL) + } + if err != nil { + return nil, NewExitError(fmt.Sprintf("could not renew the browser "+ + "sign-in for %s: %v", apiURL, err), ExitAuth) + } + claims, err := auth.ParseClaims(toks.AccessToken) + if err != nil { + return nil, NewExitError(fmt.Sprintf("renewing the browser sign-in "+ + "returned an access token the CLI cannot read: %v", err), + ExitAuth) + } + expiresAt := claims.ExpiresAt() + if expiresAt.IsZero() { + expiresAt = time.Now().Add(time.Duration(toks.ExpiresIn) * + time.Second) + } + tok, err := SaveSession(rt, apiURL, toks.AccessToken, user, expiresAt) + if err != nil { + return nil, NewExitError(fmt.Sprintf("save renewed session: %v", + err), ExitAuth) + } + if toks.RefreshToken != "" { + if err := store.SaveSessionRefreshToken(toks.RefreshToken); err != nil { + fmt.Fprintf(rt.Stderr, "Warning: could not save the renewed "+ + "sign-in to the OS keychain (%s); run '%s' when this "+ + "session ends.\n", output.Sanitize(err.Error()), + output.Sanitize(loginCommand(rt))) + } + } + return tok, nil +} + // SaveSession writes a browser sign-in's access token to the profile's // cache, bound to apiURL and the token's subject. func SaveSession(rt *module.Runtime, apiURL, accessToken, user string, @@ -705,15 +779,16 @@ func TenantUsageError(tenant string, reachable []string) *ExitError { // Resolve produces an authenticated connection for the runtime's // active profile, honouring per-invocation flag overrides. It reuses a // cached token that is more than auth.RefreshWindow from expiry and -// otherwise exchanges credentials for a fresh one; a browser session -// has nothing to exchange, so it fails instead. +// otherwise exchanges credentials, or a browser session's refresh +// token, for a fresh one. func Resolve(rt *module.Runtime, f Flags) (*Conn, error) { return resolve(rt, f, persistToken) } // ResolveEphemeral is Resolve for `starfleet doctor`, which must not // change what it reports: it reuses a valid cached token but never -// writes one. Writing would let doctor report "token expired or missing" +// writes one, and never renews a browser session, which would rotate +// its refresh token. Writing would let doctor report "token expired or missing" // and leave a fresh token behind, and `doctor --api-url ` would // evict the profile's working token, since a cached token is bound to // its API URL. @@ -741,7 +816,7 @@ func resolve(rt *module.Runtime, f Flags, policy tokenPolicy, return nil, NewExitError(err.Error(), ExitAuth) } if res.Session { - return resolveSession(rt, res, f.Tenant, timeout) + return resolveSession(rt, res, f.Tenant, timeout, policy) } if f.Tenant != "" { return nil, ErrTenantWithAPIClient() @@ -790,9 +865,9 @@ func resolve(rt *module.Runtime, f Flags, policy tokenPolicy, // resolveSession builds a browser session's connection from its cached // token. Every request carries SelectedTenantHeader. func resolveSession(rt *module.Runtime, res *Resolved, flagTenant string, - timeout time.Duration, + timeout time.Duration, policy tokenPolicy, ) (*Conn, error) { - tok, claims, err := LoadSession(rt, res.APIURL) + tok, claims, err := loadSession(rt, res.APIURL, timeout, policy) if err != nil { return nil, err } @@ -806,13 +881,19 @@ func resolveSession(rt *module.Runtime, res *Resolved, flagTenant string, TenantIDs: claims.Tenants(), ExpiresAt: tok.ExpiresAt, } - // A wait that outlives the token ends with the same message, not a - // 401: there is nothing to re-mint with. + // A wait that outlives the token renews it as the first call would. src := &tokenSource{ value: tok.AccessToken, expiry: tok.ExpiresAt, - mint: func(context.Context) (string, time.Time, error) { - return "", time.Time{}, SessionExpiredError(rt, res.APIURL) + mint: func(ctx context.Context) (string, time.Time, error) { + if policy == ephemeralToken { + return "", time.Time{}, SessionExpiredError(rt, res.APIURL) + } + t, err := renewSession(ctx, rt, res.APIURL, tok.User, timeout) + if err != nil { + return "", time.Time{}, err + } + return t.AccessToken, t.ExpiresAt, nil }, } return &Conn{ diff --git a/internal/starfleet/conn/session_test.go b/internal/starfleet/conn/session_test.go index b01b6c9..db7d610 100644 --- a/internal/starfleet/conn/session_test.go +++ b/internal/starfleet/conn/session_test.go @@ -8,8 +8,10 @@ import ( "testing" "time" + "github.com/pgEdge/pgedge-cli/internal/apidefaults" "github.com/pgEdge/pgedge-cli/internal/auth" "github.com/pgEdge/pgedge-cli/internal/config" + "github.com/pgEdge/pgedge-cli/internal/keychain" "github.com/pgEdge/pgedge-cli/internal/module" "github.com/pgEdge/pgedge-cli/internal/testsupport" "github.com/spf13/pflag" @@ -147,11 +149,25 @@ func TestSessionWaitThatOutlivesTheTokenFailsWithTheSameMessage(t *testing.T) { wantExit(t, err, ExitAuth, "has expired or is missing") } +// TestFlagsOverASessionLeaveItsCacheAlone uses a session due for +// renewal, so flags that touched it would renew or clear its refresh +// token. func TestFlagsOverASessionLeaveItsCacheAlone(t *testing.T) { stub := newTokenStub(t, "client-tok") + auth0 := newTokenStub(t, "unused") + saved := SignInApp + t.Cleanup(func() { SignInApp = saved }) + SignInApp = func(string) (apidefaults.Auth0App, string, bool) { + return apidefaults.Auth0App{ClientID: "cid"}, auth0.url(), true + } rt, _, _ := testsupport.NewRuntime(t, "", "text") + kc := &keychain.Fake{} + rt.Keychain = kc sess := testsupport.SeedSession(t, rt, sessionAPI, "u1", - time.Now().Add(time.Hour), "t-a") + time.Now().Add(auth.RefreshWindow/2), "t-a") + if err := Store(rt).SaveSessionRefreshToken("rt"); err != nil { + t.Fatal(err) + } c, err := Resolve(rt, Flags{ClientID: "id", ClientSecret: "s", APIURL: stub.url(), Timeout: RequestTimeout}) if err != nil { @@ -164,8 +180,16 @@ func TestFlagsOverASessionLeaveItsCacheAlone(t *testing.T) { if err != nil || tok.AccessToken != sess || !tok.SessionFor(sessionAPI) { t.Fatalf("the session cache was overwritten: %+v, %v", tok, err) } - if _, err := Resolve(rt, Flags{}); err != nil { - t.Errorf("the session no longer resolves: %v", err) + if v, err := Store(rt).SessionRefreshToken(); v != "rt" || + kc.Sets != 1 || kc.Deletes != 0 || auth0.hits() != 0 { + t.Fatalf("flags touched the refresh token: %q, %v, %d sets, "+ + "%d deletes, %d renewals", v, err, kc.Sets, kc.Deletes, + auth0.hits()) + } + // Positive control: without flags the session does renew. + _, _ = Resolve(rt, Flags{}) + if auth0.hits() != 1 { + t.Errorf("the session did not try to renew: %d", auth0.hits()) } } diff --git a/internal/starfleet/signin/signin.go b/internal/starfleet/signin/signin.go index af3a37f..46cc946 100644 --- a/internal/starfleet/signin/signin.go +++ b/internal/starfleet/signin/signin.go @@ -66,11 +66,13 @@ type Flow struct { Out io.Writer } -// Tokens is what the token endpoint returns. +// Tokens is what the token endpoint returns. RefreshToken is empty +// when the Auth0 app does not grant refresh tokens. type Tokens struct { - AccessToken string `json:"access_token"` - IDToken string `json:"id_token"` - ExpiresIn int64 `json:"expires_in"` + AccessToken string `json:"access_token"` + IDToken string `json:"id_token"` + RefreshToken string `json:"refresh_token"` + ExpiresIn int64 `json:"expires_in"` } type callback struct { @@ -178,7 +180,7 @@ func (f *Flow) authorizeURL(redirect, challenge, state string) string { "client_id": {f.ClientID}, "redirect_uri": {redirect}, "audience": {f.Audience}, - "scope": {"openid profile email"}, + "scope": {"openid profile email offline_access"}, "code_challenge": {challenge}, "code_challenge_method": {"S256"}, "state": {state}, @@ -249,37 +251,60 @@ func callbackHandler(state string, results chan<- callback) http.Handler { // exchange trades the code for tokens at /oauth/token. func (f *Flow) exchange(ctx context.Context, code, redirect, verifier string) (*Tokens, error) { - form := url.Values{ + return postToken(ctx, f.HTTPClient, f.BaseURL, url.Values{ "grant_type": {"authorization_code"}, "client_id": {f.ClientID}, "code": {code}, "redirect_uri": {redirect}, "code_verifier": {verifier}, - } - req, err := http.NewRequestWithContext(ctx, http.MethodPost, - strings.TrimRight(f.BaseURL, "/")+"/oauth/token", - strings.NewReader(form.Encode())) + }) +} + +// Refresh trades a refresh token for new tokens. Auth0 rotates refresh +// tokens, so the old one is dead once this succeeds. A refusal is a +// *DeniedError; IsInvalidGrant says whether the token itself is dead. +func Refresh(ctx context.Context, client *http.Client, + baseURL, clientID, refreshToken string) (*Tokens, error) { + return postToken(ctx, client, baseURL, url.Values{ + "grant_type": {"refresh_token"}, + "client_id": {clientID}, + "refresh_token": {refreshToken}, + }) +} + +// IsInvalidGrant reports Auth0 refusing a refresh token: expired, +// reused after rotation, revoked, or its user blocked. +func IsInvalidGrant(err error) bool { + var d *DeniedError + return errors.As(err, &d) && d.Code == "invalid_grant" +} + +// Revoke revokes a refresh token at /oauth/revoke. +func Revoke(ctx context.Context, client *http.Client, + baseURL, clientID, token string) error { + resp, raw, err := postForm(ctx, client, baseURL, "/oauth/revoke", + url.Values{"client_id": {clientID}, "token": {token}}) if err != nil { - return nil, fmt.Errorf("signin: token request: %w", err) + return fmt.Errorf("revoke: %w", err) } - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, err := f.HTTPClient.Do(req) - if err != nil { - return nil, fmt.Errorf("token exchange: %w", err) + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + if d := deniedFrom(raw); d != nil { + return d + } + return fmt.Errorf("revoke returned %s", resp.Status) } - defer func() { _ = resp.Body.Close() }() - raw, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + return nil +} + +func postToken(ctx context.Context, client *http.Client, baseURL string, + form url.Values) (*Tokens, error) { + resp, raw, err := postForm(ctx, client, baseURL, "/oauth/token", form) if err != nil { - return nil, fmt.Errorf("token exchange: read response: %w", err) + return nil, fmt.Errorf("token exchange: %w", err) } if resp.StatusCode < 200 || resp.StatusCode >= 300 { - var e struct { - Error string `json:"error"` - Description string `json:"error_description"` - } - if json.Unmarshal(raw, &e) == nil && e.Error != "" { - return nil, &DeniedError{Code: e.Error, - Description: e.Description} + if d := deniedFrom(raw); d != nil { + return nil, d } return nil, fmt.Errorf("token exchange returned %s", resp.Status) } @@ -293,6 +318,39 @@ func (f *Flow) exchange(ctx context.Context, return &tok, nil } +func postForm(ctx context.Context, client *http.Client, baseURL, + path string, form url.Values) (*http.Response, []byte, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodPost, + strings.TrimRight(baseURL, "/")+path, + strings.NewReader(form.Encode())) + if err != nil { + return nil, nil, err + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + resp, err := client.Do(req) + if err != nil { + return nil, nil, err + } + defer func() { _ = resp.Body.Close() }() + raw, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return nil, nil, fmt.Errorf("read response: %w", err) + } + return resp, raw, nil +} + +// deniedFrom reads an OAuth error body, or returns nil. +func deniedFrom(raw []byte) *DeniedError { + var e struct { + Error string `json:"error"` + Description string `json:"error_description"` + } + if json.Unmarshal(raw, &e) == nil && e.Error != "" { + return &DeniedError{Code: e.Error, Description: e.Description} + } + return nil +} + // browserCommand is the command that opens target on goos. func browserCommand(goos, target string) (name string, args []string) { switch goos { diff --git a/internal/starfleet/signin/signin_test.go b/internal/starfleet/signin/signin_test.go index 35d40a3..edb9892 100644 --- a/internal/starfleet/signin/signin_test.go +++ b/internal/starfleet/signin/signin_test.go @@ -115,16 +115,13 @@ func TestRunExchangesTheCodeWithTheVerifier(t *testing.T) { "response_type": "code", "client_id": "cid", "audience": "https://api.example", - "scope": "openid profile email", + "scope": "openid profile email offline_access", "code_challenge_method": "S256", } { if q.Get(k) != want { t.Errorf("authorize %s = %q, want %q", k, q.Get(k), want) } } - if strings.Contains(q.Get("scope"), "offline_access") { - t.Error("step 1 must not ask for offline_access") - } if !strings.HasPrefix(q.Get("redirect_uri"), "http://127.0.0.1:") || !strings.HasSuffix(q.Get("redirect_uri"), "/callback") { t.Errorf("redirect_uri = %q", q.Get("redirect_uri")) From e3d3f310136b1a03af491d2ca135c69affc84687 Mon Sep 17 00:00:00 2001 From: Antony Pegg Date: Mon, 19 Oct 2026 16:26:12 -0400 Subject: [PATCH 2/6] docs: describe browser sessions that renew --- docs/auth-and-profiles.md | 15 ++++--- docs/ci.md | 2 +- docs/doctor.md | 14 ++++--- docs/getting-started.md | 7 ++-- internal/starfleet/llms.txt | 10 +++-- internal/starfleet/llms/auth.txt | 41 +++++++++++-------- .../pgedge/skills/pgedge-starfleet/SKILL.md | 6 +-- 7 files changed, 57 insertions(+), 38 deletions(-) diff --git a/docs/auth-and-profiles.md b/docs/auth-and-profiles.md index bc44dba..08981f6 100644 --- a/docs/auth-and-profiles.md +++ b/docs/auth-and-profiles.md @@ -22,9 +22,11 @@ A profile signs in one of two ways: `--api-client` to be prompted for them. This is the path for CI and scripts, because it needs no browser and the token renews itself. -A browser session lasts until the time login prints. After that, -commands fail with exit status 5 and name the login command to run -again. A profile holds one kind of credential, so signing in one way +A browser session renews itself while you use it and ends after a +period without use. Login keeps what renews it in the OS keychain; +where no keychain can be used, the session lasts until the time login +prints. Once the session ends, commands fail with exit status 5 and +name the login command, which starts a new session. A profile holds one kind of credential, so signing in one way removes what the other way stored. When you belong to several tenants, login asks which one to use, or @@ -225,8 +227,11 @@ never silently runs as a previous credential. `pgedge starfleet auth logout` removes the active profile's cached token file (and any staging file an interrupted write left beside it), and deletes the stored `client_id` and `client_secret` from the profile -and the secret from the OS keychain. Other profiles' tokens and -credentials are left alone. +and the secret from the OS keychain. For a browser session, logout +first asks pgEdge to end the session, then removes it from the OS +keychain; if that request fails, logout warns and still removes the +session from this machine. Other profiles' tokens and credentials are +left alone. `pgedge starfleet auth status` reports the resolution state without making a server call, and `pgedge starfleet doctor` reports the whole diff --git a/docs/ci.md b/docs/ci.md index 6ddee33..892b68f 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -17,7 +17,7 @@ The page leans on these terms: An unattended run signs in with an API client, never through the browser: a browser sign-in needs a person at the sign-in page, and its -session ends without renewing itself. +session ends after a period without use. Set `PGEDGE_CLIENT_ID` and `PGEDGE_CLIENT_SECRET` from your secret store. The CLI treats the pair as a profile that lasts one invocation. diff --git a/docs/doctor.md b/docs/doctor.md index a9e9146..b77c787 100644 --- a/docs/doctor.md +++ b/docs/doctor.md @@ -122,12 +122,14 @@ login will not help. `pgedge starfleet auth status` answers a narrower question than doctor: which credential resolved, without calling any endpoint. It -prints `authenticated`, `source`, `problem`, `client_id`, `api_url`, -`token_valid`, `token_bound` and `expires_at`, and the client secret's -value never appears in any format. Only the three booleans are always -there: the five string fields are omitted when empty, so a run with no -credentials configured answers with `authenticated`, `token_valid`, -`token_bound` and the `api_url` that resolved anyway. +prints `authenticated`, `source`, `problem`, `client_id`, `user`, +`tenant_id`, `api_url`, `token_valid`, `token_bound`, `expires_at` and +`renews`, and the client secret's value never appears in any format. +`user`, `tenant_id` and `renews` describe a browser session. Only the +four booleans are always there: the seven string fields are omitted +when empty, so a run with no credentials configured answers with +`authenticated`, `token_valid`, `token_bound`, `renews` and the +`api_url` that resolved anyway. Because it calls nothing, `authenticated: true` means the credentials resolved, not that the server accepts them: a wrong secret that merely diff --git a/docs/getting-started.md b/docs/getting-started.md index 8353eb1..06f249c 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -45,9 +45,10 @@ describes each one and what it needs. ## Running Your First Commands Sign in once per profile. Login opens your browser on the pgEdge -sign-in page, and from then on the CLI works as you, in your tenant, -until the session expires. Login prints when that is; run it again -after that. It saves the profile in `~/.pgedge/cli/config.yaml`: +sign-in page, and from then on the CLI works as you, in your tenant. +The session renews itself while you use it and ends after a period +without use; run login again to start a new one. Login saves the +profile in `~/.pgedge/cli/config.yaml`: pgedge starfleet auth login --profile dev diff --git a/internal/starfleet/llms.txt b/internal/starfleet/llms.txt index 0265937..895e4cf 100644 --- a/internal/starfleet/llms.txt +++ b/internal/starfleet/llms.txt @@ -33,10 +33,12 @@ A profile signs in one of two ways: - **As a user, through the browser.** `pgedge starfleet auth login` with no credential flags opens the browser, and every command then - runs as that user, in the tenant the profile names, until the - session expires. Nothing renews it: once it expires, a command - fails at exit 5 and tells the user to run `auth login` again. An - agent cannot complete a browser sign-in, so ask the user to run it. + runs as that user, in the tenant the profile names. The session + renews itself while in use and ends after a period without use, or + at the time login printed when no OS keychain could hold it. Once + it ends, a command fails at exit 5 and tells the user to run + `auth login` again. An agent cannot complete a browser sign-in, so + ask the user to run it. - **As an API client,** for CI, scripts and agents. Supply a client ID and secret through one of the sources below. diff --git a/internal/starfleet/llms/auth.txt b/internal/starfleet/llms/auth.txt index 2066a15..683132f 100644 --- a/internal/starfleet/llms/auth.txt +++ b/internal/starfleet/llms/auth.txt @@ -44,9 +44,12 @@ It opens the pgEdge sign-in page and prints the same URL on stderr, then waits for the browser to return. A person has to complete the page, so an agent asks the user to run this command. The profile stores `api_url`, `auth: browser` and `tenant_id`; the user's access -token goes to the token cache. Every later command runs as that user -until the session expires. Nothing renews it, so after that a command -fails at exit 5 and names the `auth login` command to run. +token goes to the token cache and the token that renews the session +to the OS keychain. Every later command runs as that user. The +session renews itself while in use and ends after a period without +use; where no keychain could hold it, it ends at the time login +printed. Once it ends, a command fails at exit 5 and names the +`auth login` command to run. A user in several tenants is asked which to use when stdin and stdout are a terminal. Otherwise login uses the account's default tenant and @@ -96,8 +99,8 @@ pgedge starfleet auth login --api-client --insecure-storage ``` A browser sign-in ends with four lines on stderr: who signed in -(`Signed in as .`), the tenant, when the session ends, and the -file the profile was saved to. An API-client sign-in ends with +(`Signed in as .`), the tenant, whether the session renews +itself or the time it ends, and the file the profile was saved to. An API-client sign-in ends with `Authenticated. Token expires