codeswholesale / codeswholesale-sdk-php

A PHP wrapper for CodesWholesale's API
Apache License 2.0
36 stars 21 forks source link

AccessToken does not refresh itself (sandbox) #26

Closed rchoffardet closed 6 years ago

rchoffardet commented 6 years ago

Hey,

I'm still doing some testes from your sandbox. At first, everything seemed to work properly, then, everything broke for no reason. I dug into your code and find that the access token doesn't refresh itself.

The main reason is because you're storing (at least in my env) the whole timestamp instead of the difference. So i guess the column is not properly named in your import.sql file. It should be expires instead of expires_in.

Even if you were storing the difference, the expiration's calculation is not properly done :

elseif (isset($data['expires_in'])) {
    $this->expires = new \DateTime();
    $this->expires->add(new \DateInterval(sprintf('PT%sS', $data['expires_in'])));
}

Datetime class take an optional first argument which is initialized to the current timestamp by default. So with this code, your token can't expire... This code is part of the package sainsburys/plugin-guzzle-oauth2 which is not maintained as wrote here : https://github.com/Sainsburys/guzzle-oauth2-plugin/issues/11#issuecomment-279336635

Anyway, so I changed everything in your code to rename the column. It seemed that it was configurable but it's not because expires_in is hardcoded. Here is the occurences: CodeWholesale\Storage\TokenDatabaseStorage line 43 and 53

Then I got another problem, after refresh, the token seems to be duplicated in database, not updated. The problem is yours on this one :

if(false === $accessToken || $accessToken->isExpired()) {
    $storage->storeAccessToken($middleware->getAccessToken(), $this->clientConfigId);
}

At Class CodeWholesale\CodesWholesaleApi line 57 The code above only insert the new token, but does not delete the old one and thus throws an error.

To fix this, here is a tweak which is not good because it ends up with a bit of code duplication (I didn't want to mess up much with your code) but it works :

if(false === $accessToken) {
    $storage->storeAccessToken($middleware->getAccessToken(), $this->clientConfigId);
} elseif($accessToken->isExpired()) {
    $storage->deleteAccessToken($accessToken, $this->clientConfigId);
    $storage->storeAccessToken($middleware->getAccessToken(), $this->clientConfigId);
}
codeswholesale commented 6 years ago

Hello,

Problem with tokens not refreshing should be already fixed with our newest release.

rchoffardet commented 6 years ago

My library is up to date and the issue is still here