apple: macOS app: Some bugfixes (#1937)

Fixes the following issues with the macOS app: 1. Every alternate launch of the app caused the tunnel to go down 2. When signing out and signing back in, resources were not getting updated 3. While the tunnel is up, when we quit the app and restart the app, the tunnel was brought down
2026-01-27 10:18:54 +00:00 · 2023-08-22 20:55:16 +05:30
parent 54e1a79a50
commit e1ac496545
10 changed files with 111 additions and 81 deletions
--- a/swift/apple/FirezoneKit/Sources/FirezoneKit/AuthClient/AuthClient.swift
+++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/AuthClient/AuthClient.swift
@@ -81,12 +81,8 @@ private final class WebAuthenticationSession: NSObject,
          return
        }

-        do {
-          let authResponse = try AuthResponse(portalURL: host, token: token, actorName: actorName)
-          continuation.resume(returning: authResponse)
-        } catch {
-          continuation.resume(throwing: AuthClientError.authResponseError(error))
-        }
+        let authResponse = AuthResponse(portalURL: host, token: token, actorName: actorName)
+        continuation.resume(returning: authResponse)
      }

      session.presentationContextProvider = self
--- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Features/MainView.swift
+++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Features/MainView.swift
@@ -18,7 +18,10 @@ final class MainViewModel: ObservableObject {
  private let appStore: AppStore

  var authResponse: AuthResponse? {
-    appStore.auth.authResponse
+    switch appStore.auth.loginStatus {
+      case .signedIn(let authResponse): return authResponse
+      default: return nil
+    }
  }

  var status: NEVPNStatus {
--- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Features/WelcomeView.swift
+++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Features/WelcomeView.swift
@@ -59,16 +59,17 @@ final class WelcomeViewModel: ObservableObject {
      destination = .undefinedSettingsAlert(.undefinedSettings)
    }

-    appStore.auth.$authResponse
+    appStore.auth.$loginStatus
      .receive(on: mainQueue)
-      .sink(receiveValue: { [weak self] authResponse in
+      .sink(receiveValue: { [weak self] loginStatus in
        guard let self else {
          return
        }

-        if authResponse != nil {
+        switch loginStatus {
+        case .signedIn:
          self.state = .authenticated(MainViewModel(appStore: self.appStore))
-        } else {
+        default:
          self.state = .unauthenticated(AuthViewModel())
        }
      })
--- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/AuthResponse.swift
+++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/AuthResponse.swift
@@ -16,7 +16,7 @@ struct AuthResponse {
  // The opaque auth token
  let token: String

-  init(portalURL: URL, token: String, actorName: String?) throws {
+  init(portalURL: URL, token: String, actorName: String?) {
    self.portalURL = portalURL
    self.actorName = actorName
    self.token = token
@@ -26,14 +26,14 @@ struct AuthResponse {
 #if DEBUG
  extension AuthResponse {
    static let invalid =
-      try! AuthResponse(
+      AuthResponse(
        portalURL: URL(string: "http://localhost:4568")!,
        token: "",
        actorName: nil
      )

    static let valid =
-      try! AuthResponse(
+      AuthResponse(
        portalURL: URL(string: "http://localhost:4568")!,
        token: "b1zwwwAdf=",
        actorName: "foobar"
--- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/AppStore.swift
+++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/AppStore.swift
@@ -31,26 +31,29 @@ final class AppStore: ObservableObject {
    }
    .store(in: &cancellables)

-    auth.$authResponse
+    auth.$loginStatus
      .receive(on: mainQueue)
-      .sink { [weak self] authResponse in
+      .sink { [weak self] loginStatus in
        Task { [weak self] in
-          await self?.handleAuthResponseChanged(authResponse)
+          await self?.handleLoginStatusChanged(loginStatus)
        }
      }
      .store(in: &cancellables)
  }

-  private func handleAuthResponseChanged(_ authResponse: AuthResponse?) async {
-    if let authResponse = authResponse {
+  private func handleLoginStatusChanged(_ loginStatus: AuthStore.LoginStatus) async {
+    switch loginStatus {
+    case .signedIn(let authResponse):
      do {
        try await tunnel.start(authResponse: authResponse)
      } catch {
        logger.error("Error starting tunnel: \(String(describing: error)) -- signing out")
        auth.signOut()
      }
-    } else {
+    case .signedOut:
      tunnel.stop()
+    case .uninitialized:
+      break
    }
  }

--- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/AuthStore.swift
+++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/AuthStore.swift
@@ -26,6 +26,12 @@ final class AuthStore: ObservableObject {

  static let shared = AuthStore()

+  enum LoginStatus {
+    case uninitialized
+    case signedOut
+    case signedIn(AuthResponse)
+  }
+
  @Dependency(\.keychain) private var keychain
  @Dependency(\.auth) private var auth
  @Dependency(\.settingsClient) private var settingsClient
@@ -33,43 +39,44 @@ final class AuthStore: ObservableObject {
  private let authBaseURL: URL
  private var cancellables = Set<AnyCancellable>()

-  @Published private(set) var authResponse: AuthResponse?
+  @Published private(set) var loginStatus: LoginStatus

  private init() {
    self.authBaseURL = Self.getAuthBaseURLFromInfoPlist()
+    self.loginStatus = .uninitialized
    Task {
-      self.authResponse = await { () -> AuthResponse? in
+      self.loginStatus = await { () -> LoginStatus in
        guard let teamId = settingsClient.fetchSettings()?.teamId else {
          logger.debug("No team-id found in settings")
-          return nil
+          return .signedOut
        }
        guard let token = try? await keychain.token() else {
          logger.debug("Token not found in keychain")
-          return nil
+          return .signedOut
        }
        guard let actorName = try? await keychain.actorName() else {
          logger.debug("Actor not found in keychain")
-          return nil
+          return .signedOut
        }
        let portalURL = self.authURL(teamId: teamId)
-        guard let authResponse = try? AuthResponse(portalURL: portalURL, token: token, actorName: actorName) else {
-          logger.debug("Token or Actor recovered from keychain is invalid")
-          return nil
-        }
+        let authResponse = AuthResponse(portalURL: portalURL, token: token, actorName: actorName)
        logger.debug("Token recovered from keychain.")
-        return authResponse
+        return .signedIn(authResponse)
      }()
    }

-    $authResponse.dropFirst()
-      .sink { [weak self] authResponse in
+    $loginStatus
+      .sink { [weak self] loginStatus in
        Task { [weak self] in
-          if let authResponse {
-            try? await self?.keychain.save(token: authResponse.token, actorName: authResponse.actorName)
-            self?.logger.debug("authResponse saved on keychain.")
-          } else {
-            try? await self?.keychain.deleteAuthResponse()
-            self?.logger.debug("token deleted from keychain.")
+          switch loginStatus {
+            case .signedIn(let authResponse):
+              try? await self?.keychain.save(token: authResponse.token, actorName: authResponse.actorName)
+              self?.logger.debug("authResponse saved on keychain.")
+            case .signedOut:
+              try? await self?.keychain.deleteAuthResponse()
+              self?.logger.debug("token deleted from keychain.")
+            case .uninitialized:
+              break
          }
        }
      }
@@ -81,7 +88,7 @@ final class AuthStore: ObservableObject {

    let portalURL = authURL(teamId: teamId)
    let authResponse = try await auth.signIn(portalURL)
-    self.authResponse = authResponse
+    self.loginStatus = .signedIn(authResponse)
  }

  func signIn() async throws {
@@ -98,7 +105,7 @@ final class AuthStore: ObservableObject {
  func signOut() {
    logger.trace("\(#function)")

-    authResponse = nil
+    loginStatus = .signedOut
  }

  static func getAuthBaseURLFromInfoPlist() -> URL {
--- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/TunnelStore.swift
+++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/TunnelStore.swift
@@ -20,7 +20,7 @@ final class TunnelStore: ObservableObject {
    didSet { setupTunnelObservers() }
  }

-  @Published private(set) var status: NEVPNStatus = .invalid {
+  @Published private(set) var status: NEVPNStatus {
    didSet { TunnelStore.logger.info("status changed: \(self.status.description)") }
  }

@@ -41,6 +41,7 @@ final class TunnelStore: ObservableObject {
  init(tunnel: NETunnelProviderManager) {
    self.controlPlaneURL = Self.getControlPlaneURLFromInfoPlist()
    self.tunnel = tunnel
+    self.status = tunnel.connection.status
    tunnel.isEnabled = true
    setupTunnelObservers()
  }
@@ -67,6 +68,16 @@ final class TunnelStore: ObservableObject {
    // make sure we have latest preferences before starting
    try await tunnel.loadFromPreferences()

+    if tunnel.connection.status == .connected || tunnel.connection.status == .connecting {
+      if let (tunnelControlPlaneURLString, tunnelToken) = Self.getTunnelConfigurationParameters(of: tunnel) {
+        if tunnelControlPlaneURLString == self.controlPlaneURL.absoluteString && tunnelToken == authResponse.token {
+          // Already connected / connecting with the required configuration
+          TunnelStore.logger.debug("\(#function): Already connected / connecting. Nothing to do.")
+          return
+        }
+      }
+    }
+
    tunnel.protocolConfiguration = Self.makeProtocolConfiguration(
      controlPlaneURL: self.controlPlaneURL,
      token: authResponse.token
@@ -104,6 +115,10 @@ final class TunnelStore: ObservableObject {

  private func updateResources() {
    let session = tunnel.connection as! NETunnelProviderSession
+    guard session.status == .connected else {
+      self.resources = DisplayableResources()
+      return
+    }
    let resourcesQuery = resources.versionStringToData()
    do {
      try session.sendProviderMessage(resourcesQuery) { [weak self] reply in
@@ -162,6 +177,19 @@ final class TunnelStore: ObservableObject {
    return proto
  }

+  private static func getTunnelConfigurationParameters(of tunnelProvider: NETunnelProviderManager) -> (String, String)? {
+    guard let tunnelProtocol = tunnelProvider.protocolConfiguration as? NETunnelProviderProtocol else {
+      return nil
+    }
+    guard let controlPlaneURLString = tunnelProtocol.providerConfiguration?["controlPlaneURL"] as? String else {
+      return nil
+    }
+    guard let token = tunnelProtocol.providerConfiguration?["token"] as? String else {
+      return nil
+    }
+    return (controlPlaneURLString, token)
+  }
+
  private func setupTunnelObservers() {
    TunnelStore.logger.trace("\(#function)")

@@ -174,14 +202,13 @@ final class TunnelStore: ObservableObject {
          named: .NEVPNStatusDidChange,
          object: nil
        ) {
-          guard let session = notification.object as? NETunnelProviderSession,
-                let tunnelProvider = session.manager as? NETunnelProviderManager
-          else {
+          guard let session = notification.object as? NETunnelProviderSession else {
            return
          }
-          self.status = tunnelProvider.connection.status
+          let status = session.status
+          self.status = status
          if let startTunnelContinuation = self.startTunnelContinuation {
-            switch self.status {
+            switch status {
              case .connected:
                startTunnelContinuation.resume(returning: ())
                self.startTunnelContinuation = nil
@@ -192,6 +219,9 @@ final class TunnelStore: ObservableObject {
                break
            }
          }
+          if status != .connected {
+            self.resources = DisplayableResources()
+          }
        }
      }
    )
--- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/MenuBar.swift
+++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/MenuBar.swift
@@ -53,17 +53,19 @@ public final class MenuBar: NSObject {
      Task {
        let tunnel = try await TunnelStore.loadOrCreate()
        self.appStore = AppStore(tunnelStore: TunnelStore(tunnel: tunnel))
+        updateStatusItemIcon()
      }
    }

    private func setupObservers() {
-      appStore?.auth.$authResponse
+      appStore?.auth.$loginStatus
        .receive(on: mainQueue)
-        .sink { [weak self] authResponse in
-          if let authResponse {
-            self?.showSignedIn(authResponse.actorName)
-          } else {
-            self?.showSignedOut()
+        .sink { [weak self] loginStatus in
+          switch loginStatus {
+            case .signedIn(let authResponse):
+              self?.showSignedIn(authResponse.actorName)
+            default:
+              self?.showSignedOut()
          }
        }
        .store(in: &cancellables)
@@ -71,17 +73,8 @@ public final class MenuBar: NSObject {
      appStore?.tunnel.$status
        .receive(on: mainQueue)
        .sink { [weak self] status in
-          if status == .connected {
-            self?.connectionMenuItem.title = "Disconnect"
-            self?.statusItem.button?.image = self?.connectedIcon
-          } else {
-            self?.connectionMenuItem.title = "Connect"
-            self?.statusItem.button?.image = self?.disconnectedIcon
-          }
+          self?.updateStatusItemIcon()
          self?.handleMenuVisibilityOrStatusChanged()
-          if status != .connected {
-            self?.setOrderedResources([])
-          }
        }
        .store(in: &cancellables)

@@ -96,14 +89,6 @@ public final class MenuBar: NSObject {

    private lazy var menu = NSMenu()

-    private lazy var connectionMenuItem = createMenuItem(
-      menu,
-      title: "Connect",
-      action: #selector(connectButtonTapped),
-      isHidden: true,
-      target: self
-    )
-
    private lazy var signInMenuItem = createMenuItem(
      menu,
      title: "Sign in",
@@ -152,7 +137,6 @@ public final class MenuBar: NSObject {
    }()

    private func createMenu() {
-      menu.addItem(connectionMenuItem)
      menu.addItem(signInMenuItem)
      menu.addItem(signOutMenuItem)
      menu.addItem(NSMenuItem.separator())
@@ -208,7 +192,7 @@ public final class MenuBar: NSObject {
        appStore?.tunnel.stop()
      } else {
        Task {
-          if let authResponse = appStore?.auth.authResponse {
+          if case .signedIn(let authResponse) = appStore?.auth.loginStatus {
            do {
              try await appStore?.tunnel.start(authResponse: authResponse)
            } catch {
@@ -249,6 +233,14 @@ public final class MenuBar: NSObject {
      NSWorkspace.shared.open(URL(string: "firezone://settings")!)
    }

+    private func updateStatusItemIcon() {
+      if self.appStore?.tunnel.status == .connected {
+        self.statusItem.button?.image = self.connectedIcon
+      } else {
+        self.statusItem.button?.image = self.disconnectedIcon
+      }
+    }
+
    private func handleMenuVisibilityOrStatusChanged() {
      guard let appStore = appStore else { return }
      let status = appStore.tunnel.status
--- a/swift/apple/FirezoneNetworkExtension/Adapter.swift
+++ b/swift/apple/FirezoneNetworkExtension/Adapter.swift
@@ -101,7 +101,7 @@ public class Adapter {
    // Shutdown the tunnel
    if case .tunnelReady(let wrappedSession) = self.state {
      logger.debug("Adapter.deinit: Shutting down connlib")
-      _ = wrappedSession.disconnect()
+      wrappedSession.disconnect()
    }
  }

@@ -148,15 +148,14 @@ public class Adapter {
        case .tunnelReady(let session):
          self.logger.debug("Adapter.stop: Shutting down connlib")
          self.state = .stoppingTunnel(session: session, onStopped: completionHandler)
-          _ = session.disconnect()
+          session.disconnect()
        case .startingTunnel(let session, let onStarted):
          self.logger.debug("Adapter.stop: Shutting down connlib before tunnel ready")
          self.state = .stoppingTunnel(session: session, onStopped: {
            onStarted?(AdapterError.stoppedByRequestWhileStarting)
            completionHandler()
          })
-          // FIXME: Is it ok to call disconnect() while we haven't got onTunnelReady?
-          _ = session.disconnect()
+          session.disconnect()
        case .stoppingTunnelTemporarily(let session, let onStopped):
          self.state = .stoppingTunnel(session: session, onStopped: {
            onStopped?()
@@ -210,8 +209,7 @@ extension Adapter {
          onStarted?(nil)
          self.packetTunnelProvider?.reasserting = true
          self.state = .stoppingTunnelTemporarily(session: session, onStopped: nil)
-          // FIXME: Is it ok to call disconnect() while we haven't got onTunnelReady?
-          _ = session.disconnect()
+          session.disconnect()
        }

      case .tunnelReady(let session):
@@ -225,7 +223,7 @@ extension Adapter {
          self.logger.debug("Adapter.didReceivePathUpdate: Offline. Shutting down connlib.")
          self.packetTunnelProvider?.reasserting = true
          self.state = .stoppingTunnelTemporarily(session: session, onStopped: nil)
-          _ = session.disconnect()
+          session.disconnect()
        }

      case .stoppingTunnelTemporarily:
--- a/swift/apple/FirezoneNetworkExtension/NetworkSettings.swift
+++ b/swift/apple/FirezoneNetworkExtension/NetworkSettings.swift
@@ -110,7 +110,7 @@ class NetworkSettings {
          }
          let validNetworkPrefixLength = Self.validNetworkPrefixLength(fromString: networkPrefixLengthString, maximumValue: 32)
          let ipv4SubnetMask = Self.ipv4SubnetMask(networkPrefixLength: validNetworkPrefixLength)
-          logger.debug("NetworkSettings.apply: Adding IPv4 route: \(address) (subnet mask: \(ipv4SubnetMask))")
+          logger.debug("NetworkSettings.apply: Adding IPv4 route: \(address, privacy: .public) (subnet mask: \(ipv4SubnetMask, privacy: .public))")
          tunnelIPv4Routes.append(NEIPv4Route(destinationAddress: address, subnetMask: ipv4SubnetMask))
        }
        if groupSeparator == ":" { // IPv6 address
@@ -119,7 +119,7 @@ class NetworkSettings {
            continue
          }
          let validNetworkPrefixLength = Self.validNetworkPrefixLength(fromString: networkPrefixLengthString, maximumValue: 128)
-          logger.debug("NetworkSettings.apply: Adding IPv6 route: \(address) (prefix length: \(validNetworkPrefixLength))")
+          logger.debug("NetworkSettings.apply: Adding IPv6 route: \(address, privacy: .public) (prefix length: \(validNetworkPrefixLength, privacy: .public))")
          tunnelIPv6Routes.append(NEIPv6Route(destinationAddress: address, networkPrefixLength: NSNumber(integerLiteral: validNetworkPrefixLength)))
        }
      } else {