diff --git a/adapters/linear/README.md b/adapters/linear/README.md index 58d6588..c79beeb 100644 --- a/adapters/linear/README.md +++ b/adapters/linear/README.md @@ -92,7 +92,7 @@ The adapter owns its own rate-limit retry (`RetryPolicy`) and Linear-specific ex ### Tokens and tenants - The adapter refreshes a token when it is empty or within 1 hour of expiry. [linear.go:ensureToken,tokenRefreshBuffer] -- A token response without a positive `expires_in` gets no expiry, so the adapter never refreshes that token. This holds for the single-install token and for cached per-tenant tokens. [linear.go:exchangeClientCredentials,ensureToken,tenantToken] +- A token response without a positive `expires_in` gets Linear's documented 30-day lifetime, so the adapter refreshes it 1 hour before 30 days pass. This holds for the single-install token and for cached per-tenant tokens. The adapter does not refresh a token after a 401. [linear.go:exchangeClientCredentials,defaultTokenLifetime,ensureToken,tenantToken] - A client-credentials response must contain `access_token` and grant every requested scope. [linear.go:exchangeClientCredentials,verifyGrantedScopes] - An install credential must be a `LinearInstall` or a non-nil `*LinearInstall`. A multi-tenant install needs a webhook secret. A pre-supplied `AccessToken` is used as is. [linear.go:linearCredential,resolveWebhookInstall,tenantToken] - `GraphQL`, `CreateSessionOnIssue`, and `CreateSessionOnComment` fail in multi-tenant mode. Use the `ForTenant` forms. [session.go:GraphQL,CreateSessionOnIssue,CreateSessionOnComment] diff --git a/adapters/linear/linear.go b/adapters/linear/linear.go index 9363fa7..5148387 100644 --- a/adapters/linear/linear.go +++ b/adapters/linear/linear.go @@ -40,6 +40,7 @@ const ( defaultSignatureTolerance = time.Minute maxWebhookBodyBytes = 1 << 20 tokenRefreshBuffer = time.Hour + defaultTokenLifetime = 30 * 24 * time.Hour ) // ClientCredentials configures Linear app-actor authentication. @@ -638,7 +639,7 @@ func (a *Adapter) verifyTimestamp(timestamp int64) error { func (a *Adapter) ensureToken(ctx context.Context) error { a.mu.Lock() - needsRefresh := a.accessToken == "" || (!a.tokenExpiry.IsZero() && a.now().After(a.tokenExpiry.Add(-tokenRefreshBuffer))) + needsRefresh := a.accessToken == "" || a.now().After(a.tokenExpiry.Add(-tokenRefreshBuffer)) a.mu.Unlock() if !needsRefresh { return nil @@ -686,11 +687,13 @@ func (a *Adapter) exchangeClientCredentials(ctx context.Context, creds ClientCre if err := verifyGrantedScopes(scopes, resp.Scope); err != nil { return "", time.Time{}, err } - var expiry time.Time + // expires_in is optional (RFC 6749 section 5.1), so fall back to Linear's + // documented lifetime: https://linear.app/developers/oauth-2-0-authentication + lifetime := defaultTokenLifetime if resp.ExpiresIn > 0 { - expiry = a.now().Add(time.Duration(resp.ExpiresIn) * time.Second) + lifetime = time.Duration(resp.ExpiresIn) * time.Second } - return resp.AccessToken, expiry, nil + return resp.AccessToken, a.now().Add(lifetime), nil } // tenantToken resolves the access token for one Platform Tenant in multi-tenant @@ -706,7 +709,7 @@ func (a *Adapter) tenantToken(ctx context.Context, tenant string, cred LinearIns } a.tenantTokensMu.Lock() cached := a.tenantTokens[tenant] - if cached != nil && cached.accessToken != "" && (cached.expiry.IsZero() || a.now().Before(cached.expiry.Add(-tokenRefreshBuffer))) { + if cached != nil && cached.accessToken != "" && a.now().Before(cached.expiry.Add(-tokenRefreshBuffer)) { token := cached.accessToken a.tenantTokensMu.Unlock() return token, nil diff --git a/adapters/linear/token_lifetime_test.go b/adapters/linear/token_lifetime_test.go new file mode 100644 index 0000000..70ac502 --- /dev/null +++ b/adapters/linear/token_lifetime_test.go @@ -0,0 +1,115 @@ +package linear_test + +import ( + "context" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "github.com/coder/chat" + "github.com/coder/chat/adapters/linear" +) + +type tokenWithoutExpiryServer struct { + *httptest.Server + mu sync.Mutex + tokens int +} + +func newTokenWithoutExpiryServer(t *testing.T) *tokenWithoutExpiryServer { + t.Helper() + api := &tokenWithoutExpiryServer{} + api.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/oauth/token": + api.mu.Lock() + api.tokens++ + api.mu.Unlock() + writeJSON(t, w, map[string]any{"access_token": "token", "scope": "read write app:mentionable app:assignable"}) + case "/graphql": + writeJSON(t, w, map[string]any{"data": map[string]any{}}) + default: + t.Fatalf("unexpected path: %s", r.URL.Path) + } + })) + t.Cleanup(api.Close) + return api +} + +func (api *tokenWithoutExpiryServer) tokenRequests() int { + api.mu.Lock() + defer api.mu.Unlock() + return api.tokens +} + +func assertTokenRefreshedAfterDefaultLifetime(t *testing.T, api *tokenWithoutExpiryServer, clock *mutableClock, query func() error) { + t.Helper() + const lifetime, refreshBuffer = 30 * 24 * time.Hour, time.Hour + if err := query(); err != nil { + t.Fatalf("first query: %v", err) + } + if got := api.tokenRequests(); got != 1 { + t.Fatalf("token requests after first query = %d, want 1", got) + } + clock.advance(lifetime - refreshBuffer - time.Minute) + if err := query(); err != nil { + t.Fatalf("query within lifetime: %v", err) + } + if got := api.tokenRequests(); got != 1 { + t.Fatalf("token requests within lifetime = %d, want 1 (cached)", got) + } + clock.advance(2 * time.Minute) + if err := query(); err != nil { + t.Fatalf("query past lifetime: %v", err) + } + if got := api.tokenRequests(); got != 2 { + t.Fatalf("token requests past 30 days minus refresh buffer = %d, want 2 (refreshed)", got) + } +} + +func TestSingleInstallRefreshesTokenWithoutExpiresIn(t *testing.T) { + t.Parallel() + api := newTokenWithoutExpiryServer(t) + clock := newMutableClock(time.UnixMilli(1_700_000_000_000)) + adapter, err := linear.New(context.Background(), linear.Options{ + WebhookSecret: "whsec", + ClientCredentials: linear.ClientCredentials{ClientID: "client", ClientSecret: "secret"}, + APIBaseURL: api.URL, + Client: api.Client(), + Now: clock.now, + }) + if err != nil { + t.Fatalf("adapter: %v", err) + } + assertTokenRefreshedAfterDefaultLifetime(t, api, clock, func() error { + var dest map[string]any + return adapter.GraphQL(context.Background(), `query Q { viewer { id } }`, nil, &dest) + }) +} + +func TestMultiTenantRefreshesTokenWithoutExpiresIn(t *testing.T) { + t.Parallel() + api := newTokenWithoutExpiryServer(t) + clock := newMutableClock(time.UnixMilli(1_700_000_000_000)) + store := newFakeInstallStore() + store.set("ORG_A", chat.Install{ + Tenant: "ORG_A", + BotActorID: "APP_A", + Credential: linear.LinearInstall{WebhookSecret: "secretA", ClientCredentials: linear.ClientCredentials{ClientID: "clientA", ClientSecret: "csA"}}, + }) + adapter, err := linear.New(context.Background(), linear.Options{ + InstallStore: store, + APIBaseURL: api.URL, + Client: api.Client(), + Now: clock.now, + }) + if err != nil { + t.Fatalf("adapter: %v", err) + } + assertTokenRefreshedAfterDefaultLifetime(t, api, clock, func() error { + var dest map[string]any + return adapter.GraphQLForTenant(context.Background(), "ORG_A", `query Q { viewer { id } }`, nil, &dest) + }) +}