Skip to content

feat(auth): fetch and evaluate the project password policy - #2555

Draft
demolaf wants to merge 1 commit into
version-10.0.1from
auth/password-policy-fetch
Draft

demolaf wants to merge 1 commit into
version-10.0.1from
auth/password-policy-fetch

Conversation

@demolaf

@demolaf demolaf commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

Adds an internal client for the Identity Toolkit v2/passwordPolicy endpoint, the one the JS SDK's validatePassword() uses, plus an evaluator for its requirements. Nothing calls it yet.

Changes

  • PasswordPolicyRepository fetches with HttpURLConnection and org.json (no new dependencies), sends X-Android-Package and X-Android-Cert, adds tenantId when set, and caches per app and tenant.
  • Every failure (offline, timeout, any non-2xx including the Auth emulator's 501, bad JSON, a schemaVersion other than 1) returns no policy instead of throwing. Logs carry exception class names only, since connection errors include the URL with the API key.
  • PasswordPolicy.validate() mirrors JS password_policy_impl.ts: ASCII character classes, symbols count only when listed in allowedNonAlphanumericCharacters, min defaults to 6, max applies only when set.
  • Tests use a fake HTTP client and a loopback server, never the network.

Maintainer note: Part of internal CPRN-449

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a password policy configuration and validation mechanism for Firebase UI Auth, including classes to parse, validate, fetch, and cache the policy from the Identity Toolkit API. The implementation is well-tested across various scenarios. However, a critical concurrency issue was identified in PasswordPolicyRepository.lockFor where the non-atomic ConcurrentHashMap.getOrPut extension function is used, which could lead to duplicate concurrent network requests under high load. Replacing this with putIfAbsent is recommended to ensure thread safety.

}
}

private fun lockFor(key: Key): Mutex = locks.getOrPut(key) { Mutex() }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The use of ConcurrentHashMap.getOrPut is not thread-safe because getOrPut is an inline extension function on MutableMap that does not execute atomically. Under high concurrency, multiple threads calling lockFor for the same key can simultaneously observe a null value, instantiate separate Mutex instances, and overwrite each other in the map. This defeats the purpose of the lock and can lead to duplicate concurrent network requests.\n\nTo ensure atomicity across all Android API levels without relying on API 24+ computeIfAbsent (which requires library desugaring), you can use putIfAbsent to safely and atomically initialize the Mutex.

    private fun lockFor(key: Key): Mutex {
        val existing = locks[key]
        if (existing != null) return existing
        val newMutex = Mutex()
        val old = locks.putIfAbsent(key, newMutex)
        return old ?: newMutex
    }

@demolaf
demolaf removed this pull request from stack #2557 November 5, 2026 13:24
@demolaf
demolaf changed the base branch from master to version-10.0.1 November 5, 2026 13:24
@demolaf
demolaf changed the base branch from version-10.0.1 to master November 5, 2026 13:24
@demolaf
demolaf added this pull request to stack #2558 November 5, 2026 13:24
@demolaf
demolaf removed this pull request from stack #2558 November 5, 2026 13:25
@demolaf
demolaf changed the base branch from master to version-10.0.1 November 5, 2026 13:25
@demolaf
demolaf added this pull request to stack #2559 November 5, 2026 13:25
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.

1 participant