Skip to content

Conversation

@subhankarmaiti
Copy link

@subhankarmaiti subhankarmaiti commented Sep 8, 2025

Changes

This PR introduces configurable biometric authentication policies for SecureCredentialsManager, allowing developers to control when biometric prompts are shown when accessing stored credentials. This enhancement improves user experience by reducing authentication friction while maintaining security.

References

auth0/react-native-auth0#687 (comment)

Testing

  1. Run the included unit tests: ./gradlew :auth0:testDebugUnitTest --tests "*BiometricPolicyTest"
  2. Use the sample app to test different policies:
    • Always Policy: Biometric prompt appears every time
    • Session Policy (5 min): Prompt appears once, then cached for 5 minutes
    • AppLifecycle Policy: Prompt appears once until app restart or manual clear
  • 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

@subhankarmaiti subhankarmaiti requested a review from a team as a code owner September 8, 2025 06:43
@pmathew92 pmathew92 requested a review from Copilot September 9, 2025 07:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds configurable biometric authentication policies to SecureCredentialsManager, allowing developers to control when biometric prompts are shown based on different authentication strategies (Always, Session-based, or App Lifecycle-based).

Key changes:

  • Introduction of BiometricPolicy sealed class with three policy types
  • Enhanced LocalAuthenticationOptions to include policy configuration
  • Session management functionality in SecureCredentialsManager

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
BiometricPolicy.kt New sealed class defining three biometric authentication policies
LocalAuthenticationOptions.kt Added policy parameter and builder method for biometric policy configuration
SecureCredentialsManager.kt Integrated session validation logic and biometric session management
SecureCredentialsManagerBiometricPolicyTest.kt Comprehensive test coverage for new biometric policy functionality
EXAMPLES.md Updated documentation with examples and explanations of BiometricPolicy usage

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +1188 to +1189
private const val NO_SESSION = -1L
private val lastBiometricAuthTime = AtomicLong(NO_SESSION)
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lastBiometricAuthTime is declared as a companion object property, making it shared across all instances of SecureCredentialsManager. This creates a potential issue where multiple manager instances would interfere with each other's session state. Consider making this an instance variable instead of a companion object property.

Copilot uses AI. Check for mistakes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, why does lastBiometricAuthTime defined as companion object property. This needs to be class property

import java.util.concurrent.Executor

@RunWith(RobolectricTestRunner::class)
public class SecureCredentialsManagerBiometricPolicyTest {
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The public modifier is unnecessary for test classes in Kotlin. Test classes should have default (internal) visibility.

Suggested change
public class SecureCredentialsManagerBiometricPolicyTest {
class SecureCredentialsManagerBiometricPolicyTest {

Copilot uses AI. Check for mistakes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it requires to run the test cases.

private val testExecutor = Executor { command -> command.run() }

@Before
public fun setUp() {
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The public modifier is unnecessary for test methods in Kotlin. Test methods should have default (internal) visibility.

Suggested change
public fun setUp() {
fun setUp() {

Copilot uses AI. Check for mistakes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it requires to run the test cases.

* The session is invalidated by calling clearBiometricSession() or after the default timeout.
* @param timeoutInSeconds The duration for which the session remains valid. Defaults to 3600 seconds (1 hour).
*/
public data class AppLifecycle(val timeoutInSeconds: Int = 3600) : BiometricPolicy() // Default 1 hour
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs the @jvmoverloads annotation to work with seamlessly java

is BiometricPolicy.AppLifecycle -> {
val timeoutMillis = biometricPolicy.timeoutInSeconds * 1000L
System.currentTimeMillis() - lastAuth < timeoutMillis
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two cases can be combined

return when (biometricPolicy) {
            is BiometricPolicy.Session ,
            is BiometricPolicy.AppLifecycle -> {
                val timeoutMillis = biometricPolicy.timeoutInSeconds * 1000L
                System.currentTimeMillis() - lastAuth < timeoutMillis
            }
            is BiometricPolicy.Always -> false
            }
    ```        


assert(policy1 === policy2) // Same instance
assert(policy1 == policy2) // Equal
}
Copy link
Contributor

@pmathew92 pmathew92 Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case is not needed

}

@Test
public fun `BiometricPolicy Session should be data class with timeout parameter`() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be removed

}

@Test
public fun `BiometricPolicy AppLifecycle should be data class with default timeout`() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be removed

@pmathew92 pmathew92 requested a review from Copilot September 30, 2025 06:04
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

// =========================

@Test
public fun `BiometricPolicy Always should be object type`() {
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The public visibility modifier is unnecessary for test methods in Kotlin and should be removed. Kotlin defaults to public visibility for functions, making this redundant.

Copilot uses AI. Check for mistakes.

}

@Test
public fun `BiometricPolicy Session should be data class with timeout parameter`() {
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The public visibility modifier is unnecessary for test methods in Kotlin and should be removed. Kotlin defaults to public visibility for functions, making this redundant.

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants