From fb422c0866fbefc0a1cdfbdb2c26540df77b2721 Mon Sep 17 00:00:00 2001 From: evgeniyChepelev <68751844+evgeniyChepelev@users.noreply.github.com> Date: Sun, 26 Jul 2026 23:22:44 +0200 Subject: [PATCH 1/8] fix(ios): present the SSO login browser as a sheet --- .../App/Views/Components/SafariView.swift | 188 +++++++++++------- .../App/Views/iOS/ProfilesListView.swift | 13 ++ .../App/Views/iOS/iOSConnectionView.swift | 79 ++++++-- NetbirdKit/GlobalConstants.swift | 5 + NetbirdKit/NetworkExtensionAdapter.swift | 65 +++++- NetbirdKit/Preferences.swift | 32 +++ 6 files changed, 282 insertions(+), 100 deletions(-) diff --git a/NetBird/Source/App/Views/Components/SafariView.swift b/NetBird/Source/App/Views/Components/SafariView.swift index 89e572cd..75875074 100644 --- a/NetBird/Source/App/Views/Components/SafariView.swift +++ b/NetBird/Source/App/Views/Components/SafariView.swift @@ -2,104 +2,148 @@ // SafariView.swift // NetBird // -// iOS-only: Wraps ASWebAuthenticationSession for in-app web authentication. -// Uses ephemeral session so each login starts fresh (no shared cookies), -// which is required for multi-profile support. +// iOS-only: the in-app web-auth browser for interactive SSO login. One logic +// for every profile, default included: persistent cookies scoped to the +// profile (the IdP's trusted-device 2FA cookie survives, so a re-login skips +// the OTP prompt) and auto-close shortly after the OAuth loopback response — +// the behavior the pre-multi-profile SFSafariViewController version had, +// now with per-profile isolation. // import SwiftUI -// Safari is only available on iOS +// Only used on iOS (tvOS logs in via TVAuthView's device-code flow) #if os(iOS) -import AuthenticationServices +import WebKit -struct SafariView: UIViewControllerRepresentable { +/// Login browser used by every profile. Persistent cookies and auto-close a few +/// seconds after the OAuth loopback redirect, with each profile's cookies in its +/// own persistent WKWebsiteDataStore — the IdP's trusted-device 2FA cookie +/// survives re-logins per profile while profiles stay fully isolated from one +/// another. +struct ProfileLoginWebView: View { @Binding var isPresented: Bool + let profileName: String let url: URL - /// Called when the web auth session ends (success, user cancel, or error). - /// Note: with the NetBird PKCE loopback flow the completion fires with a nil - /// callbackURL even on success — the loopback redirect is consumed by the Go HTTP - /// server, not the auth session — so the caller must determine success from the - /// SDK's login callback, not from this handler. - let didFinish: () -> Void - - func makeUIViewController(context: Context) -> UIViewController { - let vc = UIViewController() - // Start the auth session after the VC is presented - DispatchQueue.main.async { - context.coordinator.startSession(from: vc) + /// `userCancelled` is true only for the explicit Cancel button. The auto-close + /// after the loopback redirect passes false — at that point the login may + /// STILL be completing on the SDK side (first-time profile registration can + /// outlast the close grace), so the caller must not treat it as a cancel. + let didFinish: (_ userCancelled: Bool) -> Void + + var body: some View { + VStack(spacing: 0) { + HStack { + Button("Cancel") { + isPresented = false + didFinish(true) + } + .padding(.horizontal, 16) + .padding(.vertical, 12) + Spacer() + } + .background(Color("BgMenu")) + + LoginWebViewRepresentable( + url: url, + storeIdentifier: Preferences.webStoreIdentifier(for: profileName), + onSuccessRedirect: { + isPresented = false + didFinish(false) + } + ) } - return vc + .background(Color("BgMenu").ignoresSafeArea()) } +} + +private struct LoginWebViewRepresentable: UIViewRepresentable { + let url: URL + let storeIdentifier: UUID + /// Called (on the main queue) shortly after the loopback response finished + /// loading — i.e. once the token exchange is done. The management login may + /// still be running; the caller defers to the SDK's result for that part. + let onSuccessRedirect: () -> Void - func updateUIViewController(_ uiViewController: UIViewController, context: Context) {} + func makeUIView(context: Context) -> WKWebView { + let config = WKWebViewConfiguration() + if #available(iOS 17.0, *) { + // Persistent, identifier-scoped store: this profile's cookies only. + config.websiteDataStore = WKWebsiteDataStore(forIdentifier: storeIdentifier) + } else { + // Pre-iOS 17 has no per-identifier persistent stores. Fall back to an + // isolated in-memory store — still fully isolated, but the trusted-device + // cookie won't survive, so these users re-enter the OTP on each login. + config.websiteDataStore = .nonPersistent() + } + let webView = WKWebView(frame: .zero, configuration: config) + webView.navigationDelegate = context.coordinator + webView.load(URLRequest(url: url)) + return webView + } + + func updateUIView(_ uiView: WKWebView, context: Context) {} func makeCoordinator() -> Coordinator { Coordinator(self) } - class Coordinator: NSObject, ASWebAuthenticationPresentationContextProviding { - let parent: SafariView - private var session: ASWebAuthenticationSession? + class Coordinator: NSObject, WKNavigationDelegate { + let parent: LoginWebViewRepresentable + private var successURLSeen = false + private var closeScheduled = false - init(_ parent: SafariView) { + init(_ parent: LoginWebViewRepresentable) { self.parent = parent } - func startSession(from viewController: UIViewController) { - // The NetBird SDK uses a PKCE flow with an http://localhost redirect URI. - // ASWebAuthenticationSession intercepts that navigation before the browser - // follows it, so "http" works as a callback scheme in practice. - // A proper long-term fix requires the SDK to expose a custom-scheme - // redirect URI (e.g. "netbird://") for mobile OAuth flows. - let completionHandler: ASWebAuthenticationSession.CompletionHandler = { [weak self] callbackURL, error in - guard let self else { return } - - DispatchQueue.main.async { - if let callbackURL = callbackURL { - print("Auth callback URL: \(callbackURL.absoluteString)") - } - if let error = error as? ASWebAuthenticationSessionError, - error.code == .canceledLogin { - print("User cancelled login") - } - self.parent.isPresented = false - self.parent.didFinish() - } + func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction, decisionHandler: @escaping (WKNavigationActionPolicy) -> Void) { + if !successURLSeen, + let urlString = navigationAction.request.url?.absoluteString, + Self.isSuccessURL(urlString) { + successURLSeen = true + print("Url is: \(urlString)") } + // Always allow — the loopback redirect must reach the SDK's local HTTP + // server, which is what actually receives the authorization code. + decisionHandler(.allow) + } - let session: ASWebAuthenticationSession - if #available(iOS 17.4, *) { - session = ASWebAuthenticationSession( - url: parent.url, - callback: .customScheme("http"), - completionHandler: completionHandler - ) - } else { - session = ASWebAuthenticationSession( - url: parent.url, - callbackURLScheme: "http", - completionHandler: completionHandler - ) - } + // Close only after the loopback RESPONSE arrived (the page finished + // loading), not on a timer from when the request started: the SDK's local + // server answers that request only once the token exchange is done, and + // tearing the web view down earlier cancels the request — which cancels + // the exchange itself and kills the login. After the response, only the + // management login remains; the adapter starts the VPN when it completes, + // so the window may close after a short glance at the success page. + func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { + scheduleCloseIfNeeded() + } - // Ephemeral = no shared cookies, fresh login every time - session.prefersEphemeralWebBrowserSession = true - session.presentationContextProvider = self - self.session = session - session.start() + // If the loopback request fails instead (e.g. the flow was already torn + // down server-side), don't leave the user staring at a blank page. + func webView(_ webView: WKWebView, didFail navigation: WKNavigation!, withError error: Error) { + scheduleCloseIfNeeded() } - func presentationAnchor(for session: ASWebAuthenticationSession) -> ASPresentationAnchor { - guard let keyWindow = UIApplication.shared.connectedScenes - .compactMap({ $0 as? UIWindowScene }) - .flatMap({ $0.windows }) - .first(where: { $0.isKeyWindow }) - else { - assertionFailure("No key window found — auth session may fail to present") - return UIWindow() + func webView(_ webView: WKWebView, didFailProvisionalNavigation navigation: WKNavigation!, withError error: Error) { + scheduleCloseIfNeeded() + } + + private func scheduleCloseIfNeeded() { + guard successURLSeen, !closeScheduled else { return } + closeScheduled = true + DispatchQueue.main.asyncAfter(deadline: .now() + 1.5) { + self.parent.onSuccessRedirect() } - return keyWindow + } + + // Same success pattern the pre-multi-profile SFSafariViewController + // version matched (minus its empty-string case, which cannot occur for + // a WKNavigationAction URL). + static func isSuccessURL(_ string: String) -> Bool { + let pattern = "^(http|https)://(localhost:53000/\\?code=.*|[a-zA-Z0-9.-]+/device/success)$" + return string.range(of: pattern, options: .regularExpression) != nil } } } diff --git a/NetBird/Source/App/Views/iOS/ProfilesListView.swift b/NetBird/Source/App/Views/iOS/ProfilesListView.swift index d548db8d..c1e36cbc 100644 --- a/NetBird/Source/App/Views/iOS/ProfilesListView.swift +++ b/NetBird/Source/App/Views/iOS/ProfilesListView.swift @@ -4,6 +4,7 @@ // import SwiftUI +import WebKit #if os(iOS) @@ -186,6 +187,18 @@ struct ProfilesListView: View { private func removeProfile(_ profile: Profile) { do { try ProfileManager.shared.removeProfile(profile.name) + // Delete the profile's login web store (cookies incl. the IdP's + // trusted-device session) so a future profile with the same name + // cannot inherit it. Done here rather than in ProfileManager because + // WebKit is unavailable in the network-extension target. + if let storeID = Preferences.removeWebStoreIdentifier(for: profile.name), + #available(iOS 17.0, *) { + WKWebsiteDataStore.remove(forIdentifier: storeID) { error in + if let error { + AppLogger.shared.log("Failed to remove web store for profile '\(profile.name)': \(error.localizedDescription)") + } + } + } loadProfiles() } catch { errorMessage = error.localizedDescription diff --git a/NetBird/Source/App/Views/iOS/iOSConnectionView.swift b/NetBird/Source/App/Views/iOS/iOSConnectionView.swift index 09682466..8cb3eac8 100644 --- a/NetBird/Source/App/Views/iOS/iOSConnectionView.swift +++ b/NetBird/Source/App/Views/iOS/iOSConnectionView.swift @@ -16,6 +16,10 @@ struct iOSConnectionView: View { @State private var ipv4Copied = false @State private var ipv6Copied = false @State private var showAddressDetails = false + /// True once loginBrowserDidFinish ran for the current login-browser sheet. + /// Lets the sheet's onDismiss distinguish a swipe-down (nothing handled yet → + /// user cancel) from a dismissal that followed the Cancel button / auto-close. + @State private var loginBrowserCompletionHandled = false var body: some View { ZStack { @@ -144,31 +148,70 @@ struct iOSConnectionView: View { } } - // Safari login view — shown regardless of statusDetailsValid - if viewModel.networkExtensionAdapter.showBrowser, - let loginURLString = viewModel.networkExtensionAdapter.loginURL, + } + .navigationBarTitleDisplayMode(.inline) + .navigationBarHidden(true) + // Login browser sheet — presented regardless of statusDetailsValid. One + // logic for every profile: persistent cookies scoped to the profile's own + // web store (the IdP's trusted-device 2FA cookie survives → no repeated + // OTP prompt) and auto-close after the OAuth loopback redirect. Profiles + // can never see each other's IdP sessions. + .sheet( + isPresented: $viewModel.networkExtensionAdapter.showBrowser, + onDismiss: { + // A swipe-down dismissal bypasses the browser's own callbacks — + // treat it as a user cancel so the pending login doesn't dangle. + // When the Cancel button or the auto-close already ran, + // loginBrowserDidFinish handled completion before the dismissal. + if !loginBrowserCompletionHandled { + loginBrowserDidFinish(userCancelled: true) + } + } + ) { + if let loginURLString = viewModel.networkExtensionAdapter.loginURL, let loginURL = URL(string: loginURLString) { - SafariView( + ProfileLoginWebView( isPresented: $viewModel.networkExtensionAdapter.showBrowser, + profileName: ProfileManager.shared.getActiveProfileName(), url: loginURL, - didFinish: { - if viewModel.networkExtensionAdapter.loginSucceeded { - print("Finish login") - viewModel.networkExtensionAdapter.startVPNConnection() - } else { - // User closed the browser without completing login. Do NOT start - // the VPN — that would launch the extension, trip its needs-login - // path, and pop a spurious "Login required" alert/notification. - print("Login cancelled by user") - viewModel.cancelPendingLogin() - } - } + didFinish: loginBrowserDidFinish ) + .onAppear { loginBrowserCompletionHandled = false } + } + } + } + + /// Completion for the login browser. + private func loginBrowserDidFinish(userCancelled: Bool) { + loginBrowserCompletionHandled = true + let adapter = viewModel.networkExtensionAdapter + if userCancelled { + // Explicit Cancel tap. Do NOT start the VPN — that would launch the + // extension, trip its needs-login path, and pop a spurious + // "Login required" alert/notification. + print("Login cancelled by user") + viewModel.cancelPendingLogin() + return + } + if adapter.loginSucceeded { + print("Finish login") + adapter.startVPNConnection() + return + } + // The browser auto-closed after the loopback redirect, but the SDK hasn't + // reported success yet — a first-time profile registration can outlast the + // browser's close grace. The adapter starts the VPN itself when success + // arrives (see performLogin's onSuccess); here only arm a fallback reset + // for the case where the login errors out instead of succeeding. + print("Login still completing after browser closed - deferring to SDK result") + DispatchQueue.main.asyncAfter(deadline: .now() + 20) { + let adapter = viewModel.networkExtensionAdapter + if !adapter.loginSucceeded && !adapter.showBrowser { + print("Login did not complete after browser closed - resetting") + viewModel.cancelPendingLogin() } } - .navigationBarTitleDisplayMode(.inline) - .navigationBarHidden(true) } @ViewBuilder diff --git a/NetbirdKit/GlobalConstants.swift b/NetbirdKit/GlobalConstants.swift index 16a571c9..396f60a0 100644 --- a/NetbirdKit/GlobalConstants.swift +++ b/NetbirdKit/GlobalConstants.swift @@ -34,6 +34,11 @@ struct GlobalConstants { static let stateFileName = "state.json" static let serverURLFileName = "netbird_server_url" + // Map of profile name → UUID of the persistent WKWebsiteDataStore used by the + // login browser. Every profile (default included) has its own store, so IdP + // sessions never leak between profiles. + static let keyProfileWebStoreIDs = "netbird.profileWebStoreIDs" + // Local notification identifiers static let notificationLoginRequired = "netbird.login.required" } diff --git a/NetbirdKit/NetworkExtensionAdapter.swift b/NetbirdKit/NetworkExtensionAdapter.swift index b094f45c..f15b379b 100644 --- a/NetbirdKit/NetworkExtensionAdapter.swift +++ b/NetbirdKit/NetworkExtensionAdapter.swift @@ -77,12 +77,14 @@ public class NetworkExtensionAdapter: ObservableObject { #if os(iOS) private var pendingAuth: NetBirdSDKAuth? /// Set to true by the SDK's onLoginSuccess callback (which fires once the Go PKCE - /// localhost server receives the OAuth callback). The browser-finished handler reads - /// this to tell a genuine login from the user dismissing the browser: the - /// ASWebAuthenticationSession completion fires with a nil callbackURL even on success - /// (the loopback redirect is consumed by the Go HTTP server, not the auth session), - /// so the SafariView callback alone cannot distinguish success from cancellation. + /// flow completes the token exchange and management login). The browser-finished + /// handler reads this to tell a genuine login apart from the user cancelling the + /// browser — the browser itself cannot make that distinction. public private(set) var loginSucceeded = false + /// True while an automatic identity-reset retry is in flight (see the + /// ownership-conflict handling in performLogin's error callback). Guards + /// against retrying more than once per login attempt. + private var identityResetAttempted = false #endif @Published var userCode: String? @@ -490,6 +492,9 @@ public class NetworkExtensionAdapter: ObservableObject { logger.info("performLogin: using management URL '\(activeManagementURL, privacy: .public)' for profile '\(activeProfile, privacy: .public)'") if let configPath = Preferences.configFile(), !configPath.isEmpty, let auth = NetBirdSDKNewAuth(configPath, activeManagementURL, nil) { + // A stale flow from an abandoned attempt would keep the loopback port + // bound (and its WaitToken goroutine alive) — stop it first. + self.pendingAuth?.stop() self.pendingAuth = auth self.loginSucceeded = false let urlOpener = MainAppLoginURLOpener() @@ -537,15 +542,54 @@ public class NetworkExtensionAdapter: ObservableObject { // observes it and starts the VPN instead of treating the browser // dismissal as a cancellation. DispatchQueue.main.async { - self?.loginSucceeded = true - self?.pendingAuth = nil + guard let self else { return } + self.logger.info("performLogin: SDK login succeeded") + self.identityResetAttempted = false + self.loginSucceeded = true + self.pendingAuth = nil + // If the browser already auto-closed (its close grace ended + // before the registration finished — typical for a first-time + // profile login), the finished-handler deferred to us: start + // the VPN now that the login is fully done. When the browser + // is still open, its finished-handler starts the VPN instead. + if !self.showBrowser { + self.logger.info("performLogin: login completed after browser closed - starting VPN") + self.startVPNConnection() + } } } errListener.onSuccessCallback = { urlOpener.onSuccess?() } - errListener.onErrorCallback = { [weak self] _ in - // onError runs on a background goroutine; mutate pendingAuth on the + errListener.onErrorCallback = { [weak self] error in + // Surface the reason in the logs — a login that dies AFTER the + // browser part (token exchange or management login failing) + // otherwise looks identical to "nothing happened". + let message = error?.localizedDescription ?? "unknown login error" + AppLogger.shared.log("performLogin: SDK login failed: \(message)") + // onError runs on a background goroutine; mutate state on the // main queue to stay consistent with onSuccess and cancelLogin(). - DispatchQueue.main.async { self?.pendingAuth = nil } + DispatchQueue.main.async { [weak self] in + guard let self else { return } + self.logger.error("performLogin: SDK login failed: \(message, privacy: .public)") + self.pendingAuth = nil + // "peer is already registered by a different User or a Setup + // Key": the profile's stored WireGuard key belongs to a peer + // the just-authenticated account doesn't own, so every retry + // with this identity is doomed. Self-heal: drop the profile's + // local identity (config + state; the server URL survives) and + // rerun the login once — the SDK then generates a fresh key + // and registers a NEW peer under the correct account, leaving + // the conflicting peer untouched. The retry's browser closes + // by itself: the IdP session cookie from the just-completed + // interactive login makes it a silent SSO redirect. + if message.contains("registered by a different User"), !self.identityResetAttempted { + self.identityResetAttempted = true + let profile = ProfileManager.shared.getActiveProfileName() + self.logger.warning("performLogin: peer identity conflicts with the logged-in account — resetting identity for '\(profile, privacy: .public)' and retrying login") + AppLogger.shared.log("performLogin: resetting identity for '\(profile)' after ownership conflict; retrying login") + try? ProfileManager.shared.logoutProfile(profile) + Task { await self.performLogin() } + } + } resume(nil) } // Pass the device name explicitly. The plain login() path uses an empty @@ -591,6 +635,7 @@ public class NetworkExtensionAdapter: ObservableObject { pendingAuth?.stop() pendingAuth = nil loginSucceeded = false + identityResetAttempted = false showBrowser = false } #endif diff --git a/NetbirdKit/Preferences.swift b/NetbirdKit/Preferences.swift index c9efdd6f..e2396435 100644 --- a/NetbirdKit/Preferences.swift +++ b/NetbirdKit/Preferences.swift @@ -178,6 +178,38 @@ class Preferences { return sharedUserDefaults()?.string(forKey: managementURLKey) } + // MARK: - Per-Profile Login Web-Store Identifiers + // + // Every profile logs in through a WKWebView whose persistent + // WKWebsiteDataStore is keyed by a stable per-profile UUID (iOS 17+). That + // store keeps the IdP's trusted-device 2FA cookie between re-logins, while + // every profile stays fully isolated in its own cookie jar. + + /// Stable identifier of the profile's persistent login web store, created on + /// first use. + static func webStoreIdentifier(for profile: String) -> UUID { + let defaults = sharedUserDefaults() + var map = defaults?.dictionary(forKey: GlobalConstants.keyProfileWebStoreIDs) as? [String: String] ?? [:] + if let existing = map[profile], let uuid = UUID(uuidString: existing) { + return uuid + } + let uuid = UUID() + map[profile] = uuid.uuidString + defaults?.set(map, forKey: GlobalConstants.keyProfileWebStoreIDs) + return uuid + } + + /// Forgets the profile's web-store identifier, returning it so the caller can + /// delete the underlying WKWebsiteDataStore (WebKit is not linked here — this + /// file is also compiled into the network extension). + static func removeWebStoreIdentifier(for profile: String) -> UUID? { + let defaults = sharedUserDefaults() + var map = defaults?.dictionary(forKey: GlobalConstants.keyProfileWebStoreIDs) as? [String: String] ?? [:] + guard let existing = map.removeValue(forKey: profile) else { return nil } + defaults?.set(map, forKey: GlobalConstants.keyProfileWebStoreIDs) + return UUID(uuidString: existing) + } + /// Restore config from UserDefaults to the config file path. /// iOS only - needed because the Go SDK reads from the file path. #if os(iOS) From 75d1a0b5d777a23a9c719ee8261390c4ef702cda Mon Sep 17 00:00:00 2001 From: evgeniyChepelev <68751844+evgeniyChepelev@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:48:25 +0200 Subject: [PATCH 2/8] fix(ios): address review findings in the SSO login flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit don't log the loopback redirect URL — its query carries the OAuth authorization code; log only the fact and host - rearm the login-sheet completion flag in onDismiss instead of the content's onAppear, and dismiss-and-cancel defensively if the sheet is ever presented without a valid login URL - tag each login attempt with a token and check it in the 20 s post-dismissal fallback, so a stale timer can't cancel a newer attempt (including the automatic identity-reset retry) - harden the ownership-conflict self-heal: skip the retry when the identity reset itself fails (try? previously hid the error and the retry re-presented the same conflicting key) and gate it on the browser phase having started, so it can't race the IPC fallback - document why navigation-failure handlers act only after the loopback redirect: pre-success failures include benign NSURLErrorCancelled from superseded navigations, and reacting to them would abort healthy logins --- .../App/Views/Components/SafariView.swift | 18 ++++++++---- .../App/Views/iOS/iOSConnectionView.swift | 20 +++++++++++-- NetbirdKit/NetworkExtensionAdapter.swift | 29 +++++++++++++++---- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/NetBird/Source/App/Views/Components/SafariView.swift b/NetBird/Source/App/Views/Components/SafariView.swift index 75875074..79342ef4 100644 --- a/NetBird/Source/App/Views/Components/SafariView.swift +++ b/NetBird/Source/App/Views/Components/SafariView.swift @@ -99,10 +99,12 @@ private struct LoginWebViewRepresentable: UIViewRepresentable { func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction, decisionHandler: @escaping (WKNavigationActionPolicy) -> Void) { if !successURLSeen, - let urlString = navigationAction.request.url?.absoluteString, - Self.isSuccessURL(urlString) { + let url = navigationAction.request.url, + Self.isSuccessURL(url.absoluteString) { successURLSeen = true - print("Url is: \(urlString)") + // Don't log the URL itself — its query carries the OAuth + // authorization code. + print("Login success redirect detected (host: \(url.host ?? "?"))") } // Always allow — the loopback redirect must reach the SDK's local HTTP // server, which is what actually receives the authorization code. @@ -120,8 +122,14 @@ private struct LoginWebViewRepresentable: UIViewRepresentable { scheduleCloseIfNeeded() } - // If the loopback request fails instead (e.g. the flow was already torn - // down server-side), don't leave the user staring at a blank page. + // Post-success failures only (scheduleCloseIfNeeded guards on + // successURLSeen): if the loopback request fails AFTER the redirect was + // seen (e.g. the flow was already torn down server-side), close instead + // of leaving the user on a blank page. Failures BEFORE the redirect are + // deliberately ignored: WebKit reports benign NSURLErrorCancelled (-999) + // whenever one navigation supersedes another mid-redirect-chain, so + // reacting to pre-success failures would abort healthy logins. If the + // IdP genuinely fails to load, the user backs out via Cancel. func webView(_ webView: WKWebView, didFail navigation: WKNavigation!, withError error: Error) { scheduleCloseIfNeeded() } diff --git a/NetBird/Source/App/Views/iOS/iOSConnectionView.swift b/NetBird/Source/App/Views/iOS/iOSConnectionView.swift index 8cb3eac8..299fb4ee 100644 --- a/NetBird/Source/App/Views/iOS/iOSConnectionView.swift +++ b/NetBird/Source/App/Views/iOS/iOSConnectionView.swift @@ -166,6 +166,10 @@ struct iOSConnectionView: View { if !loginBrowserCompletionHandled { loginBrowserDidFinish(userCancelled: true) } + // Rearm here, not in the content's onAppear: every dismissal path + // ends in this handler, so the next presentation always starts + // unhandled even if the content branch never rendered. + loginBrowserCompletionHandled = false } ) { if let loginURLString = viewModel.networkExtensionAdapter.loginURL, @@ -177,7 +181,14 @@ struct iOSConnectionView: View { url: loginURL, didFinish: loginBrowserDidFinish ) - .onAppear { loginBrowserCompletionHandled = false } + } else { + // Defensive: the adapter sets loginURL before showBrowser, so this + // should be unreachable. Dismiss immediately instead of showing a + // blank sheet; onDismiss then cancels the pending login. + Color.clear.onAppear { + print("Login browser presented without a valid URL - dismissing") + viewModel.networkExtensionAdapter.showBrowser = false + } } } } @@ -205,9 +216,14 @@ struct iOSConnectionView: View { // arrives (see performLogin's onSuccess); here only arm a fallback reset // for the case where the login errors out instead of succeeding. print("Login still completing after browser closed - deferring to SDK result") + // Capture the attempt token so this timer can only act on the attempt it + // was armed for — a retry within the window would otherwise be killed by + // the stale timer once its own browser has closed. + let attemptToken = adapter.loginAttemptToken DispatchQueue.main.asyncAfter(deadline: .now() + 20) { let adapter = viewModel.networkExtensionAdapter - if !adapter.loginSucceeded && !adapter.showBrowser { + if adapter.loginAttemptToken == attemptToken, + !adapter.loginSucceeded, !adapter.showBrowser { print("Login did not complete after browser closed - resetting") viewModel.cancelPendingLogin() } diff --git a/NetbirdKit/NetworkExtensionAdapter.swift b/NetbirdKit/NetworkExtensionAdapter.swift index f15b379b..e9444f80 100644 --- a/NetbirdKit/NetworkExtensionAdapter.swift +++ b/NetbirdKit/NetworkExtensionAdapter.swift @@ -85,6 +85,11 @@ public class NetworkExtensionAdapter: ObservableObject { /// ownership-conflict handling in performLogin's error callback). Guards /// against retrying more than once per login attempt. private var identityResetAttempted = false + /// Incremented at every performLogin entry. Deferred actions armed for one + /// attempt (e.g. the view's post-dismissal fallback cancel) capture the + /// current value and compare before acting, so a stale timer can never + /// abort a newer attempt. + public private(set) var loginAttemptToken = 0 #endif @Published var userCode: String? @@ -496,9 +501,15 @@ public class NetworkExtensionAdapter: ObservableObject { // bound (and its WaitToken goroutine alive) — stop it first. self.pendingAuth?.stop() self.pendingAuth = auth + self.loginAttemptToken += 1 self.loginSucceeded = false let urlOpener = MainAppLoginURLOpener() let errListener = MainAppLoginErrListener() + // Set (on the main queue) once the browser actually opened. Gates the + // ownership-conflict retry below: an error BEFORE the browser phase + // falls through to the IPC fallback, and running a retry concurrently + // with it would race two login flows. + var browserPhaseStarted = false let receivedURL: String? = await withCheckedContinuation { continuation in var resumed = false @@ -516,6 +527,7 @@ public class NetworkExtensionAdapter: ObservableObject { // in parallel with this browser login. Ordering them guarantees the // await caller sees showBrowser == true. DispatchQueue.main.async { + browserPhaseStarted = true self?.loginURL = url self?.showBrowser = true resume(url) @@ -581,13 +593,20 @@ public class NetworkExtensionAdapter: ObservableObject { // the conflicting peer untouched. The retry's browser closes // by itself: the IdP session cookie from the just-completed // interactive login makes it a silent SSO redirect. - if message.contains("registered by a different User"), !self.identityResetAttempted { + if message.contains("registered by a different User"), !self.identityResetAttempted, browserPhaseStarted { self.identityResetAttempted = true let profile = ProfileManager.shared.getActiveProfileName() - self.logger.warning("performLogin: peer identity conflicts with the logged-in account — resetting identity for '\(profile, privacy: .public)' and retrying login") - AppLogger.shared.log("performLogin: resetting identity for '\(profile)' after ownership conflict; retrying login") - try? ProfileManager.shared.logoutProfile(profile) - Task { await self.performLogin() } + do { + try ProfileManager.shared.logoutProfile(profile) + self.logger.warning("performLogin: peer identity conflicts with the logged-in account — resetting identity for '\(profile, privacy: .public)' and retrying login") + AppLogger.shared.log("performLogin: resetting identity for '\(profile)' after ownership conflict; retrying login") + Task { await self.performLogin() } + } catch { + // Retrying without a successful reset would present the + // same conflicting identity again — log and stop instead. + self.logger.error("performLogin: identity reset for '\(profile, privacy: .public)' failed: \(error.localizedDescription, privacy: .public)") + AppLogger.shared.log("performLogin: identity reset for '\(profile)' failed: \(error.localizedDescription)") + } } } resume(nil) From 876c5a4103aa3e3b365f3993312a7a0f2b9748fe Mon Sep 17 00:00:00 2001 From: evgeniyChepelev <68751844+evgeniyChepelev@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:51:59 +0200 Subject: [PATCH 3/8] fix(ios): run SSO login in the system auth session instead of a webview --- NetBird/Info.plist | 10 + .../App/Views/Components/SafariView.swift | 248 +++++++++--------- .../App/Views/iOS/ProfilesListView.swift | 13 - .../App/Views/iOS/iOSConnectionView.swift | 107 +++----- NetbirdKit/GlobalConstants.swift | 9 +- NetbirdKit/NetworkExtensionAdapter.swift | 239 ++++++++++++++--- NetbirdKit/Preferences.swift | 51 ++-- NetbirdKit/ProfileManager.swift | 6 + 8 files changed, 407 insertions(+), 276 deletions(-) diff --git a/NetBird/Info.plist b/NetBird/Info.plist index 9db8a085..9040594f 100644 --- a/NetBird/Info.plist +++ b/NetBird/Info.plist @@ -18,5 +18,15 @@ + + NSAppTransportSecurity + + NSAllowsLocalNetworking + + diff --git a/NetBird/Source/App/Views/Components/SafariView.swift b/NetBird/Source/App/Views/Components/SafariView.swift index 79342ef4..264c8197 100644 --- a/NetBird/Source/App/Views/Components/SafariView.swift +++ b/NetBird/Source/App/Views/Components/SafariView.swift @@ -2,156 +2,162 @@ // SafariView.swift // NetBird // -// iOS-only: the in-app web-auth browser for interactive SSO login. One logic -// for every profile, default included: persistent cookies scoped to the -// profile (the IdP's trusted-device 2FA cookie survives, so a re-login skips -// the OTP prompt) and auto-close shortly after the OAuth loopback response — -// the behavior the pre-multi-profile SFSafariViewController version had, -// now with per-profile isolation. +// iOS-only: runs the interactive OAuth/SSO login in ASWebAuthenticationSession — +// the system authentication session, i.e. an external user-agent in the sense of +// RFC 8252. The page runs in Safari's own process, outside the app's reach, which +// is what keeps IdP-side features working: Google refuses OAuth in embedded +// webviews (disallowed_useragent), passkeys/WebAuthn and hardware security keys +// need a Safari-class agent, and enterprise policies (Okta, Entra conditional +// access) may block embedded webviews. +// +// The session is NOT ephemeral: it shares Safari's cookie jar, so the IdP's +// trusted-device cookie survives and a re-login does not re-prompt for the second +// factor. That single jar is shared by every profile, so profile isolation is +// enforced on the request instead — the adapter adds prompt=select_account when a +// login targets a different profile than the last authenticated one. // import SwiftUI -// Only used on iOS (tvOS logs in via TVAuthView's device-code flow) +// Safari is only available on iOS #if os(iOS) -import WebKit - -/// Login browser used by every profile. Persistent cookies and auto-close a few -/// seconds after the OAuth loopback redirect, with each profile's cookies in its -/// own persistent WKWebsiteDataStore — the IdP's trusted-device 2FA cookie -/// survives re-logins per profile while profiles stay fully isolated from one -/// another. -struct ProfileLoginWebView: View { - @Binding var isPresented: Bool - let profileName: String - let url: URL - /// `userCancelled` is true only for the explicit Cancel button. The auto-close - /// after the loopback redirect passes false — at that point the login may - /// STILL be completing on the SDK side (first-time profile registration can - /// outlast the close grace), so the caller must not treat it as a cancel. - let didFinish: (_ userCancelled: Bool) -> Void - - var body: some View { - VStack(spacing: 0) { - HStack { - Button("Cancel") { - isPresented = false - didFinish(true) - } - .padding(.horizontal, 16) - .padding(.vertical, 12) - Spacer() - } - .background(Color("BgMenu")) - - LoginWebViewRepresentable( - url: url, - storeIdentifier: Preferences.webStoreIdentifier(for: profileName), - onSuccessRedirect: { - isPresented = false - didFinish(false) - } - ) - } - .background(Color("BgMenu").ignoresSafeArea()) - } +import AuthenticationServices + +/// How the login browser ended. The session cannot report whether the login as a +/// whole succeeded — the OAuth code is only the first half, the SDK still has to +/// exchange it and register with the management server — so the caller resolves +/// the final outcome through the SDK's callbacks. +enum LoginBrowserOutcome { + /// The session captured the loopback redirect itself instead of letting the + /// browser follow it. The URL carries the authorization code and has been + /// replayed to the SDK's local server, so the flow continues. + case redirectCaptured + /// The browser was dismissed. Either the user cancelled, or they closed the + /// SDK's success page after the redirect already went through — the two are + /// indistinguishable here. + case closed + /// The session itself failed (could not present, or an OAuth error came back). + case failed(Error) } -private struct LoginWebViewRepresentable: UIViewRepresentable { +struct SafariView: UIViewControllerRepresentable { + @Binding var isPresented: Bool let url: URL - let storeIdentifier: UUID - /// Called (on the main queue) shortly after the loopback response finished - /// loading — i.e. once the token exchange is done. The management login may - /// still be running; the caller defers to the SDK's result for that part. - let onSuccessRedirect: () -> Void - - func makeUIView(context: Context) -> WKWebView { - let config = WKWebViewConfiguration() - if #available(iOS 17.0, *) { - // Persistent, identifier-scoped store: this profile's cookies only. - config.websiteDataStore = WKWebsiteDataStore(forIdentifier: storeIdentifier) - } else { - // Pre-iOS 17 has no per-identifier persistent stores. Fall back to an - // isolated in-memory store — still fully isolated, but the trusted-device - // cookie won't survive, so these users re-enter the OTP on each login. - config.websiteDataStore = .nonPersistent() + let didFinish: (LoginBrowserOutcome) -> Void + + func makeUIViewController(context: Context) -> UIViewController { + let vc = UIViewController() + // Start the auth session after the VC is presented + DispatchQueue.main.async { + context.coordinator.startSession(from: vc) } - let webView = WKWebView(frame: .zero, configuration: config) - webView.navigationDelegate = context.coordinator - webView.load(URLRequest(url: url)) - return webView + return vc } - func updateUIView(_ uiView: WKWebView, context: Context) {} + func updateUIViewController(_ uiViewController: UIViewController, context: Context) {} + + static func dismantleUIViewController(_ uiViewController: UIViewController, coordinator: Coordinator) { + // Programmatic teardown (e.g. the login was aborted elsewhere): the session + // presents its own window, which outlives this hosting VC unless cancelled. + coordinator.cancelSession() + } func makeCoordinator() -> Coordinator { Coordinator(self) } - class Coordinator: NSObject, WKNavigationDelegate { - let parent: LoginWebViewRepresentable - private var successURLSeen = false - private var closeScheduled = false + class Coordinator: NSObject, ASWebAuthenticationPresentationContextProviding { + let parent: SafariView + private var session: ASWebAuthenticationSession? - init(_ parent: LoginWebViewRepresentable) { + init(_ parent: SafariView) { self.parent = parent } - func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction, decisionHandler: @escaping (WKNavigationActionPolicy) -> Void) { - if !successURLSeen, - let url = navigationAction.request.url, - Self.isSuccessURL(url.absoluteString) { - successURLSeen = true - // Don't log the URL itself — its query carries the OAuth - // authorization code. - print("Login success redirect detected (host: \(url.host ?? "?"))") + func startSession(from viewController: UIViewController) { + let completionHandler: ASWebAuthenticationSession.CompletionHandler = { [weak self] callbackURL, error in + guard let self else { return } + + let outcome: LoginBrowserOutcome + if let callbackURL { + // The session matched the loopback redirect and swallowed it, so + // the SDK's local HTTP server never saw the authorization code. + // Replay the request to hand the code over; without this the PKCE + // flow would wait until it expires. Harmless when the browser + // already delivered it — the SDK's server is gone by then and the + // request simply fails. + Self.replayToLoopback(callbackURL) + outcome = .redirectCaptured + } else if let error = error as? ASWebAuthenticationSessionError, + error.code == .canceledLogin { + outcome = .closed + } else if let error { + outcome = .failed(error) + } else { + outcome = .closed + } + + DispatchQueue.main.async { + self.session = nil + self.parent.isPresented = false + self.parent.didFinish(outcome) + } + } + + // The SDK's PKCE flow redirects to http://localhost:. Declaring + // "http" as the callback scheme covers the case where the session + // intercepts that navigation; when it instead lets the browser follow + // the redirect, the SDK's local server receives the code directly and + // this session ends via .closed once the user dismisses the success page. + // Either path is handled — see the completion handler above. + let session: ASWebAuthenticationSession + if #available(iOS 17.4, *) { + session = ASWebAuthenticationSession( + url: parent.url, + callback: .customScheme("http"), + completionHandler: completionHandler + ) + } else { + session = ASWebAuthenticationSession( + url: parent.url, + callbackURLScheme: "http", + completionHandler: completionHandler + ) } - // Always allow — the loopback redirect must reach the SDK's local HTTP - // server, which is what actually receives the authorization code. - decisionHandler(.allow) - } - // Close only after the loopback RESPONSE arrived (the page finished - // loading), not on a timer from when the request started: the SDK's local - // server answers that request only once the token exchange is done, and - // tearing the web view down earlier cancels the request — which cancels - // the exchange itself and kills the login. After the response, only the - // management login remains; the adapter starts the VPN when it completes, - // so the window may close after a short glance at the success page. - func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { - scheduleCloseIfNeeded() + // Non-ephemeral: shares Safari's cookie jar so the IdP's trusted-device + // cookie survives between logins and the second factor is not re-prompted + // on every re-login. Cross-profile isolation is enforced by the + // prompt=select_account the adapter adds when the profile changes. + session.prefersEphemeralWebBrowserSession = false + session.presentationContextProvider = self + self.session = session + session.start() } - // Post-success failures only (scheduleCloseIfNeeded guards on - // successURLSeen): if the loopback request fails AFTER the redirect was - // seen (e.g. the flow was already torn down server-side), close instead - // of leaving the user on a blank page. Failures BEFORE the redirect are - // deliberately ignored: WebKit reports benign NSURLErrorCancelled (-999) - // whenever one navigation supersedes another mid-redirect-chain, so - // reacting to pre-success failures would abort healthy logins. If the - // IdP genuinely fails to load, the user backs out via Cancel. - func webView(_ webView: WKWebView, didFail navigation: WKNavigation!, withError error: Error) { - scheduleCloseIfNeeded() + /// Dismisses the session UI. Safe to call after it already completed. + func cancelSession() { + session?.cancel() + session = nil } - func webView(_ webView: WKWebView, didFailProvisionalNavigation navigation: WKNavigation!, withError error: Error) { - scheduleCloseIfNeeded() + /// Hands an intercepted authorization code to the SDK's loopback server. + private static func replayToLoopback(_ callbackURL: URL) { + var request = URLRequest(url: callbackURL) + request.timeoutInterval = 10 + URLSession.shared.dataTask(with: request).resume() } - private func scheduleCloseIfNeeded() { - guard successURLSeen, !closeScheduled else { return } - closeScheduled = true - DispatchQueue.main.asyncAfter(deadline: .now() + 1.5) { - self.parent.onSuccessRedirect() + func presentationAnchor(for session: ASWebAuthenticationSession) -> ASPresentationAnchor { + guard let keyWindow = UIApplication.shared.connectedScenes + .compactMap({ $0 as? UIWindowScene }) + .flatMap({ $0.windows }) + .first(where: { $0.isKeyWindow }) + else { + assertionFailure("No key window found — auth session may fail to present") + return UIWindow() } - } - - // Same success pattern the pre-multi-profile SFSafariViewController - // version matched (minus its empty-string case, which cannot occur for - // a WKNavigationAction URL). - static func isSuccessURL(_ string: String) -> Bool { - let pattern = "^(http|https)://(localhost:53000/\\?code=.*|[a-zA-Z0-9.-]+/device/success)$" - return string.range(of: pattern, options: .regularExpression) != nil + return keyWindow } } } diff --git a/NetBird/Source/App/Views/iOS/ProfilesListView.swift b/NetBird/Source/App/Views/iOS/ProfilesListView.swift index c1e36cbc..d548db8d 100644 --- a/NetBird/Source/App/Views/iOS/ProfilesListView.swift +++ b/NetBird/Source/App/Views/iOS/ProfilesListView.swift @@ -4,7 +4,6 @@ // import SwiftUI -import WebKit #if os(iOS) @@ -187,18 +186,6 @@ struct ProfilesListView: View { private func removeProfile(_ profile: Profile) { do { try ProfileManager.shared.removeProfile(profile.name) - // Delete the profile's login web store (cookies incl. the IdP's - // trusted-device session) so a future profile with the same name - // cannot inherit it. Done here rather than in ProfileManager because - // WebKit is unavailable in the network-extension target. - if let storeID = Preferences.removeWebStoreIdentifier(for: profile.name), - #available(iOS 17.0, *) { - WKWebsiteDataStore.remove(forIdentifier: storeID) { error in - if let error { - AppLogger.shared.log("Failed to remove web store for profile '\(profile.name)': \(error.localizedDescription)") - } - } - } loadProfiles() } catch { errorMessage = error.localizedDescription diff --git a/NetBird/Source/App/Views/iOS/iOSConnectionView.swift b/NetBird/Source/App/Views/iOS/iOSConnectionView.swift index 299fb4ee..a3af6549 100644 --- a/NetBird/Source/App/Views/iOS/iOSConnectionView.swift +++ b/NetBird/Source/App/Views/iOS/iOSConnectionView.swift @@ -16,10 +16,6 @@ struct iOSConnectionView: View { @State private var ipv4Copied = false @State private var ipv6Copied = false @State private var showAddressDetails = false - /// True once loginBrowserDidFinish ran for the current login-browser sheet. - /// Lets the sheet's onDismiss distinguish a swipe-down (nothing handled yet → - /// user cancel) from a dismissal that followed the Cancel button / auto-close. - @State private var loginBrowserCompletionHandled = false var body: some View { ZStack { @@ -148,83 +144,56 @@ struct iOSConnectionView: View { } } - } - .navigationBarTitleDisplayMode(.inline) - .navigationBarHidden(true) - // Login browser sheet — presented regardless of statusDetailsValid. One - // logic for every profile: persistent cookies scoped to the profile's own - // web store (the IdP's trusted-device 2FA cookie survives → no repeated - // OTP prompt) and auto-close after the OAuth loopback redirect. Profiles - // can never see each other's IdP sessions. - .sheet( - isPresented: $viewModel.networkExtensionAdapter.showBrowser, - onDismiss: { - // A swipe-down dismissal bypasses the browser's own callbacks — - // treat it as a user cancel so the pending login doesn't dangle. - // When the Cancel button or the auto-close already ran, - // loginBrowserDidFinish handled completion before the dismissal. - if !loginBrowserCompletionHandled { - loginBrowserDidFinish(userCancelled: true) - } - // Rearm here, not in the content's onAppear: every dismissal path - // ends in this handler, so the next presentation always starts - // unhandled even if the content branch never rendered. - loginBrowserCompletionHandled = false - } - ) { - if let loginURLString = viewModel.networkExtensionAdapter.loginURL, + // System auth session — started regardless of statusDetailsValid. It + // presents its own modal window, so this view only hosts the launcher. + if viewModel.networkExtensionAdapter.showBrowser, + let loginURLString = viewModel.networkExtensionAdapter.loginURL, let loginURL = URL(string: loginURLString) { - ProfileLoginWebView( + SafariView( isPresented: $viewModel.networkExtensionAdapter.showBrowser, - profileName: ProfileManager.shared.getActiveProfileName(), url: loginURL, didFinish: loginBrowserDidFinish ) - } else { - // Defensive: the adapter sets loginURL before showBrowser, so this - // should be unreachable. Dismiss immediately instead of showing a - // blank sheet; onDismiss then cancels the pending login. - Color.clear.onAppear { - print("Login browser presented without a valid URL - dismissing") - viewModel.networkExtensionAdapter.showBrowser = false - } } } + .navigationBarTitleDisplayMode(.inline) + .navigationBarHidden(true) } - /// Completion for the login browser. - private func loginBrowserDidFinish(userCancelled: Bool) { - loginBrowserCompletionHandled = true + /// Resolves what the login browser's end means for the VPN. + private func loginBrowserDidFinish(_ outcome: LoginBrowserOutcome) { let adapter = viewModel.networkExtensionAdapter - if userCancelled { - // Explicit Cancel tap. Do NOT start the VPN — that would launch the - // extension, trip its needs-login path, and pop a spurious - // "Login required" alert/notification. - print("Login cancelled by user") + + switch outcome { + case .failed(let error): + print("Login browser failed: \(error.localizedDescription)") + AppLogger.shared.log("Login browser failed: \(error.localizedDescription)") viewModel.cancelPendingLogin() - return - } - if adapter.loginSucceeded { - print("Finish login") - adapter.startVPNConnection() - return - } - // The browser auto-closed after the loopback redirect, but the SDK hasn't - // reported success yet — a first-time profile registration can outlast the - // browser's close grace. The adapter starts the VPN itself when success - // arrives (see performLogin's onSuccess); here only arm a fallback reset - // for the case where the login errors out instead of succeeding. - print("Login still completing after browser closed - deferring to SDK result") - // Capture the attempt token so this timer can only act on the attempt it - // was armed for — a retry within the window would otherwise be killed by - // the stale timer once its own browser has closed. - let attemptToken = adapter.loginAttemptToken - DispatchQueue.main.asyncAfter(deadline: .now() + 20) { - let adapter = viewModel.networkExtensionAdapter - if adapter.loginAttemptToken == attemptToken, - !adapter.loginSucceeded, !adapter.showBrowser { - print("Login did not complete after browser closed - resetting") + + case .redirectCaptured: + // The authorization code was handed to the SDK; the rest of the login + // runs there. The adapter starts the VPN once it reports success. + print("Login redirect captured - waiting for the SDK to finish") + adapter.resolveLoginAfterBrowserClose { + print("Login did not complete after redirect - resetting") + viewModel.cancelPendingLogin() + } + + case .closed: + if adapter.loginSucceeded { + print("Finish login") + adapter.startVPNConnection() + return + } + // Ambiguous: the user may have cancelled, or closed the SDK's success + // page while registration was still running. Never start the VPN here — + // that would launch the extension, trip its needs-login path and pop a + // spurious "Login required" alert. Let the adapter decide, and only + // reset the UI if it concludes the login is not in flight. + print("Login browser closed without a reported success - resolving") + adapter.resolveLoginAfterBrowserClose { + print("Login cancelled or failed - resetting") viewModel.cancelPendingLogin() } } diff --git a/NetbirdKit/GlobalConstants.swift b/NetbirdKit/GlobalConstants.swift index 396f60a0..6e0820f1 100644 --- a/NetbirdKit/GlobalConstants.swift +++ b/NetbirdKit/GlobalConstants.swift @@ -34,10 +34,11 @@ struct GlobalConstants { static let stateFileName = "state.json" static let serverURLFileName = "netbird_server_url" - // Map of profile name → UUID of the persistent WKWebsiteDataStore used by the - // login browser. Every profile (default included) has its own store, so IdP - // sessions never leak between profiles. - static let keyProfileWebStoreIDs = "netbird.profileWebStoreIDs" + // Profile whose account the login browser last authenticated. The system auth + // session shares one Safari cookie jar across all profiles, so a login for any + // other profile must force account selection instead of silently reusing that + // session. + static let keyLastAuthenticatedProfile = "netbird.lastAuthenticatedProfile" // Local notification identifiers static let notificationLoginRequired = "netbird.login.required" diff --git a/NetbirdKit/NetworkExtensionAdapter.swift b/NetbirdKit/NetworkExtensionAdapter.swift index e9444f80..993d7361 100644 --- a/NetbirdKit/NetworkExtensionAdapter.swift +++ b/NetbirdKit/NetworkExtensionAdapter.swift @@ -9,6 +9,7 @@ import Foundation import NetworkExtension import SwiftUI import Combine +import Network import NetBirdSDK import os @@ -76,20 +77,21 @@ public class NetworkExtensionAdapter: ObservableObject { @Published var loginURL: String? #if os(iOS) private var pendingAuth: NetBirdSDKAuth? - /// Set to true by the SDK's onLoginSuccess callback (which fires once the Go PKCE - /// flow completes the token exchange and management login). The browser-finished - /// handler reads this to tell a genuine login apart from the user cancelling the - /// browser — the browser itself cannot make that distinction. + /// Set to true by the SDK's onLoginSuccess callback, which fires only once the + /// whole flow is done: authorization code exchanged AND the peer registered with + /// the management server. The browser cannot report this — its completion looks + /// the same whether the user cancelled or closed the SDK's success page — so + /// every "did the login work" decision reads this flag. public private(set) var loginSucceeded = false - /// True while an automatic identity-reset retry is in flight (see the - /// ownership-conflict handling in performLogin's error callback). Guards - /// against retrying more than once per login attempt. + /// Guards the ownership-conflict self-heal so it retries at most once per login. private var identityResetAttempted = false - /// Incremented at every performLogin entry. Deferred actions armed for one - /// attempt (e.g. the view's post-dismissal fallback cancel) capture the - /// current value and compare before acting, so a stale timer can never + /// Incremented on every performLogin entry. Deferred work armed for one attempt + /// captures the value and compares before acting, so a stale timer can never /// abort a newer attempt. public private(set) var loginAttemptToken = 0 + /// Authorization URL of the in-flight login, used to locate the SDK's loopback + /// listener when deciding whether a closed browser means "cancelled". + private var pendingAuthorizeURL: String? #endif @Published var userCode: String? @@ -495,20 +497,28 @@ public class NetworkExtensionAdapter: ObservableObject { } let activeManagementURL = resolvedURL ?? "" logger.info("performLogin: using management URL '\(activeManagementURL, privacy: .public)' for profile '\(activeProfile, privacy: .public)'") + // The system auth session shares one Safari cookie jar across every profile, + // and the app cannot scope or clear it. So when this login targets a profile + // other than the one that jar was last authenticated for, force the IdP to + // ask which account to use rather than silently reusing the live session. + let lastAuthenticated = Preferences.loadLastAuthenticatedProfile() + let needsAccountSelection = lastAuthenticated != activeProfile + if needsAccountSelection { + logger.info("performLogin: profile changed (last authenticated: \(lastAuthenticated ?? "none", privacy: .public)) — forcing account selection") + } if let configPath = Preferences.configFile(), !configPath.isEmpty, let auth = NetBirdSDKNewAuth(configPath, activeManagementURL, nil) { - // A stale flow from an abandoned attempt would keep the loopback port - // bound (and its WaitToken goroutine alive) — stop it first. + // A stale flow from an abandoned attempt would keep its loopback port + // bound and its WaitToken goroutine alive — stop it first. self.pendingAuth?.stop() self.pendingAuth = auth self.loginAttemptToken += 1 self.loginSucceeded = false let urlOpener = MainAppLoginURLOpener() let errListener = MainAppLoginErrListener() - // Set (on the main queue) once the browser actually opened. Gates the - // ownership-conflict retry below: an error BEFORE the browser phase - // falls through to the IPC fallback, and running a retry concurrently - // with it would race two login flows. + // Set once the browser actually opened. Gates the ownership-conflict + // self-heal below: an error before the browser phase falls through to the + // IPC fallback, and retrying concurrently with it would race two flows. var browserPhaseStarted = false let receivedURL: String? = await withCheckedContinuation { continuation in @@ -526,9 +536,13 @@ public class NetworkExtensionAdapter: ObservableObject { // the extension, trips its needsLogin path, and pops the auth alert // in parallel with this browser login. Ordering them guarantees the // await caller sees showBrowser == true. + let browserURL = needsAccountSelection + ? Self.urlForcingAccountSelection(url) + : url DispatchQueue.main.async { browserPhaseStarted = true - self?.loginURL = url + self?.pendingAuthorizeURL = browserURL + self?.loginURL = browserURL self?.showBrowser = true resume(url) } @@ -549,21 +563,28 @@ public class NetworkExtensionAdapter: ObservableObject { ProfileManager.shared.saveServerURL(activeManagementURL, for: activeProfile) Preferences.saveManagementURL(activeManagementURL) } + // This profile's account now owns the shared browser session — + // record it so its own re-logins stay silent while a login for + // any other profile forces account selection. + Preferences.saveLastAuthenticatedProfile(activeProfile) + AppLogger.shared.log("performLogin: SDK login succeeded for '\(activeProfile)'") // onSuccess runs on a background goroutine. Mark success on the main // queue so the browser-finished handler (also main-queue) reliably - // observes it and starts the VPN instead of treating the browser - // dismissal as a cancellation. + // observes it. DispatchQueue.main.async { guard let self else { return } + // Success is delivered twice (urlOpener.onLoginSuccess and the + // result listener); act on the first only. + guard !self.loginSucceeded else { return } self.logger.info("performLogin: SDK login succeeded") - self.identityResetAttempted = false self.loginSucceeded = true + self.identityResetAttempted = false self.pendingAuth = nil - // If the browser already auto-closed (its close grace ended - // before the registration finished — typical for a first-time - // profile login), the finished-handler deferred to us: start - // the VPN now that the login is fully done. When the browser - // is still open, its finished-handler starts the VPN instead. + self.pendingAuthorizeURL = nil + // If the browser is already gone, the view's completion handler + // deferred the decision to us — the login only finished now, so + // start the VPN here. While it is still open, the view starts it + // when the user dismisses the success page. if !self.showBrowser { self.logger.info("performLogin: login completed after browser closed - starting VPN") self.startVPNConnection() @@ -572,38 +593,38 @@ public class NetworkExtensionAdapter: ObservableObject { } errListener.onSuccessCallback = { urlOpener.onSuccess?() } errListener.onErrorCallback = { [weak self] error in - // Surface the reason in the logs — a login that dies AFTER the - // browser part (token exchange or management login failing) - // otherwise looks identical to "nothing happened". + // Surface the reason: a login that dies after the browser phase + // (failed token exchange or management registration) is otherwise + // indistinguishable from "nothing happened". let message = error?.localizedDescription ?? "unknown login error" AppLogger.shared.log("performLogin: SDK login failed: \(message)") - // onError runs on a background goroutine; mutate state on the - // main queue to stay consistent with onSuccess and cancelLogin(). + // onError runs on a background goroutine; mutate state on the main + // queue to stay consistent with onSuccess and cancelLogin(). DispatchQueue.main.async { [weak self] in guard let self else { return } self.logger.error("performLogin: SDK login failed: \(message, privacy: .public)") self.pendingAuth = nil + self.pendingAuthorizeURL = nil // "peer is already registered by a different User or a Setup - // Key": the profile's stored WireGuard key belongs to a peer - // the just-authenticated account doesn't own, so every retry - // with this identity is doomed. Self-heal: drop the profile's + // Key": the profile's stored WireGuard key belongs to a peer the + // just-authenticated account does not own, so every retry with + // this identity is refused. Self-heal once: drop the profile's // local identity (config + state; the server URL survives) and - // rerun the login once — the SDK then generates a fresh key - // and registers a NEW peer under the correct account, leaving - // the conflicting peer untouched. The retry's browser closes - // by itself: the IdP session cookie from the just-completed - // interactive login makes it a silent SSO redirect. - if message.contains("registered by a different User"), !self.identityResetAttempted, browserPhaseStarted { + // rerun the login, which generates a fresh key and registers a + // new peer under the correct account. + if message.contains("registered by a different User"), + !self.identityResetAttempted, + browserPhaseStarted { self.identityResetAttempted = true let profile = ProfileManager.shared.getActiveProfileName() do { try ProfileManager.shared.logoutProfile(profile) - self.logger.warning("performLogin: peer identity conflicts with the logged-in account — resetting identity for '\(profile, privacy: .public)' and retrying login") + self.logger.warning("performLogin: peer identity conflicts with the logged-in account — resetting identity for '\(profile, privacy: .public)' and retrying") AppLogger.shared.log("performLogin: resetting identity for '\(profile)' after ownership conflict; retrying login") Task { await self.performLogin() } } catch { // Retrying without a successful reset would present the - // same conflicting identity again — log and stop instead. + // same conflicting identity again — stop and report. self.logger.error("performLogin: identity reset for '\(profile, privacy: .public)' failed: \(error.localizedDescription, privacy: .public)") AppLogger.shared.log("performLogin: identity reset for '\(profile)' failed: \(error.localizedDescription)") } @@ -639,10 +660,42 @@ public class NetworkExtensionAdapter: ObservableObject { logger.error("performLogin: no login URL received from extension, aborting") return } + #if os(iOS) + // Same shared-cookie-jar reasoning as the main-app path above. + let fallbackURL = Preferences.loadLastAuthenticatedProfile() == ProfileManager.shared.getActiveProfileName() + ? url + : Self.urlForcingAccountSelection(url) + self.pendingAuthorizeURL = fallbackURL + self.loginURL = fallbackURL + #else self.loginURL = url + #endif self.showBrowser = true } + #if os(iOS) + /// Returns `urlString` with an OIDC `prompt` that makes the IdP ask which account + /// to use, so a login for one profile cannot silently inherit the browser session + /// another profile left in the shared cookie jar. An existing `prompt=login` is + /// left alone — re-authentication is already stronger than account selection. + static func urlForcingAccountSelection(_ urlString: String) -> String { + guard var components = URLComponents(string: urlString) else { return urlString } + var items = components.queryItems ?? [] + if let index = items.firstIndex(where: { $0.name == "prompt" }) { + let existing = items[index].value ?? "" + let values = existing.split(separator: " ").map(String.init) + guard !values.contains("login"), !values.contains("select_account") else { + return urlString + } + items[index] = URLQueryItem(name: "prompt", value: (values + ["select_account"]).joined(separator: " ")) + } else { + items.append(URLQueryItem(name: "prompt", value: "select_account")) + } + components.queryItems = items + return components.string ?? urlString + } + #endif + #if os(iOS) /// Aborts an in-progress interactive login (e.g. the user dismissed the OAuth /// browser without completing it). Stopping the SDK auth cancels its context, @@ -653,10 +706,114 @@ public class NetworkExtensionAdapter: ObservableObject { logger.info("cancelLogin: aborting in-progress login") pendingAuth?.stop() pendingAuth = nil + pendingAuthorizeURL = nil loginSucceeded = false identityResetAttempted = false showBrowser = false } + + /// Decides what a dismissed login browser means and calls `abort` only when the + /// login is definitely not in flight. + /// + /// The system auth session reports the same "cancelled" completion whether the + /// user backed out of the IdP page or closed the SDK's success page after the + /// redirect already went through, so the dismissal alone cannot be trusted. The + /// SDK's loopback listener settles it: it stays bound while the flow is still + /// waiting for the authorization code and goes away once the code arrives. A + /// listener that is still up on two probes means nothing was delivered — a real + /// cancel. Anything else defers to the SDK, with a bounded fallback so a login + /// that dies silently cannot leave the UI stuck on "Connecting…". + public func resolveLoginAfterBrowserClose(abort: @escaping () -> Void) { + let token = loginAttemptToken + // `abort` must never fire for an attempt other than the one being resolved. + let abortIfStillCurrent: () -> Void = { [weak self] in + guard let self, self.loginAttemptToken == token, + !self.loginSucceeded, !self.showBrowser else { return } + abort() + } + + guard let endpoint = pendingAuthorizeURL.flatMap(Self.loopbackEndpoint(fromAuthorizeURL:)) else { + logger.info("resolveLoginAfterBrowserClose: no loopback endpoint known, deferring to SDK result") + DispatchQueue.main.asyncAfter(deadline: .now() + Self.loginResolutionTimeout, execute: abortIfStillCurrent) + return + } + + Self.probeListener(host: endpoint.host, port: endpoint.port) { [weak self] listening in + guard let self else { return } + guard listening else { + self.logger.info("resolveLoginAfterBrowserClose: loopback listener gone — code delivered, waiting for the SDK") + DispatchQueue.main.asyncAfter(deadline: .now() + Self.loginResolutionTimeout, execute: abortIfStillCurrent) + return + } + // Still listening: either nothing was delivered, or the code arrived and + // the token exchange is running with the listener briefly still up. + // Re-probe once before treating it as a cancel so a live exchange is + // never killed. + DispatchQueue.main.asyncAfter(deadline: .now() + Self.loopbackRecheckDelay) { + Self.probeListener(host: endpoint.host, port: endpoint.port) { stillListening in + DispatchQueue.main.async { + if stillListening { + self.logger.info("resolveLoginAfterBrowserClose: loopback still waiting for the code — treating as cancelled") + abortIfStillCurrent() + } else { + self.logger.info("resolveLoginAfterBrowserClose: code delivered late, waiting for the SDK") + DispatchQueue.main.asyncAfter(deadline: .now() + Self.loginResolutionTimeout, execute: abortIfStillCurrent) + } + } + } + } + } + } + + /// How long to wait for the SDK's verdict once the code is known to be delivered. + /// Covers a first-time peer registration, which can outlast the browser session. + private static let loginResolutionTimeout: TimeInterval = 20 + /// Gap between loopback probes, long enough to cover a token exchange. + private static let loopbackRecheckDelay: TimeInterval = 3 + + /// Extracts the loopback host/port the SDK told the IdP to redirect to. + static func loopbackEndpoint(fromAuthorizeURL urlString: String) -> (host: String, port: UInt16)? { + guard let components = URLComponents(string: urlString), + let redirect = components.queryItems?.first(where: { $0.name == "redirect_uri" })?.value, + let redirectComponents = URLComponents(string: redirect), + let host = redirectComponents.host, + let port = redirectComponents.port, + let port16 = UInt16(exactly: port) + else { return nil } + return (host, port16) + } + + /// Reports whether something accepts TCP connections at host:port. + private static func probeListener(host: String, port: UInt16, completion: @escaping (Bool) -> Void) { + guard let nwPort = NWEndpoint.Port(rawValue: port) else { + completion(false) + return + } + let connection = NWConnection(host: NWEndpoint.Host(host), port: nwPort, using: .tcp) + var settled = false + let settle: (Bool) -> Void = { listening in + guard !settled else { return } + settled = true + connection.cancel() + completion(listening) + } + connection.stateUpdateHandler = { state in + switch state { + case .ready: + settle(true) + case .failed, .cancelled: + settle(false) + case .waiting: + // Connection refused surfaces as .waiting with a retry — for loopback + // that means nothing is bound. + settle(false) + default: + break + } + } + connection.start(queue: DispatchQueue.global(qos: .userInitiated)) + DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + 2) { settle(false) } + } #endif public func startVPNConnection() { diff --git a/NetbirdKit/Preferences.swift b/NetbirdKit/Preferences.swift index e2396435..9ee8d325 100644 --- a/NetbirdKit/Preferences.swift +++ b/NetbirdKit/Preferences.swift @@ -178,36 +178,31 @@ class Preferences { return sharedUserDefaults()?.string(forKey: managementURLKey) } - // MARK: - Per-Profile Login Web-Store Identifiers + // MARK: - Last Authenticated Profile // - // Every profile logs in through a WKWebView whose persistent - // WKWebsiteDataStore is keyed by a stable per-profile UUID (iOS 17+). That - // store keeps the IdP's trusted-device 2FA cookie between re-logins, while - // every profile stays fully isolated in its own cookie jar. - - /// Stable identifier of the profile's persistent login web store, created on - /// first use. - static func webStoreIdentifier(for profile: String) -> UUID { - let defaults = sharedUserDefaults() - var map = defaults?.dictionary(forKey: GlobalConstants.keyProfileWebStoreIDs) as? [String: String] ?? [:] - if let existing = map[profile], let uuid = UUID(uuidString: existing) { - return uuid + // The login browser (ASWebAuthenticationSession) shares Safari's cookie jar, + // which the app cannot scope per profile or clear. Recording which profile that + // jar was last authenticated for lets the adapter add prompt=select_account + // whenever a login targets a different profile, so the IdP re-asks which + // account to use instead of silently signing in the previous one. + + /// Records the profile the login browser last authenticated. + static func saveLastAuthenticatedProfile(_ name: String) { + sharedUserDefaults()?.set(name, forKey: GlobalConstants.keyLastAuthenticatedProfile) + } + + /// The profile the login browser last authenticated, or nil if none has yet. + static func loadLastAuthenticatedProfile() -> String? { + return sharedUserDefaults()?.string(forKey: GlobalConstants.keyLastAuthenticatedProfile) + } + + /// Forgets the marker if it names `profile`. Called on logout and profile + /// removal so the next login re-asks which account to use. + static func clearLastAuthenticatedProfile(ifEquals profile: String) { + guard let defaults = sharedUserDefaults() else { return } + if defaults.string(forKey: GlobalConstants.keyLastAuthenticatedProfile) == profile { + defaults.removeObject(forKey: GlobalConstants.keyLastAuthenticatedProfile) } - let uuid = UUID() - map[profile] = uuid.uuidString - defaults?.set(map, forKey: GlobalConstants.keyProfileWebStoreIDs) - return uuid - } - - /// Forgets the profile's web-store identifier, returning it so the caller can - /// delete the underlying WKWebsiteDataStore (WebKit is not linked here — this - /// file is also compiled into the network extension). - static func removeWebStoreIdentifier(for profile: String) -> UUID? { - let defaults = sharedUserDefaults() - var map = defaults?.dictionary(forKey: GlobalConstants.keyProfileWebStoreIDs) as? [String: String] ?? [:] - guard let existing = map.removeValue(forKey: profile) else { return nil } - defaults?.set(map, forKey: GlobalConstants.keyProfileWebStoreIDs) - return UUID(uuidString: existing) } /// Restore config from UserDefaults to the config file path. diff --git a/NetbirdKit/ProfileManager.swift b/NetbirdKit/ProfileManager.swift index 1c1d6d69..4dbacf6e 100644 --- a/NetbirdKit/ProfileManager.swift +++ b/NetbirdKit/ProfileManager.swift @@ -167,6 +167,7 @@ class ProfileManager { try fileManager.removeItem(atPath: dir) ProfileConnectionCache().remove(for: name) + Preferences.clearLastAuthenticatedProfile(ifEquals: name) } /// Clears authentication data for a profile by removing its config and state files. @@ -186,6 +187,11 @@ class ProfileManager { } cache.clearConnectionData(for: name) + // An explicit logout must not silently sign back in through the browser + // session this profile left behind: dropping the marker makes the next + // login force account selection. + Preferences.clearLastAuthenticatedProfile(ifEquals: name) + if fileManager.fileExists(atPath: statePath) { try fileManager.removeItem(atPath: statePath) } From 524f85a978ef1c8e2a678904f874b415c293d03a Mon Sep 17 00:00:00 2001 From: evgeniyChepelev <68751844+evgeniyChepelev@users.noreply.github.com> Date: Fri, 31 Jul 2026 14:41:04 +0200 Subject: [PATCH 4/8] fix(ios): reuse the browser session on re-login instead of clearing it --- NetBird.xcodeproj/project.pbxproj | 16 +- .../App/Views/Components/SafariView.swift | 7 +- .../App/Views/iOS/iOSConnectionView.swift | 9 ++ NetbirdKit/GlobalConstants.swift | 13 +- NetbirdKit/NetworkExtensionAdapter.swift | 142 +++++++++--------- NetbirdKit/Preferences.swift | 18 +++ NetbirdKit/ProfileManager.swift | 12 +- 7 files changed, 128 insertions(+), 89 deletions(-) diff --git a/NetBird.xcodeproj/project.pbxproj b/NetBird.xcodeproj/project.pbxproj index 3b1d6441..b2ee7342 100644 --- a/NetBird.xcodeproj/project.pbxproj +++ b/NetBird.xcodeproj/project.pbxproj @@ -59,7 +59,6 @@ 44F3E3992EE2F90900C87FEC /* libresolv.tbd in Frameworks */ = {isa = PBXBuildFile; fileRef = 44DCF5B82EDF4D900026078E /* libresolv.tbd */; }; 44F3E39B2EE2F9FA00C87FEC /* TVAuthView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 44F3E39A2EE2F9FA00C87FEC /* TVAuthView.swift */; }; 4849965EC2515950756C8F10 /* VPNOnDemandView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9CD257EF78F038560FF3112D /* VPNOnDemandView.swift */; }; - BB001A012F99000000000001 /* TroubleshootView.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB001A002F99000000000001 /* TroubleshootView.swift */; }; 50003BBC2AFBCA6B00E5EB6B /* FirebasePerformance in Frameworks */ = {isa = PBXBuildFile; productRef = 50003BBB2AFBCA6B00E5EB6B /* FirebasePerformance */; }; 50003BBE2AFBCA7900E5EB6B /* FirebasePerformance in Frameworks */ = {isa = PBXBuildFile; productRef = 50003BBD2AFBCA7900E5EB6B /* FirebasePerformance */; }; 50003BC42AFBD7D500E5EB6B /* PacketTunnelProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50245A562A80431C0034792B /* PacketTunnelProvider.swift */; }; @@ -134,16 +133,16 @@ 50E608132A7958B100BAF09B /* MainViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50E608122A7958B100BAF09B /* MainViewModel.swift */; }; 50E608242A79966600BAF09B /* AboutView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50E608232A79966600BAF09B /* AboutView.swift */; }; 50E608262A79968500BAF09B /* AdvancedView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50E608252A79968500BAF09B /* AdvancedView.swift */; }; + 5573F6EE2F9F523D00E63A73 /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = 5573F6ED2F9F523D00E63A73 /* GoogleService-Info.plist */; }; + 5573F6EF2F9F523D00E63A73 /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = 5573F6ED2F9F523D00E63A73 /* GoogleService-Info.plist */; }; + 5573F6F02F9F523D00E63A73 /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = 5573F6ED2F9F523D00E63A73 /* GoogleService-Info.plist */; }; + 5573F6F12F9F523D00E63A73 /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = 5573F6ED2F9F523D00E63A73 /* GoogleService-Info.plist */; }; 558553FB2FE34921004FB58D /* jetbrains-mono-variable.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 558553FA2FE34921004FB58D /* jetbrains-mono-variable.ttf */; }; 558553FC2FE34921004FB58D /* inter-variable.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 558553F92FE34921004FB58D /* inter-variable.ttf */; }; 558553FD2FE34921004FB58D /* jetbrains-mono-variable.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 558553FA2FE34921004FB58D /* jetbrains-mono-variable.ttf */; }; 558553FE2FE34921004FB58D /* inter-variable.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 558553F92FE34921004FB58D /* inter-variable.ttf */; }; 558553FF2FE34921004FB58D /* jetbrains-mono-variable.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 558553FA2FE34921004FB58D /* jetbrains-mono-variable.ttf */; }; 558554002FE34921004FB58D /* inter-variable.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 558553F92FE34921004FB58D /* inter-variable.ttf */; }; - 5573F6EE2F9F523D00E63A73 /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = 5573F6ED2F9F523D00E63A73 /* GoogleService-Info.plist */; }; - 5573F6EF2F9F523D00E63A73 /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = 5573F6ED2F9F523D00E63A73 /* GoogleService-Info.plist */; }; - 5573F6F02F9F523D00E63A73 /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = 5573F6ED2F9F523D00E63A73 /* GoogleService-Info.plist */; }; - 5573F6F12F9F523D00E63A73 /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = 5573F6ED2F9F523D00E63A73 /* GoogleService-Info.plist */; }; 55B5E81B2F39158200852AA7 /* InternetStatusView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 55B5E81A2F39158200852AA7 /* InternetStatusView.swift */; }; 55D865852F70982000A2EFF8 /* WidgetKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 55D865842F70982000A2EFF8 /* WidgetKit.framework */; }; 55D865872F70982000A2EFF8 /* SwiftUI.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 55D865862F70982000A2EFF8 /* SwiftUI.framework */; }; @@ -184,6 +183,7 @@ AA0009042F22000900000001 /* ProfileConnectionCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA0009002F22000900000001 /* ProfileConnectionCache.swift */; }; AA1B2C022F4E5A0100D1E2F3 /* TVGradientBackground.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA1B2C012F4E5A0100D1E2F3 /* TVGradientBackground.swift */; }; B1A2C3D42F3A000100000001 /* PeerDetailSheet.swift in Sources */ = {isa = PBXBuildFile; fileRef = B1A2C3D32F3A000100000001 /* PeerDetailSheet.swift */; }; + BB001A012F99000000000001 /* TroubleshootView.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB001A002F99000000000001 /* TroubleshootView.swift */; }; BB3D4E022F4E5A0200D1E2F3 /* TVPreSharedKeyButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB3D4E012F4E5A0200D1E2F3 /* TVPreSharedKeyButton.swift */; }; CC5F6A022F4E5A0300D1E2F3 /* TVQRCodeSheet.swift in Sources */ = {isa = PBXBuildFile; fileRef = CC5F6A012F4E5A0300D1E2F3 /* TVQRCodeSheet.swift */; }; E1A0B0012F5E000100000001 /* EmptyTabPlaceholder.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1A0B0002F5E000100000001 /* EmptyTabPlaceholder.swift */; }; @@ -345,9 +345,9 @@ 50E608232A79966600BAF09B /* AboutView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AboutView.swift; sourceTree = ""; }; 50E608252A79968500BAF09B /* AdvancedView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AdvancedView.swift; sourceTree = ""; }; 53CB9305A9DC6CAD1895495A /* SharedUserDefaultsTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = SharedUserDefaultsTests.swift; sourceTree = ""; }; + 5573F6ED2F9F523D00E63A73 /* GoogleService-Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = "GoogleService-Info.plist"; sourceTree = ""; }; 558553F92FE34921004FB58D /* inter-variable.ttf */ = {isa = PBXFileReference; lastKnownFileType = file; path = "inter-variable.ttf"; sourceTree = ""; }; 558553FA2FE34921004FB58D /* jetbrains-mono-variable.ttf */ = {isa = PBXFileReference; lastKnownFileType = file; path = "jetbrains-mono-variable.ttf"; sourceTree = ""; }; - 5573F6ED2F9F523D00E63A73 /* GoogleService-Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = "GoogleService-Info.plist"; sourceTree = ""; }; 55B5E81A2F39158200852AA7 /* InternetStatusView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InternetStatusView.swift; sourceTree = ""; }; 55D865832F70982000A2EFF8 /* NetBirdWidgetExtensionExtension.appex */ = {isa = PBXFileReference; explicitFileType = "wrapper.app-extension"; includeInIndex = 0; path = NetBirdWidgetExtensionExtension.appex; sourceTree = BUILT_PRODUCTS_DIR; }; 55D865842F70982000A2EFF8 /* WidgetKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = WidgetKit.framework; path = System/Library/Frameworks/WidgetKit.framework; sourceTree = SDKROOT; }; @@ -359,7 +359,6 @@ 978FC46F2EEDF167002D0EB8 /* AppLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppLogger.swift; sourceTree = ""; }; 9CD257EF78F038560FF3112D /* VPNOnDemandView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VPNOnDemandView.swift; sourceTree = ""; }; A1B2C3D32F4A000100000001 /* VPNToggleView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VPNToggleView.swift; sourceTree = ""; }; - BB001A002F99000000000001 /* TroubleshootView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TroubleshootView.swift; sourceTree = ""; }; A1B2C3D42EEDF500001A2B3C /* ConfigurationProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConfigurationProvider.swift; sourceTree = ""; }; A1C3D5E72F000001001A2B3C /* WiFiOnDemandPolicy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WiFiOnDemandPolicy.swift; sourceTree = ""; }; A1C3D5E82F000002001A2B3C /* CellularOnDemandPolicy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CellularOnDemandPolicy.swift; sourceTree = ""; }; @@ -371,6 +370,7 @@ AA0009002F22000900000001 /* ProfileConnectionCache.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProfileConnectionCache.swift; sourceTree = ""; }; AA1B2C012F4E5A0100D1E2F3 /* TVGradientBackground.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TVGradientBackground.swift; sourceTree = ""; }; B1A2C3D32F3A000100000001 /* PeerDetailSheet.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PeerDetailSheet.swift; sourceTree = ""; }; + BB001A002F99000000000001 /* TroubleshootView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TroubleshootView.swift; sourceTree = ""; }; BB3D4E012F4E5A0200D1E2F3 /* TVPreSharedKeyButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TVPreSharedKeyButton.swift; sourceTree = ""; }; C7A1CFF65CC44187912007EC /* iOSNetworksView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = iOSNetworksView.swift; sourceTree = ""; }; CC5F6A012F4E5A0300D1E2F3 /* TVQRCodeSheet.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TVQRCodeSheet.swift; sourceTree = ""; }; @@ -561,7 +561,7 @@ 50A8910E2A792A15007C48FC = { isa = PBXGroup; children = ( - 5573F6ED2F9F523D00E63A73 /* GoogleService-Info.plist */, + 5573F6ED2F9F523D00E63A73 /* GoogleService-Info.plist */, 50D402932BD9143900D4AC5B /* NetBirdSDK.xcframework */, 50245A0A2A7AA9390034792B /* NetBird-Bridging-Header.h */, 50A891192A792A15007C48FC /* NetBird */, diff --git a/NetBird/Source/App/Views/Components/SafariView.swift b/NetBird/Source/App/Views/Components/SafariView.swift index 264c8197..3787222e 100644 --- a/NetBird/Source/App/Views/Components/SafariView.swift +++ b/NetBird/Source/App/Views/Components/SafariView.swift @@ -43,6 +43,11 @@ enum LoginBrowserOutcome { struct SafariView: UIViewControllerRepresentable { @Binding var isPresented: Bool let url: URL + /// True starts the session with an empty cookie jar. Set for profiles whose + /// account conflicts with the one the shared session holds — the only reliable + /// way to reach a different account when the IdP ignores both login_hint and + /// prompt=select_account. + let prefersEphemeralSession: Bool let didFinish: (LoginBrowserOutcome) -> Void func makeUIViewController(context: Context) -> UIViewController { @@ -129,7 +134,7 @@ struct SafariView: UIViewControllerRepresentable { // cookie survives between logins and the second factor is not re-prompted // on every re-login. Cross-profile isolation is enforced by the // prompt=select_account the adapter adds when the profile changes. - session.prefersEphemeralWebBrowserSession = false + session.prefersEphemeralWebBrowserSession = parent.prefersEphemeralSession session.presentationContextProvider = self self.session = session session.start() diff --git a/NetBird/Source/App/Views/iOS/iOSConnectionView.swift b/NetBird/Source/App/Views/iOS/iOSConnectionView.swift index a3af6549..2d8b7153 100644 --- a/NetBird/Source/App/Views/iOS/iOSConnectionView.swift +++ b/NetBird/Source/App/Views/iOS/iOSConnectionView.swift @@ -153,12 +153,21 @@ struct iOSConnectionView: View { SafariView( isPresented: $viewModel.networkExtensionAdapter.showBrowser, url: loginURL, + prefersEphemeralSession: viewModel.networkExtensionAdapter.useEphemeralBrowserSession, didFinish: loginBrowserDidFinish ) } } .navigationBarTitleDisplayMode(.inline) .navigationBarHidden(true) + .alert("Login failed", isPresented: Binding( + get: { viewModel.networkExtensionAdapter.loginErrorMessage != nil }, + set: { if !$0 { viewModel.networkExtensionAdapter.loginErrorMessage = nil } } + )) { + Button("OK") { viewModel.networkExtensionAdapter.loginErrorMessage = nil } + } message: { + Text(viewModel.networkExtensionAdapter.loginErrorMessage ?? "") + } } /// Resolves what the login browser's end means for the VPN. diff --git a/NetbirdKit/GlobalConstants.swift b/NetbirdKit/GlobalConstants.swift index 6e0820f1..2631a64e 100644 --- a/NetbirdKit/GlobalConstants.swift +++ b/NetbirdKit/GlobalConstants.swift @@ -34,12 +34,17 @@ struct GlobalConstants { static let stateFileName = "state.json" static let serverURLFileName = "netbird_server_url" - // Profile whose account the login browser last authenticated. The system auth - // session shares one Safari cookie jar across all profiles, so a login for any - // other profile must force account selection instead of silently reusing that - // session. + // Profile the persistent login browser session belongs to. The system auth + // session has a single cookie store shared by all profiles, so this records + // whose SSO and trusted-device state is currently in it: that profile's + // re-logins reuse the session, any other profile's login starts a fresh one. static let keyLastAuthenticatedProfile = "netbird.lastAuthenticatedProfile" + // Set when the stored browser session must not be reused — after an explicit + // logout, where signing straight back in through the old session would defeat + // logging out. Cleared by the next completed login. + static let keyNeedsFreshBrowserSession = "netbird.needsFreshBrowserSession" + // Local notification identifiers static let notificationLoginRequired = "netbird.login.required" } diff --git a/NetbirdKit/NetworkExtensionAdapter.swift b/NetbirdKit/NetworkExtensionAdapter.swift index 993d7361..1b417e5c 100644 --- a/NetbirdKit/NetworkExtensionAdapter.swift +++ b/NetbirdKit/NetworkExtensionAdapter.swift @@ -83,8 +83,12 @@ public class NetworkExtensionAdapter: ObservableObject { /// the same whether the user cancelled or closed the SDK's success page — so /// every "did the login work" decision reads this flag. public private(set) var loginSucceeded = false - /// Guards the ownership-conflict self-heal so it retries at most once per login. - private var identityResetAttempted = false + /// Whether the pending login must start from a fresh browser session rather + /// than the stored one. Decided per login by the policy in performLogin. + @Published public private(set) var useEphemeralBrowserSession = false + /// Reason the last login failed, surfaced to the user. Nil when there is nothing + /// to report. + @Published public var loginErrorMessage: String? /// Incremented on every performLogin entry. Deferred work armed for one attempt /// captures the value and compares before acting, so a stale timer can never /// abort a newer attempt. @@ -497,15 +501,27 @@ public class NetworkExtensionAdapter: ObservableObject { } let activeManagementURL = resolvedURL ?? "" logger.info("performLogin: using management URL '\(activeManagementURL, privacy: .public)' for profile '\(activeProfile, privacy: .public)'") - // The system auth session shares one Safari cookie jar across every profile, - // and the app cannot scope or clear it. So when this login targets a profile - // other than the one that jar was last authenticated for, force the IdP to - // ask which account to use rather than silently reusing the live session. - let lastAuthenticated = Preferences.loadLastAuthenticatedProfile() - let needsAccountSelection = lastAuthenticated != activeProfile - if needsAccountSelection { - logger.info("performLogin: profile changed (last authenticated: \(lastAuthenticated ?? "none", privacy: .public)) — forcing account selection") - } + // Browser session policy. The system auth session has a single cookie store, + // shared by every profile, that the app can neither scope nor clear — the + // only control it offers is whether a session starts from that store or from + // an empty one. Reusing it is what keeps the IdP's SSO session and its + // trusted-device cookie alive, so a re-login does not re-prompt for the + // second factor. Starting empty is the only way to keep one profile's + // account out of another's login. + // + // So the stored session is reused for a plain re-login, and a fresh one is + // started in exactly the three cases where reuse would be wrong: + // - switching profiles, and the first login of a new profile: the stored + // session belongs to a different profile (sessionOwner != activeProfile); + // - after logging out: signing straight back in through the session that + // was just logged out of would make the logout meaningless. + // A nil owner means nothing has claimed the store yet, i.e. the first login + // on this install — nothing to inherit, so it may claim it. + let sessionOwner = Preferences.loadLastAuthenticatedProfile() + let ownsStoredSession = sessionOwner == nil || sessionOwner == activeProfile + let useEphemeralSession = Preferences.needsFreshBrowserSession() || !ownsStoredSession + logger.info("performLogin: '\(activeProfile, privacy: .public)' — \(useEphemeralSession ? "fresh" : "stored", privacy: .public) browser session (owner: \(sessionOwner ?? "none", privacy: .public))") + AppLogger.shared.log("performLogin: '\(activeProfile)' uses a \(useEphemeralSession ? "fresh" : "stored") browser session") if let configPath = Preferences.configFile(), !configPath.isEmpty, let auth = NetBirdSDKNewAuth(configPath, activeManagementURL, nil) { // A stale flow from an abandoned attempt would keep its loopback port @@ -536,11 +552,11 @@ public class NetworkExtensionAdapter: ObservableObject { // the extension, trips its needsLogin path, and pops the auth alert // in parallel with this browser login. Ordering them guarantees the // await caller sees showBrowser == true. - let browserURL = needsAccountSelection - ? Self.urlForcingAccountSelection(url) - : url + let browserURL = url DispatchQueue.main.async { browserPhaseStarted = true + self?.useEphemeralBrowserSession = useEphemeralSession + self?.loginErrorMessage = nil self?.pendingAuthorizeURL = browserURL self?.loginURL = browserURL self?.showBrowser = true @@ -563,10 +579,19 @@ public class NetworkExtensionAdapter: ObservableObject { ProfileManager.shared.saveServerURL(activeManagementURL, for: activeProfile) Preferences.saveManagementURL(activeManagementURL) } - // This profile's account now owns the shared browser session — - // record it so its own re-logins stay silent while a login for - // any other profile forces account selection. - Preferences.saveLastAuthenticatedProfile(activeProfile) + // An ephemeral login leaves no cookies behind, so the shared + // session still holds whichever account was there — only a + // shared-session login may claim it. + // This login just put its account's SSO and trusted-device state + // into the shared browser store, so the profile now owns it and + // its re-logins may reuse it. A fresh session leaves nothing + // behind, so it cannot take ownership — claiming from one would + // send this profile's next login into a store still holding + // another profile's account. + if !useEphemeralSession { + Preferences.saveLastAuthenticatedProfile(activeProfile) + } + Preferences.clearNeedsFreshBrowserSession() AppLogger.shared.log("performLogin: SDK login succeeded for '\(activeProfile)'") // onSuccess runs on a background goroutine. Mark success on the main // queue so the browser-finished handler (also main-queue) reliably @@ -578,7 +603,6 @@ public class NetworkExtensionAdapter: ObservableObject { guard !self.loginSucceeded else { return } self.logger.info("performLogin: SDK login succeeded") self.loginSucceeded = true - self.identityResetAttempted = false self.pendingAuth = nil self.pendingAuthorizeURL = nil // If the browser is already gone, the view's completion handler @@ -605,30 +629,25 @@ public class NetworkExtensionAdapter: ObservableObject { self.logger.error("performLogin: SDK login failed: \(message, privacy: .public)") self.pendingAuth = nil self.pendingAuthorizeURL = nil + guard browserPhaseStarted else { return } // "peer is already registered by a different User or a Setup - // Key": the profile's stored WireGuard key belongs to a peer the - // just-authenticated account does not own, so every retry with - // this identity is refused. Self-heal once: drop the profile's - // local identity (config + state; the server URL survives) and - // rerun the login, which generates a fresh key and registers a - // new peer under the correct account. - if message.contains("registered by a different User"), - !self.identityResetAttempted, - browserPhaseStarted { - self.identityResetAttempted = true - let profile = ProfileManager.shared.getActiveProfileName() - do { - try ProfileManager.shared.logoutProfile(profile) - self.logger.warning("performLogin: peer identity conflicts with the logged-in account — resetting identity for '\(profile, privacy: .public)' and retrying") - AppLogger.shared.log("performLogin: resetting identity for '\(profile)' after ownership conflict; retrying login") - Task { await self.performLogin() } - } catch { - // Retrying without a successful reset would present the - // same conflicting identity again — stop and report. - self.logger.error("performLogin: identity reset for '\(profile, privacy: .public)' failed: \(error.localizedDescription, privacy: .public)") - AppLogger.shared.log("performLogin: identity reset for '\(profile)' failed: \(error.localizedDescription)") - } + // Key" means the account that signed in does not own this + // profile's peer. Report it rather than "repairing" it: the + // app cannot tell a stale local key from a login under the + // wrong account, and deleting the profile's identity to fix + // the latter destroys a working registration and can + // re-register the peer under the wrong account. Removing an + // identity stays an explicit user action — Profiles → Log out. + if message.contains("registered by a different User") { + self.loginErrorMessage = """ + This profile belongs to a different NetBird account. \ + Sign in with the account that owns it, or log the \ + profile out (Profiles → Log out) to register it again. + """ + } else { + self.loginErrorMessage = message } + self.showBrowser = false } resume(nil) } @@ -661,41 +680,19 @@ public class NetworkExtensionAdapter: ObservableObject { return } #if os(iOS) - // Same shared-cookie-jar reasoning as the main-app path above. - let fallbackURL = Preferences.loadLastAuthenticatedProfile() == ProfileManager.shared.getActiveProfileName() - ? url - : Self.urlForcingAccountSelection(url) - self.pendingAuthorizeURL = fallbackURL - self.loginURL = fallbackURL - #else - self.loginURL = url + // Same session policy as the main-app path above. This path cannot observe + // login success, so it never claims the stored session — which also means it + // must not reuse one it does not already own. + let fallbackOwner = Preferences.loadLastAuthenticatedProfile() + let fallbackProfile = ProfileManager.shared.getActiveProfileName() + self.useEphemeralBrowserSession = Preferences.needsFreshBrowserSession() + || (fallbackOwner != nil && fallbackOwner != fallbackProfile) + self.pendingAuthorizeURL = url #endif + self.loginURL = url self.showBrowser = true } - #if os(iOS) - /// Returns `urlString` with an OIDC `prompt` that makes the IdP ask which account - /// to use, so a login for one profile cannot silently inherit the browser session - /// another profile left in the shared cookie jar. An existing `prompt=login` is - /// left alone — re-authentication is already stronger than account selection. - static func urlForcingAccountSelection(_ urlString: String) -> String { - guard var components = URLComponents(string: urlString) else { return urlString } - var items = components.queryItems ?? [] - if let index = items.firstIndex(where: { $0.name == "prompt" }) { - let existing = items[index].value ?? "" - let values = existing.split(separator: " ").map(String.init) - guard !values.contains("login"), !values.contains("select_account") else { - return urlString - } - items[index] = URLQueryItem(name: "prompt", value: (values + ["select_account"]).joined(separator: " ")) - } else { - items.append(URLQueryItem(name: "prompt", value: "select_account")) - } - components.queryItems = items - return components.string ?? urlString - } - #endif - #if os(iOS) /// Aborts an in-progress interactive login (e.g. the user dismissed the OAuth /// browser without completing it). Stopping the SDK auth cancels its context, @@ -708,7 +705,6 @@ public class NetworkExtensionAdapter: ObservableObject { pendingAuth = nil pendingAuthorizeURL = nil loginSucceeded = false - identityResetAttempted = false showBrowser = false } diff --git a/NetbirdKit/Preferences.swift b/NetbirdKit/Preferences.swift index 9ee8d325..ada9d4c7 100644 --- a/NetbirdKit/Preferences.swift +++ b/NetbirdKit/Preferences.swift @@ -178,6 +178,24 @@ class Preferences { return sharedUserDefaults()?.string(forKey: managementURLKey) } + // MARK: - Fresh Browser Session + + /// Whether the stored browser session must not be reused by the next login. + static func needsFreshBrowserSession() -> Bool { + return sharedUserDefaults()?.bool(forKey: GlobalConstants.keyNeedsFreshBrowserSession) ?? false + } + + /// Requires the next login to start a fresh browser session. Set on logout, so + /// signing back in cannot silently reuse the session that was just logged out of. + static func setNeedsFreshBrowserSession() { + sharedUserDefaults()?.set(true, forKey: GlobalConstants.keyNeedsFreshBrowserSession) + } + + /// Clears the requirement once a login has run with a fresh session. + static func clearNeedsFreshBrowserSession() { + sharedUserDefaults()?.removeObject(forKey: GlobalConstants.keyNeedsFreshBrowserSession) + } + // MARK: - Last Authenticated Profile // // The login browser (ASWebAuthenticationSession) shares Safari's cookie jar, diff --git a/NetbirdKit/ProfileManager.swift b/NetbirdKit/ProfileManager.swift index 4dbacf6e..8c51d9d7 100644 --- a/NetbirdKit/ProfileManager.swift +++ b/NetbirdKit/ProfileManager.swift @@ -167,6 +167,11 @@ class ProfileManager { try fileManager.removeItem(atPath: dir) ProfileConnectionCache().remove(for: name) + // If the removed profile owned the stored browser session, nothing owns it + // now — the next login must start fresh rather than inherit that account. + if Preferences.loadLastAuthenticatedProfile() == name { + Preferences.setNeedsFreshBrowserSession() + } Preferences.clearLastAuthenticatedProfile(ifEquals: name) } @@ -187,9 +192,10 @@ class ProfileManager { } cache.clearConnectionData(for: name) - // An explicit logout must not silently sign back in through the browser - // session this profile left behind: dropping the marker makes the next - // login force account selection. + // Logging out must actually log out: without this the next login would sign + // straight back in through the browser session this profile left behind, so + // require a fresh one and drop the ownership marker with it. + Preferences.setNeedsFreshBrowserSession() Preferences.clearLastAuthenticatedProfile(ifEquals: name) if fileManager.fileExists(atPath: statePath) { From 8f7b771d9ecb958cc72aa40bdb217a5f4ac27928 Mon Sep 17 00:00:00 2001 From: evgeniyChepelev <68751844+evgeniyChepelev@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:39:41 +0200 Subject: [PATCH 5/8] fix(ios): serialize the loopback probe and clarify the callback design --- .../App/Views/Components/SafariView.swift | 23 ++++++++++++++----- NetbirdKit/NetworkExtensionAdapter.swift | 9 ++++++-- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/NetBird/Source/App/Views/Components/SafariView.swift b/NetBird/Source/App/Views/Components/SafariView.swift index 3787222e..bcd302f5 100644 --- a/NetBird/Source/App/Views/Components/SafariView.swift +++ b/NetBird/Source/App/Views/Components/SafariView.swift @@ -109,12 +109,23 @@ struct SafariView: UIViewControllerRepresentable { } } - // The SDK's PKCE flow redirects to http://localhost:. Declaring - // "http" as the callback scheme covers the case where the session - // intercepts that navigation; when it instead lets the browser follow - // the redirect, the SDK's local server receives the code directly and - // this session ends via .closed once the user dismisses the success page. - // Either path is handled — see the completion handler above. + // The SDK's PKCE flow uses a loopback redirect (RFC 8252 §7.3): the + // authorization code arrives at an HTTP server the SDK runs on + // 127.0.0.1. That server is the primary path and needs no interception — + // the browser simply follows the redirect to it, and this session then + // ends via .closed when the user dismisses the SDK's success page. + // + // A session must still declare a callback, and "http" is the closest + // match for that redirect. Should the session capture the navigation + // instead of letting the browser follow it, the completion handler + // replays the URL to the same local server, so the code still arrives. + // Neither path leaves the flow hanging: resolveLoginAfterBrowserClose + // probes the loopback listener and resolves the login either way. + // + // Apple intends this API for custom schemes or (iOS 17.4+) HTTPS + // host/path callbacks. Moving to either would mean the management server + // issuing a different redirect URI — a core/server change, not one the + // app can make on its own. let session: ASWebAuthenticationSession if #available(iOS 17.4, *) { session = ASWebAuthenticationSession( diff --git a/NetbirdKit/NetworkExtensionAdapter.swift b/NetbirdKit/NetworkExtensionAdapter.swift index 1b417e5c..2b21f186 100644 --- a/NetbirdKit/NetworkExtensionAdapter.swift +++ b/NetbirdKit/NetworkExtensionAdapter.swift @@ -786,6 +786,11 @@ public class NetworkExtensionAdapter: ObservableObject { return } let connection = NWConnection(host: NWEndpoint.Host(host), port: nwPort, using: .tcp) + // Both the connection's state updates and the timeout below run here. A + // serial queue is what makes `settled` safe: on a concurrent queue the + // watchdog could run alongside a state update, and the check-then-set would + // let both through — cancelling twice and reporting the result twice. + let queue = DispatchQueue(label: "io.netbird.loopback-probe") var settled = false let settle: (Bool) -> Void = { listening in guard !settled else { return } @@ -807,8 +812,8 @@ public class NetworkExtensionAdapter: ObservableObject { break } } - connection.start(queue: DispatchQueue.global(qos: .userInitiated)) - DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + 2) { settle(false) } + connection.start(queue: queue) + queue.asyncAfter(deadline: .now() + 2) { settle(false) } } #endif From c3ebc079cdbeeff73e379dfc1e38af5134355793 Mon Sep 17 00:00:00 2001 From: evgeniyChepelev <68751844+evgeniyChepelev@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:29:36 +0200 Subject: [PATCH 6/8] fix(ios): bind the account to the profile instead of the cookie jar --- .../App/Views/Components/SafariView.swift | 39 +++-- .../App/Views/iOS/ProfilesListView.swift | 34 ++-- .../App/Views/iOS/iOSConnectionView.swift | 11 +- NetbirdKit/GlobalConstants.swift | 17 +- NetbirdKit/NetworkExtensionAdapter.swift | 154 ++++++++++++------ NetbirdKit/Preferences.swift | 79 +++++---- NetbirdKit/ProfileManager.swift | 54 ++++-- 7 files changed, 254 insertions(+), 134 deletions(-) diff --git a/NetBird/Source/App/Views/Components/SafariView.swift b/NetBird/Source/App/Views/Components/SafariView.swift index bcd302f5..dee09ebd 100644 --- a/NetBird/Source/App/Views/Components/SafariView.swift +++ b/NetBird/Source/App/Views/Components/SafariView.swift @@ -12,9 +12,12 @@ // // The session is NOT ephemeral: it shares Safari's cookie jar, so the IdP's // trusted-device cookie survives and a re-login does not re-prompt for the second -// factor. That single jar is shared by every profile, so profile isolation is -// enforced on the request instead — the adapter adds prompt=select_account when a -// login targets a different profile than the last authenticated one. +// factor. That single jar is shared by every profile, and the app deliberately +// does not try to partition it. Which account a login lands on is decided on the +// request instead: the SDK sends the profile's own account as an OIDC login_hint, +// and the adapter adds prompt=select_account when the login targets a different +// profile than the session last signed in as (see NetworkExtensionAdapter's +// authorizeURL(_:selectingAccount:)). // import SwiftUI @@ -43,11 +46,6 @@ enum LoginBrowserOutcome { struct SafariView: UIViewControllerRepresentable { @Binding var isPresented: Bool let url: URL - /// True starts the session with an empty cookie jar. Set for profiles whose - /// account conflicts with the one the shared session holds — the only reliable - /// way to reach a different account when the IdP ignores both login_hint and - /// prompt=select_account. - let prefersEphemeralSession: Bool let didFinish: (LoginBrowserOutcome) -> Void func makeUIViewController(context: Context) -> UIViewController { @@ -141,11 +139,13 @@ struct SafariView: UIViewControllerRepresentable { ) } - // Non-ephemeral: shares Safari's cookie jar so the IdP's trusted-device - // cookie survives between logins and the second factor is not re-prompted - // on every re-login. Cross-profile isolation is enforced by the - // prompt=select_account the adapter adds when the profile changes. - session.prefersEphemeralWebBrowserSession = parent.prefersEphemeralSession + // Never ephemeral. Sharing Safari's cookie jar is the whole point: the + // IdP's trusted-device cookie survives between logins, so the second + // factor is not re-prompted on every re-login — for every profile, not + // just one. Which account the session resolves to is decided by the + // login_hint in the authorize URL, so an empty jar buys no isolation + // here, it only throws the trusted-device state away. + session.prefersEphemeralWebBrowserSession = false session.presentationContextProvider = self self.session = session session.start() @@ -161,7 +161,18 @@ struct SafariView: UIViewControllerRepresentable { private static func replayToLoopback(_ callbackURL: URL) { var request = URLRequest(url: callbackURL) request.timeoutInterval = 10 - URLSession.shared.dataTask(with: request).resume() + URLSession.shared.dataTask(with: request) { _, _, error in + // Log the outcome without the URL — its query carries the live + // authorization code. A failure here is not fatal on its own: it also + // happens on the normal path, where the browser already delivered the + // code and the SDK's server is gone. It is the one signal that + // separates the two, so it is worth recording. + if let error { + AppLogger.shared.log("Login redirect replay to the loopback server failed: \(error.localizedDescription)") + } else { + AppLogger.shared.log("Login redirect replayed to the loopback server") + } + }.resume() } func presentationAnchor(for session: ASWebAuthenticationSession) -> ASPresentationAnchor { diff --git a/NetBird/Source/App/Views/iOS/ProfilesListView.swift b/NetBird/Source/App/Views/iOS/ProfilesListView.swift index d548db8d..455059a6 100644 --- a/NetBird/Source/App/Views/iOS/ProfilesListView.swift +++ b/NetBird/Source/App/Views/iOS/ProfilesListView.swift @@ -35,11 +35,7 @@ struct ProfilesListView: View { Text(active.name) .font(.body.bold()) .foregroundColor(Color("TextPrimary")) - if let url = ProfileManager.shared.managementURL(for: active.name) { - Text(url) - .font(.footnote) - .foregroundColor(Color("TextSecondary")) - } + profileSubtitle(for: active) } Spacer() Text("Active") @@ -78,11 +74,7 @@ struct ProfilesListView: View { Text(profile.name) .font(.body) .foregroundColor(Color("TextPrimary")) - if let url = ProfileManager.shared.managementURL(for: profile.name) { - Text(url) - .font(.footnote) - .foregroundColor(Color("TextSecondary")) - } + profileSubtitle(for: profile) } } .swipeActions(edge: .trailing, allowsFullSwipe: false) { @@ -159,6 +151,28 @@ struct ProfilesListView: View { } } + // MARK: - Rows + + /// Server and account lines under a profile's name. The account is the one the + /// profile last signed in with — it is also what goes out as the login_hint on + /// the next login, so showing it makes visible which account a re-login returns + /// to. A profile that never completed an SSO login, or was logged out, has none. + @ViewBuilder + private func profileSubtitle(for profile: Profile) -> some View { + if let url = ProfileManager.shared.managementURL(for: profile.name) { + Text(url) + .font(.footnote) + .foregroundColor(Color("TextSecondary")) + } + if let email = ProfileManager.shared.accountEmail(for: profile.name) { + Text(email) + .font(.footnote) + .foregroundColor(Color("TextSecondary")) + .lineLimit(1) + .truncationMode(.middle) + } + } + // MARK: - Actions private func loadProfiles() { diff --git a/NetBird/Source/App/Views/iOS/iOSConnectionView.swift b/NetBird/Source/App/Views/iOS/iOSConnectionView.swift index 2d8b7153..2fbb82c8 100644 --- a/NetBird/Source/App/Views/iOS/iOSConnectionView.swift +++ b/NetBird/Source/App/Views/iOS/iOSConnectionView.swift @@ -153,9 +153,18 @@ struct iOSConnectionView: View { SafariView( isPresented: $viewModel.networkExtensionAdapter.showBrowser, url: loginURL, - prefersEphemeralSession: viewModel.networkExtensionAdapter.useEphemeralBrowserSession, didFinish: loginBrowserDidFinish ) + } else if viewModel.networkExtensionAdapter.showBrowser { + // A login was started but its authorize URL is missing or unparsable, + // so no browser can be presented. Nothing would ever report an + // outcome, leaving the SDK flow pending until it expires — cancel it + // here instead. + Color.clear.onAppear { + let raw = viewModel.networkExtensionAdapter.loginURL ?? "" + AppLogger.shared.log("Login browser: unusable login URL (\(raw)) — cancelling") + viewModel.cancelPendingLogin() + } } } .navigationBarTitleDisplayMode(.inline) diff --git a/NetbirdKit/GlobalConstants.swift b/NetbirdKit/GlobalConstants.swift index 2631a64e..48152561 100644 --- a/NetbirdKit/GlobalConstants.swift +++ b/NetbirdKit/GlobalConstants.swift @@ -34,16 +34,13 @@ struct GlobalConstants { static let stateFileName = "state.json" static let serverURLFileName = "netbird_server_url" - // Profile the persistent login browser session belongs to. The system auth - // session has a single cookie store shared by all profiles, so this records - // whose SSO and trusted-device state is currently in it: that profile's - // re-logins reuse the session, any other profile's login starts a fresh one. - static let keyLastAuthenticatedProfile = "netbird.lastAuthenticatedProfile" - - // Set when the stored browser session must not be reused — after an explicit - // logout, where signing straight back in through the old session would defeat - // logging out. Cleared by the next completed login. - static let keyNeedsFreshBrowserSession = "netbird.needsFreshBrowserSession" + // Profile whose account the login browser's shared cookie jar last signed in + // with. Only decides whether the next login asks the IdP for an account chooser + // (prompt=select_account) — the account a profile belongs to is bound by the + // login_hint the SDK sends, not by this. An empty string means "an account no + // profile may silently reuse": set on logout, so signing back in cannot land on + // the account just left. + static let keyLastBrowserLoginProfile = "netbird.lastBrowserLoginProfile" // Local notification identifiers static let notificationLoginRequired = "netbird.login.required" diff --git a/NetbirdKit/NetworkExtensionAdapter.swift b/NetbirdKit/NetworkExtensionAdapter.swift index 2b21f186..37e266db 100644 --- a/NetbirdKit/NetworkExtensionAdapter.swift +++ b/NetbirdKit/NetworkExtensionAdapter.swift @@ -83,9 +83,6 @@ public class NetworkExtensionAdapter: ObservableObject { /// the same whether the user cancelled or closed the SDK's success page — so /// every "did the login work" decision reads this flag. public private(set) var loginSucceeded = false - /// Whether the pending login must start from a fresh browser session rather - /// than the stored one. Decided per login by the policy in performLogin. - @Published public private(set) var useEphemeralBrowserSession = false /// Reason the last login failed, surfaced to the user. Nil when there is nothing /// to report. @Published public var loginErrorMessage: String? @@ -501,27 +498,41 @@ public class NetworkExtensionAdapter: ObservableObject { } let activeManagementURL = resolvedURL ?? "" logger.info("performLogin: using management URL '\(activeManagementURL, privacy: .public)' for profile '\(activeProfile, privacy: .public)'") - // Browser session policy. The system auth session has a single cookie store, - // shared by every profile, that the app can neither scope nor clear — the - // only control it offers is whether a session starts from that store or from - // an empty one. Reusing it is what keeps the IdP's SSO session and its - // trusted-device cookie alive, so a re-login does not re-prompt for the - // second factor. Starting empty is the only way to keep one profile's - // account out of another's login. + // Every profile logs in through the same persistent browser session, which is + // what keeps the IdP's SSO session and its trusted-device cookie alive so a + // re-login is not asked for the second factor again. Which account that + // session lands on is not left to the shared cookie jar: the SDK sends the + // profile's own account as an OIDC login_hint (see the Go binding's + // profile_state.go), so a re-login targets the account the profile already + // belongs to, and a profile with no stored account — fresh, or logged out — + // deliberately leaves the choice to the IdP, which is how accounts change. // - // So the stored session is reused for a plain re-login, and a fresh one is - // started in exactly the three cases where reuse would be wrong: - // - switching profiles, and the first login of a new profile: the stored - // session belongs to a different profile (sessionOwner != activeProfile); - // - after logging out: signing straight back in through the session that - // was just logged out of would make the logout meaningless. - // A nil owner means nothing has claimed the store yet, i.e. the first login - // on this install — nothing to inherit, so it may claim it. - let sessionOwner = Preferences.loadLastAuthenticatedProfile() - let ownsStoredSession = sessionOwner == nil || sessionOwner == activeProfile - let useEphemeralSession = Preferences.needsFreshBrowserSession() || !ownsStoredSession - logger.info("performLogin: '\(activeProfile, privacy: .public)' — \(useEphemeralSession ? "fresh" : "stored", privacy: .public) browser session (owner: \(sessionOwner ?? "none", privacy: .public))") - AppLogger.shared.log("performLogin: '\(activeProfile)' uses a \(useEphemeralSession ? "fresh" : "stored") browser session") + // A hint is advisory, though: an IdP holding a live session for another + // account may sign in with that session instead, which ends with the peer's + // key and the token belonging to different accounts ("peer is already + // registered by a different User"). Two cases have to ask the IdP for an + // account chooser rather than let the session resolve itself: + // + // - the profile has no account bound yet. Either it never completed an SSO + // login, or it was logged out, or it last logged in before this app + // version existed — in all three the app has nothing to steer with and no + // way to tell whose account the session is holding. It costs one chooser + // screen, once: the login binds the account and every re-login after it + // goes out with the hint and stays silent. + // - the session last signed in as a different profile, so its account is + // known to be the wrong one for this login. + // + // See authorizeURL(_:promptingForAccount:) for why that ends up as + // prompt=login rather than the friendlier prompt=select_account. + let boundAccount = ProfileManager.shared.accountEmail(for: activeProfile) + let promptForAccount = boundAccount == nil || Preferences.browserSessionHoldsAnotherProfile(activeProfile) + if promptForAccount { + let reason = boundAccount == nil ? "no account bound to the profile" : "the browser session last signed in as another profile" + logger.info("performLogin: '\(activeProfile, privacy: .public)' asks the IdP to re-decide the account — \(reason, privacy: .public)") + AppLogger.shared.log("performLogin: '\(activeProfile)' asks the IdP to re-decide the account (\(reason))") + } else { + AppLogger.shared.log("performLogin: '\(activeProfile)' reuses the browser session with a login_hint") + } if let configPath = Preferences.configFile(), !configPath.isEmpty, let auth = NetBirdSDKNewAuth(configPath, activeManagementURL, nil) { // A stale flow from an abandoned attempt would keep its loopback port @@ -552,10 +563,11 @@ public class NetworkExtensionAdapter: ObservableObject { // the extension, trips its needsLogin path, and pops the auth alert // in parallel with this browser login. Ordering them guarantees the // await caller sees showBrowser == true. - let browserURL = url + let rewritten = Self.authorizeURL(url, promptingForAccount: promptForAccount) + let browserURL = rewritten.url + AppLogger.shared.log("performLogin: authorize URL account prompt — \(rewritten.outcome.rawValue)") DispatchQueue.main.async { browserPhaseStarted = true - self?.useEphemeralBrowserSession = useEphemeralSession self?.loginErrorMessage = nil self?.pendingAuthorizeURL = browserURL self?.loginURL = browserURL @@ -579,19 +591,13 @@ public class NetworkExtensionAdapter: ObservableObject { ProfileManager.shared.saveServerURL(activeManagementURL, for: activeProfile) Preferences.saveManagementURL(activeManagementURL) } - // An ephemeral login leaves no cookies behind, so the shared - // session still holds whichever account was there — only a - // shared-session login may claim it. - // This login just put its account's SSO and trusted-device state - // into the shared browser store, so the profile now owns it and - // its re-logins may reuse it. A fresh session leaves nothing - // behind, so it cannot take ownership — claiming from one would - // send this profile's next login into a store still holding - // another profile's account. - if !useEphemeralSession { - Preferences.saveLastAuthenticatedProfile(activeProfile) - } - Preferences.clearNeedsFreshBrowserSession() + // The account this login ran under is recorded by the SDK itself, + // keyed by the config path it was handed, so the next login for + // this profile can go out with it as the login_hint. What the SDK + // cannot see is the browser session it went through, so record + // here which profile that session now holds — the next login of a + // different profile uses it to ask for the account chooser. + Preferences.saveLastBrowserLoginProfile(activeProfile) AppLogger.shared.log("performLogin: SDK login succeeded for '\(activeProfile)'") // onSuccess runs on a background goroutine. Mark success on the main // queue so the browser-finished handler (also main-queue) reliably @@ -624,7 +630,7 @@ public class NetworkExtensionAdapter: ObservableObject { AppLogger.shared.log("performLogin: SDK login failed: \(message)") // onError runs on a background goroutine; mutate state on the main // queue to stay consistent with onSuccess and cancelLogin(). - DispatchQueue.main.async { [weak self] in + DispatchQueue.main.async { guard let self else { return } self.logger.error("performLogin: SDK login failed: \(message, privacy: .public)") self.pendingAuth = nil @@ -680,19 +686,73 @@ public class NetworkExtensionAdapter: ObservableObject { return } #if os(iOS) - // Same session policy as the main-app path above. This path cannot observe - // login success, so it never claims the stored session — which also means it - // must not reuse one it does not already own. - let fallbackOwner = Preferences.loadLastAuthenticatedProfile() + // Same account-chooser policy as the main-app path. This path cannot observe + // login success, so it never records which profile the session ended up on — + // which also means the profile never gets an account bound and every login + // here asks, rather than silently resolving through the session. let fallbackProfile = ProfileManager.shared.getActiveProfileName() - self.useEphemeralBrowserSession = Preferences.needsFreshBrowserSession() - || (fallbackOwner != nil && fallbackOwner != fallbackProfile) - self.pendingAuthorizeURL = url - #endif + let rewritten = Self.authorizeURL( + url, + promptingForAccount: ProfileManager.shared.accountEmail(for: fallbackProfile) == nil + || Preferences.browserSessionHoldsAnotherProfile(fallbackProfile) + ) + AppLogger.shared.log("performLogin: authorize URL account prompt — \(rewritten.outcome.rawValue)") + self.pendingAuthorizeURL = rewritten.url + self.loginURL = rewritten.url + #else self.loginURL = url + #endif self.showBrowser = true } + #if os(iOS) + /// What asking the IdP to re-decide the account did to an authorize URL. Reported + /// so the log says what actually reached the IdP, not merely what was intended — + /// a request that was skipped looks identical from the outside otherwise. + enum AccountPromptOutcome: String { + /// `prompt=login` was added. + case added + /// The flow already asked for a prompt of its own; it is left alone. + case alreadyPrompting + /// The URL could not be parsed, so it goes out untouched. + case urlNotParsable + /// This login may resolve through the existing session. + case notRequested + } + + /// Asks the IdP to re-decide which account signs in, for logins that must not be + /// resolved by whatever session the browser already holds. + /// + /// The value is `login`, not `select_account`. `select_account` is the friendlier + /// request — pick an account, no re-authentication — but only Google, Microsoft + /// and Okta implement it; Auth0 and Zitadel ignore it and sign in with the session + /// they already have, which is exactly the failure this is meant to prevent. + /// `prompt=login` is the one value every OIDC provider honours. It costs a + /// password on an account switch, but not the second factor: it re-authenticates + /// the user, while the trusted-device cookie that gates 2FA stays in the jar. + /// + /// A `prompt` the flow itself put there (the management server drives this through + /// its login flag) wins — overriding a server-chosen prompt is not this layer's + /// call. + static func authorizeURL( + _ urlString: String, + promptingForAccount: Bool + ) -> (url: String, outcome: AccountPromptOutcome) { + guard promptingForAccount else { return (urlString, .notRequested) } + guard var components = URLComponents(string: urlString) else { + return (urlString, .urlNotParsable) + } + var items = components.queryItems ?? [] + guard !items.contains(where: { $0.name == "prompt" }) else { + return (urlString, .alreadyPrompting) + } + items.append(URLQueryItem(name: "prompt", value: "login")) + components.queryItems = items + guard let rewritten = components.string else { return (urlString, .urlNotParsable) } + return (rewritten, .added) + } + #endif + #if os(iOS) /// Aborts an in-progress interactive login (e.g. the user dismissed the OAuth /// browser without completing it). Stopping the SDK auth cancels its context, diff --git a/NetbirdKit/Preferences.swift b/NetbirdKit/Preferences.swift index ada9d4c7..99b496eb 100644 --- a/NetbirdKit/Preferences.swift +++ b/NetbirdKit/Preferences.swift @@ -178,49 +178,44 @@ class Preferences { return sharedUserDefaults()?.string(forKey: managementURLKey) } - // MARK: - Fresh Browser Session - - /// Whether the stored browser session must not be reused by the next login. - static func needsFreshBrowserSession() -> Bool { - return sharedUserDefaults()?.bool(forKey: GlobalConstants.keyNeedsFreshBrowserSession) ?? false - } - - /// Requires the next login to start a fresh browser session. Set on logout, so - /// signing back in cannot silently reuse the session that was just logged out of. - static func setNeedsFreshBrowserSession() { - sharedUserDefaults()?.set(true, forKey: GlobalConstants.keyNeedsFreshBrowserSession) - } - - /// Clears the requirement once a login has run with a fresh session. - static func clearNeedsFreshBrowserSession() { - sharedUserDefaults()?.removeObject(forKey: GlobalConstants.keyNeedsFreshBrowserSession) - } - - // MARK: - Last Authenticated Profile + // MARK: - Login Browser Account Tracking // - // The login browser (ASWebAuthenticationSession) shares Safari's cookie jar, - // which the app cannot scope per profile or clear. Recording which profile that - // jar was last authenticated for lets the adapter add prompt=select_account - // whenever a login targets a different profile, so the IdP re-asks which - // account to use instead of silently signing in the previous one. - - /// Records the profile the login browser last authenticated. - static func saveLastAuthenticatedProfile(_ name: String) { - sharedUserDefaults()?.set(name, forKey: GlobalConstants.keyLastAuthenticatedProfile) - } - - /// The profile the login browser last authenticated, or nil if none has yet. - static func loadLastAuthenticatedProfile() -> String? { - return sharedUserDefaults()?.string(forKey: GlobalConstants.keyLastAuthenticatedProfile) - } - - /// Forgets the marker if it names `profile`. Called on logout and profile - /// removal so the next login re-asks which account to use. - static func clearLastAuthenticatedProfile(ifEquals profile: String) { - guard let defaults = sharedUserDefaults() else { return } - if defaults.string(forKey: GlobalConstants.keyLastAuthenticatedProfile) == profile { - defaults.removeObject(forKey: GlobalConstants.keyLastAuthenticatedProfile) - } + // The login browser has one cookie jar shared by every profile. login_hint tells + // the IdP which account a profile wants, but a hint is advisory — an IdP with a + // live session for another account signs in with that session instead, which is + // how a profile ends up holding a peer key and a token from two different + // accounts. Recording which profile last completed a login through that jar lets + // the next login of a different profile ask the IdP to re-decide, instead of + // hoping the hint is honoured. Drift only ever costs one extra prompt, so nothing + // depends on this being exact. + + /// Profile whose account the shared browser session last signed in with. Nil when + /// no login has completed yet; "" when the session holds an account no profile may + /// silently reuse (see `requireAccountSelectionOnNextLogin`). + static func loadLastBrowserLoginProfile() -> String? { + return sharedUserDefaults()?.string(forKey: GlobalConstants.keyLastBrowserLoginProfile) + } + + /// Records the profile a completed login signed in as. + static func saveLastBrowserLoginProfile(_ name: String) { + sharedUserDefaults()?.set(name, forKey: GlobalConstants.keyLastBrowserLoginProfile) + } + + /// Makes the next login — of any profile — ask the IdP to re-decide the account. + /// Called on logout and on removing a profile: the browser session still holds the + /// account that was just left, and no profile should be signed back into it + /// silently. Stores "" because no profile can be named that, so the "profile + /// changed" test below matches every profile. + static func requireAccountSelectionOnNextLogin() { + sharedUserDefaults()?.set("", forKey: GlobalConstants.keyLastBrowserLoginProfile) + } + + /// Whether a login for `profile` must make the IdP re-decide which account signs + /// in: the shared session last signed in as a different profile, or as an account + /// that was logged out. The first login on an install has nothing to disambiguate. + static func browserSessionHoldsAnotherProfile(_ profile: String) -> Bool { + guard let last = loadLastBrowserLoginProfile() else { return false } + return last != profile } /// Restore config from UserDefaults to the config file path. diff --git a/NetbirdKit/ProfileManager.swift b/NetbirdKit/ProfileManager.swift index 8c51d9d7..4e395013 100644 --- a/NetbirdKit/ProfileManager.swift +++ b/NetbirdKit/ProfileManager.swift @@ -8,6 +8,7 @@ // import Foundation +import NetBirdSDK // MARK: - Profile Model @@ -165,14 +166,16 @@ class ProfileManager { try writeMeta(meta) } + // The profile's account file lives inside `dir`, so removing the directory + // already takes the stored login_hint with it — a future profile with the + // same name starts with no account bound to it. try fileManager.removeItem(atPath: dir) ProfileConnectionCache().remove(for: name) - // If the removed profile owned the stored browser session, nothing owns it - // now — the next login must start fresh rather than inherit that account. - if Preferences.loadLastAuthenticatedProfile() == name { - Preferences.setNeedsFreshBrowserSession() + // The browser session may still hold this profile's account, and nothing + // names it any more — the next login has to ask which account to use. + if Preferences.loadLastBrowserLoginProfile() == name { + Preferences.requireAccountSelectionOnNextLogin() } - Preferences.clearLastAuthenticatedProfile(ifEquals: name) } /// Clears authentication data for a profile by removing its config and state files. @@ -192,11 +195,14 @@ class ProfileManager { } cache.clearConnectionData(for: name) - // Logging out must actually log out: without this the next login would sign - // straight back in through the browser session this profile left behind, so - // require a fresh one and drop the ownership marker with it. - Preferences.setNeedsFreshBrowserSession() - Preferences.clearLastAuthenticatedProfile(ifEquals: name) + // Logging out must actually log out. While the account email is on disk it + // goes out as the login_hint, which would steer the next login straight back + // into the account just logged out of — dropping it hands the account choice + // back to the IdP, which is how a profile changes accounts. The browser + // session still holds that account, though, so also require the next login to + // ask which account to use rather than resolving silently through it. + clearAccountEmail(for: name) + Preferences.requireAccountSelectionOnNextLogin() if fileManager.fileExists(atPath: statePath) { try fileManager.removeItem(atPath: statePath) @@ -264,6 +270,34 @@ class ProfileManager { return ProfileConnectionCache().managementURL(for: profile) } + // MARK: - Account Binding + // + // The SDK records the account a profile logged in with next to that profile's + // config file, and reads it back as the OIDC login_hint on every later login, + // so a re-login returns to the same account without a fresh password + OTP + // prompt. The app owns the directory layout, so it goes through the SDK by + // config path rather than duplicating the file naming here. + + /// Account the profile last logged in with, or nil if it never completed an SSO + /// login or was logged out. Display-only — an unresolvable path reads as nil. + func accountEmail(for profile: String) -> String? { + guard let cfgPath = configPath(for: profile) else { return nil } + let email = NetBirdSDKProfileAccountEmail(cfgPath) + return email.isEmpty ? nil : email + } + + /// Forgets the account bound to a profile, so its next login carries no + /// login_hint and the IdP asks which account to use. + func clearAccountEmail(for profile: String) { + guard let cfgPath = configPath(for: profile) else { return } + var err: NSError? + NetBirdSDKClearProfileAccountEmail(cfgPath, &err) + if let err { + // Not fatal: a stale hint costs an account switch, not the logout itself. + AppLogger.shared.log("ProfileManager: failed to clear account email for '\(profile)': \(err.localizedDescription)") + } + } + /// Saves the management URL to a dedicated file inside the profile directory. /// This file is NOT deleted by logoutProfile(), so it survives logout. func saveServerURL(_ url: String, for profile: String) { From 53fafb5b3e38c673983dc917c5b0d7b5a9f73613 Mon Sep 17 00:00:00 2001 From: evgeniyChepelev <68751844+evgeniyChepelev@users.noreply.github.com> Date: Thu, 13 Aug 2026 14:33:27 +0200 Subject: [PATCH 7/8] Update submodule and fix conflicts --- .gitmodules | 8 +++++++- NetbirdKit/NetworkExtensionAdapter.swift | 4 +++- netbird-core | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/.gitmodules b/.gitmodules index a764677d..6d2268d8 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,9 @@ [submodule "netbird-core"] path = netbird-core - url = https://github.com/netbirdio/netbird.git + # TEMPORARY: points at the fork's feat/ios-login-hint, which carries the + # login_hint support this branch's SSO login needs (ProfileAccountEmail / + # ClearProfileAccountEmail in client/ios/NetBirdSDK). Revert to + # https://github.com/netbirdio/netbird.git once that lands upstream, and pin + # the submodule to an upstream commit before merging. + url = https://github.com/evgeniyChepelev/netbird.git + branch = feat/ios-login-hint diff --git a/NetbirdKit/NetworkExtensionAdapter.swift b/NetbirdKit/NetworkExtensionAdapter.swift index 6045d876..3db218db 100644 --- a/NetbirdKit/NetworkExtensionAdapter.swift +++ b/NetbirdKit/NetworkExtensionAdapter.swift @@ -619,7 +619,9 @@ public class NetworkExtensionAdapter: ObservableObject { // when the user dismisses the success page. if !self.showBrowser { self.logger.info("performLogin: login completed after browser closed - starting VPN") - self.startVPNConnection() + // The management login just completed here, so the extension + // can skip its own needs-login check (one Login RPC). + self.startVPNConnection(loginVerified: true) } } } diff --git a/netbird-core b/netbird-core index 5584f8ef..0d92607d 160000 --- a/netbird-core +++ b/netbird-core @@ -1 +1 @@ -Subproject commit 5584f8ef0a8dbd7d1db49655914d7c2c39b431cf +Subproject commit 0d92607d30cea3a36914cf5b0b191c9406361a08 From bc37825e22296a2a9087582a4146f28806e17103 Mon Sep 17 00:00:00 2001 From: evgeniyChepelev <68751844+evgeniyChepelev@users.noreply.github.com> Date: Thu, 13 Aug 2026 17:02:14 +0200 Subject: [PATCH 8/8] fix(ios): keep the authorize URL out of the log and profile lookups out of body --- .../App/Views/iOS/ProfilesListView.swift | 29 +++++++++++++++++-- .../App/Views/iOS/iOSConnectionView.swift | 7 +++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/NetBird/Source/App/Views/iOS/ProfilesListView.swift b/NetBird/Source/App/Views/iOS/ProfilesListView.swift index 455059a6..c62279f6 100644 --- a/NetBird/Source/App/Views/iOS/ProfilesListView.swift +++ b/NetBird/Source/App/Views/iOS/ProfilesListView.swift @@ -7,9 +7,17 @@ import SwiftUI #if os(iOS) +/// Per-profile display values shown under a profile's name. Resolved when the +/// list is loaded rather than while a row renders — see `loadProfiles()`. +private struct ProfileDisplayDetails { + let serverURL: String? + let account: String? +} + struct ProfilesListView: View { @EnvironmentObject var viewModel: ViewModel @State private var profiles: [Profile] = [] + @State private var profileDetails: [String: ProfileDisplayDetails] = [:] @State private var showAddSheet = false @State private var showSwitchAlert = false @State private var showRemoveAlert = false @@ -157,14 +165,18 @@ struct ProfilesListView: View { /// profile last signed in with — it is also what goes out as the login_hint on /// the next login, so showing it makes visible which account a re-login returns /// to. A profile that never completed an SSO login, or was logged out, has none. + /// + /// Reads only what `loadProfiles()` already resolved: both lookups touch the + /// filesystem, and a body may be evaluated any number of times. @ViewBuilder private func profileSubtitle(for profile: Profile) -> some View { - if let url = ProfileManager.shared.managementURL(for: profile.name) { + let details = profileDetails[profile.name] + if let url = details?.serverURL { Text(url) .font(.footnote) .foregroundColor(Color("TextSecondary")) } - if let email = ProfileManager.shared.accountEmail(for: profile.name) { + if let email = details?.account { Text(email) .font(.footnote) .foregroundColor(Color("TextSecondary")) @@ -176,7 +188,18 @@ struct ProfilesListView: View { // MARK: - Actions private func loadProfiles() { - profiles = ProfileManager.shared.listProfiles() + let loaded = ProfileManager.shared.listProfiles() + // Resolved here, not while a row renders: managementURL(for:) reads the + // profile's config and writes the resolved URL back to the server-URL file + // and the connection cache, and accountEmail(for:) goes through the SDK to + // disk. Doing either inside `body` turns every redraw into file I/O. + profileDetails = Dictionary(uniqueKeysWithValues: loaded.map { profile in + (profile.name, ProfileDisplayDetails( + serverURL: ProfileManager.shared.managementURL(for: profile.name), + account: ProfileManager.shared.accountEmail(for: profile.name) + )) + }) + profiles = loaded } private func switchToProfile(_ profile: Profile) { diff --git a/NetBird/Source/App/Views/iOS/iOSConnectionView.swift b/NetBird/Source/App/Views/iOS/iOSConnectionView.swift index c8e899f5..6755ac5d 100644 --- a/NetBird/Source/App/Views/iOS/iOSConnectionView.swift +++ b/NetBird/Source/App/Views/iOS/iOSConnectionView.swift @@ -164,8 +164,11 @@ struct iOSConnectionView: View { // outcome, leaving the SDK flow pending until it expires — cancel it // here instead. Color.clear.onAppear { - let raw = viewModel.networkExtensionAdapter.loginURL ?? "" - AppLogger.shared.log("Login browser: unusable login URL (\(raw)) — cancelling") + // The URL never goes to the log: it carries the OAuth state, the + // redirect target and the login_hint — the user's email address. + // Which of the two failure modes it was is the diagnostic part. + let reason = viewModel.networkExtensionAdapter.loginURL == nil ? "missing" : "unparsable" + AppLogger.shared.log("Login browser: \(reason) authorize URL — cancelling") viewModel.cancelPendingLogin() } }