From 9b213f3f650f535f8111bbdb34d9a2fb9ada76e3 Mon Sep 17 00:00:00 2001 From: Benny Cao Date: Thu, 7 Dec 2023 08:59:54 +1100 Subject: [PATCH 1/7] catch nullpointerexception when getting credentials during deserialization of json --- .../storage/SecureCredentialsManager.kt | 28 +++++++++++---- .../storage/SecureCredentialsManagerTest.kt | 35 +++++++++++++++++++ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt index de4a1703..d3e5c255 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt @@ -16,8 +16,6 @@ import androidx.annotation.VisibleForTesting import androidx.lifecycle.Lifecycle import com.auth0.android.Auth0Exception import com.auth0.android.authentication.AuthenticationAPIClient -import com.auth0.android.authentication.AuthenticationException -import com.auth0.android.callback.AuthenticationCallback import com.auth0.android.callback.Callback import com.auth0.android.request.internal.GsonProvider import com.auth0.android.result.Credentials @@ -146,7 +144,14 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT return false } if (resultCode == Activity.RESULT_OK) { - continueGetCredentials(scope, minTtl, emptyMap(), emptyMap(), forceRefresh, decryptCallback!!) + continueGetCredentials( + scope, + minTtl, + emptyMap(), + emptyMap(), + forceRefresh, + decryptCallback!! + ) } else { decryptCallback!!.onFailure(CredentialsManagerException("The user didn't pass the authentication challenge.")) decryptCallback = null @@ -547,7 +552,16 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT decryptCallback = null return@execute } - val bridgeCredentials = gson.fromJson(json, OptionalCredentials::class.java) + + val bridgeCredentials: OptionalCredentials + try { + bridgeCredentials = gson.fromJson(json, OptionalCredentials::class.java) + } catch (ex: NullPointerException) { + callback.onFailure(CredentialsManagerException("Credentials could not be retrieved.")) + decryptCallback = null + return@execute + } + /* OPTIONAL CREDENTIALS * This bridge is required to prevent users from being logged out when * migrating from Credentials with optional Access Token and ID token @@ -641,8 +655,9 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT callback.onSuccess(freshCredentials) } catch (error: CredentialsManagerException) { val exception = CredentialsManagerException( - "An error occurred while saving the refreshed Credentials.", error) - if(error.cause is IncompatibleDeviceException || error.cause is CryptoException) { + "An error occurred while saving the refreshed Credentials.", error + ) + if (error.cause is IncompatibleDeviceException || error.cause is CryptoException) { exception.refreshedCredentials = freshCredentials } callback.onFailure(exception) @@ -655,6 +670,7 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT private val TAG = SecureCredentialsManager::class.java.simpleName private const val KEY_CREDENTIALS = "com.auth0.credentials" private const val KEY_EXPIRES_AT = "com.auth0.credentials_access_token_expires_at" + // This is no longer used as we get the credentials expiry from the access token only, // but we still store it so users can rollback to versions where it is required. private const val LEGACY_KEY_CACHE_EXPIRES_AT = "com.auth0.credentials_expires_at" diff --git a/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt b/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt index 17d6075a..2779a998 100644 --- a/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt +++ b/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt @@ -460,6 +460,41 @@ public class SecureCredentialsManagerTest { MatcherAssert.assertThat(retrievedCredentials.scope, Is.`is`("different scope")) } + @Test + public fun shouldFailOnGetCredentialsWhenNullPointerExceptionIsThrown() { + verifyNoMoreInteractions(client) + val expiresAt = Date(CredentialsMock.ONE_HOUR_AHEAD_MS) + val storedJson = insertTestCredentials( + hasIdToken = true, + hasAccessToken = true, + hasRefreshToken = true, + willExpireAt = expiresAt, + scope = "scope" + ) + + Mockito.`when`(crypto.decrypt(storedJson.toByteArray())) + .thenReturn("".toByteArray()) + + manager.getCredentials(callback) + verify(callback).onFailure( + exceptionCaptor.capture() + ) + val exception = exceptionCaptor.firstValue + MatcherAssert.assertThat(exception, Is.`is`(Matchers.notNullValue())) + MatcherAssert.assertThat( + exception, IsInstanceOf.instanceOf( + CredentialsManagerException::class.java + ) + ) + MatcherAssert.assertThat( + exception.message, + Is.`is`("Credentials could not be retrieved.") + ) + verify(storage, never()).remove("com.auth0.credentials") + verify(storage, never()).remove("com.auth0.credentials_expires_at") + verify(storage, never()).remove("com.auth0.credentials_can_refresh") + } + @Test public fun shouldFailOnSavingRefreshedCredentialsInGetCredentialsWhenIncompatibleDeviceExceptionIsThrown() { val expiresAt = Date(CredentialsMock.ONE_HOUR_AHEAD_MS) // non expired credentials From 64264720bfe2168b823e4fec7d3ec3dd0965f0cd Mon Sep 17 00:00:00 2001 From: Benny Cao Date: Thu, 7 Dec 2023 09:05:12 +1100 Subject: [PATCH 2/7] fix formatting --- .../storage/SecureCredentialsManager.kt | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt index d3e5c255..d2fbf6b5 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt @@ -16,6 +16,8 @@ import androidx.annotation.VisibleForTesting import androidx.lifecycle.Lifecycle import com.auth0.android.Auth0Exception import com.auth0.android.authentication.AuthenticationAPIClient +import com.auth0.android.authentication.AuthenticationException +import com.auth0.android.callback.AuthenticationCallback import com.auth0.android.callback.Callback import com.auth0.android.request.internal.GsonProvider import com.auth0.android.result.Credentials @@ -144,14 +146,7 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT return false } if (resultCode == Activity.RESULT_OK) { - continueGetCredentials( - scope, - minTtl, - emptyMap(), - emptyMap(), - forceRefresh, - decryptCallback!! - ) + continueGetCredentials(scope, minTtl, emptyMap(), emptyMap(), forceRefresh, decryptCallback!!) } else { decryptCallback!!.onFailure(CredentialsManagerException("The user didn't pass the authentication challenge.")) decryptCallback = null @@ -552,7 +547,6 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT decryptCallback = null return@execute } - val bridgeCredentials: OptionalCredentials try { bridgeCredentials = gson.fromJson(json, OptionalCredentials::class.java) @@ -561,7 +555,6 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT decryptCallback = null return@execute } - /* OPTIONAL CREDENTIALS * This bridge is required to prevent users from being logged out when * migrating from Credentials with optional Access Token and ID token @@ -655,9 +648,8 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT callback.onSuccess(freshCredentials) } catch (error: CredentialsManagerException) { val exception = CredentialsManagerException( - "An error occurred while saving the refreshed Credentials.", error - ) - if (error.cause is IncompatibleDeviceException || error.cause is CryptoException) { + "An error occurred while saving the refreshed Credentials.", error) + if(error.cause is IncompatibleDeviceException || error.cause is CryptoException) { exception.refreshedCredentials = freshCredentials } callback.onFailure(exception) @@ -670,7 +662,6 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT private val TAG = SecureCredentialsManager::class.java.simpleName private const val KEY_CREDENTIALS = "com.auth0.credentials" private const val KEY_EXPIRES_AT = "com.auth0.credentials_access_token_expires_at" - // This is no longer used as we get the credentials expiry from the access token only, // but we still store it so users can rollback to versions where it is required. private const val LEGACY_KEY_CACHE_EXPIRES_AT = "com.auth0.credentials_expires_at" From 37e8d4b3a68e8b11816a89ae55a5a6a58fc11c39 Mon Sep 17 00:00:00 2001 From: Benny Cao Date: Thu, 7 Dec 2023 13:38:35 +1100 Subject: [PATCH 3/7] pass ex as cause --- .../android/authentication/storage/SecureCredentialsManager.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt index d2fbf6b5..76ad3f82 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt @@ -551,7 +551,7 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT try { bridgeCredentials = gson.fromJson(json, OptionalCredentials::class.java) } catch (ex: NullPointerException) { - callback.onFailure(CredentialsManagerException("Credentials could not be retrieved.")) + callback.onFailure(CredentialsManagerException("Credentials could not be retrieved.", ex)) decryptCallback = null return@execute } From 522f7bcadf251b945d693da132e2c80dcea8ea3d Mon Sep 17 00:00:00 2001 From: Benny Cao Date: Wed, 3 Jan 2024 09:08:20 +1100 Subject: [PATCH 4/7] Revert "pass ex as cause" This reverts commit 37e8d4b3a68e8b11816a89ae55a5a6a58fc11c39. --- .../android/authentication/storage/SecureCredentialsManager.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt index 76ad3f82..d2fbf6b5 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt @@ -551,7 +551,7 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT try { bridgeCredentials = gson.fromJson(json, OptionalCredentials::class.java) } catch (ex: NullPointerException) { - callback.onFailure(CredentialsManagerException("Credentials could not be retrieved.", ex)) + callback.onFailure(CredentialsManagerException("Credentials could not be retrieved.")) decryptCallback = null return@execute } From ca4846845de1783e40d38e97e13ff3b509555d91 Mon Sep 17 00:00:00 2001 From: Benny Cao Date: Wed, 3 Jan 2024 09:08:30 +1100 Subject: [PATCH 5/7] Revert "fix formatting" This reverts commit 64264720bfe2168b823e4fec7d3ec3dd0965f0cd. --- .../storage/SecureCredentialsManager.kt | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt index d2fbf6b5..d3e5c255 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt @@ -16,8 +16,6 @@ import androidx.annotation.VisibleForTesting import androidx.lifecycle.Lifecycle import com.auth0.android.Auth0Exception import com.auth0.android.authentication.AuthenticationAPIClient -import com.auth0.android.authentication.AuthenticationException -import com.auth0.android.callback.AuthenticationCallback import com.auth0.android.callback.Callback import com.auth0.android.request.internal.GsonProvider import com.auth0.android.result.Credentials @@ -146,7 +144,14 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT return false } if (resultCode == Activity.RESULT_OK) { - continueGetCredentials(scope, minTtl, emptyMap(), emptyMap(), forceRefresh, decryptCallback!!) + continueGetCredentials( + scope, + minTtl, + emptyMap(), + emptyMap(), + forceRefresh, + decryptCallback!! + ) } else { decryptCallback!!.onFailure(CredentialsManagerException("The user didn't pass the authentication challenge.")) decryptCallback = null @@ -547,6 +552,7 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT decryptCallback = null return@execute } + val bridgeCredentials: OptionalCredentials try { bridgeCredentials = gson.fromJson(json, OptionalCredentials::class.java) @@ -555,6 +561,7 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT decryptCallback = null return@execute } + /* OPTIONAL CREDENTIALS * This bridge is required to prevent users from being logged out when * migrating from Credentials with optional Access Token and ID token @@ -648,8 +655,9 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT callback.onSuccess(freshCredentials) } catch (error: CredentialsManagerException) { val exception = CredentialsManagerException( - "An error occurred while saving the refreshed Credentials.", error) - if(error.cause is IncompatibleDeviceException || error.cause is CryptoException) { + "An error occurred while saving the refreshed Credentials.", error + ) + if (error.cause is IncompatibleDeviceException || error.cause is CryptoException) { exception.refreshedCredentials = freshCredentials } callback.onFailure(exception) @@ -662,6 +670,7 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT private val TAG = SecureCredentialsManager::class.java.simpleName private const val KEY_CREDENTIALS = "com.auth0.credentials" private const val KEY_EXPIRES_AT = "com.auth0.credentials_access_token_expires_at" + // This is no longer used as we get the credentials expiry from the access token only, // but we still store it so users can rollback to versions where it is required. private const val LEGACY_KEY_CACHE_EXPIRES_AT = "com.auth0.credentials_expires_at" From 5ec108117189f67a6b1d83171760e79031c7afea Mon Sep 17 00:00:00 2001 From: Benny Cao Date: Wed, 3 Jan 2024 09:08:40 +1100 Subject: [PATCH 6/7] Revert "catch nullpointerexception when getting credentials during deserialization of json" This reverts commit 9b213f3f650f535f8111bbdb34d9a2fb9ada76e3. --- .../storage/SecureCredentialsManager.kt | 28 ++++----------- .../storage/SecureCredentialsManagerTest.kt | 35 ------------------- 2 files changed, 6 insertions(+), 57 deletions(-) diff --git a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt index d3e5c255..de4a1703 100644 --- a/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt +++ b/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt @@ -16,6 +16,8 @@ import androidx.annotation.VisibleForTesting import androidx.lifecycle.Lifecycle import com.auth0.android.Auth0Exception import com.auth0.android.authentication.AuthenticationAPIClient +import com.auth0.android.authentication.AuthenticationException +import com.auth0.android.callback.AuthenticationCallback import com.auth0.android.callback.Callback import com.auth0.android.request.internal.GsonProvider import com.auth0.android.result.Credentials @@ -144,14 +146,7 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT return false } if (resultCode == Activity.RESULT_OK) { - continueGetCredentials( - scope, - minTtl, - emptyMap(), - emptyMap(), - forceRefresh, - decryptCallback!! - ) + continueGetCredentials(scope, minTtl, emptyMap(), emptyMap(), forceRefresh, decryptCallback!!) } else { decryptCallback!!.onFailure(CredentialsManagerException("The user didn't pass the authentication challenge.")) decryptCallback = null @@ -552,16 +547,7 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT decryptCallback = null return@execute } - - val bridgeCredentials: OptionalCredentials - try { - bridgeCredentials = gson.fromJson(json, OptionalCredentials::class.java) - } catch (ex: NullPointerException) { - callback.onFailure(CredentialsManagerException("Credentials could not be retrieved.")) - decryptCallback = null - return@execute - } - + val bridgeCredentials = gson.fromJson(json, OptionalCredentials::class.java) /* OPTIONAL CREDENTIALS * This bridge is required to prevent users from being logged out when * migrating from Credentials with optional Access Token and ID token @@ -655,9 +641,8 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT callback.onSuccess(freshCredentials) } catch (error: CredentialsManagerException) { val exception = CredentialsManagerException( - "An error occurred while saving the refreshed Credentials.", error - ) - if (error.cause is IncompatibleDeviceException || error.cause is CryptoException) { + "An error occurred while saving the refreshed Credentials.", error) + if(error.cause is IncompatibleDeviceException || error.cause is CryptoException) { exception.refreshedCredentials = freshCredentials } callback.onFailure(exception) @@ -670,7 +655,6 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT private val TAG = SecureCredentialsManager::class.java.simpleName private const val KEY_CREDENTIALS = "com.auth0.credentials" private const val KEY_EXPIRES_AT = "com.auth0.credentials_access_token_expires_at" - // This is no longer used as we get the credentials expiry from the access token only, // but we still store it so users can rollback to versions where it is required. private const val LEGACY_KEY_CACHE_EXPIRES_AT = "com.auth0.credentials_expires_at" diff --git a/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt b/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt index 2779a998..17d6075a 100644 --- a/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt +++ b/auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt @@ -460,41 +460,6 @@ public class SecureCredentialsManagerTest { MatcherAssert.assertThat(retrievedCredentials.scope, Is.`is`("different scope")) } - @Test - public fun shouldFailOnGetCredentialsWhenNullPointerExceptionIsThrown() { - verifyNoMoreInteractions(client) - val expiresAt = Date(CredentialsMock.ONE_HOUR_AHEAD_MS) - val storedJson = insertTestCredentials( - hasIdToken = true, - hasAccessToken = true, - hasRefreshToken = true, - willExpireAt = expiresAt, - scope = "scope" - ) - - Mockito.`when`(crypto.decrypt(storedJson.toByteArray())) - .thenReturn("".toByteArray()) - - manager.getCredentials(callback) - verify(callback).onFailure( - exceptionCaptor.capture() - ) - val exception = exceptionCaptor.firstValue - MatcherAssert.assertThat(exception, Is.`is`(Matchers.notNullValue())) - MatcherAssert.assertThat( - exception, IsInstanceOf.instanceOf( - CredentialsManagerException::class.java - ) - ) - MatcherAssert.assertThat( - exception.message, - Is.`is`("Credentials could not be retrieved.") - ) - verify(storage, never()).remove("com.auth0.credentials") - verify(storage, never()).remove("com.auth0.credentials_expires_at") - verify(storage, never()).remove("com.auth0.credentials_can_refresh") - } - @Test public fun shouldFailOnSavingRefreshedCredentialsInGetCredentialsWhenIncompatibleDeviceExceptionIsThrown() { val expiresAt = Date(CredentialsMock.ONE_HOUR_AHEAD_MS) // non expired credentials From 9e459dca36e856bce58884a5fdb2bf602daeaa99 Mon Sep 17 00:00:00 2001 From: Benny Cao Date: Wed, 3 Jan 2024 09:43:38 +1100 Subject: [PATCH 7/7] check for null f --- .../request/internal/JsonRequiredTypeAdapterFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth0/src/main/java/com/auth0/android/request/internal/JsonRequiredTypeAdapterFactory.java b/auth0/src/main/java/com/auth0/android/request/internal/JsonRequiredTypeAdapterFactory.java index a62b12e2..d7e14428 100755 --- a/auth0/src/main/java/com/auth0/android/request/internal/JsonRequiredTypeAdapterFactory.java +++ b/auth0/src/main/java/com/auth0/android/request/internal/JsonRequiredTypeAdapterFactory.java @@ -32,7 +32,7 @@ public T read(JsonReader in) throws IOException { Field[] fields = pojo.getClass().getDeclaredFields(); for (Field f : fields) { - if (f.getAnnotation(JsonRequired.class) != null) { + if (f != null && f.getAnnotation(JsonRequired.class) != null) { try { f.setAccessible(true); if (f.get(pojo) == null) {