fix(tokens): Fixes the potential of deleting a working refresh_token when no new one is available

[drzraf]

All Submissions:

• Have you followed the plugin Contributing guideline?
• Does your code follow the WordPress' coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

save_refresh_token() disregard the existing and still valid refresh_token and replace it with false (since the response to a renewal does not contain it)

Closes #404

How to test the changes in this Pull Request:

1. Use Google OIDC with refresh token

        add_filter( 'openid-connect-generic-auth-url', function( string $url ) {
            return $url . '&access_type=offline&prompt=consent';
        });

2. Connect
3. See from your logs that, without this PR you're disconnect after 1h (the first call torefresh_token)

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully run tests with your changes locally?

Changelog entry

Do not empty refresh_token if renewed access tokens do not come with new refresh_token.

[drzraf]

ping?

[drzraf]

We have been using this in -production to fix our GSuite 1h-disconnection problems and this has been confirmed to work.

[drzraf]

ping

[timnolte]

@drzraf so actually, in doing some digging on this subject to make sure that things are being handled properly, and to specs. I happened upon an actual reason for why you were having this issue. It has been on the table to quite some time to replace much of the core functionality with a standard Composer package. I happened upon this issue:

jumbojett/OpenID-Connect-PHP#286

Which further took me here:

https://stackoverflow.com/a/65702100

Basically, what is happening is that once you authenticate the first time with Google you won't get a new refresh token un later requests without using prompt=consent or approval_prompt=force and access_type=offline. This is apparently not well documented with Google/GSuite. I'm not inclined to include this PR as I think there is more to the fix that is really required.

[timnolte]

An additional follow-up that I happened to come across this MU Plugin that is essentially an add-on for this plugin that addresses the Google refresh token issue.

https://gitlab.com/animalequality/wp-openid/-/blob/master/wp-openid.php

[drzraf]

https://gitlab.com/animalequality/wp-openid/-/blob/master/wp-openid.php

I wrote this code, and having a filter to customize the request (adding prompt=consent) is nice but it doesn't free this plugin from the responsibility of behaving correctly by not emptying an existing/valid refresh token which is what this PR is about.

[timnolte]

@drzraf from what I see this isn't some clear problem specifically with this plugin as an issue with Google specifically. I didn't close the PR or issue I simply postponed it until I can find some very clear documentation on all of the handling and can test with other IDPs. The very fact the the code submitted has a comment regarding the scenario of what if the token was actually expired and the comment said "dunno" is not a very confident response to the proper way this should be handled.

All of the notes I made were in regards to doing actual research into this issue to ensure that the correct solution is implemented and not just something that appears to be a fox for 1 IDP or use case.

[czarny]

Hi it is not matter only of one IDP. AWS Cognito also does not have refresh tokens rotation and that is fine with spec. See https://datatracker.ietf.org/doc/html/rfc6749#section-6

The authorization server MAY issue a new refresh token, in which case
the client MUST discard the old refresh token and replace it with the
new refresh token.
@drzraf I think you should update your pull request. Especially comments. If token is expired auth server will return error and plugin will end user session.

[drzraf]

Just to remind the actual use-case: My users connect to WP using GSuite. Without such a modification of the refresh-token handling, they get disconnected after one hour what is understandably problematic and frustrating. I didn't read the standards and whether IDP is in charge of disconnecting (IMHO identity providing <> session management/duration).

[timnolte]

Just to remind the actual use-case: My users connect to WP using GSuite. Without such a modification of the refresh-token handling, they get disconnected after one hour what is understandably problematic and frustrating. I didn't read the standards and whether IDP is in charge of disconnecting (IMHO identity providing <> session management/duration).

@drzraf so it's very possible the refresh token issue you are seeing is due to a different bug that has already been fixed but it hasn't been released for this version of the plugin. If you install the version I linked to in the pinned issue about no more updates going to WP.org you might find that your 1 hour timeout issue is fixed.

https://github.com/forumone/openid-connect-wp-dist