Add FXIOS-16685 [FxA Pairflow] Support pairing v2 via WebChannel OAuth Part 1 - #35460
Conversation
a738c72 to
6db6386
Compare
6db6386 to
48f8874
Compare
48f8874 to
35973c0
Compare
|
|
||
| private func onPairOAuthStart(id: Int, webView: WKWebView) { | ||
| pairingOAuthHandler.start { [weak self, weak webView] result in | ||
| guard let self, let webView else { return } |
There was a problem hiding this comment.
Nit: Let's add a log here it might be useful for us in the future for debugging.
| guard let self, let webView else { return } | |
| guard let self, let webView else { | |
| self?.logger.log("Pair OAuth reply dropped. View model or webview deallocated", | |
| level: .info, | |
| category: .sync) | |
| return | |
| } |
| } | ||
|
|
||
| /// Serialize a WebChannel payload for injection into the reply script. | ||
| private func webChannelJSONString(from object: Any) -> String? { |
There was a problem hiding this comment.
Nit: Let's move this method inside FxAWebViewModel and make it static
| } | ||
| } | ||
|
|
||
| private func messageID(from value: Any?) -> Int? { |
There was a problem hiding this comment.
Can we add a simple test for this just to prevent future regressions something like:
XCTAssertEqual(viewModel.messageID(from: 42), 42)
XCTAssertEqual(viewModel.messageID(from: "42"), 42)
XCTAssertNil(viewModel.messageID(from: "abc"))| // Without an account manager the web view never loads its first page, so the modal would | ||
| // present empty with no error and no way out. A cold-launch deep link can arrive before the | ||
| // account manager finishes initializing, so wait for it rather than dropping the route. | ||
| AppEventQueue.wait(for: .accountManagerInitialized) { [weak self] in |
There was a problem hiding this comment.
can we add a cancel in deinit just to be on the safer side something like:
if let token = pairingWaitToken {
AppEventQueue.cancelAction(token: token)
}
💪 Quality guardian2 tests files modified. You're a champion of test coverage! 🚀 🧩 Neat PieceThis PR changes 659 lines. It's a substantial update, 💬 Description craftsmanGreat PR description! Reviewers salute you 🫡 🎉 BrowserViewController got smallerNice! ✅ Code coverageAll new and modified files meet their coverage thresholds. Client.app: Coverage: 46.08
Generated by 🚫 Danger Swift against 9b60ced |
Load the pairing deep link directly in the WebChannel web view, instead of converting it to a supplicant URL through beginPairingAuthentication. The page then asks the app for OAuth parameters with fxaccounts:pair_oauth_start, and FxAPairingOAuthHandler answers with the state, scope, code_challenge, code_challenge_method and keys_jwk it reads back from the flow. Advertise pairingVersion 2 in the fxaccounts:fxa_status capabilities, so the content server can choose the v2 path. The server gates the flow on its own pairing.version config as well, so both sides must agree before v2 runs. Give the deep link its own FxAPageType. The qrCode case also serves the in-app scanner and the debug setting. Both start their own OAuth flow before the page loads, so neither may satisfy the pair_oauth_start gate. Wait for the account manager before presenting the modal, so a cold-launch deep link is not dropped. The wait runs a @sendable closure off the main actor, so hop back through ensureMainThread, matching the other AppEventQueue.wait call sites in that file. Track the wait token and cancel it in deinit. Reject WebChannel messages from subframes, and reply to a pair_oauth_start the app cannot serve rather than leaving the page waiting. Log when a reply is dropped because the view model or the web view went away. Extract the WebChannel reply envelope into webChannelReplyScript and test it. The envelope is a protocol contract with the content server: the page matches the reply against the id it sent, so messageId is emitted as a number rather than a string. Cover the redirect policy after login, the title composition, the user script setup and message id parsing, none of which had tests. Co-Authored-By: Matt Lichtenstein <lichtensteinmp@gmail.com>
e32b263 to
9b60ced
Compare
|
🚀 PR merged to |
📜 Tickets
Jira ticket
Github issue
💡 Description
In v2 the page drives pairing: it asks the browser for OAuth parameters over the WebChannel rather than the app minting a supplicant URL itself. Follows the Android implementation.
What changed
FxAPairingOAuthHandler(new) answersfxaccounts:pair_oauth_start, callingbeginAuthenticationand returningstate,scope,code_challenge,code_challenge_methodandkeys_jwk. Scopes areprofile,oldsyncandsession.FxAWebViewModeladvertisespairingVersion: 2in thefxaccounts:fxa_statusreply. That reply is now built withJSONSerializationinstead of string interpolation, removing an escaping hazard in the engines list.presentPairingViewControllerno longer routes throughbeginPairingAuthentication. The page starts pairing, so the app loads the pairing URL directly.FxAPageType.pairingV2(new) is the only page type allowed to satisfypair_oauth_start. See below..accountManagerInitializedrather than dropping a cold-launch deep link that arrives before the account manager is ready.IntorString.Acceptance criteria
pairingVersion: 2in thefxaccounts:fxa_statusresponsefxaccounts:pair_oauth_startand return the OAuth parameters required to continue pairing, with theprofile,oldsyncandsessionscopes; pairing is started by the webpagePairingTests/testPairingV2, which lands in the follow-upbeginPairingAuthenticationpath and cannot triggerpair_oauth_startFxAPairingURLParserreturns.notPairingwithoutv=2, so they fall through to normal URL handlingTesting
37 unit tests pass on this branch, covering the OAuth handler, the
fxa_statuscapabilities, the pair OAuth reply shapes and the page-type gate.The full flow was verified end to end before the split, driven by a Playwright functional test in
mozilla/fxa(pairingFlowV2iOS.spec.ts): a real Firefox Nightly authority over Marionette, an iOS Simulator supplicant and a live local FxA stack. The authority reachedsync_success, and the device registered on the account and appeared in Connected Services.That test cannot run against this branch alone. It pairs against
http://localhost:3030, which the currentFxAPairingURLParserallowlist rejects, so the parser change and the UI test ship together in the follow-up.Depends on the FxA-side counterpart for the
pair_oauth_startround trip.📝 Checklist