House cleaning.

[tfrommen]

Hi there! 👋

I had some time, so I went through the current code base (i.e., the auth-code-flow branch, which this PR thus targets), with an army of house elves, and fixed, updated and improved a few things here and there.

Sorry for not splitting up commits. I did this on purpose, though, because otherwise we would've ended up with 42 (!) commits, ... or so. I did not do any real changes to functionality, though, but see for yourself...

What Does This Pull Request Include?

• General:
  • Replace almost all mentions of OAuth1 with OAuth2.
  • Require WordPress version 4.8 or higher.
  • Bump tested WordPress version to 4.8.
• Code:
  • Remove unused stuff (e.g., variables, namespace imports).
  • Add/Adapt phpDoc blocks for methods with either parameters or return values, or both.
  • Add missing namespace imports.
  • Add a few type hints.
  • Make WP\OAuth2\Client::check_redirect_uri() return true or false.
  • Improve and streamline WP\OAuth2\Endpoints\Authorization::handle_request().
  • Ensure authorization code expiration is numeric, and return an int.
  • Make WP\OAuth2\Tokens\Token::is_valid() abstract, and not static anymore.
  • Introduce abstract WP\OAuth2\Tokens\Token::get_meta_prefix() method.
  • Make PhpStorm (or any other decent IDE) aware of $client in the login template.

What's Left?

• Coding Standards:
  • Every third file looks different than the others.
  • Where to put blank lines (both in code and comments)?
  • Import classes in the global namespace, yes or no?
• admin.php:
  • I did not touch this file at all.
  • It is broken, though.
  • The referenced file lib/class-wp-rest-oauth1-admin.php does not exist in this plugin.
  • oauth1 all over.
• inc/class-scopes.php:
  • I did not touch this file at all.
  • I don't get it. 😅
  • It's not referenced/used at all.
  • Properties are either defined and not used, or not defined, but set. 🤔
• inc/endpoints/class-authorization.php:
  • The handle_request() method uses OAuth2\get_grant_types(), which returns the unvalidated result of a filter, oauth2.grant_types.
  • There is direct, but unchecked access to assumed methods.
  • In my opinion, the OAuth2\get_grant_types() function should both validate the data (and remove invalid handlers) and state WP\OAuth2\Types\Type[] instead of array as return type.
• inc/tokens/class-authorization-code.php:
  • I assume the $args parameter in WP\OAuth2\Tokens\Authorization_Code::validate() is for inheritance...?
• Possibly other stuff...

Again, this PR targets the auth-code-flow branch. If you want me to make it target master instead, just tell me.

Cheers, Thorsten

[rmccue]

Sorry for missing your other questions here!

Every third file looks different than the others.

Yeah: half of this code comes from the original OAuth 1 project and follows that code style, half of it is new and follows the HM coding standards (essentially, modernised WP standards).

Where to put blank lines (both in code and comments)?

Same as WP generally; generally a blank line before a comment line.

Import classes in the global namespace, yes or no?

Yes: all classes that aren't in the current namespace should be imported.

admin.php:

Kept this around for #13, will eventually go 💀

inc/class-scopes.php:

Not used yet, see #10. The code there is just the skeleton that I created in a rush to get my thoughts down.

the OAuth2\get_grant_types() function should both validate the data (and remove invalid handlers) and state WP\OAuth2\Types\Type[] instead of array as return type

Makes sense to me; will merge #14 in a moment. :)

I assume the $args parameter in WP\OAuth2\Tokens\Authorization_Code::validate() is for inheritance...?

Currently unused, but we need to validate a few things to be spec compliant, notably redirect_uri. Haven't written that code yet. :)

[tfrommen]

Sorry, too, because not all of the things in What's Left were real questions, or at least not questions to get answered (just thought about and eventually resolved).

The coding style parts were just to enumerate what I encountered (inconsistent blank lines, inconsistent namespace imports, etc.).

I just updated #14, although you still have to decide about the intended behavior when encountering an invalid type.

Just to have said that: for me, this PR is done (merged anyway, and all questions answered). Thanks. 🙂