Page MenuHomePhabricator

OAuthClient should check for error before validating JWT
Open, Needs TriagePublic

Description

When certain OAuth requests that normally return some JWT data result in an error from the server, OAuthClient blindly assumes they were successful, tries to verify the JWT signature, and fails, masking the real error.

Event Timeline

Also happens in some non-JWT-related situations:

17:02 <+bd808> "Undefined property: stdClass::$key in /data/project/olympics/public_html/vendor/mediawiki/oauthclient/src/Client.php on line 179" -- that doesn't look good
17:04 <+bd808> that looks like the json_decode() of the server response failing
17:04 <+bd808> (crappy error handling there too)

I had this error the other day when attempting to auth to a wiki that was incorrectly set-up. The exposed error was "Notice: Trying to get property 'alg' of non-object" from oauthclient/src/Client.php (line 314), because the result from the server was a pile of HTML error (and the curl call is only checking for an empty result being the indicator of an error).

Change 486706 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/oauthclient-php@master] Improve JSON syntax error handling

https://gerrit.wikimedia.org/r/486706

Change 486706 merged by jenkins-bot:
[mediawiki/oauthclient-php@master] Improve JSON syntax error handling

https://gerrit.wikimedia.org/r/486706

Tgr claimed this task.

Hm, on second thought I think this fixes Sam's issue but not the original issue which is that the server returns a valid JSON object with an error message in it instead of a JWT.