Add OAuth authentication

[iamdharmesh]

Description of the Change

The PR replaces the existing API key-based authentication flow with OAuth authentication to improve the overall connection flow experience. Here are the details of the tasks covered in this PR.

• Utilize the mailchimpapp.com service to generate an access token
• Encrypt the access token before we store it
• Switch all API requests to use the access token instead of an API key
• Handle potential error states where an access token expires or is revoked
• Backward compatibility for sites that are currently using an API key (We don’t have a UI for viewing or modifying an existing key; the user has to log out to set a new key. Therefore, we can completely replace the API key form with the connect button.)

Note

• For the logout process, no API is called to revoke the access token or disconnect the integration on the Mailchimp side. I checked the Mailchimp for WooCommerce plugin and didn’t find any API call for this. I also reviewed the API documentation but didn’t find any API for revoking tokens. Since Mailchimp uses non-expiring tokens, it is crucial to revoke the token upon logout.
• The PR uses the woocommerce.mailchimpapp.com OAuth middleware server to connect with Mailchimp. We will need to update it to the WordPress server once it is set up.

Closes #9

How to test the Change

New setup:

1. Go to the Mailchimp settings page; the "Connect Account" button should be visible there.
2. Click on the "Connect Account" button. This will open a pop-up with the Mailchimp login form.
3. Complete the process in the pop-up window. The settings page should reload with the logged-in UI and settings options.
4. Verify that the list and form fields data are loaded correctly based on the selected field.
5. For browsers with pop-ups blocked, a modal with information will be shown, where the user can try again after allowing pop-ups.
6. Verify that closing the pop-up without logging in or during intermediate steps is handled correctly.
7. Regression testing: Perform overall regression testing to ensure all functionality works properly with the new OAuth authentication.
   • Admin settings page options
   • Adding/removing form fields from Mailchimp
   • Subscription form widget and block on the front end (rendering and submission as per settings)
8. Click the "Logout" button.
9. Verify that the settings page starts displaying the login page to connect the Mailchimp account.

Existing Users

1. Set up the plugin with the trunk branch and configure it using API key authentication.
2. Check out this PR branch and ensure that everything continues to work as it did (overall regression testing).
3. Click "Logout."
4. Connect your account by clicking the Connect Account button and ensure everything works as expected.

Changelog Entry

Added - OAuth authentication for connecting a Mailchimp account with WordPress.

Credits

Props @jeffpaul @dkotter @iamdharmesh

Checklist:

• I agree to follow this project's Code of Conduct.
• I have updated the documentation accordingly.
• I have added tests to cover my change.
• All new and existing tests pass.

[dkotter]

@iamdharmesh We'll also want to ensure the readmes get updated with information on how encryption works and the need to set the custom defines, with a warning that if this isn't done, decryption may fail (if the LOGGED_IN_SALT changes) and the authentication flow will need to be gone through again

[dkotter]

@iamdharmesh Code looks good here and tests well, nice work!

I do have feedback on a few things that I think is worth addressing:

1. In simulating an access token failure (I just directly modified the token in the database so it was invalid) the forms on the front-end still show, though the forms won't work. My opinion here is we should hide those forms if we don't have valid credentials. Note this will need to work for a shortcode form, widget form and block form.

2. In testing what happens if decryption fails (I modified the salt we use to encrypt things) I noticed the block form and widget form don't show (there is a PHP warning attempt to read property datacenter on bool that I'm wondering is what's causing those not to render) but the shortcode form does show. Ideally as above, we don't show any of those forms if we don't have valid credentials

3. This is more an edge case due to how I was testing but if I modified the access token, I get an admin message letting me know I need to re-authenticate. If I modify the access token to be valid again, that message still shows. Again, this is an edge case but seems like once we have a valid API call made, we should clear out that error message

4. The admin notice we show overlaps weird in our settings page. I'm assuming some minor markup or CSS changes would fix this and ensure the notice shows above our header:

5. Also wondering if we should change that notice from a warning to an error, since things won't work until the connection is reestablished.

[dkotter]

^ The above are things I think we for sure should look at addressing. Here's a few things I'm raising for discussion purposes (cc / @jeffpaul @iamdharmesh):

1. For existing users, we allow them to continue to use their API key with no problems, which is the correct approach. There is no prompt though to update to the new connection method. Wondering if we want to show an admin notice letting them know there's a new authentication method they should update to? Not sure how hard we want to push that, especially initially. I think the ideal would be to get everyone using OAuth but maybe this is something we add in a follow up release?

2. For our encryption, we look for two custom constants first (MAILCHIMP_SF_ENCRYPTION_KEY and MAILCHIMP_SF_ENCRYPTION_SALT). This is documented in the readme but the reality here is most sites won't set these. We then try using some constants that are typically set across all WordPress sites (LOGGED_IN_KEY and LOGGED_IN_SALT). If these aren't set, we then fallback to a hardcoded string.

   Our experience on a couple other projects has shown us that some security plugins will periodically rotate those core WordPress salts. This causes the decryption to no longer work for sites that aren't using the custom constants and will require the site to go through the authentication process again.

   There's a couple options to consider here. We can leave this as-is, noting it is the most secure implementation and we have documentation on how to avoid this issue. Or we could remove the use of those core WordPress salts, meaning we would only rely on the custom constants or fallback to the hardcoded strings. This would prevent situations where decryption no longer works (so a better user experience) but is less secure for those that don't set those constants (which will probably be most sites)

[jeffpaul]

For existing users, we allow them to continue to use their API key with no problems, which is the correct approach. There is no prompt though to update to the new connection method. Wondering if we want to show an admin notice letting them know there's a new authentication method they should update to? Not sure how hard we want to push that, especially initially. I think the ideal would be to get everyone using OAuth but maybe this is something we add in a follow up release?

I agree that some sort of admin notice, sigh adding another one to folks already likely clogged up admin notices, to direct folks who are using the API Key to disconnect and reconnect via OAuth would be ideal and within this release.

There's a couple options to consider here. We can leave this as-is, noting it is the most secure implementation and we have documentation on how to avoid this issue. Or we could remove the use of those core WordPress salts, meaning we would only rely on the custom constants or fallback to the hardcoded strings. This would prevent situations where decryption no longer works (so a better user experience) but is less secure for those that don't set those constants (which will probably be most sites)

I think its fine to leave as-is. Ensuring we've got a good FAQ item in the readme as well as details for the Woo.com product documentation on the topic will be helpful to direct folks to that run into the core salts being rotated and then, finally, properly adding the MC constants instead.

[iamdharmesh]

Thanks for the detailed feedback @dkotter. I have address those in 57c0058.

I agree that some sort of admin notice, sigh adding another one to folks already likely clogged up admin notices, to direct folks who are using the API Key to disconnect and reconnect via OAuth would be ideal and within this release.

Thanks for confirming @jeffpaul. Could you please help with adjust the wording of the notice on how hard we want to push the users to switch to OAuth. eg: is deprecated vs is going to deprecated in future.

(Heads up! It looks like you're using an API key to connect with Mailchimp, which is now deprecated. Please log out and reconnect your Mailchimp account using the new OAuth authentication by clicking the "Connect Account" button.)

For encryption (#2), @dkotter I am more inclined towards keeping it as it is.

Thank you.

[jeffpaul]

@iamdharmesh I'm fine with that existing notice noting deprecated.

[iamdharmesh]

@iamdharmesh I'm fine with that existing notice noting deprecated.

@jeffpaul This is added in 4175774

[dkotter]

For the logout process, no API is called to revoke the access token or disconnect the integration on the Mailchimp side. I checked the Mailchimp for WooCommerce plugin and didn’t find any API call for this. I also reviewed the API documentation but didn’t find any API for revoking tokens. Since Mailchimp uses non-expiring tokens, it is crucial to revoke the token upon logout.

@dkotter can you coordinate with @justin0440 / Vextras (in case we need to proxy through their wordpress.mailchimpapp.com service) to confirm what API should be used on logout to disconnect the respective WordPress site from the connected Mailchimp account?

Sounds like there isn't a public facing API to do this, so the approach we're taking here is the best we can do for now