Repository navigation
Fix native pairing sign-in failures #6457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 5 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
1ed2476
Add native sign-in callback regression tests
azooz2003-bit 7b904eb
Surface native sign-in callback failures
azooz2003-bit 13e4654
Localize native sign-in callback strings
azooz2003-bit 4d323fc
Merge remote-tracking branch 'origin/main' into task-connect-device-a…
azooz2003-bit ade58d7
Split browser sign-in flow tests
azooz2003-bit db5af51
Bound host browser sign-in test waits
azooz2003-bit 5873031
Localize iOS invalid auth callbacks
azooz2003-bit 82a73c9
Ignore stale auth callbacks after sign-out
azooz2003-bit 8591f77
Merge remote-tracking branch 'origin/main' into task-connect-device-a…
azooz2003-bit 0c49f5f
Refresh Swift length budget after main merge
azooz2003-bit 5e20598
Add fallback callback retry regression
azooz2003-bit c3cdbb5
Preserve fallback callback state until sign-in succeeds
azooz2003-bit d1554ef
Add hosted browser failure message regression
azooz2003-bit daaf8f3
Use generic hosted browser callback failure
azooz2003-bit d19aae0
Merge remote-tracking branch 'origin/main' into task-connect-device-a…
azooz2003-bit 1f4e2ee
Refresh Swift length budget after main update
azooz2003-bit 08ce03c
Keep browser sign-in helpers private
azooz2003-bit c45f703
Merge remote-tracking branch 'origin/main' into task-connect-device-a…
azooz2003-bit ba5768b
Clean up browser sign-in deadline helper
azooz2003-bit 01cde11
Keep host browser test helpers on harness
azooz2003-bit 7caef00
Localize pairing browser fallback
azooz2003-bit fe931bc
Add stale sign-out callback failure regression
azooz2003-bit 15dba5d
Ignore stale callback failures after sign-out
azooz2003-bit 1f00653
Surface browser sign-in session failures
azooz2003-bit 5c00324
Bound host browser sign-in test waits
azooz2003-bit 2a000cd
Merge remote-tracking branch 'origin/main' into task-connect-device-a…
azooz2003-bit a068307
Split host browser auth session result type
azooz2003-bit b2b321c
Keep browser auth result classification on factory instance
azooz2003-bit bd4b75f
Merge origin/main into task-connect-device-auth-hang
azooz2003-bit 8245509
Localize browser sign-in failure message
azooz2003-bit File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
28 changes: 28 additions & 0 deletions
28
.../CmuxAuthRuntime/Sources/CmuxAuthRuntime/BrowserSignIn/HostBrowserSignInFlowHelpers.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import Foundation | ||
|
|
||
| /// MainActor-confined once-guard for racing continuation resumes. | ||
| @MainActor | ||
| final class ResumeOnceFlag { | ||
| var fired = false | ||
| } | ||
|
|
||
| func authCallbackState(from url: URL) -> String? { | ||
| URLComponents(url: url, resolvingAgainstBaseURL: false)? | ||
| .queryItems? | ||
| .first(where: { $0.name == "cmux_auth_state" })? | ||
| .value | ||
| } | ||
|
|
||
| func redactedAuthState(_ state: String) -> String { | ||
| "\(state.prefix(8))..." | ||
| } | ||
|
|
||
| func authCallbackSummary(_ url: URL) -> String { | ||
| let scheme = url.scheme ?? "nil" | ||
| let target = url.host ?? url.path.trimmingCharacters(in: CharacterSet(charactersIn: "/")) | ||
| let queryItems = URLComponents(url: url, resolvingAgainstBaseURL: false)? | ||
| .queryItems? | ||
| .map(\.name) | ||
| .joined(separator: ",") ?? "" | ||
| return "scheme=\(scheme) target=\(target.isEmpty ? "nil" : target) queryKeys=\(queryItems.isEmpty ? "none" : queryItems)" | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
68 changes: 68 additions & 0 deletions
68
...Shared/CmuxAuthRuntime/Tests/CmuxAuthRuntimeTests/HostBrowserSignInFlowFailureTests.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| import CMUXAuthCore | ||
| import Foundation | ||
| import Testing | ||
| @testable import CmuxAuthRuntime | ||
|
|
||
| @MainActor | ||
| @Suite struct HostBrowserSignInFlowFailureTests { | ||
| @Test func invalidCallbackPayloadIsRejected() async { | ||
| let harness = makeHostBrowserSignInFlowHarness(user: CMUXAuthUser(id: "u1", primaryEmail: nil, displayName: nil)) | ||
|
|
||
| let attempt = Task { await harness.flow.signIn(timeout: 60) } | ||
| await waitForHostBrowserSession(harness.factory) | ||
| harness.factory.sessions[0].deliver(URL(string: "cmux-dev://auth-callback?other=1&cmux_auth_state=\(hostBrowserCallbackState(harness.factory.sessions[0]))")!) | ||
|
|
||
| #expect(await attempt.value == false) | ||
| #expect(harness.coordinator.isAuthenticated == false) | ||
| #expect(await harness.tokenStore.getStoredRefreshToken() == nil) | ||
| #expect(harness.flow.lastFailure == .invalidCallback) | ||
| } | ||
|
|
||
| @Test func externalCallbackStateMismatchRecordsInvalidCallbackFailure() async { | ||
| let harness = makeHostBrowserSignInFlowHarness(user: CMUXAuthUser(id: "u1", primaryEmail: nil, displayName: nil)) | ||
|
|
||
| let attempt = Task { await harness.flow.signIn(timeout: 60) } | ||
| await waitForHostBrowserSession(harness.factory) | ||
|
|
||
| let result = await harness.flow.handleCallbackURL(hostBrowserCallbackURL(state: "other-state")) | ||
|
|
||
| #expect(result == false) | ||
| #expect(harness.flow.isSigningIn) | ||
| #expect(harness.coordinator.isAuthenticated == false) | ||
| #expect(harness.flow.lastFailure == .invalidCallback) | ||
|
|
||
| harness.factory.sessions[0].cancel() | ||
| #expect(await attempt.value == false) | ||
| } | ||
|
|
||
| @Test func callbackTokensThatDoNotValidateRecordUnauthorizedFailure() async { | ||
| let harness = makeHostBrowserSignInFlowHarness(user: nil) | ||
|
|
||
| let attempt = Task { await harness.flow.signIn(timeout: 60) } | ||
| await waitForHostBrowserSession(harness.factory) | ||
| harness.factory.sessions[0].deliver(hostBrowserCallbackURL(state: hostBrowserCallbackState(harness.factory.sessions[0]))) | ||
|
|
||
| #expect(await attempt.value == false) | ||
| #expect(harness.coordinator.isAuthenticated == false) | ||
| #expect(await harness.tokenStore.getStoredRefreshToken() == "refresh-1") | ||
| #expect(await harness.tokenStore.getStoredAccessToken() == "access-1") | ||
| #expect(harness.flow.lastFailure == .unauthorized) | ||
| } | ||
|
|
||
| @Test func abandonedBrowserAttemptTimesOut() async throws { | ||
| let clock = ManualTestClock() | ||
| let harness = makeHostBrowserSignInFlowHarness(browserAttemptTimeout: 1, clock: clock) | ||
|
|
||
| harness.flow.beginSignIn() | ||
| await waitForHostBrowserSession(harness.factory) | ||
| await clock.waitUntilSleepers(count: 2) | ||
|
|
||
| clock.advance(by: .seconds(1)) | ||
|
|
||
| await waitForHostBrowserCondition { harness.flow.isSigningIn == false } | ||
| #expect(harness.factory.sessions[0].cancelled) | ||
| #expect(harness.flow.isSigningIn == false) | ||
| #expect(harness.coordinator.isAuthenticated == false) | ||
| #expect(harness.flow.lastFailure == .timedOut) | ||
| } | ||
| } |
85 changes: 85 additions & 0 deletions
85
.../Shared/CmuxAuthRuntime/Tests/CmuxAuthRuntimeTests/HostBrowserSignInFlowTestSupport.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| import CMUXAuthCore | ||
| import Foundation | ||
| @testable import CmuxAuthRuntime | ||
|
|
||
| @MainActor | ||
| struct HostBrowserSignInFlowHarness { | ||
| let flow: HostBrowserSignInFlow | ||
| let coordinator: AuthCoordinator | ||
| let client: FlowFakeAuthClient | ||
| let tokenStore: FlowInMemoryTokenStore | ||
| let factory: FakeBrowserAuthSessionFactory | ||
| } | ||
|
|
||
| @MainActor | ||
| func makeHostBrowserSignInFlowHarness( | ||
| user: CMUXAuthUser? = nil, | ||
| browserAttemptTimeout: TimeInterval = 5 * 60, | ||
| slowSignInThreshold: TimeInterval = 30, | ||
| clock: (any Clock<Duration>)? = nil | ||
| ) -> HostBrowserSignInFlowHarness { | ||
| let store = FakeKeyValueStore() | ||
| // The fake client reads and clears the SAME token store the flow seeds, like | ||
| // production. Split stores would hide seed/capture/clear races. | ||
| let tokenStore = FlowInMemoryTokenStore() | ||
| let client = FlowFakeAuthClient(user: user, store: tokenStore) | ||
| let coordinator = AuthCoordinator( | ||
| client: client, | ||
| sessionCache: CMUXAuthSessionCache(keyValueStore: store, key: "has_tokens"), | ||
| userCache: CMUXAuthIdentityStore(keyValueStore: store, key: "cached_user"), | ||
| teamSelection: CMUXAuthTeamSelectionStore(keyValueStore: store, key: "selected_team"), | ||
| anchor: FakeAnchor(), | ||
| config: .test, | ||
| launch: .plain() | ||
| ) | ||
| let factory = FakeBrowserAuthSessionFactory() | ||
| let flow = HostBrowserSignInFlow( | ||
| coordinator: coordinator, | ||
| tokenStore: tokenStore, | ||
| sessionFactory: factory, | ||
| callbackRouter: AuthCallbackRouter(), | ||
| makeSignInURL: { URL(string: "https://example.test/handler/sign-in?cmux_auth_state=\($0)")! }, | ||
| callbackScheme: { "cmux-dev" }, | ||
| clock: clock ?? ContinuousClock(), | ||
| browserAttemptTimeout: browserAttemptTimeout, | ||
| slowSignInThreshold: slowSignInThreshold | ||
| ) | ||
| return HostBrowserSignInFlowHarness( | ||
| flow: flow, | ||
| coordinator: coordinator, | ||
| client: client, | ||
| tokenStore: tokenStore, | ||
| factory: factory | ||
| ) | ||
| } | ||
|
|
||
| func hostBrowserCallbackURL(state: String) -> URL { | ||
| URL(string: "cmux-dev://auth-callback?stack_refresh=refresh-1&stack_access=access-1&cmux_auth_state=\(state)")! | ||
| } | ||
|
|
||
| func hostBrowserFallbackCallbackURL() -> URL { | ||
| URL(string: "cmux-dev://auth-callback?stack_refresh=refresh-1&stack_access=access-1")! | ||
| } | ||
|
|
||
| func hostBrowserCallbackState(_ session: FakeBrowserAuthSession) -> String { | ||
| URLComponents(url: session.signInURL, resolvingAgainstBaseURL: false)? | ||
| .queryItems? | ||
| .first(where: { $0.name == "cmux_auth_state" })? | ||
| .value ?? "" | ||
| } | ||
|
|
||
| @MainActor | ||
| func waitForHostBrowserSession(_ factory: FakeBrowserAuthSessionFactory, count: Int = 1) async { | ||
| // The attempt task runs on the same main actor; yielding lets it reach the | ||
| // browser-session continuation deterministically. | ||
| while factory.sessions.count < count { | ||
| await Task.yield() | ||
| } | ||
| } | ||
|
|
||
| @MainActor | ||
| func waitForHostBrowserCondition(until condition: @MainActor () -> Bool) async { | ||
| while !condition() { | ||
| await Task.yield() | ||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.