openid / AppAuth-iOS

iOS and macOS SDK for communicating with OAuth 2.0 and OpenID Connect providers.
https://openid.github.io/AppAuth-iOS
Apache License 2.0
1.75k stars 766 forks source link

Crash when using TVOS device authorization workflow when an error occurs during token retrieval #863

Closed ronnybremer closed 3 months ago

ronnybremer commented 3 months ago

Describe the bug I have been trying to get device authorization going on tvOS 17.2 with Swift 5 in a demo app. If the auth server responds with an error, the demo app crashes:

RESP: Optional("TEOPK6c0")
*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[__NSArrayI objectForKeyedSubscript:]: unrecognized selector sent to instance 0x600000264b40'
*** First throw call stack:
(
    0   CoreFoundation                      0x00007ff80082ebcf __exceptionPreprocess + 242
    1   libobjc.A.dylib                     0x00007ff80005a8d9 objc_exception_throw + 48
    2   CoreFoundation                      0x00007ff800843852 +[NSObject(NSObject) instanceMethodSignatureForSelector:] + 0
    3   CoreFoundation                      0x00007ff8008332f5 ___forwarding___ + 1430
    4   CoreFoundation                      0x00007ff8008354a8 _CF_forwarding_prep_0 + 120
    5   TestApp                             0x000000010ed46f3d __86+[OIDAuthorizationService performTokenRequest:originalAuthorizationResponse:callback:]_block_invoke + 957
    6   CFNetwork                           0x00007ff800e12ebe CFNetwork + 36542
    7   CFNetwork                           0x00007ff800e308b7 _CFHTTPMessageSetResponseProxyURL + 16951
    8   libdispatch.dylib                   0x000000010f84500c _dispatch_call_block_and_release + 12
    9   libdispatch.dylib                   0x000000010f8462f8 _dispatch_client_callout + 8
    10  libdispatch.dylib                   0x000000010f84defc _dispatch_lane_serial_drain + 1085
    11  libdispatch.dylib                   0x000000010f84ec8d _dispatch_lane_invoke + 441
    12  libdispatch.dylib                   0x000000010f85b902 _dispatch_root_queue_drain_deferred_wlh + 318
    13  libdispatch.dylib                   0x000000010f85ad80 _dispatch_workloop_worker_thread + 571
    14  libsystem_pthread.dylib             0x00000001102cfb84 _pthread_wqthread + 327
    15  libsystem_pthread.dylib             0x00000001102ceacf start_wqthread + 15
)
libc++abi: terminating due to uncaught exception of type NSException

The RESP print comes from the initialization closure, which is correctly called and handled, I am getting back all required fields. In this case, this is the user_code. After 5 seconds the library calls the token endpoint, which fails as it has an issue with the code, it returns this error structure with a http status of 400:

{
   "error": "invalid_request",
   "error_description": "unknown device code provided"
}

I would assume this is a correctly formatted error response, but somehow the library expects something different.

To Reproduce Steps to reproduce the behavior:

  1. initiate device authorization request
  2. wait 5 seconds until the token request gets called
  3. observe crash in Xcode

Expected behavior The error response should be handled correctly, even if it is malformed.

Screenshots

Environment

Additional context

ronnybremer commented 3 months ago

Same issue when the device code has not yet been approved, the server responds with 403 and the json error

{
  "error": "authorization_pending",
  "error_description": "pending approval"
}
ronnybremer commented 3 months ago

This line (and similar calls) are causing the crash: https://github.com/openid/AppAuth-iOS/blob/c89ed571ae140f8eb1142735e6e23d7bb8c34cb2/Sources/AppAuthCore/OIDAuthorizationService.m#L480 as it is assumed the Json is a valid Json object response, however, after digging deeper inside of the debugger the Json response from the server seems to be invalid, as it has been deserialized as an array of array, hence the crash.

mdmathias commented 3 months ago

@ronnybremer is it possible to reach out to the IdP owner of the server to ask about their invalid JSON?

ronnybremer commented 3 months ago

@mdmathias I did that and they have scheduled a fix.

mdmathias commented 3 months ago

Closing since this seems like an issue with the IdP sending back invalid JSON.