Investigate any breaking changes in SDK caused by V3 API

[iamstuartwilson]

Quick read suggests not as lib doesn't care about token storage or scope schemas

[adriangibbons]

I may be wrong, but I think the changes to the Authentication may require some changes?

New parameter in the token exchange, grant_type, needs to be set to either:

• authorization_code
• refresh_token

For initial authentication, must always be "authorization_code". When refreshing an access token, must always be "refresh_token".

Then when refreshing tokens, we need another new parameter in the token exchange, refresh_token (but we don't need code)

In addition to private $accessToken; we probably also need some new attributes in the class:

• private $refreshToken; for refresh_token returned in the response for a token
• private $expiresAt; for expires_at

Maybe some internal logic somewhere to check if there is less than one hour before the access token expires, e.g.

if ($this->expiresAt - time() < 3600) {
    $this->tokenExchangeRefresh();
}

Haven't tried this but something like: https://github.com/iamstuartwilson/strava/blob/master/src/Iamstuartwilson/StravaApi.php#L203

        public function tokenExchange($code)
        {
            $parameters = array(
                'client_id'     => $this->clientId,
                'client_secret' => $this->clientSecret,
                'code'          => $code,
                'grant_type'  => 'authorization_code'
            );
            // WRINKLE: we need to set $this->refresh_token and $this->expires_at using the response returned from the below :'(
            // Maybe instead of returning the CURL output we set $this->accessToken, $this->refresh_token and $this->expires_at?
            return $this->request(
                $this->authUrl . 'token',
                $parameters
            );
        }

        public function tokenExchangeRefresh()
        {
            $parameters = array(
                'client_id'     => $this->clientId,
                'client_secret' => $this->clientSecret,
                'refresh_token'  => $this->refresh_token,
                'grant_type'  => 'refresh_token'
            );
            // WRINKLE: as above
            return $this->request(
                $this->authUrl . 'token',
                $parameters
            );
        }

Further, the setAccessToken() method should perhaps take expiresAt as a parameter. Now that tokens expire we can't just retrieve from a database and set it here before using; unless there's some logic somewhere that checks it hasn't expired and refreshes it.

My head hurts now, have managed to tie myself up in knots, so hope some of this is useful. If I find some time to progress some commits I will be sure to send a PR.

[iamstuartwilson]

@adriangibbons this looks good! If you have a time to test if this approach will work then it LGTM 👍

Thanks a lot for investigating

[adriangibbons]

@iamstuartwilson I have spent an hour or so on this. Haven't tested it at all but changes are fairly simple so fingers crossed :)

https://github.com/adriangibbons/strava/commit/d016a90fb00515de7de2653d403aaa069efcb06a

The above commit broken down:

• Lines 44/45 - added two new private attributes.
• Lines 127-130 - throws Exception if token has expired. For now, whilst in the transitionary period, $refreshToken and $expiresAt are optional because there are still "forever tokens" until 15 October 2019. Rather than refresh the token without intervention, an Exception is thrown, and application logic will be needed to handle this. The thinking behind this is that many applications out there probably currently store the $accessToken and they'll need to know of new tokens, expiry dates, and refresh tokens.
• Line 215: added grant_type per Strava documentation
• Line 231: added tokenExchangeRefresh() method. This should be called by the application when the Exception relating to expired tokens is called, followed by setAccessToken() with the new token, expiry date, and refresh token.
• Lines 270-277: added handling for optional $refreshToken and $expiresAt.

Of course, the documentation will need a refresh on how to use all this. I think the URLs to the Strava documentation need updating throughout as well.

If you're able to give this some thought (and hopefully some testing) then I'll send through the PR when the time comes! 👍

[adriangibbons]

Hi @iamstuartwilson, after some initial testing I have added two further commits over on my fork of your repo 👍 I'll run it for a while on my prod site and if all good then I'll make the README updates and submit a PR. Any further feedback welcome!

[iamstuartwilson]

@adriangibbons LGTM 👍 Will try and find time to test this when I get some time away from work (probably over Christmas holidays) if that's cool with you? Thanks once again for your time on this. I hope you're building great things 🙌

[nathanczachur]

Hey @iamstuartwilson , is this package still supported? If so, do you want some help getting this merged in?

Thanks!

[mjaschen]

Hey Nathan,

can you create a pull request with your changes? I'd like to review it and merge it into master if everything works as expected :-)

Marcus

[iamstuartwilson]

Hey @mjaschen @nathanczachur,

It looks as though @adriangibbons has already worked on a fix for the imminent refresh token requirement for the API. I'm quite short of time (and our of practice with PHP these days 😅) to verify this fix myself, but it's here if you care to look.

Thanks for reviving this issue! :)

[mjaschen]

Hi together,

I'll take a closer look on Monday and keep you all updated!

Marcus

[adriangibbons]

Just pushed a small fix to my fork

[adriangibbons]

To use, needs code to handle the token refresh e.g.:

/*
 * Database fields
 * access_token - varchar(255) utf8_unicode_ci
 * refresh_token - varchar(255) utf8_unicode_ci
 * expires_at - int(10)
 */

try {
  $db_connection = new PDO('mysql:host='. DB_HOST .';dbname='. DB_NAME . ';charset=utf8', DB_USER, DB_PASS);
  $stmt = $db_connection->prepare('SELECT `access_token`,`refresh_token`,`expires_at` FROM `athletes` WHERE `athlete_id`=:athlete_id');
  $stmt->bindValue(':athlete_id', 1234567890, PDO::PARAM_INT);
  $stmt->execute();
  $db_result = $stmt->fetchObject();

  $strava_api = new Iamstuartwilson\StravaApi(0000, 'your_api_key');
  $strava_api->setAccessToken($db_result->access_token, $db_result->refresh_token, $db_result->expires_at);
} catch (Exception $e) {
  if ($e->getMessage() === 'Strava access token needs to be refreshed') {
    $results = $strava_api->tokenExchangeRefresh();
    $strava_api->setAccessToken($results->access_token, $results->refresh_token, $results->expires_at);
  }
}

[mjaschen]

Hi @adriangibbons

I did take a look on your fix now. It works fine, thanks for your work.

There's only one little thing I'd like to see improved:

In the method request() there's a new condition:

if (strpos($url, 'token') === false && isset($this->expiresAt) && ($this->expiresAt - time() < 3600)) {
    throw new \Exception('Strava access token needs to be refreshed');
}

Here we check for the existence of an URL segment (token). I'd like to see this comparison to be stricter (i.e. comparing the full URL path instead of just the string token). I'll change this.

The README will be updated with some examples on how to use the updated class.

As we're backwards compatible, a new minor version will be tagged after that (1.4.0).

Marcus

[mjaschen]

Hi @iamstuartwilson

I've integrated all changes regarding the new oAuth2 flow, added an example script which demonstrates the authentication and updated the README.

Pull-request #36 is pending, maybe you find 5 minutes to take a look. If everything is fine for you I'd merge + tag a new release.