fix(dpop): cache key pair to drop per-request keychain lookup and lock

docs/auth/token-lifecycle.md

@@ -168,6 +168,15 @@ transition, that overload can fall back to credential-scoped key material. This
 not consult the global preference. Token-endpoint proofs omit `ath`; resource proofs bind the proof
 to the current access token with `ath`.
 
+`DPoPKeyStore` caches each credential-scoped `SecKey` handle in process memory, keyed by the derived
+Keychain key name. The first "load or create the credential-scoped EC P-256 keypair" step for a
+scope performs the Keychain lookup (or Secure Enclave key generation on first use) and caches the
+resulting handle; every subsequent proof build for that scope reuses the cached handle directly,
+skipping the Keychain (`SecItemCopyMatching`/`securityd`) round-trip and the exclusive lock that
+previously serialized every DPoP proof build across every credential. Proof signing still happens
+per request against the Secure-Enclave/Keychain-backed private key — only the *lookup* is cached, so
+no private-key material is exported or held in plaintext.
+
 ---
 
 ## 5. DPoP Nonce Lifecycle
@@ -198,6 +207,9 @@ refresh, where the token endpoint can provide a fresh nonce before the resource
 
 Account deletion clears both the credential-scoped DPoP keypair and cached nonces. Credential
 migration clears the old credential's DPoP state after the new credential state is established.
+Both paths evict the corresponding entry from `DPoPKeyStore`'s in-process key-pair cache before
+removing the persisted Keychain entry, so no later lookup for that scope can return a stale handle
+to a now-deleted key.
 
 ---
 

libs/SalesforceSDKCore/SalesforceSDKCore/Classes/OAuth/DPoP/DPoPKeyStore.swift

@@ -57,6 +57,12 @@ public final class DPoPKeyStore: NSObject {
     // Use dispatch queue to prevent a potential double create race
     private let queue = DispatchQueue(label: "com.salesforce.dpop.keystore", attributes: .concurrent)
 
+    // Process-local cache of key-pair handles, keyed by the derived Keychain key name.
+    // The key material never changes for the lifetime of a credential, so once loaded or
+    // generated, the handle is safe to reuse for the rest of the process's lifetime.
+    // Accessed only under `queue` — concurrent reads, exclusive (.barrier) writes.
+    private var keyPairCache: [String: DPoPKeyPair] = [:]
+
     private override init() { super.init() }
 
     /// Returns the keypair bound to the given scope identifier, generating it on first call.
@@ -66,13 +72,24 @@ public final class DPoPKeyStore: NSObject {
             throw DPoPKeyStoreError.missingScopeIdentifier
         }
         let name = Self.keyName(for: scope)
-        // In this specific file, only .barrier work runs on this queue, so it's behaviorally
-        // identical to a serial queue. The concurrent-with-barriers pattern is the standard Swift
-        // idiom for reader/writer locks — concurrent reads, exclusive write.
+
+        // Warm path: concurrent (non-barrier) read. No Keychain I/O, no exclusive lock.
+        if let cached = queue.sync(execute: { keyPairCache[name] }) {
+            return cached
+        }
+
+        // Cold path: exclusive barrier. Double-check the cache (a concurrent cold caller may
+        // have populated it while this call was waiting), then load-or-generate exactly once
+        // and cache the handle.
         return try queue.sync(flags: .barrier) {
+            if let cached = keyPairCache[name] {
+                return cached
+            }
             do {
                 let pair = try KeyGenerator.ecKeyPair(name: name)
-                return DPoPKeyPair(publicKey: pair.publicKey, privateKey: pair.privateKey)
+                let keyPair = DPoPKeyPair(publicKey: pair.publicKey, privateKey: pair.privateKey)
+                keyPairCache[name] = keyPair
+                return keyPair
             } catch {
                 SFSDKCoreLogger.e(Self.self, message: "DPoP keypair generation failed: \(error.localizedDescription)")
                 throw DPoPKeyStoreError.keyGenerationFailed
@@ -86,15 +103,22 @@ public final class DPoPKeyStore: NSObject {
         return try keyPair(forScope: credentials.identifier)
     }
 
-    /// Returns `true` iff a private key is already present in the Keychain for `scope`.
-    /// Side-effect-free — never generates a key on miss. Used to gate DPoP proof attachment
-    /// on the presence of previously-minted key material for the credential.
+    /// Returns `true` if key material for `scope` is available — from the in-process cache,
+    /// or (on a cache miss) from the Keychain. Side-effect-free — never generates a key on
+    /// miss. Used to gate DPoP proof attachment on the presence of previously-minted key
+    /// material for the credential.
+    ///
+    /// Note: a cache hit reports `true` without touching the Keychain, so if the persistent
+    /// key were removed out of band (i.e. not via `delete(forScope:)`) this can report `true`
+    /// until the in-process cache is evicted. Every in-SDK deletion routes through
+    /// `delete(forScope:)`, which evicts the cache, keeping the two views consistent.
     public func hasKeyPair(forScope scope: String) -> Bool {
         guard !scope.isEmpty else { return false }
         let name = Self.keyName(for: scope)
         // Non-barrier read: safe to run concurrently with other reads.
         // All mutating methods in this class use .barrier for exclusive access.
         return queue.sync {
+            if keyPairCache[name] != nil { return true }   // cache-first short-circuit
             guard let privateTag = try? KeyGenerator.keyTag(name: name, prefix: KeyGenerator.ecPrivateKeyTagPrefix) else {
                 // A throw here isn't necessarily "key absent" — it can be a Keychain access
                 // denied, entitlement, or device-locked error. Returning `false` means
@@ -117,6 +141,7 @@ public final class DPoPKeyStore: NSObject {
         guard !scope.isEmpty else { return }
         let name = Self.keyName(for: scope)
         queue.sync(flags: .barrier) {
+            keyPairCache.removeValue(forKey: name)   // evict BEFORE removing the persistent key
             do {
                 try KeyGenerator.removeECKeyPair(name: name)
             } catch {
@@ -130,6 +155,14 @@ public final class DPoPKeyStore: NSObject {
         delete(forScope: credentials.identifier)
     }
 
+    /// Clears process-local key-pair handles without deleting their Keychain entries.
+    /// Test-only (simulates a process restart / cold load). Not `@objc` — invisible to consumers.
+    internal func clearInMemoryCache() {
+        queue.sync(flags: .barrier) {
+            keyPairCache.removeAll()
+        }
+    }
+
     // MARK: - Internal helpers
 
     static func keyName(for scope: String) -> String {

libs/SalesforceSDKCore/SalesforceSDKCoreTests/SFSDKDPoPTests.swift

@@ -422,6 +422,108 @@ class SFSDKDPoPTests: XCTestCase {
         }
     }
 
+    // MARK: - In-memory key-pair cache
+
+    /// Warm hit must return the exact same cached instance (object identity), not merely
+    /// an equivalent-but-distinct wrapper — a fresh `DPoPKeyPair` would mean the lookup
+    /// re-hit the Keychain and re-acquired the barrier lock.
+    func test_givenCachedKeyPair_whenFetchedAgain_thenSameInstanceReturned() throws {
+        let scope = "cache-warm-\(UUID().uuidString)"
+        defer { DPoPKeyStore.shared.delete(forScope: scope) }
+        let first = try DPoPKeyStore.shared.keyPair(forScope: scope)
+        let second = try DPoPKeyStore.shared.keyPair(forScope: scope)
+        XCTAssertTrue(first === second, "warm hit must return the identical cached instance")
+    }
+
+    /// N concurrent cold callers for a not-yet-cached scope must converge on exactly one
+    /// generated instance — the barrier's double-check must prevent a double-mint race.
+    func test_givenConcurrentCallers_whenGenerateOrLoad_thenSameCachedInstance() throws {
+        let scope = "cache-concurrent-\(UUID().uuidString)"
+        defer { DPoPKeyStore.shared.delete(forScope: scope) }
+        let iterations = 50
+        var results: [DPoPKeyPair?] = Array(repeating: nil, count: iterations)
+        let lock = NSLock()
+
+        // `concurrentPerform` genuinely runs the closures in parallel (unlike serial-prone
+        // `async` dispatch), so multiple cold callers actually race the barrier's double-check.
+        DispatchQueue.concurrentPerform(iterations: iterations) { i in
+            if let kp = try? DPoPKeyStore.shared.keyPair(forScope: scope) {
+                lock.lock(); results[i] = kp; lock.unlock()
+            }
+        }
+
+        let resolved = results.compactMap { $0 }
+        XCTAssertEqual(resolved.count, iterations, "every concurrent caller should get a keypair")
+        let first = try XCTUnwrap(resolved.first)
+        for result in resolved.dropFirst() {
+            XCTAssertTrue(result === first, "all concurrent cold callers must converge on the same cached instance")
+        }
+    }
+
+    /// Deleting a scope must evict its cached handle — the next fetch must not resurrect
+    /// the pre-delete instance, and must mint a genuinely new key pair.
+    func test_givenCachedKeyPair_whenDeleted_thenNextFetchGeneratesNew() throws {
+        let scope = "cache-delete-\(UUID().uuidString)"
+        defer { DPoPKeyStore.shared.delete(forScope: scope) }
+        let first = try DPoPKeyStore.shared.keyPair(forScope: scope)
+        DPoPKeyStore.shared.delete(forScope: scope)
+        let regenerated = try DPoPKeyStore.shared.keyPair(forScope: scope)
+        XCTAssertFalse(first === regenerated, "delete must evict the cache, not just leave a stale handle live")
+        let firstJwk = try Encryptor.jwkP256(from: first.publicKey)
+        let regenJwk = try Encryptor.jwkP256(from: regenerated.publicKey)
+        XCTAssertNotEqual(firstJwk, regenJwk, "post-delete key pair must have a different public key")
+    }
+
+    /// Clearing the in-memory cache (simulating a process restart) must reload the same
+    /// persistent Keychain-backed key on the next fetch, and re-cache it for subsequent
+    /// warm hits.
+    func test_givenMemoryCacheCleared_whenFetched_thenSamePersistentKeyReloaded() throws {
+        let scope = "cache-reset-\(UUID().uuidString)"
+        defer { DPoPKeyStore.shared.delete(forScope: scope) }
+        let first = try DPoPKeyStore.shared.keyPair(forScope: scope)
+        let firstJwk = try Encryptor.jwkP256(from: first.publicKey)
+
+        DPoPKeyStore.shared.clearInMemoryCache()
+
+        let reloaded = try DPoPKeyStore.shared.keyPair(forScope: scope)
+        let reloadedJwk = try Encryptor.jwkP256(from: reloaded.publicKey)
+        XCTAssertEqual(firstJwk, reloadedJwk, "cache-reset reload must return the same persistent key")
+
+        let rewarmed = try DPoPKeyStore.shared.keyPair(forScope: scope)
+        XCTAssertTrue(reloaded === rewarmed, "the reloaded instance must be re-cached for subsequent warm hits")
+    }
+
+    /// `hasKeyPair` must short-circuit on a cache hit. To prove this is genuinely the
+    /// cache-first path (and not just the pre-change Keychain lookup — which would also
+    /// pass while the persisted key exists), remove the persisted key *out of band* via
+    /// `KeyGenerator`, bypassing `delete(forScope:)` so the in-memory cache is NOT evicted:
+    /// the cache-first path must still report `true`, whereas the Keychain-only
+    /// implementation would report `false`. Clearing the cache then falls back to the
+    /// Keychain and reports the now-absent key.
+    func test_givenCachedKeyPair_whenPersistentKeyRemovedOutOfBand_thenCacheStillReportsPresentUntilCleared() throws {
+        let scope = "cache-haskey-\(UUID().uuidString)"
+        defer { DPoPKeyStore.shared.delete(forScope: scope) }
+
+        // Absent before any key exists; empty scope is always false.
+        XCTAssertFalse(DPoPKeyStore.shared.hasKeyPair(forScope: scope))
+        XCTAssertFalse(DPoPKeyStore.shared.hasKeyPair(forScope: ""))
+
+        // Warm the in-process cache.
+        _ = try DPoPKeyStore.shared.keyPair(forScope: scope)
+        XCTAssertTrue(DPoPKeyStore.shared.hasKeyPair(forScope: scope))
+
+        // Remove the persisted key directly, WITHOUT going through delete(forScope:), so the
+        // cache entry survives. The Keychain-only implementation would now return false.
+        _ = try KeyGenerator.removeECKeyPair(name: DPoPKeyStore.keyName(for: scope))
+        XCTAssertTrue(DPoPKeyStore.shared.hasKeyPair(forScope: scope),
+                      "a cache hit must satisfy hasKeyPair even after the persistent key is gone")
+
+        // Evict the cache: hasKeyPair now falls back to the Keychain and sees the absence.
+        DPoPKeyStore.shared.clearInMemoryCache()
+        XCTAssertFalse(DPoPKeyStore.shared.hasKeyPair(forScope: scope),
+                       "after clearing the cache, hasKeyPair must reflect the now-absent persistent key")
+    }
+
     // MARK: - Nonce cache
 
     func test_givenScopedNonce_whenSameUrlSameScope_thenReturnsCachedValue() {