fix(token): re-check user is active on refresh_token grant

[appleboy]

Summary

The refresh_token grant never re-checked whether the token's owner still exists and is active before minting new tokens. Commit #204 added that guard to the device_code and authorization_code redemption paths but missed refresh — the longest-lived, most frequently replayed credential. Any path that disables/deletes a user without fully revoking their tokens (e.g. SetUserActiveStatus, external auth-source sync) left their refresh token able to keep issuing fresh access tokens. This PR closes that gap and returns the correct OAuth error code.

Architecture / flow

sequenceDiagram
    participant C as Client
    participant H as handleRefreshTokenGrant
    participant T as RefreshAccessToken
    participant S as Store
    participant A as AuditService
    participant P as TokenProvider

    C->>H: POST /oauth/token (grant_type=refresh_token)
    H->>T: RefreshAccessToken(...)
    Note over T: existing: token valid, client auth (active + secret)
    rect rgb(223,240,216)
    T->>S: GetUserByID(refreshToken.UserID)
    alt user missing or !IsActive
        T->>A: LogSync(SUSPICIOUS_ACTIVITY, WARNING, ResourceToken)
        T-->>H: ErrUserInactive
        H-->>C: 400 access_denied
    end
    end
    Note over T: compose claims, issue / rotate
    T->>P: RefreshAccessToken(...)
    T->>S: RunInTransaction(create/rotate token)
    T-->>H: new access (+ refresh) token
    H-->>C: 200 access_token

Loading

The green block is new. It runs after client authentication and before composing claims / calling the provider, so a rejected refresh never rotates or revokes the existing token.

AI Authorship

• No AI was used in this PR
• AI was used. Details:
  • Tool / model: Claude Opus 4.8 (Claude Code), driven from plan.md
  • AI-authored files: all changed files (internal/services/token_refresh.go, internal/services/token.go, internal/handlers/token.go, and the three test files)
  • Human line-by-line reviewed: none yet — the author relied on the written plan, the test suite, and an automated max-effort code review. Reviewers should read the core files line-by-line.

Change classification

• Leaf node (local impact)
• Core code (broad impact — needs line-by-line review)

This is on the OAuth token-issuance / auth path. A mistake here could either let disabled users keep minting tokens (security gap) or wrongly reject legitimate refreshes (availability).

Plan reference

A plan.md drove the work (kept local, not committed). Goal: re-check the refresh token's owner is present and active before issuance, mirroring the device/auth-code paths from #204; on failure log a SUSPICIOUS_ACTIVITY event and reject without rotating/revoking the token. The shared assertUserStillActive helper refactor was explicitly out of scope (separate issue).

Two scope decisions were made with the human during execution:

1. Seeding active users in pre-existing refresh tests that used synthetic user IDs (the new guard correctly rejected them) — approved.
2. Returning a dedicated ErrUserInactive sentinel mapped to OAuth access_denied (instead of invalid_client), which required touching internal/handlers/token.go (outside the plan's original "May modify" list) — approved, to match the plan's intent and the sibling grants.

Verification

• Unit tests (service layer)
• Integration tests (handler layer)
• At least 3 e2e tests (1 happy path + 2 errors): happy refresh, disabled-owner rejected (and not rotated), deleted-owner rejected, audit-event emitted, plus a handler test asserting 400 access_denied
• Stress / soak test: N/A (single primary-key lookup, no concurrency change)
• Manual verification (optional): start the server, obtain a refresh token via device-cli, disable the user at /admin/users/:id/disable, then attempt grant_type=refresh_token → expect access_denied; confirm a SUSPICIOUS_ACTIVITY entry in /admin/audit.

make generate / make fmt / make lint (0 issues) / make test (full suite) all pass.

Verifiability check

• Inputs and outputs are documented (godoc on RefreshAccessToken, comments on the new sentinel + guard)
• Reviewer can judge correctness from interface + tests alone — no, core path: read it line-by-line
• Failures will surface in monitoring (RecordTokenRefresh(false) metric + SUSPICIOUS_ACTIVITY audit log)

Security check

• No secrets in code
• All external inputs validated (no new external inputs; uses existing refreshToken.UserID)
• Permission checks tested (disabled + deleted owner rejection covered)
• Rate limits applied (existing /oauth/token rate limit unchanged)
• Errors don't leak internals (returns standard OAuth access_denied, no user state detail)

Risk & rollback

• Risk: one extra GetUserByID primary-key lookup per refresh (refresh already does a token + client lookup; negligible). If a legitimate refresh token's UserID did not resolve to an active user row it would now be rejected — guarded against by the happy-path test and by confirming client_credentials issues no refresh tokens, so refresh tokens always carry a real user.
• Rollback: revert this commit. Single inline guard + one new error sentinel + one handler case; no schema, config, or migration changes.

Reviewer guide

• Read carefully: internal/services/token_refresh.go (the new guard block and its placement relative to client auth and token rotation), internal/services/token.go (the ErrUserInactive sentinel), internal/handlers/token.go (the new access_denied mapping ordering before the invalid_client case).
• Spot-check OK: the three test files — token_refresh_test.go (services + handlers), token_resource_test.go (user-seeding additions).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!