Crystal: oauth2 errors are making some incorrect json assumptions for Facebook

Created on 21 Feb 2019  路  1Comment  路  Source: crystal-lang/crystal

Hello thanks for all your hard work on crystal!

I use the multi_auth shard which itself use oauth2 from the crystal stdlib.

the OAuth2::Error class is trying to decode the error from the payload expecting something like this:

json { "error": "some oauth error" }

This is what I understand according to the code in src/oauth2/error.cr:

 pull.read_object do |key|
      case key
      when "error"             then error = pull.read_string
      when "error_description" then error_description = pull.read_string
      else
        raise "Unknown key in oauth2 error json: #{key}"
      end
    end

I suppose that must be the case with some oauth2 providers but this assumption is not valid for Facebook, which returns in case of error something like this:

{"error":{"message":"This authorization code has expired.","type":"OAuthException","code":100,"fbtrace_id":"GyzZxJWzHMP"}}

Because of error being an object instead of the string it makes this json reader crash with this error:

Unhandled exception: Expected string but was begin_object at 1:11 (JSON::ParseException)
  from /usr/share/crystal/src/json/pull_parser.cr:518:5 in 'parse_exception'
  from /usr/share/crystal/src/json/pull_parser.cr:510:5 in 'expect_kind'
  from /usr/share/crystal/src/json/pull_parser.cr:180:5 in 'read_string'
  from /usr/share/crystal/src/json/pull_parser.cr:83:5 in 'read_object_key'
  from /usr/share/crystal/src/oauth2/error.cr:89:13 in 'new'
  from /usr/share/crystal/src/json/from_json.cr:13:3 in 'from_json'
  from /usr/share/crystal/src/oauth2/client.cr:154:7 in 'get_access_token_using_authorization_code'
[... more stack ]

I would have gladly fixed it myself but I'm a beginner crystal developer and I'm clearly not good enough yet to fix this issue.

bug topicnetworking

Most helpful comment

Yes, this is stupid Facebook that doesn't respect standards:

error
         REQUIRED.  A single ASCII [USASCII] error code

I guess we'll have to change this.

We can probably make the class hold the JSON source as a string to be parsed, if needed, by clients. We can probably make the entire JSON string the error. I'll send a PR to fix this later today.

>All comments

Yes, this is stupid Facebook that doesn't respect standards:

error
         REQUIRED.  A single ASCII [USASCII] error code

I guess we'll have to change this.

We can probably make the class hold the JSON source as a string to be parsed, if needed, by clients. We can probably make the entire JSON string the error. I'll send a PR to fix this later today.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

nabeelomer picture nabeelomer  路  3Comments

ArthurZ picture ArthurZ  路  3Comments

relonger picture relonger  路  3Comments

oprypin picture oprypin  路  3Comments

will picture will  路  3Comments