Skip to content

Harden OAuth token-response parsing and refresh-error classification #180

Description

@santhil-cyber

Problem

A few off-spec OAuth token-endpoint responses are handled less defensively than the rest of the auth code, all in src/services/auth-service.ts.

1. Unsafe casts on token fields

parseTokenResponse / parseRefreshTokenResponse read fields with as string behind a truthiness check:

accessToken: d.access_token as string,

A truthy-but-non-string access_token (a number, an object) passes the presence check and is mistyped as a string, surfacing later as a confusing downstream failure instead of a clear parse error. The rest of the auth code (e.g. auth-config.ts) validates with Zod .safeParse; this path doesn't.

2. expires_in coercion has a NaN path

expiresIn: (d.expires_in as number) || 3600,

This handles a missing value, but a non-numeric string like "soon" becomes NaN. That flows into new Date(Date.now() + expiresIn * 1000) in login.ts, and new Date(NaN).toISOString() throws a RangeError. A numeric string ("3600") and a negative value are also not handled cleanly.

3. Refresh-error classification depends on description wording

classifyTerminalRefreshError requires invalid_grant plus a specific phrase in the error description to flag a dead refresh token:

oauthError === "invalid_grant" &&
  (text.includes("invalid refresh token") || text.includes("already used") || ...)

Per RFC 6749 §5.2, invalid_grant on a refresh request already means the refresh token is expired/revoked/reused. If the backend rewords its error_description, classification silently fails and the user isn't prompted to re-authenticate.

Proposed solution

  • Validate both token responses with Zod .safeParse (matching the existing auth-config.ts pattern), preserving the current "Token response missing required fields" error.
  • Coerce expires_in safely: accept numeric strings, fall back to the default lifetime for non-finite / zero / negative values.
  • Classify invalid_grant on the structured code, keeping the substring checks as a fallback for servers that don't send a clean code.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions