auth0 / Auth0.Android

Android toolkit for Auth0 API
https://auth0.com
MIT License
216 stars 133 forks source link

Is there missing proguard rules ? #676

Closed bennycao closed 10 months ago

bennycao commented 1 year ago

Checklist

Description

Hi, we are seeing in our error logs exception being thrown

java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.annotation.Annotation java.lang.reflect.Field.getAnnotation(java.lang.Class)' on a null object reference at com.auth0.android.request.internal.JsonRequiredTypeAdapterFactory$1.read(JsonRequiredTypeAdapterFactory.java:35) at com.google.gson.TypeAdapter$1.read(TypeAdapter.java:199) at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$1.read(ReflectiveTypeAdapterFactory.java:131) at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:222) at com.auth0.android.request.internal.JsonRequiredTypeAdapterFactory$1.read(JsonRequiredTypeAdapterFactory.java:31) at com.google.gson.TypeAdapter$1.read(TypeAdapter.java:199) at com.google.gson.Gson.fromJson(Gson.java:963) at com.google.gson.Gson.fromJson(Gson.java:928) at com.google.gson.Gson.fromJson(Gson.java:877) at com.google.gson.Gson.fromJson(Gson.java:848) at com.auth0.android.authentication.storage.SecureCredentialsManager.continueGetCredentials$lambda-3(SecureCredentialsManager.kt:492) at com.auth0.android.authentication.storage.SecureCredentialsManager.$r8$lambda$6WE7o-y4gxPLV-YF4cOhR7EawOA(SecureCredentialsManager.kt:1) at com.auth0.android.authentication.storage.SecureCredentialsManager$$InternalSyntheticLambda$1$509647de71d71de33da01830616eb550f5518df260cce6beb4062f02e8cc048d$0.run(SecureCredentialsManager:13) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641) at java.lang.Thread.run(Thread.java:923)

It seems to have increased from version 2.9.3 and for some reason only seeing issues for android 10,11 and 12.

Is there some proguard rules missing ?

Reproduction

cannot seem to reproduce locally

Additional context

No response

Auth0.Android version

2.9.3

Android version(s)

10,11,12

poovamraj commented 1 year ago

@bennycao The issue is not reproducing and this happens OptionalCredentials. This is different from how we deserialize Credentials.

Are you able to reproduce this consistently? Do you have 1 fat APK or do you have seperate ones based on architecture and is there any pattern on this.

Any details for us to reproduce this would be great.

bennycao commented 1 year ago

@bennycao The issue is not reproducing and this happens OptionalCredentials. This is different from how we deserialize Credentials.

Are you able to reproduce this consistently? Do you have 1 fat APK or do you have seperate ones based on architecture and is there any pattern on this.

Any details for us to reproduce this would be great.

Hi @poovamraj , we haven't been able to locally reproduce the error. We have the 1 apk. Could a first step be to guard against the NullPointerException here https://github.com/auth0/Auth0.Android/blob/main/auth0/src/main/java/com/auth0/android/request/internal/JsonRequiredTypeAdapterFactory.java#L35 ?

poovamraj commented 1 year ago

Patching for an issue without understanding the underlying cause could backfire in the future. I am not sure how to proceed further since we already have proguard rules in place to avoid issues. Could this issue be on the application end?

poovamraj commented 1 year ago

@bennycao anything on this?

bennycao commented 12 months ago

Hi @poovamraj , we still are unable to reproduce this locally but it still does happen in production. The concern here is that it throws a unhandled exception.

If it is on the application end, is there something that we need to add to proguard or the like ? I wouldn't of thought we needed to.

The interesting part is it only shows up on our production logs for OS versions 10, 11 and 12. Is there something there ?

zsoltboldizsar commented 12 months ago

Hi @bennycao. First of all thank you for opening this issue. I've recently updated to 2.10.2 and I'm wondering if your issue appears on all versions (2.9.3+) or 2.9.3 specifically? Thank you for your time.

bennycao commented 12 months ago

Hi @bennycao. First of all thank you for opening this issue. I've recently updated to 2.10.2 and I'm wondering if your issue appears on all versions (2.9.3+) or 2.9.3 specifically? Thank you for your time.

Hi @zsoltboldizsar , we are already on 2.10.1 and it still happens on this version. Also its been happening before version 2.9.3, can't pinpoint a particular version as our logs only retain so much but definitely well before 2.9.3

zsoltboldizsar commented 12 months ago

Hi @zsoltboldizsar , we are already on 2.10.1 and it still happens on this version. Also its been happening before version 2.9.3, can't pinpoint a particular version as our logs only retain so much but definitely well before 2.9.3

Thanks for your answer @bennycao, interesting situation for sure 🤔.

Worth noting, that in a 9 month time-frame we never experienced this issue using v2.9.0 in an app with a small user base of 500+ users. Now that I updated to 2.10.2 I was worried whether it would happen to us as well. Will keep a close eye on it and share if the issue shows up.

poovamraj commented 10 months ago

Hi @bennycao considering this has been open for quite sometime and we are not able to close this issue and no one else has also reported about this. We will close this issue now. But feel free to comment here if you have more details and we can reopen this issue. Thanks for understanding.

mattt-seek commented 10 months ago

Based on the stacktrace there appears to be a call to the reflection API without a null check, here (but it's not obvious why we'd have a null there): https://github.com/auth0/Auth0.Android/blob/c17b87b4f67ddec78e5e6be9276c6f84a7bcd402/auth0/src/main/java/com/auth0/android/request/internal/JsonRequiredTypeAdapterFactory.java#L35

bennycao commented 10 months ago

@poovamraj we still don't have a reproduceable scenario for this issue, but exception is increasing and is not good that it produces a unhandled exception.

As the exception is within a new Thread, we cannot catch the exception as a consumer.
Therefore, can you add some sort of default exception handling whether by e.g.

        Thread.setDefaultUncaughtExceptionHandler { _, ex ->
            callback.onFailure(
                CredentialsManagerException("Handled uncaught exception", ex)
            )
        }

before this line https://github.com/auth0/Auth0.Android/blob/main/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt#L515

or

wrapping the line of concern here https://github.com/auth0/Auth0.Android/blob/main/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt#L550 with

            val bridgeCredentials: OptionalCredentials

            try {
                bridgeCredentials = gson.fromJson(json, OptionalCredentials::class.java)
            } catch (ex: NullPointerException) {
                callback.onFailure(CredentialsManagerException("Invalid credentials from storage."))
                decryptCallback = null
                return@execute
            }

This will enable us to handle the exception gracefully

poovamraj commented 10 months ago

@bennycao Would be great if you can send us a PR with the 2nd solution as we are bit tied now and this issue is absolutely not reproducing for anyone..

The error message has to be more fine tuned though but we can discuss it in the PR.

bennycao commented 10 months ago

Thanks @poovamraj i've created a PR https://github.com/auth0/Auth0.Android/pull/701