OAuth: Fix Refresh Token Scheduled Time

[n7studios]

Summary

#839 incorrectly added the expires_in response parameter value to the token's created_at time, instead of the current timestamp (time()), resulting in the WordPress cron event to refresh the token being scheduled in the past:

Tests did not detect this as tests mock making a call to refresh an 'expired' access token, setting the API's created_at response parameter as the current date and time, instead of when the access token was created.

This PR resolves by:

• scheduling the WordPress cron event to next refresh the token on expiry of the current access token, relative to the current date and time,
• updated tests to reflect API functionality when calling the refresh_token method

Testing

Existing tests pass.

Checklist

• I have written a test and included it in this PR
• I have run all tests and they pass
• The code passes when running the PHP CodeSniffer
• Code meets WordPress Coding Standards for PHP, HTML, CSS and JS
• Security and Sanitization requirements have been followed
• I have assigned a reviewer or two to review this PR (if you're not sure who to assign, we can do this step for you)

[github-actions]

WordPress Playground

🚀 Your PR has been built and is ready for testing in WordPress Playground!

Click here to test your changes in WordPress Playground

[github-actions]

WordPress Playground

🚀 Your PR has been built and is ready for testing in WordPress Playground!

Click here to test your changes in WordPress Playground

[noelherrick]

I'm not completely understanding this: why would we schedule a cron job vs. immediately refreshing if the expiration is already passed? If created_at + expired_in is not the actual expiration time, what is?

[n7studios]

I'm not completely understanding this: why would we schedule a cron job vs. immediately refreshing if the expiration is already passed?

There's an elevated number of 401 unauthorized requests to the API. Whilst the WordPress Libraries check if an API response is a 401 (and if so, refreshes the token), I'm being asked for a more proactive approach here, hence scheduling a cron event to automatically perform this step when we know the token expires, rather than waiting for the next API call to then refresh the token.

If created_at + expired_in is not the actual expiration time, what is?

expires_in is the number of seconds before the access token expires, not a timestamp of when the access token expires, and the API has never returned a fixed value. Adding it to created_at results in the exact issue reported in this PR. Adding it to time() results in the correct calculation of the token's expiry. But if I'm missing something obvious, let me know.

[noelherrick]

@n7studios I understand that the expiration could be in the past, but why wouldn't we just immediately refresh the token vs. schedule a job when that's true?

[n7studios]

@n7studios I understand that the expiration could be in the past, but why wouldn't we just immediately refresh the token vs. schedule a job when that's true?

The job is scheduled to refresh the token on its expiry, so the sites always have a valid access token.

Whilst the WordPress Libraries will check if the API response is a 401 (and if so, refresh the token), lower traffic sites won't call the API regularly. Relying on this check alone might be a cause of the elevated 401 issues that I'm being asked to provide a more proactive approach towards.

If this PR doesn't cut it, not an issue - we can go ahead and close.

[noelherrick]

Tim, I totally get what you're trying to do (schedule a cron job to renew creds on low traffic sites) and the bug we're fixing (scheduled dates for those jobs are in the past) but I get the sense there is something we're missing. Are you saying that when you hit the V4 endpoint, the created_at + expired_in can be in the past? Is this actually a time zone bug?

[n7studios]

Are you saying that when you hit the V4 endpoint, the created_at + expired_in can be in the past? Is this actually a time zone bug?

No.

created_at is when the access token was created.
expires_in is when the access token will expire, relative to when the API request was made to fetch the access token.

Example:

1. I request an access token on Monday at 00:00, as a creator using the Plugin for the first time ever.
   created_at will be Monday at 00:00. Correct
   expires_in is in 48 hours time, meaning the token expires Wednesday 00:00. Correct
   created_at + expires_in = Wednesday, 00:00. Correct. The scheduled cron job would run on the token's expiry, ensuring a new access token is issued.

2. I remove the Plugin from the site - perhaps I want to try a different product.

3. On Tuesday at 00:00, I install the Plugin again, once again going through the OAuth flow.
   created_at will still be Monday at 00:00, as I'll be issued the existing access token given it is valid. Correct.
   expires_in is now 24 hours time, meaning the token expires Wednesday 00:00. Correct.
   created_at + expires_in = Tuesday, 00:00. Incorrect. The scheduled job would run before the token's expiry, and the same access token is issued.

Using time() (the current timestamp) + expires_in honors that expires_in is relative to when the API request was made to fetch the access token (i.e. time()), ensuring the cron job is scheduled on the token's expiry, not before.

[noelherrick]

@n7studios Thanks for your patience - the reinstall scenario was what I was missing (and the fact that expires_in changes based on the time of the request)