Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow expires_at to be a UNIX timestamp #277

Merged

Conversation

ctavan
Copy link
Contributor

@ctavan ctavan commented Nov 17, 2019

This library currently only handles expires_at properties well when they
are provided in ISOString representation.

Unlike the expires_in field, the expires_at is not part of the OAuth2
spec, so implementations may vary.

Some APIs like the Strava API use a UNIX timestamp (seconds since epoch)
in the expires_at field:
http://developers.strava.com/docs/authentication/#response-parameters-1

Token refresh will yield "Invalid Date" in the expires_at field when
only accepting an ISOString.

This patch checks if expires_at is a number and if so assumes it is a
UNIX timestamp (seconds since epoch).

This library currently only handles expires_at properties well when they
are provided in ISOString representation.

Unlike the expires_in field, the expires_at is not part of the OAuth2
spec, so implementations may vary.

Some APIs like the Strava API use a UNIX timestamp (seconds since epoch)
in the expires_at field:
http://developers.strava.com/docs/authentication/#response-parameters-1

Token refresh will yield "Invalid Date" in the expires_at field when
only accepting an ISOString.

This patch checks if expires_at is a number and if so assumes it is a
UNIX timestamp (seconds since epoch).
expireMode: 'expires_at',
});
// Sometimes expires_at is also given as a UNIX timestamp in seconds:
accessTokenResponse.expires_at = Math.round(new Date(accessTokenResponse.expires_at).getTime() / 1000);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be improved if jonathansamines/chance-access-token#2 lands

Copy link
Collaborator

@jonathansamines jonathansamines Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the mentioned change would require breaking the existing public API, will probably take us more time to land. This change looks good as is, will merge it and release a new version and we could update the tests later when the change lands.

@jonathansamines jonathansamines merged commit ed57b5b into lelylan:master Nov 21, 2019
@jonathansamines
Copy link
Collaborator

@ctavan Published as simple-oauth2@3.1.0

Copy link

@fmatheus fmatheus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gitlab OAuth tokens expires_at include milliseconds, demanding a workaround of
token.expires_at = new Date(token.expires_at) // without * 1000
before calling createToken
It could be improved here by checking also the number of digits, or if the result of new Date is a valid date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants