WP-API / OAuth2

Connect applications to your WordPress site without ever giving away your password.
GNU General Public License v2.0
172 stars 42 forks source link

Validate grant types #14

Closed tfrommen closed 7 years ago

tfrommen commented 7 years ago

Hi again,

@rmccue didn't comment on my What's Left list in #8, but since this was no big deal, I just created this pull request with a proposed validation for grant type( handler)s.

The result of the grant type filter, oauth2.grant_types, will be validated, and only WP\OAuth2\Types\Type implementations make it to the consumer.

Comments?

Cheers, Thorsten

rmccue commented 7 years ago

Whoops, let me go back and answer :)

tfrommen commented 7 years ago

Could we instead just specify function ( Type $type ) and have PHP validate for us? 🤔

No, not really. PHP would not validate (and discard invalid) types, like the code that I suggested, but rather expect correct implementations, and fatal error if encountered anything else. If this is the desired behavior, then sure, we could add a type hint. But the array_filter() would then get pretty awkward to look at, in my opinion. We do not want to do anything to valid types, so the code could look something like this:

return array_filter( $types, function ( Type $type )  {
    return true;
} );

So, in fact, we wouldn't really need the callback('s body), but just PHP that kicks in for checking (and fatal erroring because of) the type.

To be honest, I would rather keep my code (or something along the lines). But like I said, the bahavior would be discard invalid types. If you want PHP to jump the user into their face, we can do that. 😀

Does this make sense? 🤔

rmccue commented 7 years ago

I'd rather not silently drop objects; it makes debugging a pain for developers. If not the fatal error, we should at least fire a _doing_it_wrong when we drop it.

tfrommen commented 7 years ago

Makes sense. Should I adapt the closure accordingly?

rmccue commented 7 years ago

That'd be great, thanks :)

tfrommen commented 7 years ago

I just updated the validation code, again.

If only we could use PHP 5.6+... :) I decided to use a regular foreach loop now, because then I'm able to include the type in the message. If this is not required, I can refactor this to use array_filter(). Another option would be to use array_walk() in combination with another variable, or array_filter() - which both might not be faster than the foreach.

Comments?