Restore AccessToken

[EugeneVdovenko]

I am store the data from AccessToken in $_SESSION.

When I want restoring AccessToken from $_SESSION

$accessToken = new AccessToken($_SESSION['accessToken']);

I can have wrong expires-field, and method hasExpires() returns «true» always.

For example, if user use access_token after expired time, constructor writes to expires-filed

time()+«time_from_SESSION»

I mean it's wrong

[shadowhand]

After looking at the code, I can make some sense of this. Basically, the issue is when you construct AccessToken from stored values, instead of completely fresh values, it will use the existing expiration time as a second value instead of as a timestamp.

I'm not really sure what to do about this. Pushing the normalization of the expires (in) outside probably makes the most sense.

[ramsey]

Our AccessToken class uses a Unix timestamp for expires, so it should always evaluate correctly. Is the problem that end-users are storing "expires_in" in the database/session and restoring from that causes problems?

[shadowhand]

@ramsey the problem is when the token expires in the past it is assumed to be "the same as" expires_in. We could probably do something sane like:

// October 2012, the initial release of OAuth 2.0
if ($expires < 1349067600) { ... }

... rather than the blind "in the past" check we are doing now. I doubt anyone is issuing tokens that expire in 3+ years.

[rtheunissen]

Hah this is such an interesting case. I can't think of a nicer way than what @shadowhand has suggested. Unless we can somehow specify on a per-provider basis whether its token should use "expires in" or "expires at".

Or as an option when constructing the token? Specifies as either TOKEN_EXPIRES_IN, TOKEN_EXPIRES_AT or TOKEN_EXPIRES_AUTO.

[stevenmaguire]

This is an interesting case indeed. When I was writing test cases to bring the package to 100% coverage, it felt odd the way the Token handles the expiration. I'd like the package to remain simple with limited complexity so I like the solution proposed by @shadowhand. I do also like the package to be receptive to consumer intent and the "modes" @rtheunissen proposed would offer a bit of that; the package could still carry a default preference.

[TrafeX]

What can we do to solve this? I'm also running into this issue which basically means you can't construct an AccessToken with an expired expiration date.
The only workaround it I could think of was using the expires_in and subtracting the time() value of what we have in our datastore.

[shadowhand]

@TrafeX @ramsey PR created.

[shadowhand]

@TrafeX This will go out in v1.0.2.

[ramsey]

And v1.0.2 is now out!

[TrafeX]

@shadowhand @ramsey That was fast, thanks guys!