[flutter_appauth] iOS app would crash when authorizationCode was nil

[kennethj]

Wrapping the offending code in a try/catch and sending it up to the dart code.

Regarding #589

[kennethj]

Hey @MaikuB!
Would you mind looking at this PR when you get a chance? I’d really appreciate your feedback. Let me know if there is anything I can do to improve this PR. Thanks!

[MaikuB]

I understand that this at least allows catching the error so app doesn't crash but do you have details on why this issue is occurring to begin with and how to reproduce it? Based on what I know, the part of code you've updated shouldn't be hit if the server had responded properly to indicate the request succeeded. That you needed to put this in suggest to me that the server isn't giving the correct response and possibly non-compliant with OAuth specs. The server may also be returning an error that is being masked as well

[kennethj]

I can reproduce consistently with the OAuth service I am using. I'll work on getting response examples and provide more details today.

[kennethj]

I am not 100% sure what is happening on the OAuth server. The login is successful, but there is an issue with the user to where it does not generate an authorization code. Instead of throwing forbidden or unauthorized, it calls the redirect URI without providing an authorization code in the query string parameters–it only includes the state in the query string.

example redirect app.schema://redirect?state=123abc

I think it is a valid OAuth flow to use the redirect URI to report errors and not provide an authorization code. This type of response puts the plugin into a state that makes the app crash.

[kennethj]

So, I determined that the OAuth server is failing to append an error parameter (e.g., app.schema://redirect? error=unauthorized_client). The plug-in appears to handle the redirect properly if the query string has an error parameter. In this case, since there is no error and no auth code, the plug-in crashes due to an unhandled null (Nil) exception.

I am working to get a fix for the OAuth server I am using implemented.

Regarding this PR, I think sending the exception to be handled by Flutter has benefits since you cannot anticipate that all auth servers will be properly implemented.

[kennethj]

Should I pursue this update further or close it? @MaikuB

I found a fix that can be applied to the OAuth server to prevent the report bug. However, this PR would make the Flutter plugin more resilient and pass the exception to the UI in a more helpful way.

I'll let you make the call.

[MaikuB]

My advice would be to see if relevant code changes could done on the AppAuth iOS SDK itself. Whilst the plugin could be made resilient, it is a wrapper for the Android and iOS SDK. I'd say it would be better caught upstream it would enabling relying off the SDK's existing approaching on detecting a successful/unsuccessful request. Other libraries and apps making use of the AppAuth iOS SDK would then benefit from this. The Android SDK appears to already handle this already (reference) so this helps add towards a case to have it done by the iOS SDK