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

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions