RefreshTokenMiddleware could be made more reliable

[LeoniePhiline]

Hi Nascom team,

thank you for open-sourcing this client!

I am using the latest v2 git 391159556bdb3018572cbe32f1121b0e3688857f .

The situation: Since Teamleader has a short access token lifetime, it might expire in the middle of a sync process.

The current implementation: \Nascom\TeamleaderApiClient\Http\Guzzle\Middleware\RefreshTokenMiddleware::__invoke() attempts to deal with such situations by checking the expiry date of the locally stored access token before sending the request. If at that time the token has expired - by the low precision DateTime available - a new access token is requested by means of the provided refresh token.

The problem: However, despite NTP-synced clock, I ran multiple times into HTTP 401 responses, like here:

   Http\Client\Exception\HttpException  : Client error: `GET https://api.teamleader.eu/contacts.info` resulted in a `401 Unauthorized` response:
{"errors":[{"title":"Access token expired","status":401}]}                                                                                                                                   

  at /path/to/vendor/php-http/guzzle6-adapter/src/Promise.php:128
    124| 
    125|         if ($exception instanceof GuzzleExceptions\RequestException) {
    126|             // Make sure we have a response for the HttpException
    127|             if ($exception->hasResponse()) {
  > 128|                 return new HttplugException\HttpException(
    129|                     $exception->getMessage(),
    130|                     $exception->getRequest(),
    131|                     $exception->getResponse(),
    132|                     $exception

  Exception trace:

  1   Http\Adapter\Guzzle6\Promise::handleException(Object(GuzzleHttp\Exception\ClientException), Object(GuzzleHttp\Psr7\Request))
     /path/to/vendor/php-http/guzzle6-adapter/src/Promise.php:64

  2   GuzzleHttp\Exception\ClientException::("Client error: `GET https://api.teamleader.eu/contacts.info` resulted in a `401 Unauthorized` response:
{"errors":[{"title":"Access token expired","status":401}]}                                                                                                                                   
")                                                                                                                                                                                           
      /path/to/vendor/guzzlehttp/guzzle/src/Exception/RequestException.php:113

  Please use the argument -v to see more details.

(The stack traces do not show it, but I was using the Nascom Teamleader Api Client stack for these requests.)

Even re-starting the sync manually in the command line (inspecting the error and re-starting the command takes a few seconds) was not enough to pass enough time to cross the database-stored key expiry DateTime. Maybe the Teamleader API server's clock is off - I don't know.

In any case, this can easily happen when requesting e.g. contact.info responses for the entire contacts.list in quick succession, since the current implementation would require the two clocks (server and client side) to be synchronized perfectly and for the retrieval and storage of the access code to be instant!

Possible workaround: A workaround I will implement for now will be to manually adjust the access token expiration time by a negative delta before storing it. However, I still wanted to report, so the implementation can be improved.

My suggestions for an improved implementation:

• By default, refresh not when the expiry time as passed, but already minutes before that.
• Catch HTTP 401 Unauthorized responses. Due to the clear error title ({"errors":[{"title":"Access token expired","status":401}]}) this case could easily be distinguished from other cases of Unauthorized responses (e.g. when not an expired but an entirely invalid access token was provided).

I am curious what you think about these suggestions.

Kind regards!

[mark-gerarts]

Hi @LeoniePhiline, thanks for the detailed writeup! We've had a similar problem for a different OAuth API, which we resolved by fetching a refresh token a few minutes before expiry. So your first suggestion is likely to resolve it for this client as well.

We'll look into implementing this, we'll keep you posted.

[mark-gerarts]

As suggested, we now consider the token expired a few minutes (5) before it actually is. The new release tag is v2.0.0-RC3.

If you still encounter any problems, feel free to let us know!