Add support for Multi-Resource Refresh Token (MRRT) #811

pmathew92 · 2025-03-18T09:01:56Z

Changes

This PR moves the Credentials Manager from a single credentials model to a multiple credentials one, supporting:

1 set of app credentials (the existing functionality)
N sets of API-specific credentials
To this end, two new public methods were added to the Credentials Manager:

// And its coroutine counter part
public fun getApiCredentials(
        audience: String,
        scope: String? = null,
        minTtl: Int = 0,
        parameters: Map<String, String> = emptyMap(),
        headers: Map<String, String> = emptyMap(),
        callback: Callback<APICredentials, CredentialsManagerException>
    )

public fun clearApiCredentials(audience: String)

Testing

Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. Since this library has unit testing, tests should be added for new functionality and existing tests should complete without errors.

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

Checklist

I have read the Auth0 general contribution guidelines
I have read the Auth0 Code of Conduct
All existing and new tests complete without errors

…getting credentials

…lass

auth0/src/main/java/com/auth0/android/authentication/storage/CredentialsManager.kt

+                val expiresAt = newCredentials.expiresAt.time
+                val willAccessTokenExpire = willExpire(expiresAt, minTtl.toLong())
+                if (willAccessTokenExpire) {
+                    val tokenLifetime = (expiresAt - currentTimeInMillis - minTtl * 1000) / -1000


To fix the issue, we need to ensure that the multiplication minTtl * 1000 is performed in a long context to avoid integer overflow. This can be achieved by explicitly casting one of the operands (minTtl or 1000) to long before the multiplication. This ensures that the result of the multiplication is a long and can safely handle larger values.

The specific change will be made on line 610, where minTtl * 1000 is replaced with minTtl.toLong() * 1000. This change does not alter the logic of the program but ensures that the multiplication is performed in a long context.

auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt

+                val expiresAt = newCredentials.expiresAt.time
+                val willAccessTokenExpire = willExpire(expiresAt, minTtl.toLong())
+                if (willAccessTokenExpire) {
+                    val tokenLifetime = (expiresAt - currentTimeInMillis - minTtl * 1000) / -1000


To fix the issue, we need to ensure that the multiplication minTtl * 1000 is performed safely without risking integer overflow. This can be achieved by casting one of the operands to Long before the multiplication. This ensures that the multiplication is performed in the long domain, which has a much larger range than int.

Specifically, we can cast minTtl to Long before multiplying it by 1000. This change is minimal and does not alter the logic of the code. The corrected line will look like this: (minTtl.toLong() * 1000).

…K-5778

Widcket · 2025-05-07T17:20:46Z

auth0/src/main/java/com/auth0/android/authentication/storage/CredentialsManager.kt

+     * @param audience the audience for which the credentials are stored
+     */
+    override fun saveApiCredentials(apiCredentials: APICredentials, audience: String) {
+        gson.toJson(apiCredentials).let {


Should this method throw an error if the serialization fails?

This shouldn't cause any serialization issue and wouldn't require to throw an exception

Widcket · 2025-05-07T17:33:08Z

auth0/src/main/java/com/auth0/android/authentication/storage/CredentialsManager.kt

@@ -305,6 +323,45 @@ public class CredentialsManager @VisibleForTesting(otherwise = VisibleForTesting
        }
    }

+    /**
+     * Retrieves API credentials from storage and automatically renews them using the refresh token if the access
+     * token is expired. Otherwise, the retrieved API credentials will be returned via the success case as they are still valid.


This method is an async wrapper, so there is no success case.

Widcket · 2025-05-07T17:34:03Z

auth0/src/main/java/com/auth0/android/authentication/storage/CredentialsManager.kt

+     * token is expired. Otherwise, the retrieved API credentials will be returned via the success case as they are still valid.
+     *
+     * If there are no stored API credentials, the refresh token will be exchanged for a new set of API credentials.
+     * New or renewed API credentials will be automatically stored in storage.


Suggested change

* New or renewed API credentials will be automatically stored in storage.

* New or renewed API credentials will be automatically persisted in storage.

Widcket · 2025-05-07T20:25:59Z

auth0/src/main/java/com/auth0/android/authentication/storage/CredentialsManager.kt

+     *  token is expired. Otherwise, the retrieved API credentials will be returned via the success case as they are still valid.
+     *
+     * If there are no stored API credentials, the refresh token will be exchanged for a new set of API credentials.
+     * New or renewed API credentials will be automatically stored in storage.


Suggested change

* New or renewed API credentials will be automatically stored in storage.

* New or renewed API credentials will be automatically persisted in storage.

Widcket · 2025-05-07T20:26:18Z

auth0/src/main/java/com/auth0/android/authentication/storage/CredentialsManager.kt

+
+    /**
+     *  Retrieves API credentials from storage and automatically renews them using the refresh token if the access
+     *  token is expired. Otherwise, the retrieved API credentials will be returned via the success case as they are still valid.


Suggested change

* token is expired. Otherwise, the retrieved API credentials will be returned via the success case as they are still valid.

* token is expired. Otherwise, the retrieved API credentials will be returned via the success callback as they are still valid.

Widcket · 2025-05-07T21:25:01Z

auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt

@@ -406,6 +437,48 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT
        }
    }

+    /**
+     * Retrieves API credentials from storage and automatically renews them using the refresh token if the access
+     * token is expired. Otherwise, the retrieved API credentials will be returned via the success case as they are still valid.


This method is an async wrapper, so there is no success case.

Widcket · 2025-05-07T21:25:16Z

auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt

+     * token is expired. Otherwise, the retrieved API credentials will be returned via the success case as they are still valid.
+     *
+     * If there are no stored API credentials, the refresh token will be exchanged for a new set of API credentials.
+     * New or renewed API credentials will be automatically stored in storage.


Suggested change

* New or renewed API credentials will be automatically stored in storage.

* New or renewed API credentials will be automatically persisted in storage.

Widcket · 2025-05-07T21:26:49Z

auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt

@@ -553,6 +625,47 @@ public class SecureCredentialsManager @VisibleForTesting(otherwise = VisibleForT
            }
        }

+    /**
+     * Retrieves API credentials from storage and automatically renews them using the refresh token if the access
+     * token is expired. Otherwise, the retrieved API credentials will be returned via the success case as they are still valid.


Suggested change

* token is expired. Otherwise, the retrieved API credentials will be returned via the success case as they are still valid.

* token is expired. Otherwise, the retrieved API credentials will be returned via the success callback as they are still valid.

Widcket · 2025-05-07T21:33:20Z

auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt

+    ) {
+        val localAuthenticationManager = localAuthenticationManagerFactory?.create(
+            activity = fragmentActivity,
+            authenticationOptions = localAuthenticationOptions!!,


Should this value be passed via argument, like fragmentActivity and biometricResultCallback?

This is a global property that will be set when the instance of the SecureCredentialsManager is created. Hence no need to pass as an argument

So is fragmentActivity.

Yes. but fragment activity is a weak reference which we check if there is a valid reference present or not before invoking the methods. Hence they are passed as arguments

Yes, but you also check that localAuthenticationOptions is not null before calling this method. Like you do for the fragment activity.

The only difference between the two is that one is weak and the other is not, but both are nullable. Hence the comment.

We do have the check of localAuthenticationOptions != null before calling . Hence we are accessing this globally. For fragment activity we have two null checks , one of the property is not null and they secondary check to ensure the reference inside the property is still non null or not. We pass this non null reference as local parameter ,to avoid any issues even if the main reference got destroyed.

if (fragmentActivity != null && localAuthenticationOptions != null && localAuthenticationManagerFactory != null) { fragmentActivity.get()?.let { fragmentActivity -> startBiometricAuthentication( fragmentActivity, biometricAuthenticationApiCredentialsCallback( audience, scope, minTtl, parameters, headers, callback ) ) } ?: run { callback.onFailure(CredentialsManagerException.BIOMETRIC_ERROR_NO_ACTIVITY) } return }

Widcket · 2025-05-07T21:34:44Z

auth0/src/main/java/com/auth0/android/authentication/storage/CredentialsManager.kt

+     * Removes the credentials for the given audience from the storage if present.
+     */
+    override fun clearApiCredentials(audience: String) {
+        storage.remove(audience)


Should this be logged like in the SecureCredentialsManager? E.g.:
Log.d(TAG, "API Credentials for $audience were just removed from the storage")

Widcket · 2025-05-07T21:35:47Z

auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt

+    internal fun continueGetApiCredentials(
+        audience: String,
+        scope: String?,
+        minTtl: Int = 0,


Does this argument need a default value here?

Not needed. Removed

Widcket · 2025-05-07T22:01:51Z

auth0/src/main/java/com/auth0/android/result/APICredentials.kt

+    @field:SerializedName("expires_at")
+    val expiresAt: Date,
+    /**
+     * Getter for the access token's granted scope. Only available if the requested scope differs from the granted one.


Only available if the requested scope differs from the granted one. This doesn't seem to be the case anymore.

Widcket · 2025-05-07T22:03:45Z

auth0/src/main/java/com/auth0/android/result/APICredentials.kt

+ * Converts a Credentials instance to an APICredentials instance.
+ */
+internal fun Credentials.toAPICredentials(): APICredentials {
+    val newScope = scope ?: "openid"


I'd suggest using an empty string instead. If the server response doesn't include a scope value, we cannot assume openid was the only requested scope.

Widcket · 2025-05-07T22:07:41Z

auth0/src/test/java/com/auth0/android/authentication/AuthenticationAPIClientTest.kt

+        val auth0 = auth0
+        val client = AuthenticationAPIClient(auth0)
+        mockAPI.willReturnSuccessfulLogin()
+        val credentials = client.renewAuth(refreshToken = "refreshToken", scope = "read:data")


To differentiate this case from shouldRenewAuthWithOAuthAudienceAndScopeEnforcingOpendId in the "enforcing openid" aspect, I'd suggest using openid read:data as the scope value here.

Also, that would make shouldRenewAuthWithOAuthAudienceAndScopeNotEnforcingOpendId redundant, so it could be removed.

Widcket · 2025-05-07T22:17:56Z

auth0/src/test/java/com/auth0/android/authentication/storage/CredentialsManagerTest.kt

+        val renewedCredentials =
+            Credentials("newId", "newAccess", "newType", newRefresh, newDate, "newScope")
+        Mockito.`when`(request.execute()).thenReturn(renewedCredentials)
+        manager.getApiCredentials("audience", "newScope", minTtl = 10, callback = apiCredentialsCallback)


I'd suggest using the same scope here to make it clear the renewal is not happening because of the different scopes, but because of the minTtl.

Widcket · 2025-05-07T22:18:30Z

auth0/src/test/java/com/auth0/android/authentication/storage/CredentialsManagerTest.kt

+        val renewedCredentials =
+            Credentials("newId", "newAccess", "newType", newRefresh, newDate, "newScope")
+        Mockito.`when`(request.execute()).thenReturn(renewedCredentials)
+        manager.getApiCredentials("audience", "newScope", callback = apiCredentialsCallback)


I'd suggest using the same scope here to make it clear that the renewal is not happening because of the different scopes but because the access token has expired.

Widcket · 2025-05-07T22:19:35Z

auth0/src/test/java/com/auth0/android/authentication/storage/CredentialsManagerTest.kt

+
+        // Verify the credentials are property stored
+        verify(storage).store("com.auth0.id_token", renewedCredentials.idToken)
+        // RefreshToken should not be replaced


Suggested change

// RefreshToken should not be replaced

// RefreshToken should be replaced

Widcket · 2025-05-07T22:27:34Z

auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt

+
+        // Verify the credentials are property stored
+        verify(storage).store(eq("com.auth0.credentials"), stringCaptor.capture())
+        MatcherAssert.assertThat(stringCaptor.firstValue, Is.`is`(Matchers.notNullValue()))


Should we assert here that the refresh token was not replaced?

Widcket · 2025-05-07T22:29:24Z

auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt

+            apiCredentialsCaptor.capture()
+        )
+
+        // Verify the returned credentials are the latest


Widcket · 2025-05-07T22:29:41Z

auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt

+            apiCredentialsCaptor.capture()
+        )
+
+        // Verify the returned credentials are the latest


Widcket · 2025-05-07T22:29:57Z

auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt

+            apiCredentialsCaptor.capture()
+        )
+
+        // Verify the returned credentials are the latest


Widcket · 2025-05-07T22:30:38Z

auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt

+
+        // Verify the returned credentials are the latest


Suggested change

// Verify the returned credentials are the latest

Widcket · 2025-05-07T22:31:14Z

auth0/src/test/java/com/auth0/android/authentication/storage/SecureCredentialsManagerTest.kt

+
+        // Verify the returned credentials are the latest
+        val exception = exceptionCaptor.firstValue
+        MatcherAssert.assertThat(exception, Is.`is`(Matchers.notNullValue()))


Should we assert on the exception message here?

The timing shown in the message can vary dynamically so can't have the text

pmathew92 added 3 commits March 11, 2025 15:16

Updated the renewAuth api to take audience and scope parameter while …

188a009

…getting credentials

Added support for getting api credentials in the CredentialsManager c…

de1bbd2

…lass

Added MRRT api support in SecureCredentialsManager class

600eb1e

pmathew92 requested a review from a team as a code owner March 18, 2025 09:01

pmathew92 added 2 commits March 19, 2025 15:56

Added UTs for the CredentialManager class

d3db663

Merge conflict with main is resolved

38ccea9

github-advanced-security bot found potential problems Mar 19, 2025

View reviewed changes

pmathew92 and others added 8 commits March 20, 2025 14:56

Added UT for the secure credentials manager class

daec7f4

Fixing the failed test

6fd8364

Merge conflicts with master resolved

9b1d161

Merge branch 'main' of https://github.com/auth0/Auth0.Android into SD…

66ddabd

…K-5778

Merge branch 'main' into SDK-5778

6f5e847

Merge branch 'main' into SDK-5778

94605c5

Made the scope in API credentials non nullable

b75899a

Merge branch 'main' of https://github.com/auth0/Auth0.Android into SD…

3320784

…K-5778

Widcket reviewed May 7, 2025

View reviewed changes

review comments addressed

aab6508

Widcket approved these changes May 8, 2025

View reviewed changes

pmathew92 merged commit 7f0c68f into main May 8, 2025
7 checks passed

pmathew92 deleted the SDK-5778 branch May 8, 2025 13:25

pmathew92 mentioned this pull request May 9, 2025

Release 3.7.0 #828

Merged

@@ -609,3 +609,3 @@
                             if (willAccessTokenExpire) {
-                                val tokenLifetime = (expiresAt - currentTimeInMillis - minTtl * 1000) / -1000
+                                val tokenLifetime = (expiresAt - currentTimeInMillis - minTtl.toLong() * 1000) / -1000
                                 val wrongTtlException = CredentialsManagerException(

@@ -942,3 +942,3 @@
                             if (willAccessTokenExpire) {
-                                val tokenLifetime = (expiresAt - currentTimeInMillis - minTtl * 1000) / -1000
+                                val tokenLifetime = (expiresAt - currentTimeInMillis - minTtl.toLong() * 1000) / -1000
                                 val wrongTtlException = CredentialsManagerException(

	* New or renewed API credentials will be automatically stored in storage.
	* New or renewed API credentials will be automatically persisted in storage.

	* token is expired. Otherwise, the retrieved API credentials will be returned via the success case as they are still valid.
	* token is expired. Otherwise, the retrieved API credentials will be returned via the success callback as they are still valid.

	// RefreshToken should not be replaced
	// RefreshToken should be replaced

Add support for Multi-Resource Refresh Token (MRRT) #811

Add support for Multi-Resource Refresh Token (MRRT) #811

Uh oh!

Conversation

Uh oh!

Changes

Testing

Checklist

Uh oh!

Check warning

Uh oh!

Copilot Autofix

Check warning

Uh oh!

Copilot Autofix

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment