DPoP: send dpop_jkt on /authorize (RFC 9449 §10 code binding) - #4106
Conversation
Binds the authorization code returned by /authorize to the DPoP key pair that will later prove possession at /token. Mirrors the Android implementation. - Add DPoPProofBuilder.jwkThumbprint(publicKey:) — RFC 7638 JWK thumbprint (43-char base64url of SHA-256 over canonical JSON with sorted keys). - Add SFSDKAuthConfigUtil.isPoolLoginHost: predicate and lift the three pool-host constants to the header for reuse. Refactor the existing pool check to consume the new predicate — single source of truth. - Append &dpop_jkt=<thumbprint> to the approval URL when SalesforceSDKManager.usesDPoP is YES, the login server is a my-domain host, and credentials.identifier is set. Pool hosts are excluded (Salesforce blocks DPoP at pool servers). - Soft-fail on crypto/keychain error: log a warning and leave the URL untouched so login proceeds; the server will surface the RFC error. Tests cover: URL shape (43-char base64url), authorize↔token key binding, pool-host exclusion (three hosts), byte-identical baseline when usesDPoP=NO, soft-fail on missing key material, migrateRefreshToken path, and both the user-agent and web-server flow entry points. Also adds jwkThumbprint unit tests against an RFC 6979 fixed test key with a precomputed RFC 7638 fixture and shape/uniqueness checks.
Generated by 🚫 Danger |
Clang Static Analysis Issues
Generated by 🚫 Danger |
|
||||||||||||||||||
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dpop #4106 +/- ##
=======================================
Coverage ? 65.95%
=======================================
Files ? 251
Lines ? 21822
Branches ? 0
=======================================
Hits ? 14392
Misses ? 7430
Partials ? 0
🚀 New features to boost your workflow:
|
| } catch { | ||
| throw DPoPProofBuilderError.jwkExportFailed | ||
| } | ||
| // RFC 7638: canonical JSON with lexicographic key ordering, UTF-8, no whitespace. |
There was a problem hiding this comment.
Nit: RFC 7638 §3.2 requires the canonical JSON to contain only the required members for the key type — for P-256 that's exactly {crv, kty, x, y}, no optional fields like kid, use, or key_ops. jwk is passed directly to JSONSerialization here, so if Encryptor.jwkP256 ever grows extra fields the thumbprint will silently diverge from what the server computes off the DPoP proof's jwk claim, breaking the authorize↔token binding. The fixture test would catch it, but worth a comment stating the invariant in-context so the dependency is visible to a future jwkP256 author.
There was a problem hiding this comment.
Addressed in f60327e — added the RFC 7638 §3.2 minimality note tying the thumbprint's correctness to jwkP256's output staying at exactly {crv, kty, x, y}, and a SFSDKCoreLogger.w line before the thumbprintFailed throw so canonicalization failures are diagnosable.
| // RFC 7638: canonical JSON with lexicographic key ordering, UTF-8, no whitespace. | ||
| guard let canonicalData = try? JSONSerialization.data(withJSONObject: jwk, | ||
| options: [.sortedKeys, .withoutEscapingSlashes]), | ||
| let digest = (canonicalData as NSData).sfsdk_sha256() else { |
There was a problem hiding this comment.
Nit: try? on the JSONSerialization call above swallows any error before this throw, so if canonicalization ever fails the failure is silent. In practice a [String: String] dict won't cause a serialization error, but a log here (matching the style of appendDPoPJktIfNeededTo:) would make failures diagnosable: SFSDKCoreLogger.w(DPoPProofBuilder.self, format: "DPoP jwkThumbprint: canonicalization or hash failed")
There was a problem hiding this comment.
Addressed in f60327e — added the RFC 7638 §3.2 minimality note tying the thumbprint's correctness to jwkP256's output staying at exactly {crv, kty, x, y}, and a SFSDKCoreLogger.w line before the thumbprintFailed throw so canonicalization failures are diagnosable.
|
||||||||||||||||||
…canonicalization failure
- Add in-context comment tying jwkThumbprint's correctness to jwkP256's
output being minimal ({crv, kty, x, y} only). Optional JWK fields would
silently break the authorize↔token binding; this makes the dependency
visible to a future jwkP256 author.
- Log a warn line when JSON canonicalization or SHA-256 hashing fails
before throwing thumbprintFailed, matching the diagnosability style of
the coordinator's appendDPoPJktIfNeededTo: soft-fail path.
2 participants
Summary
Bind the authorization code returned by
/authorizeto the DPoP key pair that will later prove possession at/token, per RFC 9449 §10. Mirrors the Android implementation.When
SalesforceSDKManager.usesDPoPisYES, the login server is a my-domain host, andcredentials.identifieris set, the approval URL now carriesdpop_jkt=<RFC 7638 thumbprint>. Pool hosts (login/test/welcome.salesforce.com) are excluded — Salesforce blocks DPoP at the pool servers.Changes
DPoPProofBuilder.jwkThumbprint(publicKey:)— new@objcstatic that returns the 43-char base64url RFC 7638 thumbprint. Composed from existing primitives (Encryptor.jwkP256,JSONSerialization(.sortedKeys),sfsdk_sha256,sfsdk_base64UrlString). New.thumbprintFailederror case.SFSDKAuthConfigUtil.isPoolLoginHost:— new predicate. The three pool-host string constants are lifted from.mfile-locals into the header asFOUNDATION_EXTERNso both the auth-config prefetch check and the dpop_jkt gate share one source of truth.SFOAuthCoordinator.approvalURLForEndpoint:…— after the existing scope/login-hint appends, invokes a new private helper-appendDPoPJktIfNeededTo:domain:credentials:that gates onusesDPoP+isPoolLoginHost+ non-empty identifier, loads the key pair viaDPoPKeyStore.keyPair(forCredentials:), and appends&dpop_jkt=…. Crypto/keychain errors soft-fail (warn-log, URL untouched, login proceeds).kSFOAuthDPoPJktParamName— new constant next tokSFOAuthCodeChallengeParamName.Tests
DPoPProofBuilder.jwkThumbprint— RFC 7638 fixture against RFC 6979 §A.2.5 P-256 test key, uniqueness across pairs, 43-char base64url shape.SFOAuthCoordinator.generateApprovalUrlString:dpop_jktpresent + shape correct under DPoP + my-domain + identifierdpop_jktvalue equalsjwkThumbprint(publicKey:)of the pairDPoPKeyStorereturns at/tokentime (authorize↔token binding invariant)login,test,welcome.../discovery) — nodpop_jktusesDPoP=NO— byte-identical baseline; nodpop_jktcredentials.identifier— soft-fail path, nodpop_jkt, no exceptionmigrateRefreshToken:construction path carriesdpop_jktTest plan
Require DPoPECA on a my-domain org via RestAPIExplorer withusesDPoP=YES; confirmdpop_jktpresent on the/authorizerequest and matches the token-endpoint JWK thumbprint.login.salesforce.com) — nodpop_jktsent, flow unchanged.