-
Notifications
You must be signed in to change notification settings - Fork 155
Description
Summary
The isValidToken method in the TokenManager class contains a logic error that causes token expiration checks to always pass, effectively disabling token expiration validation entirely.
Vulnerable Code
// src/lib/token-manager.ts, lines 320-326
isValidToken(token?: AccessToken): token is AccessToken {
return (
typeof token !== 'undefined' &&
token !== null &&
token.expires_in < Date.now() / 1000 // BUG: Wrong field and wrong comparison
)
}Root Cause
The validation logic has two compounding errors:
-
Wrong field: The code checks
expires_in(a duration in seconds, e.g.,3600) instead ofexpires_at(the actual expiration timestamp). -
Wrong comparison operator: Even if the correct field were used, the comparison should check if the expiration time is greater than the current time, not less than.
How tokens are structured
When a token is received (line 137), the code correctly calculates expires_at:
token.expires_at = Math.round(Date.now() / 1000) + token.expires_inFor a token with expires_in: 3600 (1 hour):
expires_at= current timestamp + 3600 ≈1707004600expires_in=3600
Why the bug occurs
The current check token.expires_in < Date.now() / 1000 evaluates:
3600 < 1707000000 → true (always)
This condition is always true for any realistic token, meaning:
- Expired tokens are incorrectly considered valid
- Token refresh is never triggered based on expiration
- The SDK continues using stale/expired tokens
Impact
- Expired tokens remain in use: The SDK will continue sending requests with expired OAuth tokens until an external failure occurs.
- Authentication failures: Requests made with expired tokens will fail at the Segment API, causing data loss or delivery failures.
- Silent failures: Depending on error handling, these failures may go unnoticed.
Suggested Fix
isValidToken(token?: AccessToken): token is AccessToken {
return (
typeof token !== 'undefined' &&
token !== null &&
typeof token.expires_at === 'number' &&
token.expires_at > Date.now() / 1000 // Use expires_at with correct comparison
)
}Reproduction
Any usage of the OAuth flow in @segment/analytics-node is affected. The bug can be confirmed by:
- Obtaining a valid OAuth token
- Waiting for the token to expire
- Observing that
isValidToken()still returnstrue - Observing that the cached expired token continues to be used