This plugin provides an OAuth authentication provider to authenticate users using WordPress.com Connect.
This diff corresponds to the github pull request: https://github.com/facebook/libphutil/pull/36
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Commits
- rPHU90aaeb01bb92: Add WordPress.com OAuth2 plugin
Configured Wordpress as an auth provider, saw it show up on the login screen, registered a new account, got proper defaults for my username/name/email/profile picture.
Diff Detail
Diff Detail
- Repository
- rPHU libphutil
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
A few minor inlines. This generally looks great to me, everything inline is pretty minor.
(On your test plan, have you not run this code at all?)
What's your use case here in broad terms? For example: is this purely speculative ("someone might want to do this"), or do you or your company actually want to do this, or do you work for/with Automattic and want this for your own purposes and/or because WordPress users have expressed interest in it?
| src/auth/PhutilAuthAdapterOAuthWordPress.php | ||
|---|---|---|
| 13–15 | Would making this configurable potentially be useful? Does only wordpress.com offer OAuth, or can another install of WordPress also serve as an OAuth server? For example, if I run a WordPress consulting company and we have our master install at wp.mycompany.com, is it plausible that I might want to authenticate against that install? Or is this impossible / unrealistically obscure? | |
| 55–56 | (weird indent) | |
| src/future/wordpress/PhutilWordPressFuture.php | ||
| 4–6 | You can drop this @group stuff, we've moved away from it for documentation generation but haven't cleaned everything up yet. | |
| 76–77 | You can simplify this slightly with phutil_json_decode(). | |
| 78–79 | We haven't updated all the code yet, but we're generally making exception messages translatable for consistency in newer code. You can do this with: throw new Exception(
pht('Expected JSON response from WordPress.com, got: %s', $body));And likewise below. | |
Comment Actions
Sorry, I misunderstood that I need to provide some test cases here which I didn't have.
We (at Automattic) are evaluating Phabricator, and have hacked this WordPress.com authentication provider to be able to authenticate users against WordPress.com, and it works fine.
| src/auth/PhutilAuthAdapterOAuthWordPress.php | ||
|---|---|---|
| 13–15 | This plugin is specific to WordPress.com service and uses the API which is not present in WordPress software, so it can't be used to authenticate against a WordPress.org install. | |
Comment Actions
Sorry, I misunderstood that I need to provide some test cases here which I didn't have.
"Test Plan" is just an informal description of "what I did to make sure the code works". You can read some more details in the documentation here: https://secure.phabricator.com/book/phabricator/article/differential_test_plans/
In this case it would be something like: "Configured Wordpress as an auth provider, saw it show up on the login screen, registered a new account, got proper defaults for my username/name/email/profile picture.".
This update is looks correct, but you've only included the most recent commit in the review. Do something like:
arc diff master
...to include all the changes since master. Then I can accept and pull this. Thanks!
Comment Actions
- Add WordPress.com OAuth2 plugin
- Fix indentation issues
- Implement suggestions in https://secure.phabricator.com/D9004