NPE in AuthenticationActivity about auth0.android HOT 14 CLOSED

@billgockeler @MattPhilpot There is an error case which was not being contemplated, when the browser tab is kept open after a successful authentication and the user goes back to the browser, refreshes the page and tries to log in again. This time it fails because there was no expected authentication. I've added the test case and prepared a patch for this. I'll try to make a release asap.

from auth0.android.

Thanks @lbalmaceda for the quick turn around. We will include this fix in our next release and will let you know!

from auth0.android.

@vanshg I've released 2.8.1. Should be available in the next hours.

from auth0.android.

Adding a null check there is nonsense as the method will do nothing.. :/ The activity is meant to be launched automatically by the WebAuthProvider class. Unless you're doing something in behind, this NPE should not be thrown. Can you please share any steps to reproduce it?

from auth0.android.

You are correct, the NPE should not be thrown, and yet it is - by your own code, not ours. As you said, the AuthenticationActivity is launched by OAuth internally, not by us. (actually, it's your OAuthManager that's starting the AuthenticationActivity, not WebAuthProvider as you stated).

We have not yet figured out the steps to reproduce the crash. There must be some condition in restoring the activity state that's leading to the NPE.

Regardless of how it's happening, your library should handle the error - not crash. The fact that your launchAuthenticationIntent() will do "nothing" if the bundle is null is another problem. It's called lack of error handling. It would be preferable if you could properly handle the null bundle and recover gracefully, otherwise a simple NP check would at least stop it from crashing.

BTW, the crash is happening across all devices and Android versions 4-7

Lastly, telling customers that their efforts to identify and fix bugs in your software is 'nonsense' is pretty unprofessional.

from auth0.android.

Ok let's review how this is supposed to work, step by step:

1. Using the WebAuthProvider you configure and start the authentication. This calls the internal class OAuthManager that will launch the AuthenticationActivity by passing among other things, the authorize Url. This value is generated by us and it's never null.
2. Because the activity was never launched before, the intentLaunched flag takes the false value and the authentication with the browser is requested. This is the step where I say that adding a null check will do nothing, as we need that authorize Url value to begin and it's supposed to be present at that time. Always. The intentLaunched flag turns true before opening the browser.
3. If the OS requests the activity state to be persisted, the flag value is stored for a later check when coming back like in step (2) or (4).
4. Once the callback URL is hit, the RedirectActivity activity catches the intent and launches the AuthenticationActivity that, because it's already running, will receive a call in the OnNewIntent method. The intent is set to the activity and the next method to be called is either OnCreate if the activity was recreated, where the flag value is read from the saved state or OnResume where the data is obtained from the intent and returned up to the WebAuthProvider.

The state as I see, is correctly managed as the only valuable info is the intentLaunched flag and that's already being handled 👌 . Adding null checks everywhere in the code probably won't hurt but you need to analyze how a value may end up being null before that and see if that's a valid scenario or not.

Finally, note that all the activities are public and anyone could have launched the activity either via an app that read extracts packages and activities from app's manifests or via adb. If this is the case you can ignore the report. In any other case that you find this continues to happen, please provide as much details as possible so we can solve it. A sample project showing how to reproduce it helps a lot. 👍

Thanks

from auth0.android.

We've been trying to identify and reproduce this error so that we can provide you with more info and/or a sample app to repro - no luck so far. During that process we've been able to isolate it down to your 1.9.0 release. This crash was not present before that release. Perhaps that might provide a clue as to what changes you made in that release that would cause it.

from auth0.android.

I'd also like to comment on this. We've been experiencing this same issue since we started using auth0 within our app. We love the features of the service and library, but this is particularly frustrating as it has lead to several bad reviews.

Unfortunately, this specific crash is the #1 crash for our app(s), causing more crashes than everything else combined.

As far as statistics, I can say that we see this error most often on, from most to least:
API 23 (33%)
API 24 (30.8%)
API 25 (13.2%)
It doesn't happen on Oreo much, but that's probably just because there aren't many devices on it. It happens prior to 23 as well, but not nearly as much as 23 itself. Again, this could just be confirmation bias due to most devices today having 23 installed, but it's worth making these listed above the primary target of investigation

It has occurred for us both in 1.9.0 and your latest release. We haven't tried rolling back to a previous version yet.

The most common devices are (by a long shot, these two accounting for >50%):
Samsung Galaxy S7 Edge running API 24 (70%), the rest API 25 (30%)
Asus ZenFone 2 (ZE551ML) (Z00A) running API 24 (71%), the rest API 25 (29%)

It also tends to happen a lot of x86 devices, no matter the platform (but API 24 is most common)

As a stopgap to prevent this crash, we are considering modifying the source code to just finish the activity and go back to the previous screen with a toast or broadcasting a message reporting the failure, therefore allowing us to handle the error in a safer manner than crashing.

We'd really prefer not to make this change on our side, but hopefully this information helps in your investigation of the issue.

from auth0.android.

Please try out version 1.12.1 and leave some feedback.

from auth0.android.

Hey @lbalmaceda, just wanted to let you know that the fix is looking good so far. Haven't had any crash reports.

Thanks again for the quick turnaround.

from auth0.android.

@MattPhilpot thanks for the feedback!

from auth0.android.

We're getting this crash as well, but we're using Lock. Is it safe to manually include the new auth0 dependency and override the auth0 library version Lock is using (1.11.0)?

from auth0.android.

@vanshg there shouldn't be any breaking changes since 1.11.0 at least, but I'll try to make a release of lock.android tomorrow. 👍

from auth0.android.

Great, thank you!

from auth0.android.