Misconfigured or malicious servers can easily crash an openid based app

[sonashannon]

Checklist:

• [x ] I am using the latest release
• [x ] I searched for existing GitHub issues
• [x ] I read the documentation
• [ x] I verified the client configuration matches the information in the identity provider (or I am using dynamic client registration)
• [x ] I am either using a custom URI scheme or https with App Links for client redirect.
• I can reproduce the issue in the demo app (optional)

Configuration

• Version: latest
• Integration: java
• Identity provider: custom

Issue Description

The specific error is as follows:

1. Create a misconfigured server that returns a blank refresh_token.
2. Use openID to call the token endpoint
3. App crashes with this trace:

E/AndroidRuntime(17080): java.lang.IllegalArgumentException: refresh token must not be empty if defined
E/AndroidRuntime(17080): at net.openid.appauth.Preconditions.checkArgument(Preconditions.java:116)
E/AndroidRuntime(17080): at net.openid.appauth.Preconditions.checkNotEmpty(Preconditions.java:68)
E/AndroidRuntime(17080): at net.openid.appauth.Preconditions.checkNullOrNotEmpty(Preconditions.java:89)
E/AndroidRuntime(17080): at net.openid.appauth.TokenResponse$Builder.setRefreshToken(TokenResponse.java:305)
E/AndroidRuntime(17080): at net.openid.appauth.TokenResponse$Builder.fromResponseJson(TokenResponse.java:222)
E/AndroidRuntime(17080): at net.openid.appauth.AuthorizationService$TokenRequestTask.onPostExecute(AuthorizationService.java:688)
E/AndroidRuntime(17080): at net.openid.appauth.AuthorizationService$TokenRequestTask.onPostExecute(AuthorizationService.java:579)
E/AndroidRuntime(17080): at android.os.AsyncTask.finish(AsyncTask.java:771)
E/AndroidRuntime(17080): at android.os.AsyncTask.access$900(AsyncTask.java:199)
E/AndroidRuntime(17080): at android.os.AsyncTask$InternalHandler.handleMessage(AsyncTask.java:788)
E/AndroidRuntime(17080): at android.os.Handler.dispatchMessage(Handler.java:106)
E/AndroidRuntime(17080): at android.os.Looper.loopOnce(Looper.java:226)
E/AndroidRuntime(17080): at android.os.Looper.loop(Looper.java:313)
E/AndroidRuntime(17080): at android.app.ActivityThread.main(ActivityThread.java:8751)
E/AndroidRuntime(17080): at java.lang.reflect.Method.invoke(Native Method)
E/AndroidRuntime(17080): at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:571)
E/AndroidRuntime(17080): at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1135)
I/Process (17080): Sending signal. PID: 17080 SIG: 9

The problem appears to be here at around line 688 of AuthorizationService:

            TokenResponse response;
            try {
                response = new TokenResponse.Builder(mRequest).fromResponseJson(json).build();
            } catch (JSONException jsonEx) {
                mCallback.onTokenRequestCompleted(null,
                        AuthorizationException.fromTemplate(
                                GeneralErrors.JSON_DESERIALIZATION_ERROR,
                                jsonEx));
                return;
            }

Note that the catch block is only catching json exceptions. The IllegalArgumentException is never caught.

But this raises a bigger problem with the whole onPostExecute() function. Shouldn't the whole thing be wrapped with a try-catch block? The way it works now is any exception thrown that is not caught by the specialized try-catch blocks will crash the app. It permits malicious servers to bring down apps using openid at will. That should never happen.