diff --git a/README.md b/README.md index 02a39f6ca..3ecf2f7be 100644 --- a/README.md +++ b/README.md @@ -239,8 +239,11 @@ op read "op://Vault/Item/credential" | basecamp auth login --with-client-credent The login mints once, which is what proves the client id and secret: a refused mint stores nothing. `--account` is required when the profile does not exist yet. `basecamp auth logout` forgets the credential; there is no useful -revocation, since the same client would mint another — rotate the client -secret in Basecamp to end an agent's access. +revocation, since the same client would mint another. To end its access, +disconnect its connection in Basecamp (on My > Agent for your own agent, in +Adminland for a shared one), which kills that connection's secret and every +token minted from it. Where My > Agent lists several connections, disconnect +the one this profile used: Disconnect all ends every one of them. ### Connecting an agent to this computer diff --git a/internal/auth/agent_test.go b/internal/auth/agent_test.go index f07691881..d5bc410b1 100644 --- a/internal/auth/agent_test.go +++ b/internal/auth/agent_test.go @@ -392,9 +392,9 @@ func TestAgentMintRefusesARedirect(t *testing.T) { assert.Zero(t, *sinkCalls, "the client secret followed a redirect") } -// TestAgentLogoutSaysRotatingTheSecretIsTheRemedy: revoking one self-token +// TestAgentLogoutForgetsWithoutRevoking: revoking one self-token // accomplishes nothing while the client that minted it can mint another. -func TestAgentLogoutSaysRotatingTheSecretIsTheRemedy(t *testing.T) { +func TestAgentLogoutForgetsWithoutRevoking(t *testing.T) { as := startDeviceAS(t) m := newDeviceTestManager(t, as.srv.URL) key := storeAgent(t, m, agentCredential(as.srv.URL+"/oauth/token", time.Now().Add(time.Hour))) @@ -421,7 +421,7 @@ func TestAgentRevokeIsRefusedAndKeepsTheCredential(t *testing.T) { err := m.RevokeStored(context.Background()) require.Error(t, err) assert.Equal(t, output.CodeUsage, output.AsError(err).Code) - assert.Contains(t, err.Error(), "rotate the client secret") + assert.Contains(t, output.AsError(err).Hint, "disconnect this connection in Basecamp") assert.Empty(t, as.revokeCalls(), "a revocation was sent for an agent self-token") creds, loadErr := m.store.Load(key) diff --git a/internal/auth/revoke.go b/internal/auth/revoke.go index 2c114cf6c..d7d462eca 100644 --- a/internal/auth/revoke.go +++ b/internal/auth/revoke.go @@ -44,8 +44,13 @@ const ( RevokeSkippedImported = "imported_token" // RevokeSkippedAgent: an agent self-token is minted on demand from a // client id and secret, so revoking one accomplishes nothing — the - // same client mints another on the next command. Ending an agent's - // access means rotating the client secret in Basecamp. + // same client mints another on the next command. What ends this + // credential's access is disconnecting its connection in Basecamp, + // which kills that connection's secret and every token minted from it. + // Once an agent can hold several connections, neither older remedy is + // right: rotating the client secret (Basecamp has no control for it) + // rewrites only the first connection's, and disconnecting the whole + // agent ends every other computer's connection too. RevokeSkippedAgent = "agent" ) @@ -194,7 +199,7 @@ func (m *Manager) RevokeStored(ctx context.Context) error { "Forget the credential locally instead: basecamp auth logout") case RevokeSkippedAgent: return output.ErrUsageHint("An agent self-token is not worth revoking: the client that minted it can mint another", - "Forget the credential locally instead (basecamp auth logout), and rotate the client secret in Basecamp to end the agent's access") + "Forget the credential locally instead (basecamp auth logout), and disconnect this connection in Basecamp to end its access") } if err := m.Revoke(ctx, creds); err != nil { // Keep the failure's taxonomy — a transport failure or a 5xx diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 9f3bcf4d4..ecab70202 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -1607,7 +1607,7 @@ func describeLogout(done string, result *auth.LogoutResult) (summary string, fie summary = done + " (forgot the imported token; it stays valid until revoked in Basecamp)" case result.Skipped == auth.RevokeSkippedAgent: fields["reason"] = result.Skipped - summary = done + " (forgot the agent credential; rotate the client secret in Basecamp to end its access)" + summary = done + " (forgot the agent credential; disconnect this connection in Basecamp to end its access)" case result.Err != nil: fields["reason"] = result.Err.Error() fields["remaining"] = result.Remaining diff --git a/internal/commands/auth_agent.go b/internal/commands/auth_agent.go index a70239f35..a66eeac4b 100644 --- a/internal/commands/auth_agent.go +++ b/internal/commands/auth_agent.go @@ -19,8 +19,8 @@ func newAuthAgentCmd() *cobra.Command { cmd := &cobra.Command{ Use: "agent", Short: "Connect this computer to a Basecamp agent", - Long: `Connect this computer to a Basecamp agent — an identity your Basecamp -administrator created — so commands run as that agent. + Long: `Connect this computer to a Basecamp agent — your own personal agent, or +one your Basecamp administrator created — so commands run as that agent. The connection hands this CLI the agent's own OAuth client, which mints the access tokens it spends. Nobody pastes a credential: you approve the diff --git a/internal/commands/auth_logout_test.go b/internal/commands/auth_logout_test.go index 07e1d8ec2..d4f93deaf 100644 --- a/internal/commands/auth_logout_test.go +++ b/internal/commands/auth_logout_test.go @@ -121,6 +121,19 @@ func bc5LogoutCredentials(s *revocationServer) *auth.Credentials { } } +// agentLogoutCredentials is a connected agent's stored credential: its own +// client id and secret, and a self-token minted from them. +func agentLogoutCredentials(s *revocationServer) *auth.Credentials { + return &auth.Credentials{ + AccessToken: "agent-at", + OAuthType: "agent", + ClientID: "bc-agent-1", + ClientSecret: "agent-secret", + TokenEndpoint: s.srv.URL + "/oauth/tokens", + Scope: "full", + } +} + func decodeLogoutJSON(t *testing.T, buf *bytes.Buffer) map[string]any { t.Helper() var envelope struct { @@ -181,6 +194,16 @@ func TestAuthLogoutHumanCopy(t *testing.T) { assert.False(t, app.Auth.IsAuthenticated()) }) + t.Run("agent", func(t *testing.T) { + s := startRevocationServer(t) + app, buf := newLogoutTestApp(t, s, output.FormatStyled, agentLogoutCredentials(s)) + require.NoError(t, runLogout(t, app)) + assert.Contains(t, buf.String(), "Logged out (forgot the agent credential; disconnect this connection in Basecamp to end its access)") + assert.NotContains(t, buf.String(), "rotate", "Basecamp has no rotate-secret control to send anyone to") + assert.Empty(t, s.revoked(), "an agent self-token is minted on demand; revoking one ends nothing") + assert.False(t, app.Auth.IsAuthenticated()) + }) + t.Run("not logged in", func(t *testing.T) { s := startRevocationServer(t) app, buf := newLogoutTestApp(t, s, output.FormatStyled, nil)