fix(server): introspect session-bound refresh tokens against their session

[SashaMIT]

Problem

533d177 added Handler.sessionAlive to token introspection and deliberately excluded refresh tokens, on the grounds that they outlive the session that issued them. 155557b then introduced per-client session binding: a client configured with refreshTokenLifetime: session (storage.Client.RefreshBoundToSession) has refresh tokens that explicitly do NOT outlive the session, and sessionAlive was rewritten to gate on exactly that flag.

The access-token path got the check; the refresh-token path never did. introspectAccessToken consults sessionAlive, while introspectRefreshToken returns active: true for any token that passes LookupRefreshToken, without consulting the session.

Reachability

Explicit logout is already covered: logout eagerly deletes bound clients' refresh tokens, so the lookup itself fails. But a session that ends by idle or absolute timeout revokes nothing eagerly. Between session end and the token's own expiry, a session-bound refresh token still introspects active: true, even though the refresh grant would refuse to redeem it (grants/refresh.go sessionID() reads the same flag and the same session, and refuses a dead one). Introspection and the grant disagree about the token's state, and RFC 7662 section 4 requires a revoked token to be reported inactive.

Fix

In introspectRefreshToken, after the lookup succeeds:

1. Load the client.
2. Read the token's sid from its offline-session reference, the same source the refresh grant reads.
3. Apply the same sessionAlive check the access-token path uses. A dead session means active: false.

A bound token whose offline session cannot be read is reported inactive, matching the grant's refusal in the same situation. For standalone clients the response is unchanged; the path now additionally reads the client and the offline-session reference. Also updates the IntrospectionExtra.SessionID doc comment, which still claimed nothing here consults session storage.

Tests

New TestHandleIntrospectRefreshTokenSessionBinding (server/server_introspection_test.go): a session-bound client whose session ended by timeout (row still stored, both expiries in the past) introspects its refresh token as active: false; a standalone client with the same dead session stays active: true. Verified the test fails without the fix.

• go build ./...
• go test ./server/ -run 'Introspect|Session' -count=1
• go test ./server/... -count=1

All pass.

Made with Cursor

[nabokihms]

Thanks for the PR, I looked through it. The diagnosis holds: introspection and the refresh grant answered differently about the same token, and reading the sid from the offline-session reference is the right source, since that is what the grant reads. Some suggestions.

1. With sessions disabled the same disagreement comes back. The refresh grant gates its check on sessionsEnabled, introspection does not, and Manager.Alive returns false when Config == nil. Enable sessions, hand out tokens to a bound client, disable sessions again: the grant keeps refreshing those tokens while introspection starts calling them inactive. A check on h.Sessions.Enabled() in sessionAlive would cover this and the access-token path at once.

2. GetOfflineSessions runs for standalone clients too and its result is then discarded. Read the client first and return early when RefreshBoundToSession() is false. The endpoint takes no client authentication, so on the Kubernetes backend this is one avoidable API call per unauthenticated request.

3. A deleted client now yields 500. Refresh-token introspection did not read the client before, so a token whose client is gone used to answer normally. ErrNotFound from GetClient is better reported as inactive than as a server error; such a token cannot be redeemed anyway. The access-token path has the same behaviour and predates this PR.

4. This is the second copy of the sid resolution. sessionID() in grants/refresh.go reads the same reference under the same rules, and the PR body says the two must agree. A shared helper would make that structural instead of a promise.

5. subject in sessionAlive has been unused since the session rework. This PR adds the second caller, so it is a good moment to drop the parameter.

6. The test covers both lifetimes but neither new branch: an unreadable offline session for a bound client, and a reference carrying no sid.

[SashaMIT]

Addressed review feedback:

1. sessionAlive gates on Sessions.Enabled() (nil-safe) so turning sessions off cannot flip bound tokens inactive while the refresh grant still redeems them. Covers the access-token path at once.
2. Early return for standalone clients after GetClient: skip GetOfflineSessions when RefreshBoundToSession() is false.
3. Deleted client → inactive, not 500 (storage.ErrNotFound on both refresh and access introspection paths).
4. Shared sid helper tokens.RefreshReferenceSessionID, used by introspection and grants/refresh.go sessionID().
5. Dropped unused subject from sessionAlive.
6. Tests now cover unreadable offline session (bound → inactive) and a reference with no sid (bound → active).

go test ./server/ -run 'Introspect|Session|Refresh' -count=1 and ./server/tokens/ pass locally.

[nabokihms]

Thanks, that covers all of it. One leftover of the same kind as the first point: with sessions off and a client marked session-bound, the offline-session read still runs before sessionAlive short-circuits, and an error there reports the token inactive while the grant would redeem it. Hoisting the Enabled() check above that block closes it.

[SashaMIT]

Thanks @nabokihms. Hoisted the Sessions.Enabled() check above the offline-session read for session-bound refresh introspection, so with sessions off we no longer touch offline storage (and cannot flip inactive on a read error while the grant would still redeem).

go test ./server/ -run 'Introspect|Session|Refresh' -count=1 is green locally.