Issue with 'expires' logic in AccessToken entity

[jakelehner]

I am using a custom provider I created to connect to our Oauth2 server. I believe I've identified an issue with the handling of the 'expires' parameter in the AccessToken entity constructor.

It first sets 'expires' by adding the current time to the 'expires_in' response from the server, as it should.

The issue is if the auth server sent back an 'expires' value, the $options['expires'] variable gets set to the current time PLUS the expires time from the auth server. The comment says something about this being facebook's fault.

This causes an issue when the auth server returns an 'expires' time that is an expiration timestamp. The Leauge Oauth2 server (ironically enough) returns both expires_in (seconds until expiration) AND expires (timestamp of expiration). So this issue is causing the oauth client to

I can do a fix and pull request but wanted to get an opinion on how to approach the solution first.

In my opinion, if the issue is with facebook, this should be handled in the facebook provider by adjusting the return variables before passing them on to be hydrated by the entity. The entity should then defer to expires as a true timestamp, or default to expires_in + the current time.

[philsturgeon]

Doh.

Yes, I would say that Facebook-specific logic should ideally be moved to the Facebook provider. If other providers do similar, then maybe move that stuff to their providers too.

[jakelehner]

Ok I'll be submitting a pull request shortly. I'm not sure if you'll like this approach or not.

I was thinking I would be able to add a step to the facebook provider between making the token request and hydrating the token entity. Looking closer, this was not possible. The getAccessToken() method in the abstract provider calls handleResponse() immediately after receiving the response, so there's no opportunity for the specific provider class to do anything in the middle.

This can be addressed but is a larger effort that would need some additional thought.

I'll submit this pull request and see what you think.

[jakelehner]

Merged in PR 113