Some OAuth2 providers don't issue a refresh token with the original token (most significantly, Google OAuth2 API doesn't).
That means when a token is expired and this module tries to refresh the token using the refresh_token, the module crashes because refresh_token is blank.
I've got a fix for this in my deployment, I will format a patch and submit it.

Comments

Ashley.Sommer created an issue. See original summary.

Ashley.Sommer’s picture

Ashley.Sommer’s picture

Status: Active » Needs review
m4olivei’s picture

+++ b/src/Service/Oauth2ClientService.php
@@ -102,13 +102,19 @@ class Oauth2ClientService extends Oauth2ClientServiceBase {
-        $access_token = $this->grantServices['refresh_token']->getAccessToken($clientId);

I think the issue here is that \Drupal\oauth2_client\Service\Grant\Oauth2ClientGrantServiceInterface::getAccessToken doesn't have a return value and it should.

I'm using the client_credentials grant with my plugin, and it has a similar issue when I do the following, asking for the access token for the first time:

$ drush php
>>>  \Drupal::service('oauth2_client.service')->getAccessToken('dcs_canonical_video_url')->getToken();
PHP Error:  Call to a member function getToken() on null in Psy Shell code on line 1

The \Drupal\oauth2_client\Service\Oauth2ClientService::getAccessToken method is returning null, which I don't think is the intent.

I'll try an alternate patch to illustrate.

m4olivei’s picture

Status: Needs review » Needs work
m4olivei’s picture

Assigned: Unassigned » m4olivei
m4olivei’s picture

How about this?

m4olivei’s picture

Assigned: m4olivei » Unassigned
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 7: enforce_return_get_access_token-3040828-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Ashley.Sommer’s picture

@m4olivei
Your patch looks good, but I don't see how it is related to the original issue.
This issue was created to deal with the problem that some oauth providers do not issue a refresh token, so when the stored token expires, the client_credentials grant provider tries to use the refresh_token to get a new access token, even if the refresh_token is empty or missing, this causes php errors to be generated and oauth2_client module no longer works.

Ashley.Sommer’s picture

@m4olivei
I see, looks like you intended to comment against this issue: https://www.drupal.org/project/oauth2_client/issues/3041788

Ashley.Sommer’s picture

This is a blocking bug in the application I am working on.
If there are no more issues to be raised, this patch will be merged in 48 hours.

Ashley.Sommer’s picture

Status: Needs work » Needs review
m4olivei’s picture

Ahh sorry about that. Not sure what I did. Perhaps your right and my patch was better suited to the other issue. In any case, I decided to go another route with what I was trying to do and use thephpleague/oauth2-client directly.

pbosmans’s picture

Patch #2 almost worked for me.
The first time after the expiration of the token, i got an empty access token.

This is because the getaccesstoken method of the grantservice doesn't return a token. It only sets the token.
So i added one line at the end of your patch to get the real access-token.

    if (empty($access_token)) {
      $this->grantServices['client_credentials']->getAccessToken($clientId);
      $access_token = $this->retrieveAccessToken($clientId);
    }
Ashley.Sommer’s picture

@pbobsmans

It looks like you, too, are trying for a solution to the other issue: https://www.drupal.org/project/oauth2_client/issues/3041788

This issue (3040828) is about fixing the bug relating to empty `refresh_token`, not empty `access_token`.

pbosmans’s picture

Oops, i was confused about the used method (getClientCredentialsAccessToken) and the explanation of this module at https://www.drupal.org/docs/8/modules/oauth2-client/oauth2-client-8x-2x.
Especially the last sentence : 'If a token has not been generated, the authentication process will be initiated.'
That doesn't worked when the token was expired. I will reference this on the issue you mentioned and switch to it.

  • Ashley.Sommer committed 16b8153 on 8.x-2.x
    Issue #3040828 by Ashley.Sommer: Don't try to use a refresh_token when...
Ashley.Sommer’s picture

Status: Needs review » Fixed

Merged patch #1 0001-Fall-back-to-getting-a-new-access-token-if-the-store.patch

Ashley.Sommer’s picture

Status: Fixed » Closed (fixed)