feat: added support for MRRT on Android/iOS

[subhankarmaiti]

No description provided.

[subhankarmaiti]

@claude

[github-actions]

Claude finished @subhankarmaiti's task —— View job

---

Code Review Progress

• Read project architecture documentation
• Analyze PR changes and diff
• Perform code quality review
• Perform security review
• Perform performance review
• Provide inline feedback for specific issues
• Provide overall assessment

---

Overall Assessment

❌ This PR implements MRRT (Multi-Resource Refresh Token) support for Android and iOS, but has several issues that should be addressed before merging:

Critical Issues:

• Web implementation uses ID token expiration instead of access token expiration for API credentials, which will cause incorrect token lifecycle management
• NativeCredentialsManager's clearCredentials error handling is inconsistent with native implementations and could leave the app in an inconsistent state

Major Issues:

• Missing minTtl parameter in getApiCredentials limits flexibility for token freshness control
• Inconsistent parameter spreading order between getCredentials and getApiCredentials in web implementation

Minor Issues:

• Code formatting inconsistencies (missing newlines, spacing)
• Type checking could be more robust in example screens
• Missing clarification on header support limitations

The implementation is well-structured with good test coverage and comprehensive documentation. Once the critical token expiration issue is fixed and error handling is made consistent, this will be a solid feature addition.

---

Job Run • Branch: SDK-6263_mrrt_support

[github-actions]

Missing newline at end of file

The file should end with a newline character for POSIX compliance and consistency with the rest of the codebase.

Suggested change

	}
	}
	}

[github-actions]

Missing blank line before closing brace

For consistency with the rest of the class structure, there should be a blank line before the closing method brace and the next method/property.

Suggested change

	}
	secureCredentialsManager.clearApiCredentials(audience)
	promise.resolve(true)
	}

[github-actions]

Inconsistent blank line spacing

There's an extra blank line here that's inconsistent with the rest of the file. Remove one blank line to match the spacing pattern used elsewhere in the class.

Suggested change

	@objc public func clearApiCredentials(audience: String, resolve: RCTPromiseResolveBlock, reject: RCTPromiseRejectBlock) {

[github-actions]

Incorrect token expiration used in getApiCredentials

The code uses the ID token's expiration (claims.exp) as the expiresAt value for API credentials, but API credentials should use the access token's expiration, not the ID token's. These expiration times can be different, and using the wrong one could cause:

1. Premature token refresh if ID token expires before access token
2. Using expired access tokens if ID token expires after access token
3. Incorrect isExpired() checks when calling apiCredentials.isExpired()

The native implementations correctly use the access token's expiration from the APICredentials object. The web implementation should calculate the access token expiration from tokenResponse or use a different approach to get the actual access token expiration.

Note: The @auth0/auth0-spa-js library's getTokenSilently doesn't directly provide the access token expiration in the response. You may need to:

1. Decode the access token if it's a JWT to get its expiration
2. Or request this feature from the @auth0/auth0-spa-js library
3. Or document this limitation in the web platform implementation

[github-actions]

Missing error handling for getCredentials

The runTest wrapper catches errors, but calling getCredentials() without parameters may not provide the necessary context. When testing getCredentials(), consider passing the scope and audience parameters that were used during login to ensure the test accurately reflects real-world usage. This is especially important for MRRT support where audience matters.

[github-actions]

Unsafe type narrowing and null checking

The type checking logic here is fragile:

1. It doesn't verify that result is actually a Credentials object with a valid refreshToken
2. The token could be undefined even after the check
3. The disabled state doesn't account for an empty string token

Suggested change

	'refreshToken' in result
	) {
	const token = (result as Credentials).refreshToken;
	if (token) {
	runTest(
	() => revokeRefreshToken({ refreshToken: token }),
	'Revoke Refresh Token'
	);
	}
	onPress={() => {
	if (
	typeof result === 'object' &&
	result &&
	'refreshToken' in result &&
	result.refreshToken
	) {
	runTest(
	() => revokeRefreshToken({ refreshToken: result.refreshToken! }),
	'Revoke Refresh Token'
	);
	}
	}}
	disabled={
	!(
	typeof result === 'object' &&
	result &&
	'refreshToken' in result &&
	result.refreshToken
	)
	}

[github-actions]

Potential error in clearCredentials: DPoP key clearing may fail silently

The clearCredentials method attempts to clear the DPoP key, but if clearDPoPKey() fails, the entire operation fails. This could be problematic because:

1. The credentials might have already been cleared before the DPoP key clearing fails
2. The user would see an error even though their primary credentials were cleared successfully
3. This creates an inconsistent state

Consider handling the DPoP key clearing error more gracefully, similar to how it's done in the native Android/iOS code where DPoP clearing errors are logged but don't fail the operation.

Suggested change

	await this.handleError(this.bridge.clearDPoPKey());
	await this.handleError(this.bridge.clearCredentials());
	// Also clear the DPoP key when clearing credentials
	// If DPoP key clearing fails, log it but don't fail the operation
	try {
	await this.bridge.clearDPoPKey();
	} catch (e) {
	// Log the error but don't propagate it
	console.warn('Failed to clear DPoP key:', e);
	}

[github-actions]

Missing minTtl parameter in getApiCredentials

The getApiCredentials method always passes minTtl: 0 to the bridge, ignoring the typical pattern used in getCredentials. This means:

1. The method will always return cached tokens even if they're about to expire
2. Users cannot specify a minimum TTL for API credentials
3. This is inconsistent with the getCredentials method which accepts a minTtl parameter

Consider adding a minTtl parameter to getApiCredentials for consistency and flexibility:

Suggested change

	this.bridge.getApiCredentials(audience, scope, 0, parameters)
	async getApiCredentials(
	audience: string,
	scope?: string,
	minTtl?: number,
	parameters?: Record<string, any>
	): Promise<ApiCredentials> {
	const nativeCredentials = await this.handleError(
	this.bridge.getApiCredentials(audience, scope, minTtl ?? 0, parameters)
	);

Note: This would also require updating the interface definition in ICredentialsManager.ts and all other implementations.

[github-actions]

Empty headers map comment is misleading

The comment says "headers not supported from JS yet", but headers are not being passed because the React Native bridge doesn't expose this parameter. Consider either:

1. Adding header support if it's a planned feature
2. Or updating the comment to clarify this is by design for security reasons (headers from JS could pose security risks)

[github-actions]

Inconsistent parameter handling between getCredentials and getApiCredentials

In getCredentials (line 28), parameters are spread first: { ...parameters, scope }, but in getApiCredentials (line 67), audience and scope come first: { ...parameters, audience, scope }.

The spread order matters - properties defined after the spread will override properties in the spread object. For consistency and to allow parameter overrides, consider using the same pattern in both methods:

Suggested change

	async getApiCredentials(
	audience: string,
	scope?: string,
	parameters?: Record<string, any>
	): Promise<ApiCredentials> {
	try {
	const tokenResponse = await this.client.getTokenSilently({
	authorizationParams: {
	...parameters,
	audience: audience,
	scope: scope,
	},
	detailedResponse: true,
	});
	async getApiCredentials(
	audience: string,
	scope?: string,
	parameters?: Record<string, any>
	): Promise<ApiCredentials> {
	try {
	const tokenResponse = await this.client.getTokenSilently({
	authorizationParams: {
	...parameters,
	audience,
	scope,
	},
	detailedResponse: true,
	});

This ensures that explicit audience/scope parameters always take precedence over any values in the parameters object.

[github-actions]

Missing scope parameter in getApiCredentials call

The test passes both audience and scope to getApiCredentials, but based on the FAQ and best practices, scope should match what was requested during login. Consider adding a comment or validation to ensure users understand that the scope must be compatible with the API's configuration.