SDKS-5120: Add authenticationValidityDuration support to device binding and signing

[vibhorgoswami]

JIRA Ticket

SDKS-5120

Description

• Adds authenticationValidityDuration field (default: 5 seconds) to DeviceBindingCallback and DeviceSigningVerifierCallback, giving callers control over how long the generated key remains valid for biometric/device
  authentication.
• Threads the value through to CryptoKey(userId, authenticationValidityDuration) via the DeviceAuthenticator.initialize extension, so the Android Keystore key is generated with the correct user-authentication validity window.
• Adds unit tests to DeviceBindingCallbackTest and DeviceSigningVerifierCallbackTest covering the default value and verifying the custom value propagates to CryptoKey.timeout.

Definition of Done Checklist:

• Coded to standards.
• Ensure backward compatibility.
• API reference docs is created or updated, if applicable.
• Unit tests are written or updated.
• Integration tests are written, if applicable.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.84%. Comparing base (da60801) to head (4190775).

Additional details and impacted files

@@              Coverage Diff              @@
##             develop     #520      +/-   ##
=============================================
+ Coverage      65.77%   65.84%   +0.06%     
- Complexity      1762     1768       +6     
=============================================
  Files            264      264              
  Lines           8658     8666       +8     
  Branches         969      969              
=============================================
+ Hits            5695     5706      +11     
+ Misses          2532     2530       -2     
+ Partials         431      430       -1     

Flag	Coverage Δ
unit-tests	65.84% <100.00%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[george-bafaloukas-forgerock]

Great work on this — the core implementation is clean and the propagation through initialize() → CryptoKey is well structured. A couple of small things worth addressing before merging:

DeviceSigningVerifierCallbackTest.kt — missing @Test annotations: Two pre-existing test methods in this file — testSignForForValidClaims() and testSignForForInvalidClaims() — are missing @Test annotations, so JUnit silently skips them entirely. Since this PR is already touching the file, it would be a great time to fix those here. The testSignForForInvalidClaims case is especially worth addressing as it covers the InvalidCustomClaims error path.

[george-bafaloukas-forgerock]

Re-review (latest commit: guard authentication validity duration)

The setter guard + tests are a solid addition. The core threading (callback → initialize(…, duration, …) → CryptoKey(userId, timeout)) is correct, the guard correctly rejects the dangerous 0/-1 Keystore special values, and timeout (server-sent, used for withTimeout/JWT exp) is kept properly distinct from authenticationValidityDuration. A few items worth a look:

1. Binary-incompatible change to a published class — contradicts the "backward compatibility" DoD

forgerock-core/.../CryptoKey.kt — CryptoKey is a public class in the published forgerock-core artifact. Changing the primary constructor from CryptoKey(keyId: String) to CryptoKey(keyId: String, timeout: Int = 5) changes its JVM signature from <init>(String) to <init>(String, int). Kotlin default parameters do not generate a synthetic overload — there's no @JvmOverloads, and there's no binary-compatibility validator in the build. Any pre-compiled third-party code calling new CryptoKey(userId) would hit NoSuchMethodError at runtime until recompiled. Since this PR ticks "Ensure backward compatibility," could we make a conscious call here — add @JvmOverloads, or confirm CryptoKey is not treated as consumer-facing API? (Intra-SDK callers recompile, so no internal break.)

2. authenticationValidityDuration is a no-op for NONE and APPLICATION_PIN, and the test masks it

Only the biometric authenticators consume cryptoKey.timeout (via setUserAuthenticationParameters / setUserAuthenticationValidityDurationSeconds in BiometricOnly / BiometricAndDeviceCredential). None and ApplicationPinDeviceAuthenticator never read it. That's defensible, but testAuthenticationValidityDurationCustomValuePassedToAuthenticator asserts the value on a mock<None>() — i.e. it verifies plumbing into CryptoKey while using the one authenticator type that ignores the value. Suggest documenting in the public KDoc that the setting only affects biometric-backed keys, and/or covering a biometric type in the test.

3. Property + validation + KDoc duplicated verbatim across both callbacks

DeviceBindingCallback.kt and DeviceSigningVerifierCallback.kt — the var authenticationValidityDuration + require(value > 0) setter + KDoc are copy-pasted into both classes, which both implement Binding. A Kotlin interface can't hold a backing field, but the validation could live in a shared helper (e.g. Binding.validateValidityDuration(Int)) to keep the two copies from drifting (e.g. one becoming >= 0).

4. Default 5 duplicated in three places

The magic default appears in CryptoKey(timeout: Int = 5), initialize(userId, authenticationValidityDuration: Int = 5), and both callbacks' = 5. A single shared constant would keep them from diverging.

---

No blocking correctness bug — #1 (binary compatibility) is the one I'd want an explicit decision on before merge given the DoD claim; the rest are advisory. (Nice catch incidentally adding @Test to the two previously-unannotated testSignFor* methods so they actually run now.)

[george-bafaloukas-forgerock]

Re-review of the latest changes — the setter guard + tests are a solid addition and the core threading is correct. Four inline notes below; #1 (binary compatibility) is the only one I'd want an explicit decision on before merge given the DoD claim.

[george-bafaloukas-forgerock]

LGTM

[spetrov]

Changes look good to me! 👍🏻