kamermans / guzzle-oauth2-subscriber

OAuth 2.0 Client for Guzzle 4, 5, 6 and 7 with PHP 5.4 - PHP 8.0 - no more dependency hell!
MIT License
140 stars 31 forks source link

Token deleted before the new one is confirmed #36

Closed randebnw closed 1 year ago

randebnw commented 3 years ago

https://github.com/kamermans/guzzle-oauth2-subscriber/blob/2276afd2b34917e2f75912ac129fc8b2ebfc9b82/src/OAuth2Handler.php#L210

Hi there!

Checking the order things happen, I noticed that the persistent token is deleted before the new one is confirmed. By doing it, if something goes wrong when the new token is requested (like a connection timeout or something), you won't be able to try to get the new token anymore, unless you authenticate in the browser again.

Imagine this scenario:

  1. Token is expired
  2. You delete the current token (line 210)
  3. You call $this->requestNewAccessToken(); (line 213)
  4. Something goes wrong in step 3
  5. You don't have the persistent token anymore, so you don't have the refresh token and you are stuck

I'm not sure if deleting the old token is really necessary for other types of token persistence, but talking about FileTokenPersistence, I don't think it is, since the new token will override the old one.

kamermans commented 1 year ago

Hmm, good point. I think it would be useful to keep the old token around until after confirmation of the new token on other providers as well, since it may contain other useful information, like the refresh token.