Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the missing functions required for the WPCOM OAuth flow. #2145

Conversation

jorgemd24
Copy link
Contributor

@jorgemd24 jorgemd24 commented Nov 6, 2023

Changes proposed in this Pull Request:

Closes part of #2146

Currently, in order to authorize a WPCOM App from a Jetpack site, the Jetpack plugin is required because the jetpack-connection lacks some essential functions necessary for a successful OAuth Flow. The Jetpack team is contemplating the inclusion of these functions in the jetpack-connection package. However, until that happens, we rely on these functions to enable the connection of any WPCOM App.

This PR copies the missing functions from Jetpack to this extension, allowing the complete OAuth flow to be executed without installing the Jetpack plugin. This PR is based on an experimental branch that I created. However, that branch was primarily intended to demonstrate the feasibility of authorizing a WPCOM app by simply copying functions and did not validate the Jetpack signature.

In essence, this PR adds all the necessary functions so that in the event that jetpack-connection doesn't incorporate these functions, we can use our version.

See more context here: pcTzPl-1SM-p2#comment-2672

Screenshots:

Detailed test instructions:

  1. If you currently have Jetpack installed, remove the plugin to verify that OAuth functions correctly without it.
  2. If your site is not connected, you can establish a connection with WPCOM. To do so, please visit the following link: wp-admin/admin.php?page=connection-test-admin-page
  3. If you don't already have a WPCOM App, please refer to this link (https://developer.wordpress.com/apps/) to create one.
  4. Proceed to authorize the WPCOM App that you've just created. For example, you can use a URL like this: https://public-api.wordpress.com/oauth2/authorize?client_id=YOUR_CLIENT_ID&redirect_uri=YOUR_REDIRECT_URI&response_type=code&blog=YOUR_BLOG_ID&scope=posts
  5. Once you click the "approve" button, you will be redirected to your site, where you will need to log in. After that, you will be redirected once more to the redirect URI of your WPCOM app. In the URL, you will find the code that can be used to generate the token.
  6. If you obtain the code, it indicates that the flow is functioning as intended. If you wish, you can also get a token by following the guidelines provided here: https://developer.wordpress.com/docs/oauth2/.

Additional details:

Changelog entry

Add - Missing functions for the WPCOM OAuth flow

@jorgemd24 jorgemd24 self-assigned this Nov 6, 2023
@github-actions github-actions bot added changelog: add A new feature, function, or functionality was added. type: enhancement The issue is a request for an enhancement. labels Nov 6, 2023
@jorgemd24 jorgemd24 marked this pull request as ready for review November 6, 2023 17:41
@jorgemd24 jorgemd24 requested a review from a team November 6, 2023 17:41
@puntope
Copy link
Contributor

puntope commented Nov 6, 2023

Hi @jorgemd24 Thanks for making this progress with the project.

I created my APP but in order to test this PR I have some questions.

Proceed to authorize the WPCOM App that you've just created. For example, you can use a URL like this: https://public-api.wordpress.com/oauth2/authorize?client_id=YOUR_CLIENT_ID&redirect_uri=YOUR_REDIRECT_URI&response_type=code&blog=YOUR_BLOG_ID&scope=posts

  1. Where can get the next variables?

YOUR_CLIENT_ID
YOUR_REDIRECT_URI
YOUR_BLOG_ID

  1. When creating the WPCOM App in this link (https://developer.wordpress.com/apps/) do I need to fill in all the fields? Can it be done with a local domain? I would appreciate some indication in order to be sure I'm creating the APP the right way.

/**
* Handles the login action for Authorizing the JSON API
*/
public function login_form_json_api_authorization() {
Copy link
Contributor

@puntope puntope Nov 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading the PR and P2 I found we are copying the functions from https://github.com/Automattic/jetpack/blob/trunk/projects/plugins/jetpack/class.jetpack.php

I think it would be useful to add that reference somewhere. I.e adding "Copied Code" or "Duplicated code" on each duplicated function in the class as well as in the main class PHPDOC. And also adding the link to the original jetpack class.

Otherwise, after this PR is approved, it might be difficult for other developers working on the code to know where this functions come from.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep that's right I added a link to the Jetpack functions. See here: 1f7ca4c

Comment on lines 76 to 81
/**
* If someone logs in to approve API access, store the Access Code in usermeta.
*
* @param string $user_login Unused.
* @param WP_User $user User logged in.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be duplicated.

@puntope
Copy link
Contributor

puntope commented Nov 6, 2023

Hi @jorgemd24 The required functions from https://raw.githubusercontent.com/Automattic/jetpack/trunk/projects/plugins/jetpack/class.jetpack.php are copied correctly, and the translation domains replaced. So that part is ✅

Please help me with the smoke testing. See #2145 (comment) So I can be sure I'm fully testing this PR

@jorgemd24
Copy link
Contributor Author

@puntope thanks for your time reviewing this PR!

Where can get the next variables?
YOUR_CLIENT_ID
YOUR_REDIRECT_URI
YOUR_BLOG_ID

When creating the WPCOM App in this link (https://developer.wordpress.com/apps/) do I need to fill in all the fields? Can it be done with a local domain? I would appreciate some indication in order to be sure I'm creating the APP the right way.

Once the WPCOM App is created you can get those variables from here: https://developer.wordpress.com/apps/

image

Your blog/site id can be found in /wp-admin/admin.php?page=connection-test-admin-page

image

When creating the WPCOM App in this link (https://developer.wordpress.com/apps/) do I need to fill in all the fields? Can it be done with a local domain? I would appreciate some indication in order to be sure I'm creating the APP the right way.

Yes, you need to fill in all details and the redirect URL can be http://localhost.

Copy link
Contributor

@puntope puntope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jorgemd24

Thanks a lot for the deeper explanation in regard the creation of WPCOM APP

I tested it and I was able to get the code in the URL after login in.

Also I tested with wrong params (i.e wrong URL) and I got the error as expected.

so LGTM ✅

@puntope
Copy link
Contributor

puntope commented Nov 8, 2023

Yes, you need to fill in all details and the redirect URL can be http://localhost/.

Notice this is not accurate, URL needs to be accessible or the next error appears:

Screenshot 2023-11-07 at 20 30 57

@jorgemd24 jorgemd24 merged commit 021017b into feature/google-api-project Nov 8, 2023
10 checks passed
@jorgemd24 jorgemd24 deleted the add/missing-functions-jetpack-for-wpcom-app branch November 8, 2023 11:33
@eason9487 eason9487 mentioned this pull request Mar 26, 2024
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: add A new feature, function, or functionality was added. type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants