diff --git a/.surface b/.surface index 41bf791e4..935e43236 100644 --- a/.surface +++ b/.surface @@ -2217,6 +2217,8 @@ FLAG basecamp auth token --no-stats type=bool FLAG basecamp auth token --profile type=string FLAG basecamp auth token --project type=string FLAG basecamp auth token --quiet type=bool +FLAG basecamp auth token --session-id type=string +FLAG basecamp auth token --session-label type=string FLAG basecamp auth token --stats type=bool FLAG basecamp auth token --stored type=bool FLAG basecamp auth token --styled type=bool diff --git a/internal/auth/agent.go b/internal/auth/agent.go index 551b7f89f..464f2263f 100644 --- a/internal/auth/agent.go +++ b/internal/auth/agent.go @@ -84,6 +84,10 @@ type agentMint struct { scope string resource string client *http.Client + + // launch is the session a session mint attributes its token to, or + // nil for the profile's shared token (see agent_launch.go). + launch *agentLaunch } // prepareAgentMint is the half of a mint that sends nothing: it checks what @@ -324,6 +328,7 @@ func (m *Manager) mintAgentToken(ctx context.Context, mint *agentMint) (*oauth.T if mint.resource != "" { form.Set("resource", mint.resource) } + mint.launch.addTo(form) reqCtx, cancel := context.WithTimeout(ctx, agentMintTimeout) defer cancel() @@ -422,6 +427,11 @@ func (m *Manager) mintAgentToken(ctx context.Context, mint *agentMint) (*oauth.T if err := applyTokenLifetime(&token, body); err != nil { return nil, nil, output.ErrAPI(resp.StatusCode, "minting an agent token: "+err.Error()) } + // Last, so a token that is not bound to the session it was asked for + // never leaves this function, however good it is otherwise. + if err := mint.launch.requireEcho(resp.StatusCode, body); err != nil { + return nil, nil, err + } return &token, nil, nil } diff --git a/internal/auth/agent_launch.go b/internal/auth/agent_launch.go new file mode 100644 index 000000000..04c0e4383 --- /dev/null +++ b/internal/auth/agent_launch.go @@ -0,0 +1,258 @@ +package auth + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/url" + "regexp" + "strings" + "unicode" + "unicode/utf8" + + "github.com/basecamp/basecamp-cli/internal/output" +) + +// Agent sessions: one self-token per launch, attributed to that launch. +// +// An agent profile's shared token is cached and served to every process on +// the host, so Basecamp cannot tell one sandboxed box from another: every +// request reads as the agent, from one token. A session mint asks the token +// endpoint for a token of the session's own instead, carrying a random id +// the launcher generated and an optional label it chose. Basecamp stores +// both on the token; its request logs carry the id beside the agent, never +// the label. +// +// The CLI calls this a session; on the wire, and in bc3, it is a launch +// (launch_id, launch_label), because "session" already means the web +// sign-in there. +// +// The contract is "a token bound to that session, or nothing": +// +// - The token is minted fresh on every call and never cached here. The +// caller is the per-launch cache: the sandbox's credential capture holds +// each token for a few minutes inside the launch's own proxy, which dies +// with the launch. Caching here instead would write one keyring entry +// per launch, which nothing would ever clean up, and contend every launch +// on the one credential lock for a token only one launch may use. +// +// - The shared cached token is neither served nor written: a session that +// fell back to it would be indistinguishable from every other process. +// +// - A server that does not echo the session back has not bound the token +// to it — it predates session attribution and ignored the parameters — +// and the token is discarded. Handing it out would be the failure that +// reads as working: attribution silently missing, and once Basecamp can +// stop a session, a token that stopping it would not reach. +// +// - A refusal is reported, never retried without the session. + +// launchIDPattern is a session id as bc3 accepts it: 128 random bits as +// canonical lowercase hex. +var launchIDPattern = regexp.MustCompile(`\A[0-9a-f]{32}\z`) + +// maxLaunchLabelChars is bc3's bound on a session label, in characters. +const maxLaunchLabelChars = 100 + +// agentLaunch is the session a mint attributes its token to. +type agentLaunch struct { + id string + label string +} + +// ValidateSession checks a session id and label against the rules the +// token endpoint holds them to, so a malformed one is a usage error here +// rather than a refused mint. The label is optional; the id is not. +func ValidateSession(id, label string) error { + if !launchIDPattern.MatchString(id) { + return output.ErrUsageHint("A session id must be 32 lowercase hex characters (128 random bits)", + "Generate one with: openssl rand -hex 16") + } + if label == "" { + return nil + } + if !utf8.ValidString(label) { + return output.ErrUsage("A session label must be valid UTF-8") + } + if strings.TrimSpace(label) == "" { + return output.ErrUsage("A session label must not be blank") + } + if n := utf8.RuneCountInString(label); n > maxLaunchLabelChars { + return output.ErrUsage(fmt.Sprintf("A session label must be at most %d characters (this one is %d)", maxLaunchLabelChars, n)) + } + for _, r := range label { + if !labelRune(r) { + return output.ErrUsage(fmt.Sprintf("A session label must not contain control, formatting or line-separator characters (found U+%04X)", r)) + } + } + return nil +} + +// labelRune reports whether r may appear in a session label: anything but +// the "other" category (control, format, private use, surrogate) and the +// line and paragraph separators — exactly the set bc3 refuses. The label is +// shown to people, so nothing that reorders, hides or breaks the text around +// it. Unassigned code points are deliberately allowed: a host's Unicode +// tables can be newer than this build's or the server's, and a label refused +// over that skew costs the launch its whole token. +func labelRune(r rune) bool { + return !unicode.In(r, unicode.Cc, unicode.Cf, unicode.Co, unicode.Cs, unicode.Zl, unicode.Zp) +} + +// addTo puts the session on a client_credentials form. A nil launch is the +// shared token's mint, which sends nothing extra. +func (l *agentLaunch) addTo(form url.Values) { + if l == nil { + return + } + form.Set("launch_id", l.id) + if l.label != "" { + form.Set("launch_label", l.label) + } +} + +// errLaunchNotRecorded is the cause of a session mint whose token came back +// without the session on it. +var errLaunchNotRecorded = errors.New("the token endpoint did not record the session") + +// requireEcho refuses a token response that does not carry the session it +// was asked for. bc3 echoes launch_id, and launch_label when one was given, +// once it has stored them on the token; a server that ignored them echoes +// nothing. +func (l *agentLaunch) requireEcho(status int, body []byte) error { + if l == nil { + return nil + } + var echoed struct { + ID *string `json:"launch_id"` + Label *string `json:"launch_label"` + } + _ = json.Unmarshal(body, &echoed) + + idMatches := echoed.ID != nil && *echoed.ID == l.id + labelMatches := (l.label == "" && echoed.Label == nil) || (echoed.Label != nil && *echoed.Label == l.label) + if idMatches && labelMatches { + return nil + } + + // Say which way the echo missed. Only a server that echoed no id at + // all is one that predates sessions; one that echoed this id records + // them, and what it got wrong is the label. + msg := fmt.Sprintf("Basecamp minted a token but did not bind it to session %s, so it was discarded", l.id) + hint := "A session token needs a Basecamp that records agent sessions; without one, leave out --session-id and --session-label" + switch { + case echoed.ID == nil: + msg += ": this Basecamp predates agent session attribution" + case !idMatches: + msg += ": Basecamp answered for a different session" + hint = "Run it again; if it keeps answering for another session, report it" + case echoed.Label == nil: + msg += ": Basecamp recorded the session but did not record the label" + hint = "Leave out --session-label to mint for the session alone" + default: + msg += ": Basecamp recorded a different label for the session" + hint = "Leave out --session-label to mint for the session alone" + } + e := output.ErrAPI(status, msg) + e.Hint = hint + e.Cause = errLaunchNotRecorded + return e +} + +// SessionAccessToken mints a fresh self-token for one agent session from +// the active profile's stored agent credential, and returns it without +// storing it. See the top of this file for why it is never cached and why +// a server that does not bind it to the session gets no token at all. +// +// Holds are honored and written exactly as the shared mint's are, because +// they are verdicts on the client secret both mints present: a refused or +// rate-limited secret is answered locally here too, and a refusal of the +// secret seen here is remembered for every later mint. A refusal of the +// request itself — the session's parameters, above all — is not about the +// secret and holds nothing, so one malformed launch cannot stop the rest. +func (m *Manager) SessionAccessToken(ctx context.Context, id, label string) (string, error) { + if err := ValidateSession(id, label); err != nil { + return "", err + } + + m.mu.Lock() + defer m.mu.Unlock() + + credKey := m.credentialKey() + creds, err := m.store.LoadContext(ctx, credKey) + if err != nil { + return "", m.unreadable("No stored credentials for", credKey, err) + } + m.remember(creds) + if creds.OAuthType != oauthTypeAgent { + return "", output.ErrUsage(fmt.Sprintf( + "A session token is minted from an agent's own credential, and %s holds a person's login; run without --session-id", credKey)) + } + + launch := &agentLaunch{id: id, label: label} + mint, err := m.prepareAgentMint(creds) + if err != nil { + return "", forSession(launch, m.hintLogin(err)) + } + mint.launch = launch + + // No credential lock across the round trip: nothing is written on + // success, so there is nothing for concurrent launches to be exclusive + // with. Only a hold is written, and that under the lock, as the shared + // path writes it. + token, hold, err := m.mintAgentToken(ctx, mint) + if err != nil { + if hold != nil { + err = m.rememberSessionHold(ctx, credKey, hold, err) + } + return "", forSession(launch, m.hintLogin(err)) + } + return token.AccessToken, nil +} + +// rememberSessionHold writes the hold a session mint's refusal leaves, under +// the credential lock, and returns what the mint answers with: its own +// refusal, or the verdict that stood instead. +// +// With no lock across the round trip, another launch's verdict on the same +// secret can land while this mint is in flight, and one that outlasts this +// mint's stands (rememberRenewalHold). Its answer is then this mint's too, +// so the wait a caller is told is the one the next mint will meet. +func (m *Manager) rememberSessionHold(ctx context.Context, credKey string, hold *RenewalHold, refusal error) error { + var stood *Credentials + lockErr := m.store.withKeyLock(ctx, credKey, func() error { + stood, _ = m.rememberRenewalHold(credKey, hold.Client, hold) + return nil + }) + if lockErr != nil { + m.warnf("warning: could not lock the credential for %s to remember the token endpoint's refusal, so the next command will ask again: %v", credKey, lockErr) + } + if stood == nil { + return refusal + } + held := m.heldRenewal(stood) + if held == nil { + return refusal + } + return m.agentRemedy(held, stood.ClientID, stood.Scope) +} + +// forSession names the session in a failed session mint's message, keeping +// its class, hint and cause. +func forSession(l *agentLaunch, err error) error { + if errors.Is(err, errLaunchNotRecorded) { + // Already says which session. + return err + } + var e *output.Error + if !errors.As(err, &e) { + return fmt.Errorf("session %s: %w", l.id, err) + } + named := *e + named.Message = fmt.Sprintf("Session %s: %s", l.id, e.Message) + if named.Cause == nil { + named.Cause = err + } + return &named +} diff --git a/internal/auth/agent_launch_test.go b/internal/auth/agent_launch_test.go new file mode 100644 index 000000000..9756cea45 --- /dev/null +++ b/internal/auth/agent_launch_test.go @@ -0,0 +1,327 @@ +package auth + +import ( + "context" + "errors" + "net/http" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/output" +) + +const testSessionID = "0123456789abcdef0123456789abcdef" + +// liveAgentCredential is an agent profile whose shared token is comfortably +// inside its lifetime, so anything that mints did so on purpose. +func liveAgentCredential(tokenEndpoint string) *Credentials { + creds := agentCredential(tokenEndpoint, time.Now().Add(time.Hour)) + creds.AccessToken = "shared-cached" + return creds +} + +// TestSessionTokenMintsFreshAndLeavesTheSharedTokenAlone: a session asks for +// its own token every time, with its id and label on the mint, and the +// profile's cached token is neither served nor overwritten. +func TestSessionTokenMintsFreshAndLeavesTheSharedTokenAlone(t *testing.T) { + as := startDeviceAS(t) + as.token = func(call int) (int, string) { + return http.StatusOK, `{"access_token":"session-` + string(rune('1'+call)) + `","token_type":"Bearer","expires_in":3600,"scope":"full",` + + `"launch_id":"` + testSessionID + `","launch_label":"coworker@box"}` + } + + m := newDeviceTestManager(t, as.srv.URL) + key := storeAgent(t, m, liveAgentCredential(as.srv.URL+"/oauth/token")) + before, err := m.store.Load(key) + require.NoError(t, err) + + first, err := m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.NoError(t, err) + assert.Equal(t, "session-1", first) + + second, err := m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.NoError(t, err) + assert.Equal(t, "session-2", second, "a session token is minted fresh, never cached") + + calls := as.tokenCalls() + require.Len(t, calls, 2) + // Every mint carries the session, not just the first: the mock echoes + // the id back whatever was sent, so only the request shows it. + for i, call := range calls { + assert.Equal(t, "client_credentials", call.Get("grant_type"), "mint %d", i+1) + assert.Equal(t, testSessionID, call.Get("launch_id"), "mint %d", i+1) + assert.Equal(t, "coworker@box", call.Get("launch_label"), "mint %d", i+1) + assert.Equal(t, AgentResourceURNPrefix+"42", call.Get("resource"), "mint %d", i+1) + } + + after, err := m.store.Load(key) + require.NoError(t, err) + assert.Equal(t, before, after, "a session mint must not touch the stored credential") + + shared, err := m.StoredAccessToken(context.Background()) + require.NoError(t, err) + assert.Equal(t, "shared-cached", shared) + assert.Len(t, as.tokenCalls(), 2, "the shared token was still live and needed no mint") +} + +// TestSessionTokenWithoutLabelSendsNoLabel: an id alone is a session; the +// label is optional and is not sent empty. +func TestSessionTokenWithoutLabelSendsNoLabel(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { + return http.StatusOK, `{"access_token":"session","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `"}` + } + m := newDeviceTestManager(t, as.srv.URL) + storeAgent(t, m, liveAgentCredential(as.srv.URL+"/oauth/token")) + + token, err := m.SessionAccessToken(context.Background(), testSessionID, "") + require.NoError(t, err) + assert.Equal(t, "session", token) + + calls := as.tokenCalls() + require.Len(t, calls, 1) + assert.Equal(t, testSessionID, calls[0].Get("launch_id")) + _, sent := calls[0]["launch_label"] + assert.False(t, sent, "an absent label must not be sent as an empty one") +} + +// TestSessionTokenFromAServerThatIgnoresTheSessionFailsClosed: a Basecamp +// that predates session attribution answers with an ordinary token. That +// token is not bound to the session, so it is discarded, not printed. The +// message says which way the echo missed: a server that echoed the id +// does record sessions, so it is not told it predates them. +func TestSessionTokenFromAServerThatIgnoresTheSessionFailsClosed(t *testing.T) { + for name, tc := range map[string]struct{ body, says, never string }{ + "no echo": { + `{"access_token":"unbound","token_type":"Bearer","expires_in":3600}`, + "predates agent session attribution", "different"}, + "other id": { + `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"ffffffffffffffffffffffffffffffff","launch_label":"coworker@box"}`, + "answered for a different session", "predates"}, + "other label": { + `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `","launch_label":"someone-else"}`, + "recorded a different label", "predates"}, + "label not given": { + `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `"}`, + "did not record the label", "predates"}, + } { + body := tc.body + t.Run(name, func(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { return http.StatusOK, body } + m := newDeviceTestManager(t, as.srv.URL) + key := storeAgent(t, m, liveAgentCredential(as.srv.URL+"/oauth/token")) + + token, err := m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.Error(t, err) + assert.Empty(t, token) + assert.NotContains(t, err.Error(), "unbound", "the unbound token must not leak into the error") + assert.Contains(t, err.Error(), testSessionID) + assert.Contains(t, err.Error(), tc.says) + assert.NotContains(t, err.Error(), tc.never) + + stored, loadErr := m.store.Load(key) + require.NoError(t, loadErr) + assert.Equal(t, "shared-cached", stored.AccessToken) + assert.Nil(t, stored.RenewalHold, "an unattributing server is no verdict on the credential") + }) + } +} + +// TestSessionTokenRefusedForItsSessionNamesItAndHoldsNothing: a refusal of +// the session's own parameters is about this request, not the credential, +// so nothing is held against the profile. +func TestSessionTokenRefusedForItsSessionNamesItAndHoldsNothing(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { + return http.StatusBadRequest, `{"error":"invalid_request","error_description":"launch_label is too long"}` + } + m := newDeviceTestManager(t, as.srv.URL) + key := storeAgent(t, m, liveAgentCredential(as.srv.URL+"/oauth/token")) + + _, err := m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.Error(t, err) + assert.Contains(t, err.Error(), testSessionID) + assert.Contains(t, err.Error(), "invalid_request") + + stored, loadErr := m.store.Load(key) + require.NoError(t, loadErr) + assert.Nil(t, stored.RenewalHold) + assert.Equal(t, "shared-cached", stored.AccessToken) +} + +// TestSessionTokenRefusalOfTheCredentialIsHeld: a session mint presents the +// same client secret the shared path does, so the token endpoint's verdict +// on that secret is remembered for both — a dozen sandbox launches must not +// each keep presenting a dead secret. +func TestSessionTokenRefusalOfTheCredentialIsHeld(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { + return http.StatusUnauthorized, `{"error":"invalid_client"}` + } + m := newDeviceTestManager(t, as.srv.URL) + key := storeAgent(t, m, liveAgentCredential(as.srv.URL+"/oauth/token")) + + _, err := m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.Error(t, err) + assert.ErrorIs(t, err, ErrAgentCredentialRefused) + + stored, loadErr := m.store.Load(key) + require.NoError(t, loadErr) + require.NotNil(t, stored.RenewalHold) + assert.Equal(t, renewalHoldRefused, stored.RenewalHold.Kind) + + _, err = m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.Error(t, err) + assert.ErrorIs(t, err, errRenewalHeld) + assert.Len(t, as.tokenCalls(), 1, "a held credential was presented again") +} + +// TestConcurrentVerdictsOnTheSecretKeepTheLongerHold: a session mint holds +// no lock across its round trip, so another mint's verdict on the same +// secret can land while it is in flight. Whichever is written last, the +// hold that stands is the one that holds longer, and the session is told +// that one: a dead secret is not sent again a minute later because a 429 +// from the abuse tracker it tripped landed after its invalid_client. +func TestConcurrentVerdictsOnTheSecretKeepTheLongerHold(t *testing.T) { + type answer struct { + status int + header http.Header + body string + } + invalidClient := answer{http.StatusUnauthorized, nil, `{"error":"invalid_client"}`} + invalidGrant := answer{http.StatusBadRequest, nil, `{"error":"invalid_grant"}`} + rateLimited := func(seconds string) answer { + return answer{http.StatusTooManyRequests, http.Header{"Retry-After": {seconds}}, `{}`} + } + + for name, tc := range map[string]struct { + // early is the verdict that lands while the session's mint is in + // flight, from another launch, or from the shared path when + // shared; late is the session's own. + early, late answer + shared bool + + kind string + until time.Duration // from now; zero is never + // stood is whether the session is answered with the earlier + // verdict rather than its own; retryAfter is the wait it is told. + stood bool + retryAfter int + }{ + "a 429 after invalid_client": {early: invalidClient, late: rateLimited("60"), kind: renewalHoldRefused, stood: true}, + "a 429 after the shared path's refusal": {early: invalidClient, late: rateLimited("60"), shared: true, kind: renewalHoldRefused, stood: true}, + "a 429 after invalid_grant": {early: invalidGrant, late: rateLimited("60"), kind: renewalHoldRefused, until: time.Hour, stood: true}, + "a shorter 429 after a longer one": {early: rateLimited("1800"), late: rateLimited("60"), kind: renewalHoldRateLimited, until: 30 * time.Minute, stood: true, retryAfter: 1800}, + "invalid_client after a 429": {early: rateLimited("60"), late: invalidClient, kind: renewalHoldRefused}, + "a longer 429 after a shorter one": {early: rateLimited("60"), late: rateLimited("1800"), kind: renewalHoldRateLimited, until: 30 * time.Minute, retryAfter: 1800}, + "invalid_client after the shared path 429": {early: rateLimited("60"), late: invalidClient, shared: true, kind: renewalHoldRefused}, + } { + t.Run(name, func(t *testing.T) { + e := startMintEndpoint(t) + first, key, now := heldManager(t, e) + second := newDeviceTestManager(t, e.srv.URL) + second.store, second.clock = first.store, first.clock + start := *now + + e.answer = func(call int) (int, http.Header, string) { + a := tc.early + if call == 0 { + // The session's mint is in flight: the other verdict + // lands, and is written, before it is answered. + var err error + if tc.shared { + _, err = first.AccessToken(context.Background()) + } else { + _, err = first.SessionAccessToken(context.Background(), "fedcba9876543210fedcba9876543210", "") + } + assert.Error(t, err) + a = tc.late + } + return a.status, a.header, a.body + } + + _, err := second.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.Error(t, err) + require.EqualValues(t, 2, e.calls.Load()) + assert.Contains(t, err.Error(), testSessionID) + assert.Equal(t, tc.stood, errors.Is(err, errRenewalHeld), "whether the session was answered by the hold that stood") + assert.Equal(t, tc.retryAfter, RetryAfter(err), "the wait the session is told") + if tc.kind == renewalHoldRefused { + assert.ErrorIs(t, err, ErrAgentCredentialRefused) + } + + stored, loadErr := second.store.Load(key) + require.NoError(t, loadErr) + require.NotNil(t, stored.RenewalHold) + assert.Equal(t, tc.kind, stored.RenewalHold.Kind) + var until int64 + if tc.until != 0 { + until = start.Add(tc.until).Unix() + } + assert.Equal(t, until, stored.RenewalHold.Until) + + // Past the shorter hold, nothing goes out from either launch. + *now = now.Add(2 * time.Minute) + _, err = second.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.Error(t, err) + assert.ErrorIs(t, err, errRenewalHeld) + assert.EqualValues(t, 2, e.calls.Load(), "the shorter hold was written over the longer one") + }) + } +} + +// TestSessionTokenNeedsAnAgentProfile: only an agent credential mints for +// itself; a person's login has no session to attribute and is not handed +// out in place of one. +func TestSessionTokenNeedsAnAgentProfile(t *testing.T) { + as := startDeviceAS(t) + m := newDeviceTestManager(t, as.srv.URL) + storeAgent(t, m, &Credentials{AccessToken: "person-token", RefreshToken: "r", ExpiresAt: time.Now().Add(time.Hour).Unix()}) + + token, err := m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.Error(t, err) + assert.Empty(t, token) + assert.Equal(t, output.CodeUsage, output.AsError(err).Code) + assert.Empty(t, as.tokenCalls()) +} + +// TestSessionTokenValidatesItsIdentityBeforeSending: the same rules the +// server holds a session to are checked before anything goes out. +func TestSessionTokenValidatesItsIdentityBeforeSending(t *testing.T) { + for name, tc := range map[string]struct{ id, label string }{ + "empty id": {"", "coworker@box"}, + "short id": {"0123456789abcdef", ""}, + "uppercase id": {strings.ToUpper(testSessionID), ""}, + "non-hex id": {"0123456789abcdef0123456789abcdeg", ""}, + "label too long": {testSessionID, strings.Repeat("x", 101)}, + "blank label": {testSessionID, " "}, + "control in label": {testSessionID, "coworker\nbox"}, + "bidi override": {testSessionID, "coworker\u202ebox"}, + "line separator": {testSessionID, "coworker\u2028box"}, + "invalid utf-8 label": {testSessionID, "coworker\xffbox"}, + } { + t.Run(name, func(t *testing.T) { + as := startDeviceAS(t) + m := newDeviceTestManager(t, as.srv.URL) + storeAgent(t, m, liveAgentCredential(as.srv.URL+"/oauth/token")) + + _, err := m.SessionAccessToken(context.Background(), tc.id, tc.label) + require.Error(t, err) + assert.Equal(t, output.CodeUsage, output.AsError(err).Code) + assert.Empty(t, as.tokenCalls()) + }) + } + + assert.NoError(t, ValidateSession(testSessionID, strings.Repeat("é", 100)), "100 characters, not bytes") + assert.NoError(t, ValidateSession(testSessionID, "claude · coworker")) + // A character newer than this build's Unicode tables is accepted, as bc3 + // accepts it: refusing it would cost the launch its whole token over a + // table the two sides need not share. + assert.NoError(t, ValidateSession(testSessionID, "coworker \U0001FAE9")) + assert.NoError(t, ValidateSession(testSessionID, "coworker\U000E0080box")) +} diff --git a/internal/auth/renewal_hold.go b/internal/auth/renewal_hold.go index f477818a4..cd26c91a8 100644 --- a/internal/auth/renewal_hold.go +++ b/internal/auth/renewal_hold.go @@ -233,7 +233,7 @@ func refreshClientFingerprint(refreshToken, clientID, clientSecret string) strin // refresh token and client (refreshClientFingerprint); a login writes a // credential with no hold, and a changed client is not held. func (m *Manager) holdRefusedRefreshClient(origin string, creds *Credentials, clientID, clientSecret string, err error) error { - stored := m.rememberRenewalHold(origin, refreshTokenFingerprint(creds.RefreshToken), &RenewalHold{ + _, stored := m.rememberRenewalHold(origin, refreshTokenFingerprint(creds.RefreshToken), &RenewalHold{ Kind: renewalHoldRefused, Detail: invalidClientDetail, Client: refreshClientFingerprint(creds.RefreshToken, clientID, clientSecret), @@ -462,8 +462,37 @@ func (hold *RenewalHold) holds(creds *Credentials, now time.Time) bool { } } +// outlasts reports whether hold, stored on creds, answers mints for at +// least as long as next would: it holds now, and never ends or ends after +// next does. +// +// Two mints of one secret can be in flight at once, and their verdicts land +// in either order, so the later write is not the later word. An +// invalid_client refusal says the secret is dead for good, and a 429 that +// lands after it — the abuse tracker the first refusals tripped answers +// every request after them with one, whatever the secret — must not cut +// that to a minute, after which the dead secret goes out again. Nor may a +// shorter rate limit cut short a longer one the server asked for. +func (hold *RenewalHold) outlasts(next *RenewalHold, creds *Credentials, now time.Time) bool { + switch { + case !hold.holds(creds, now): + return false + case hold.Until == 0: + return true + default: + return next.Until != 0 && hold.Until > next.Until + } +} + // rememberRenewalHold records hold on the stored credential — if the stored -// credential is still the one the verdict was about. +// credential is still the one the verdict was about, and no verdict it +// already carries on it outlasts this one. +// +// It returns the stored credential when such a verdict stands and nothing +// was written, so the caller can answer with it; nil otherwise. Only a mint +// that held no lock across its round trip meets one: a session mint +// (agent_launch.go), or any mint on a host without the lock. Under the lock, +// a standing hold answers the mint before anything is sent. // // The caller holds the credential key's cross-process lock, so no login can // land between the refusal and this write. The fingerprint is compared @@ -481,7 +510,7 @@ func (hold *RenewalHold) holds(creds *Credentials, now time.Time) bool { // A failure to write is reported and otherwise changes nothing — the // refusal is what the caller sees either way. It reports whether the hold // was stored, for a caller whose message promises what later commands do. -func (m *Manager) rememberRenewalHold(origin, about string, hold *RenewalHold) (stored bool) { +func (m *Manager) rememberRenewalHold(origin, about string, hold *RenewalHold) (stood *Credentials, stored bool) { current, err := m.store.Load(origin) if err != nil { // A credential removed since the mint (a logout) has nothing to @@ -489,17 +518,20 @@ func (m *Manager) rememberRenewalHold(origin, about string, hold *RenewalHold) ( if !errors.Is(err, ErrNoCredential) { m.warnf("warning: could not read the credential for %s to remember the token endpoint's refusal, so the next command will ask again: %v", origin, err) } - return false + return nil, false } if renewalSubject(current) != about { - return false + return nil, false + } + if current.RenewalHold.outlasts(hold, current, m.now()) { + return current, false } current.RenewalHold = hold if err := m.store.Save(origin, current); err != nil { m.warnf("warning: could not remember the token endpoint's refusal for %s, so the next command will ask again: %v", origin, err) - return false + return nil, false } - return true + return nil, true } // now is the Manager's clock: the clock field when a test set one. diff --git a/internal/commands/auth.go b/internal/commands/auth.go index c561fe0dc..5a24b8bb7 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -533,6 +533,7 @@ func newAuthRefreshCmd() *cobra.Command { func newAuthTokenCmd() *cobra.Command { var stored bool + var sessionID, sessionLabel string cmd := &cobra.Command{ Use: "token", @@ -553,6 +554,17 @@ Get tokens for different profiles: The --stored flag ignores BASECAMP_TOKEN and uses stored OAuth credentials: basecamp auth token --stored +An agent profile can mint a token of its own for one session — one launch +of a sandboxed agent, say — which Basecamp records on the token and logs +beside the agent, so concurrent sessions can be told apart: + basecamp -P agent auth token --stored \ + --session-id "$(openssl rand -hex 16)" --session-label "coworker@laptop" + +A session token is minted fresh on every call and never cached; the caller +holds it for as long as it means to. A Basecamp that does not record the +session gets no token from this: the command fails rather than hand out one +that is not bound to the session. + Output modes: basecamp auth token # Raw token (default, for shell substitution) basecamp auth token --json # JSON envelope with token in data field @@ -566,7 +578,21 @@ Output modes: var token string var err error - if stored { + // Whether a flag was GIVEN decides, not its value: an empty + // "--session-id $ID" is a launcher that failed to make one, and + // it is refused rather than handed the shared token. + idGiven := cmd.Flags().Changed("session-id") + session := idGiven || cmd.Flags().Changed("session-label") + switch { + case session && !idGiven: + return output.ErrUsage("--session-label needs --session-id") + case session && !stored: + return output.ErrUsage("--session-id needs --stored: a session token is minted from the stored agent credential, never taken from BASECAMP_TOKEN") + } + + if session { + token, err = app.Auth.SessionAccessToken(cmd.Context(), sessionID, sessionLabel) + } else if stored { // Use stored OAuth credentials (ignores BASECAMP_TOKEN env) // This also handles auto-refresh for near-expiry tokens token, err = app.Auth.StoredAccessToken(cmd.Context()) @@ -582,7 +608,14 @@ Output modes: // Output raw token by default for backwards compatibility with shell scripts. // Only use JSON envelope when --json/--agent/--jq is explicitly requested. if app.Flags.JSON || app.Flags.Agent || app.Flags.JQFilter != "" { - return app.OK(map[string]string{"token": token}) + data := map[string]string{"token": token} + if session { + data["session_id"] = sessionID + if sessionLabel != "" { + data["session_label"] = sessionLabel + } + } + return app.OK(data) } // Raw output: print token directly, with optional stats on stderr @@ -592,6 +625,8 @@ Output modes: } cmd.Flags().BoolVar(&stored, "stored", false, "Use stored OAuth token, ignoring BASECAMP_TOKEN env var") + cmd.Flags().StringVar(&sessionID, "session-id", "", "Mint a fresh agent token for this session (32 lowercase hex characters); needs --stored") + cmd.Flags().StringVar(&sessionLabel, "session-label", "", "Label for the session (at most 100 characters); needs --session-id") return cmd } diff --git a/internal/commands/auth_token_session_test.go b/internal/commands/auth_token_session_test.go new file mode 100644 index 000000000..7a92a9f3c --- /dev/null +++ b/internal/commands/auth_token_session_test.go @@ -0,0 +1,99 @@ +package commands + +import ( + "encoding/json" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/auth" + "github.com/basecamp/basecamp-cli/internal/config" + "github.com/basecamp/basecamp-cli/internal/output" +) + +const tokenSessionID = "0123456789abcdef0123456789abcdef" + +// storeAgentProfile seeds the "agent" profile with a live shared token. +func storeAgentProfile(t *testing.T, store *auth.Store, tokenEndpoint string) { + t.Helper() + require.NoError(t, store.Save("profile:agent", &auth.Credentials{ + AccessToken: "shared-cached", + OAuthType: "agent", + ClientID: "agent-client", + ClientSecret: "agent-secret", + TokenEndpoint: tokenEndpoint, + Scope: "full", + ExpiresAt: time.Now().Add(time.Hour).Unix(), + })) +} + +func TestAuthTokenSessionMintsForTheSession(t *testing.T) { + as := startAgentAS(t) + as.token = func() (int, string) { + return http.StatusOK, `{"access_token":"session-token","token_type":"Bearer","expires_in":3600,"scope":"full",` + + `"launch_id":"` + tokenSessionID + `","launch_label":"coworker@box"}` + } + app, buf := agentLoginApp(t, as, &config.Config{ActiveProfile: "agent"}) + storeAgentProfile(t, app.Auth.GetStore(), as.srv.URL+"/oauth/tokens") + app.Flags.JSON = true + + err := executeCommand(NewAuthCmd(), app, "token", "--stored", "--session-id", tokenSessionID, "--session-label", "coworker@box") + require.NoError(t, err) + + var envelope struct { + Data map[string]string `json:"data"` + } + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope), buf.String()) + assert.Equal(t, map[string]string{ + "token": "session-token", + "session_id": tokenSessionID, + "session_label": "coworker@box", + }, envelope.Data) + + calls := as.calls() + require.Len(t, calls, 1) + assert.Equal(t, tokenSessionID, calls[0].Get("launch_id")) + assert.Equal(t, "coworker@box", calls[0].Get("launch_label")) +} + +func TestAuthTokenWithoutSessionKeepsItsShape(t *testing.T) { + as := startAgentAS(t) + app, buf := agentLoginApp(t, as, &config.Config{ActiveProfile: "agent"}) + storeAgentProfile(t, app.Auth.GetStore(), as.srv.URL+"/oauth/tokens") + app.Flags.JSON = true + + require.NoError(t, executeCommand(NewAuthCmd(), app, "token", "--stored")) + + var envelope struct { + Data map[string]string `json:"data"` + } + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope), buf.String()) + assert.Equal(t, map[string]string{"token": "shared-cached"}, envelope.Data) + assert.Empty(t, as.calls()) +} + +func TestAuthTokenSessionFlagUsage(t *testing.T) { + for name, args := range map[string][]string{ + "label without id": {"token", "--stored", "--session-label", "coworker@box"}, + "id without stored": {"token", "--session-id", tokenSessionID}, + "malformed id": {"token", "--stored", "--session-id", "not-hex"}, + // An unset variable in "--session-id $ID" must not fall back to the + // shared, unattributed token. + "empty id": {"token", "--stored", "--session-id", ""}, + "empty id and label": {"token", "--stored", "--session-id", "", "--session-label", ""}, + } { + t.Run(name, func(t *testing.T) { + as := startAgentAS(t) + app, _ := agentLoginApp(t, as, &config.Config{ActiveProfile: "agent"}) + storeAgentProfile(t, app.Auth.GetStore(), as.srv.URL+"/oauth/tokens") + + err := executeCommand(NewAuthCmd(), app, args...) + require.Error(t, err) + assert.Equal(t, output.CodeUsage, output.AsError(err).Code, err.Error()) + assert.Empty(t, as.calls()) + }) + } +}