feat: renew browser sessions with refresh tokens

docs/auth-and-profiles.md

@@ -22,9 +22,12 @@ 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. It ends after a
+period without use, or after a fixed maximum however much you use
+it. 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 +228,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

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 or a fixed maximum.
 
 Set `PGEDGE_CLIENT_ID` and `PGEDGE_CLIENT_SECRET` from your secret
 store. The CLI treats the pair as a profile that lasts one invocation.

docs/doctor.md

@@ -81,7 +81,7 @@ The following table describes each row and what a warning means:
 |---|---|---|
 | Auth | Which credential source resolved, whether a token is cached, and whether that token is still valid. | Credentials resolve but the token is expired or absent, or a valid cached token was minted for a different connection. It reports `error` when no credentials resolve at all. |
 | API connectivity | An unauthenticated GET against the resolved base URL, with the status and latency. | Never. An unreachable endpoint is reported as `error`. |
-| Tenant | The tenant the credential authenticates as, its ID and its plan. | The plan is anything but `enterprise`, or the tenant could not be read. |
+| Tenant | The tenant the credential authenticates as, its ID and its plan. | The plan is anything but `enterprise`, the tenant could not be read, or a browser session must renew before it can be read; the next command renews it. |
 | Environment | The operating system, architecture, and whether `NO_COLOR` is set. | Never. |
 
 A cached token is bound to the credential and the API base URL that
@@ -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

docs/error-catalog.md

@@ -30,6 +30,7 @@ describes the messages both steps print:
 | `--client-id given without --client-secret: supply both or neither` | 2 | Half a credential pair reached the command. Pass both flags, or drop both and let the profile answer. | [Authentication and profiles](auth-and-profiles.md) |
 | `authentication failed: <reason>` | 5 | The token exchange did not complete. The reason names the cause, most often a refused credential or a token endpoint that could not be reached. | [Authentication and profiles](auth-and-profiles.md) |
 | `the browser sign-in for <api-url> has expired or is missing: run 'pgedge starfleet auth login' to sign in again` | 5 | The profile's browser session ended, or its token was signed in for another API URL. Sign in again; for unattended runs, use an API client. | [Authentication and profiles](auth-and-profiles.md) |
+| `the browser sign-in for <api-url> could not be renewed: <reason>; run 'pgedge starfleet auth login' to sign in again` | 5 | The sign-in service refused to renew the session and gave a reason. When the reason is `user is blocked`, signing in again is refused too; contact pgEdge support. | [Authentication and profiles](auth-and-profiles.md) |
 | `browser sign-in is not available for <api-url> yet` | 5 | This API endpoint has no browser sign-in. Sign in with an API client: `--client-id` and `--client-secret`, or `--api-client`. | [Authentication and profiles](auth-and-profiles.md) |
 | `--tenant <id> is not a tenant this sign-in can reach; reachable tenant IDs: <ids>` | 2 | The browser session's user does not belong to that tenant. Pick one of the IDs listed. | [Authentication and profiles](auth-and-profiles.md) |
 | `authentication error (401): <body>` | 5 | The API rejected the bearer token. Run `pgedge starfleet auth logout` and log in again to rebuild the cache. | [Authentication and profiles](auth-and-profiles.md) |

docs/getting-started.md

@@ -45,9 +45,11 @@ 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 or after a fixed maximum; run login again to start a new
+one. Login saves the
+profile in `~/.pgedge/cli/config.yaml`:
 
     pgedge starfleet auth login --profile dev
 

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;

internal/auth/lock.go

@@ -0,0 +1,60 @@
+package auth
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"time"
+)
+
+// Session lock timings. A renewal holds the lock for one bounded token
+// request and two keychain calls, so a lock older than staleLockAge was
+// left by a process that died holding it. lockWait outlasts that age so
+// a waiter recovers a dead holder's lock rather than failing.
+var (
+	staleLockAge = 45 * time.Second
+	lockWait     = 50 * time.Second
+	lockPoll     = 50 * time.Millisecond
+)
+
+// ErrLockTimeout reports that another process held the session lock
+// for the whole wait.
+var ErrLockTimeout = errors.New("auth: timed out waiting for the " +
+	"session lock")
+
+// LockSession takes this profile's session lock, a file beside the
+// token cache, so that only one process at a time renews the session:
+// two renewals sending the same rotating refresh token would have Auth0
+// revoke both. An O_EXCL file rather than flock, so one implementation
+// serves every OS.
+func (a *Auth) LockSession() (unlock func(), err error) {
+	p, err := a.cachePath()
+	if err != nil {
+		return nil, err
+	}
+	p += ".lock"
+	if err := os.MkdirAll(filepath.Dir(p), 0o700); err != nil {
+		return nil, fmt.Errorf("auth: create cache dir: %w", err)
+	}
+	deadline := time.Now().Add(lockWait)
+	for {
+		f, err := os.OpenFile(p, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) //nolint:gosec // G304: cachePath validates both name segments
+		if err == nil {
+			_ = f.Close()
+			return func() { _ = os.Remove(p) }, nil
+		}
+		if !errors.Is(err, os.ErrExist) {
+			return nil, fmt.Errorf("auth: session lock: %w", err)
+		}
+		if fi, err := os.Stat(p); err == nil &&
+			time.Since(fi.ModTime()) > staleLockAge {
+			_ = os.Remove(p)
+			continue
+		}
+		if time.Now().After(deadline) {
+			return nil, ErrLockTimeout
+		}
+		time.Sleep(lockPoll)
+	}
+}

internal/auth/lock_test.go

@@ -0,0 +1,92 @@
+package auth
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/pgEdge/pgedge-cli/internal/keychain"
+)
+
+// TestLockSessionIsPerProfile checks that a held lock shuts out the
+// same profile only, and that a dead holder's lock is recovered.
+func TestLockSessionIsPerProfile(t *testing.T) {
+	savedWait, savedAge := lockWait, staleLockAge
+	t.Cleanup(func() { lockWait, staleLockAge = savedWait, savedAge })
+	lockWait, staleLockAge = 100*time.Millisecond, time.Hour
+
+	dir := t.TempDir()
+	dev := &Auth{Profile: "dev", Module: "starfleet", CacheDir: dir}
+	unlock, err := dev.LockSession()
+	if err != nil {
+		t.Fatalf("LockSession: %v", err)
+	}
+	if _, err := (&Auth{Profile: "dev", Module: "starfleet",
+		CacheDir: dir}).LockSession(); !errors.Is(err, ErrLockTimeout) {
+		t.Fatalf("second lock on dev: err = %v, want ErrLockTimeout", err)
+	}
+	other, err := (&Auth{Profile: "prod", Module: "starfleet",
+		CacheDir: dir}).LockSession()
+	if err != nil {
+		t.Fatalf("lock on another profile: %v", err)
+	}
+	other()
+	unlock()
+	if _, err := dev.LockSession(); err != nil {
+		t.Fatalf("lock after unlock: %v", err)
+	}
+
+	// The holder dies without unlocking: its lock goes stale.
+	old := time.Now().Add(-2 * time.Hour)
+	if err := os.Chtimes(filepath.Join(dir, "dev-starfleet.json.lock"),
+		old, old); err != nil {
+		t.Fatal(err)
+	}
+	staleLockAge = time.Minute
+	if _, err := dev.LockSession(); err != nil {
+		t.Fatalf("stale lock was not recovered: %v", err)
+	}
+}
+
+// TestSessionRefreshTokenHasItsOwnKeychainEntry checks that the refresh
+// token neither overwrites nor is cleared with the profile's client
+// secret, and that it is refused rather than kept elsewhere when there
+// is no keychain.
+func TestSessionRefreshTokenHasItsOwnKeychainEntry(t *testing.T) {
+	kc := &keychain.Fake{}
+	a := &Auth{Profile: "dev", ConfigPath: "/cfg/config.yaml", Keychain: kc}
+	if err := kc.Set(a.KeychainAccount(), "client-secret"); err != nil {
+		t.Fatal(err)
+	}
+	if err := a.SaveSessionRefreshToken("rt"); err != nil {
+		t.Fatalf("SaveSessionRefreshToken: %v", err)
+	}
+	if got, err := a.SessionRefreshToken(); err != nil || got != "rt" {
+		t.Errorf("SessionRefreshToken = %q, %v", got, err)
+	}
+	if err := a.ClearSessionRefreshToken(); err != nil {
+		t.Fatalf("ClearSessionRefreshToken: %v", err)
+	}
+	if err := a.ClearSessionRefreshToken(); err != nil {
+		t.Errorf("clearing an absent token: %v", err)
+	}
+	if got, err := a.KeychainSecret(); err != nil || got != "client-secret" {
+		t.Errorf("client secret = %q, %v after the session's churn", got,
+			err)
+	}
+
+	none := &Auth{Profile: "dev"}
+	if err := none.SaveSessionRefreshToken("rt"); !errors.Is(err,
+		keychain.ErrUnavailable) {
+		t.Errorf("save without a keychain: err = %v", err)
+	}
+	if _, err := none.SessionRefreshToken(); !errors.Is(err,
+		keychain.ErrUnavailable) {
+		t.Errorf("read without a keychain: err = %v", err)
+	}
+	if err := none.ClearSessionRefreshToken(); err != nil {
+		t.Errorf("clear without a keychain: %v", err)
+	}
+}

internal/clitest/stderr_sanitize_test.go

@@ -299,8 +299,13 @@ 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.
+// 215 -> 218: a renewal that falls back to the current token, a
+// replaced session that could not be revoked, and an API-client login
+// that could not remove the session it replaced.
 const (
-	stderrArgSites      = 211
+	stderrArgSites      = 218
 	stderrComposedSites = 22
 )
 

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