Avoid null pointer exception because of error description

[poovamraj]

Changes

To avoid causing an exception in case error description is null, we will pass unknown error description

References

https://github.com/auth0/Auth0.Android/issues/662

Testing

• [ ] This change adds unit test coverage
• [ ] This change adds integration test coverage
• [x] This change has been tested on the latest version of the platform/language or why not

[Fraid]

@poovamraj , thank this PR will prevent any other futur exception, as quick solution should be fine.

But for my case, it's not an unknown error. User need to verify the email, app should inform the user.

Knowing that I don't have access to auth0 admin, I only did the integration. If this error "error_description=Please verify your email before logging in.e=foo+t2@gmail.com" come directly from auth0 API, I think this need to be handled ? But If it's a custom message from auth0 admin, maybe prevent any admin to use custom "="?

For me an additional fix is required, as an example I was thinking about something like: Edit: Direct permalink

if(values.lenght==3){
//It's an example! merging description content
values.put(value[0],String.format("%s %s",value[1],value[2]);
}
Result: "error_description":"Please verify your email before logging in.e foo+t2@gmail.com"

But I don't know your codebase, so carefully understand the use of asMap(), I don't know why there is filtering with "values.lenght=2", must be a reason for that.

I have old screenshot when I did the debugging: breakpoint And value of "values"

[poovamraj]

Hi @Fraid, that makes sense. I am wondering the callback URL returned should be URL encoded. Are you saying it is not encoded? Then it wouldn't be possible for any parser to seperate this value without such delimiters

[Fraid]

Hi @poovamraj, I'm not sure to understand your question, but I created the following test case (Don't know if it's compiling, but I think you'll get the idea).

Add following method here:

@Test
    public void shouldParseQueryValuesWithEmail() {

//This url is returned as it is
        String uriString = "testandroid://auth.testandroid.com/android/com.testandroid.app/callback?error=unauthorized&error_description=Please%20verify%20your%20email%20before%20logging%20in.e%3Dfoo%2Bt2%40gmail.com&state=abscefg-QWERTYUIOPasdfgHJKLMNBVCXZdd";
        Uri uri = Uri.parse(uriString);
        final Map<String, String> values = CallbackHelper.getValuesFromUri(uri);

        assertThat(values, is(notNullValue()));
        assertThat(values, aMapWithSize(3));
        assertThat(values, hasEntry("error", "unauthorized"));
        assertThat(values, hasEntry("state", "abscefg-QWERTYUIOPasdfgHJKLMNBVCXZdd"));

//This will normally failed.
        assertThat(values, hasEntry("error_description", "Please verify your email before logging in.e=foo+t2@gmail.com"));
    }

With this temporary change for asMap you should pass the test (temporary because I don't know every message that this method is handling)

private static Map<String, String> asMap(@Nullable String valueString) {
        if (valueString == null) {
            return new HashMap<>();
        }
        final String[] entries = valueString.length() > 0 ? valueString.split("&") : new String[]{};
        Map<String, String> values = new HashMap<>(entries.length);
        for (String entry : entries) {
    final String[] value = entry.split("=");
    if (value.length == 2) {
        values.put(value[0], value[1]);
    } else if (value.length == 3) {
        values.put(value[0], String.format("%s=%s", value[1], value[2]));
    }
}
        return values;
    }

Hope this help.

[poovamraj]

@Fraid how is that you are getting both the state which is retured when login is succesful and error/error_description which is returned when login fails.

Are you using custom actions to achieve this work flow?

If so, you can login and then use the email and isEmailVerified returned from the UserProfile from user in Credentials to check for this.

It is interesting because I have never seen Auth0 return both error and state in the same callback URL.

[Fraid]

@poovamraj I have no clue, what you have in the test case can be reproduce by following this procedure :

1- Sign up with a new account without validating the email (ex. foo+t3@gmail.com). 2- Close app then try to login with the unvalidated email again

• On IOS you will received a failed message like "Please verify your email before logging in."
• But on Android in Auth0 2.9.3, app will crash. And with this PR app will sent "An unexpected error occurred." that's better but not right because it's not an unexpected message android user will have to contact us every time while IOS one will check their email.

[Fraid]

@poovamraj I think we may not talking about the same thing, this is how I log. User will be prompt and at the return, app will crash. So I don't even have any callback nor any token:

 Map<String, Object> params = new HashMap<>();
        params.put("prompt", "login");
        WebAuthProvider.login(auth0)
                .withParameters(params)
                .withScheme(activity.getString(R.string.com_auth0_scheme))
                .withScope("openid email")
                .start(activity,.....)

[poovamraj]

@Fraid yes I agree, the login wouldn't work in this case. The current way to parse values from callback URL is flaky. I am trying to find a way where we can dynamically get all the values from the callback URL instead of parsing using hardcoded numbers. With the suggested code, the same error will happen if we have 3 "=". So using "=" as delimiter is the real reason for the issue here

[poovamraj]

@Fraid the current change should make it work

[fpitters]

Hi, on this matter, we just noticed that using public constructor(message: String, cause: Auth0Exception? = null) : super(message, cause) and calling getDescription raises an NPE.

Let me know if a new issue is worth it :)

[poovamraj]

@fpitters description is a nullable property inside AuthenticationException. is the crash happening on the SDK or on your code?

[fpitters]

Ops, it's actually my bad.

The NPE is raised only on unit tests and while using returnDefaultValues Then AuthenticationException.getDescription which uses android TextUtils.isEmpty is trying to access description when actually null. Using a static mock solves the issue.

Sorry for the noise :(

Maybe it would be good to move to Kotlin String.isEmpty :)