provides way to receive last token

[bshaffer]

see #901

[bshaffer]

@tmatsuo PTAL

[jonnywilliamson]

@bshaffer

So I took a look at this last night, it works kinda great!

I can easily get access to the access_token and expiry time now if the API is forced to refresh the access_token because the one I supplied has expired etc.

However, I'm left wondering how best to use this. As far as I can see there's no event system (or any observation pattern) in the API, so I'm struggling to come up for the best way to know when to call the getLastReceivedToken method so I can save/update the new access_token

When I make more than one request, the getLastReceivedToken method always seems to return NULL. I'm not sure why...

Here's some pseudo code of what I mean:

Imagine my saved access_token in the database is now a day old...

    $client = new Google_Client(['myconfigvalues']);
    $client->setAccessToken('OldaccessTokenRetrievedFromDatabase');

    $cal = new Google_Service_Calendar($client);

    //This will show the expired token that was taken from the database
    var_dump($client->getAccessToken());


    //We make a request and the API magically asks and receives a NEW access_token. Brilliant.
    $request1 =  $cal->calendarList->listCalendarList();
    var_dump($client->getLastReceivedToken()); //New access_token

   //So far so good. I could technically save this data right now. But say I wanted to do a number of
   //requests....checking after EVERY request is very tedious.

    //Let's fire off another request:
    $request2 = $cal->calendarList->get('myemailaddress@gmail.com');
    var_dump($client->getLastReceivedToken());  //Null returned

    //MMMm. Now getLastReceivedToken() is showing NULL

I'm sure this is something to do with the cache, but I'm not sure.

As it stands this is a great start. However, my work flow has me making multiple requests to the google_service rather than just one (eg, get a list of users' calendars, then insert an event if they have one called "work" etc.). Unless I've coded something badly, it appears that the getLastReceivedToken method only works after the 1st request.

What I was hoping to do was override the __destruct method on the Google_Client and when it was being destroyed I was going to check for any new tokens and save them to the database for that user for the next time.

Have I picked this up wrong?

[jonnywilliamson]

So tracing through the code, I think this is where the issue

So it all works well for the 1st request, but if there is a second request, in Google_Client the authorize method gets called and this code is used:

On line 353, $this->getAccessToken is called again, but because the Google_client has never been updated with the new access_token that it managed to get in the last request, it still retrieves the old stale one that was set when Google_Client was created.

That means that the lastCredentials gets reset to just the refresh token...wiping away the previous new access_token.

Now in the AuthTokenMiddleware:

The cache gets set, and uses the last new access_token to make the second request which will work perfectly.

However at no stage is this passed back to the Google_Client.

I have no idea what to do to fix that, I'm struggling getting my head around the architecture here, but I hope that helps to explain what the issue is.

Thank you!

[net-tools]

Hi,

I had similar issue as yours (saving and refreshing the token). Why do you bother with detecting a refreshed token, whereas you could just ask the library whether the token is expired or not ? If so, you commit it to your database (for future use). If not, you continue to use it.

Here is a piece of code I'm using :

// read token from the API or from your database, if it has been saved
$t = $client->getAccessToken();
print_r($t); // should output several fields (including expires_in and refresh_token)

// if token is expired
if ( $client->isAccessTokenExpired() )
{
   // fetch refreshed token, but this one is short-lived (one hour validity) ; you CAN'T save it to your 
   // database for future use !
   $short_lived_token = $client->fetchAccessTokenWithRefreshToken($t['refresh_token']);

   // for future calls in the API and the script lifetime, set the refresh token just obtained
   $client->setAccessToken($short_lived_token);

   $t['access_token'] = $short_lived_token['access_token'];
   $t['created'] = $short_lived_token['created'];
   $t['expires_in'] = $short_lived_token['expires_in'];

   // do some stuff to save the token in your database, for future use
}

Hope this helps a bit !

[net-tools]

Are you sure you are using the last RC ? As your issue reminds me of mine described here (and addressed by a pull request)

See here : #849

[jonnywilliamson]

HI @AM63 Thanks for replying.

I'm definitely using RC6. From my composer.lock file:

I'm currently using something very similar to your code at the moment, testing the access_token when I set it with the Users saved one.

If it's expired/stale I get a new one and save it, then set that one in the Google_Client.

However all that work is done already in the API! It checks if it's expired. It checks if its invalid. It fetches a new one using the refresh token. It uses the new one for any requests (via the cache it seems).

Just trying to see why we need to write that boilerplate code to do all that when all I need is to grab the tokens that were used by the library when it's shutting down.

There's one line in your code that surprises me:

// fetch refreshed token, but this one is short-lived (one hour validity) ; you CAN'T save it to your
// database for future use !

Why do you say that?

I have plenty of requests that will be done within an hour of the last one, and so using the token again is fine. Just curious.

Thanks though!

[net-tools]

I say that because the token obtained by fetchAccessTokenWithRefreshToken does not contain any refresh_token field ; the expires_in field is set to expire the token 1 hour later. If you save the fetchAccessTokenWithRefreshToken in the database, you are loosing the refresh_token part !

This is what I learnt some weeks ago, I don't think it has changed.

[jonnywilliamson]

@AM63 Ah ok.

I'm working with Laravel, luckily it's smart enough to know that if I send an update command to the database with an array that only has an access_token and expires_in field, it will update only those fields and not remove the refresh_token etc for that user.

But as a warning for others, that's fair enough :)

[jonnywilliamson]

I'm sorry for high-jacking this PR conversation, but things just keep jumping out at me.

So @AM63 you said earlier on:

Why do you bother with detecting a refreshed token, whereas you could just ask the library whether the token is expired or not ?

And you followed it up with this code:

$t = $client->getAccessToken();
// if token is expired
if ( $client->isAccessTokenExpired() ){
[...]
}

This is actually pretty worthless IMHO.

When we have a look at the isAccessTokenExpired() method, you find that it's nothing more than a time checker.

The code looks like this (comments removed):

  public function isAccessTokenExpired()
  {
    if (!$this->token) {
      return true;
    }

    $created = 0;
    if (isset($this->token['created'])) {
      $created = $this->token['created'];
    } elseif (isset($this->token['id_token'])) {
      $idToken = $this->token['id_token'];
      if (substr_count($idToken, '.') == 2) {
        $parts = explode('.', $idToken);
        $payload = json_decode(base64_decode($parts[1]), true);
        if ($payload && isset($payload['iat'])) {
          $created = $payload['iat'];
        }
      }
    }
    // If the token is set to expire in the next 30 seconds.
    $expired = ($created
            + ($this->token['expires_in'] - 30)) < time();
    return $expired;
  }

As you can see, it literally only checks to see if the time left on the token is more than 30 secs otherwise it just says the token has expired.

This does no checking to see if the token has been revoked, if it's actually a valid token (ie, the string of characters hasn't be tampered with) etc.

If you really wanted to check is the token valid, you need to check with Google. Something like this:

    protected function isAccessTokenInvalid($token)
    {
        $http = $this->client->authorize();

        $result = json_decode(
            $http->get('https://www.googleapis.com/oauth2/v1/tokeninfo?access_token='.$token)->getBody()
        );

        if (isset($result->error)) {
            //Bad Token - invalid, corrupt etc.
            return true;
        }

        return false;
    }

Basically it seems the work flow should be simple:

1. Create Google_Client and add a token. (It might be old and invalid, it might be new and valid).
2. Make requests. Api detects what needs to happen if token is old. Gets new token and uses it. (Currently the API does this).
3. When finished all requests, return the tokens used. They can be saved if needed for the next time.

Wouldn't that bypass all the checks and bolierplate code we have to do at the moment?

[tmatsuo]

Guys thanks for the discussion.

@bshaffer Does it make sense to create the UserRefreshCredentials only when

• the original access token is expired
• isset($token['refresh_token'])
• the credential is not UserRefreshCredentials
  ?

[bshaffer]

@tmatsuo yes, but that is the current behavior. I'm not quite sure what you mean by the last point, though.

[tmatsuo]

Now the if clause is

      if ($this->isAccessTokenExpired() && isset($token['refresh_token'])) {
        $credentials = $this->createUserRefreshCredentials(
            $scopes,
            $token['refresh_token']
        );
      }

It causes the $credentials will be cleared when you call the 2nd API call.

So I'm thinking something like:

      if ($this->isAccessTokenExpired() && isset($token['refresh_token'])
          && ! $credentials instanceof UserRefreshCredentials) {
        $credentials = $this->createUserRefreshCredentials(
            $scopes,
            $token['refresh_token']
        );
      }

so that if you call the API again, the $credentials will keep holding the same UserRefreshCredentials. I think this will solve the issue described in #905 (comment)

[bshaffer]

@tmatsuo the code never allows for $credentials to be set at that point. The only other place it's set is at line e47.

[tmatsuo]

@bshaffer Oh I got it. Somehow I thought $credentials is an instance variable.

Well, then how about to have an instance variable that holds the UserRefreshCredentials and do the same check?

    /**
     * @var UserRefreshCredentials $userRefreshCredentials
     */
    private $userRefreshCredentials;
    ...
    ...
    if ($this->isAccessTokenExpired() && isset($token['refresh_token'])) {
        if (!isset($this->userRefreshCredentials)) {
            $this->userRefreshCredentials = $this->createUserRefreshCredentials(
                $scopes,
                $token['refresh_token']
            );
        }
        $credentials = $this->userRefreshCredentials;
    }

[tmatsuo]

Oh, we already has $this->lastCredentials, then we can check

    if ($this->isAccessTokenExpired() && isset($token['refresh_token'])) {
        if (! $this->lastCredentials instanceof UserRefreshCredentials) {
            $credentials = $this->createUserRefreshCredentials(
                $scopes,
                $token['refresh_token']
            );
        } else {
          $credentials = $this->lastCredentials;
        }
    }

What do you think?

[bshaffer]

Here are two options which may provide a better solution to this problem:

1. The Google_Client class could provide a callback function to AuthHandlerFactory, which would be called when a brand new access token is fetched. We would need to add support to set a callback function in the AuthTokenSubscriber and AuthTokenMiddleware classes.
2. A CacheInterface class called CallbackCache could be added. The callback cache essentially takes a callback function and an optional CacheInterface class as constructor arguments, and calls both of them when a new value is set. The auth library would not require modification for this.

[tmatsuo]

Sure, if we can set the brand new access token to the client object, that's better.

[tmatsuo]

I'm fine with both of them

[jonnywilliamson]

I would love to be able to say which of those two options seems better, but I'm not sure I'm up to speed with the entire API code base to give a constructive opinion.

I am happy to give way to your better judgement.

Thank you for reading though my long rants above :)

[bshaffer]

Does anyone on this thread think implementing PSR-6 (#943), and in doing so supporting stash, is sufficient to solve this use-case? I can add something like setTokenCallback as a catch-all for any usecase (similar to #925), but this seems hackish and I am wondering if exposing this using a cache interface isn't the better way to handle it.

All thoughts are welcome!

[jonnywilliamson]

Again apologies, but the question is above my skill level to answer!