Prompted by Xcode warning at project startup.
Most of the changes are simple migrations from entitlements files
to build settings, which is the recommended approach, and were done
automatically by Xcode.
new settings:
- REGISTER_APP_GROUPS - Automatically registers app groups with
provisioning
profile (I had to set this manually when setting up, so it's a welcome
change)
- STRING_CATALOG_GENERATE_SYMBOLS - type-safe localization (no
regression, we're not doing any localization currently)
- ENABLE_USER_SCRIPT_SANDBOXING - sandboxing all the build scripts
Note: I had to turn off the recommended `ENABLE_USER_SCRIPT_SANDBOXING`
as it
would interfere with our building of connlib during the build.
Also: make Makefile more ergonomic to use (setup LSP config during first
build)
This PR upgrades the Swift client from Swift 5 to Swift 6.2, addressing
all
concurrency-related warnings and runtime crashes that come with Swift
6's
strict concurrency checking.
## Swift 6 Concurrency Primer
**`actor`** - A new reference type that provides thread-safe, serialised
access to mutable state. Unlike classes, actors ensure that only one
piece of
code can access their mutable properties at a time. Access to actor
methods/properties requires await and automatically hops to the actor's
isolated executor.
**`@MainActor`** - An attribute that marks code to run on the main
thread.
Essential for UI updates and anything that touches UIKit/AppKit. When a
class/function is marked @MainActor, all its methods and properties
inherit
this isolation.
**`@Sendable`** - A protocol indicating that a type can be safely passed
across concurrency domains (between actors, tasks, etc.). Value types
(structs, enums) with Sendable stored properties are automatically
Sendable.
Reference types (classes) need explicit @unchecked Sendable if they
manage
thread-safety manually.
**`nonisolated`** - Opts out of the containing type's actor isolation.
For
example, a nonisolated method in a @MainActor class can be called from
any
thread without await. Useful for static methods or thread-safe
operations.
**`@concurrent`** - Used on closure parameters in delegate methods.
Indicates
the closure may be called from any thread, preventing the closure from
inheriting the surrounding context's actor isolation. Critical for
callbacks
from system frameworks that call from background threads.
**Data Races** - Swift 6 enforces at compile-time (and optionally at
runtime)
that mutable state cannot be accessed concurrently from multiple
threads. This
eliminates entire classes of bugs that were previously only caught
through
testing or production crashes.
## Swift Language Upgrade
- **Bump Swift 5 → 6.2**: Enabled strict concurrency checking throughout
the
codebase
- **Enable ExistentialAny (SE-0335)**: Adds compile-time safety by
making
protocol type erasure explicit (e.g., any Protocol instead of implicit
Protocol)
- **Runtime safety configuration**: Added environment variables to log
concurrency violations during development instead of crashing, allowing
gradual migration
## Concurrency Fixes
### Actor Isolation
- **TelemetryState actor** (Telemetry.swift:10): Extracted mutable
telemetry
state into a dedicated actor to eliminate data races from concurrent
access
- **SessionNotification @MainActor isolation**
(SessionNotification.swift:25):
Properly isolated the class to MainActor since it manages UI-related
callbacks
- **IPCClient caching** (IPCClient.swift): Fixed actor re-entrance
issues and
resource hash-based optimisation by caching the client instance in Store
### Thread-Safe Callbacks
- **WebAuthSession @concurrent delegate** (WebAuthSession.swift:46): The
authentication callback is invoked from a background thread by
ASWebAuthenticationSession. Marked the wrapper function as @concurrent
to
prevent MainActor inference on the completion handler closure, then
explicitly hopped back to MainActor for the session.start() call. This
fixes EXC_BAD_INSTRUCTION crashes at _dispatch_assert_queue_fail.
- **SessionNotification @concurrent delegate**
(SessionNotification.swift:131): Similarly marked the notification
delegate
method as @concurrent and used Task { @MainActor in } to safely invoke
the
MainActor-isolated signInHandler
### Sendable Conformances
- Added Sendable to Resource, Site, Token, Configuration, and other
model
types that are passed between actors and tasks
- **LogWriter immutability** (Log.swift): Made jsonData immutable to
prevent
capturing mutable variables in @Sendable closures
### Nonisolated Methods
- **Static notification display** (SessionNotification.swift:73): Marked
showSignedOutNotificationiOS() as nonisolated since it's called from the
Network Extension (different process) and only uses thread-safe APIs
Fixes#10674Fixes#10675
Upon moving the version string from PKG_VERSION and Cargo.toml, we lost
the bump version automation. To avoid more bugs here in the future, we
now check for the version marker across all Git-tracked files,
regardless of their extension.
Fixes#10748
---------
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
On macOS, because it uses the System Extension packaging type, the
lifecycle of the tunnel provider process is not tied directly to
connlib's session start and end, but rather managed by the system. The
process is likely running at all times, even when the GUI is not open or
signed in.
The system will start the provider process upon the first IPC call to
it, which allocates a `utun` interface. The tricky part is ensuring this
interface gets removed when the GUI app quits. Otherwise, it's likely
that upon the next launch of the GUI app, the system will allocate a
_new_ utun interface, and the old one will linger until the next system
reboot.
Here's where things get strange. The system will only remove the `utun`
interface when stopping the tunnel under the following conditions:
- The provider is currently not in a `disconnected` state (so it needs
to be in `reasserting`, `connecting`, or `connected`
- The GUI side has called `stopTunnel`, thereby invoking the provider's
`stopTunnel` override function, or
- The provider side has called `cancelTunnelWithError`, or
- The `startTunnel`'s completionHandler is called with an `Error`
The problem we had is that we make various IPC calls throughout the
lifecycle of the GUI app, for example, to gather logs, set tunnel
configuration, and the like. If the GUI app was _not_ in a connected
state when the user quit, the `utun` would linger, even though we were
issuing a final `stopTunnel` upon quit in all circumstances.
To fix the issue, we update the dry run `startTunnel` code path we added
previously in two ways:
1. We add a `dryRun` error type to the `startTunnel`'s completionHandler
2. We implement the GUI app `applicationShouldTerminate` handler in
order to trigger one final dryRun which briefly moves the provider to a
connected state so the system will clean us up when its
completionHandler is invoked.
Tested under the following conditions:
- Launch app in a signed-out state -> quit
- Launch app in a signed-out state -> sign in -> quit
- Launch app in a signed-out state -> sign in -> sign out -> quit
- Launch app in a signed-in state -> quit
- Launch app in a signed-in state -> sign out -> quit
Notably, if the GUI app is killed with `SIGKILL`, our terminate hook is
_not_ called, and the utun lingers. We'll have to accept this edge case
for now.
Along with the above, the janky `consumeStopReason` mechanism has been
removed in favor of NE's `cancelTunnelWithError` to pass the error back
to the GUI we can then use to show the signed out alert.
Fixes#10580
show an alert to the user and ask to quit previous Firezone instance
manually before starting a new one.
Resolves: #10295
---------
Signed-off-by: Mariusz Klochowicz <mariusz@klochowicz.com>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>
As recommended by the Sentry team [0], "App hang" tracking should be
disabled before calling into certain system APIs like showing alerts to
prevent false-positives.
[0]: https://github.com/getsentry/sentry-cocoa/issues/3472
When working on the Swift codebase, I noticed that running the formatter
produced a massive diff. This PR re-formats the Swift code with `swift
format . --recursive --in-place` and adds a CI check to enforce it going
forward.
Resolves: #9534
---------
Co-authored-by: Jamil Bou Kheir <jamilbk@users.noreply.github.com>
We are currently storing app configuration across three places:
- UserDefaults (favorite resources)
- VPN configuration (Settings)
- Disk (firezone id)
These can be consolidated to UserDefaults, which is the standard way to
store app configuration like this.
UserDefaults is the umbrella persistence store for regular app
configuration (`plist` files which are just XML dictionaries),
iCloud-synced app configuration across a user's devices, and managed app
configuration (MDM). They provide a cached, thread-safe, and
interprocess-supported mechanism for handling app configuration. We can
also subscribe to changes on this app configuration to react to changes.
Unfortunately, the System Extension ruins some of our fun because it
runs as root, and is confined to a different group container, meaning we
cannot share configuration directly between GUI and tunnel procs.
To address this, we use the tunnel process to store all vital
configuration and introduce IPC calls to set and fetch these.
Commit-by-commit review recommended, but things got a little crazy
towards the end when I realized that we can't share a single
UserDefaults between both procs.
When developing the macOS app, we always build the exact same version
and build code for each build. ~~This _may_ be one reason why we
constantly have to deactivate the extension before the new one will
launch.~~ Edit: Just tested, and I can verify that this does fix the
issue on dev builds, so no more having to uninstall the sysex between
builds.
Even if that's not the reason, this is a cleaner approach than building
it in our prod-only scripts.
---------
Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
The current `VPNConfigurationManager` class is too large and handles 3
separate things:
- Encoding IPC messages
- Sending IPC messages
- Loading/storing the VPN configuration
With this PR, these are split out into:
- ProviderMessage class
- IPCClient class
- VPNConfigurationManager class
These are then use directly from our `Store` in order to load their
state upon `init()`, set the relevant properties, and thus views are
updated accordingly.
A couple minor bugs are fixed as well as part of the refactor.
### Tested: macOS
- [x] Sign in
- [x] Sign out
- [x] Yanking the VPN configuration while signed in / out
- [x] Yanking the system extension while signed in / out
- [x] Denying the VPN configuration
- [x] Denying Notifications
- [x] Denying System Extension
### Tested: iOS
- [x] Sign in
- [x] Sign out
- [x] Yanking the VPN configuration while signed in / out
- [x] Yanking the system extension while signed in / out
- [x] Denying the VPN configuration
- [x] Denying Notifications
---------
Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
A regression was introduced in #8218 that removed the `menuBar` as an
environment object for `AppView`.
Unfortunately this compiles just fine, as EnvironmentObjects are loaded
at runtime, causing the "Open Menu" button to crash since it's looking
for a non-existent EnvironmentObject.
Our application state is incredibly simple, only consisting of a handful
of properties.
Throughout our codebase, we use a singular global state store called
`Store`. We then inject this as the singular piece of state into each
view's model.
This is unnecessary boilerplate and leads to lots of duplicated logic.
Instead we refactor away (nearly) all of the application's view models
and instead use an `@EnvironmentObject` to inject the store into each
view.
This convention drastically simplifies state tracking logic and
boilerplate in the views.
For some reason, this was being initialized twice, when it doesn't need
to be.
The whole reason Favorites is initialized in the FirezoneApp module is
so we can have one instance of it passed down to children.
Whether we execute a task on the main thread or a background thread
doesn't affect whether the thread is "hung" as reported by Sentry.
Instead, our options for fixing these are:
- Try to use an async version of the underlying API (the [async
version](https://developer.apple.com/documentation/appkit/nsworkspace/open(_:configuration:completionhandler:))
of `open` for example)
- If there is none, and the call could potentially block (most likely to
do disk IO contention), at least schedule this on a new thread using
`Task.detached` but with `.background` priority so that it will avoid
blocking any other execution.
The main takeaway here is that unfortunately, under some conditions,
Sentry will _always_ report an "App Hanging" alert since it's constantly
monitoring all threads for paused execution longer than 2000ms.
We'll probably end up letting some of these slide (pausing a background
or worker thread isn't necessarily a UX issue), but pausing the UI
thread is.
Luckily, we're able to use async APIs for most things. The remaining
things (like working with log files over IPC) we use a `Task.detached`
for.
On Apple, we will silently fail if the tunnel fails to start. This adds
a simple `NSAlert` modal (macOS) or `Alert` popup (iOS) that must be
dismissed before continuing if the tunnel fails to come up, so that the
user has a chance of understanding why.
The vast majority of the time this fails due to DNS lookup errors while
starting connlib.
Related: #7004
Supersedes: #7814
The Keychain on Apple platforms, while secure, is not always available.
It can be unavailable if the user has changed its permissions
accidentally, the keychain database is corrupt, there is an issue with
the secure enclave, or any number of other system-related or Apple
account-related reasons.
There are only two things we use the Keychain for:
- Storing the `firezone-id`. This is actually not a secret.
- Persisting the `token` upon sign in so that:
- the iOS system can keep the tunnel alive without the GUI running
- the macOS app can relaunch after `Disconnect & Quit` without having to
sign in again
For the first case, we move back to persisting this to a file (see
#7464).
For the second case, we simply don't care too much if the Keychain can't
be saved to. We simply move on with activating the tunnel and logging
the error so we know how often these edge cases occur.
On macOS 12, returning an empty body for a `WindowGroup` can cause the
app UI to crash with the following error:
```
*** Assertion failure in void _NSWindowSetFrameIvar(NSWindow *, NSRect)(), NSWindow.m:935
```
Since the `menuBar` is not initialized when the app initializes, this
conditional can return an empty body in a race condition.
Even though we're winding down support for macOS 12, it would be good to
fix this logic bug.
On macOS, there are two steps the user needs to allow for the VPN
profile to be active: enable the system extension and allow the VPN
profile.
The system extension must be installed before the VPN profile can be
allowed. This PR updates the flow so that the user is prompted to handle
both of those serially. Before, we tried to install the system extension
on launch and prompt the user to allow the profile at the same time.
This PR includes fixes to handle the edge cases where the user removes
the profile and/or extension while the tunnel is connected. When that
happens, the `NEVPNStatus` becomes `.invalid` and we replace the `Sign
In` link in the Menubar with a prompt to restart the grant permissions
flow. On iOS, the behavior is similar -- we move the view back to the
GrantPermissionsView.
Lastly, some refactoring is included to use consistent naming for
`VPNProfile` instead of `TunnelManager` to more closely match what Apple
calls these things.
Although interacting with the Keychain typically involves I/O, there
wasn't a good reason to make all of its functions `async` as this
infects the caller hierarchy. Instead, the calling code should decide
how to wrap the synchronous calls directly with their own preferred
mechanism for dealing with blocking operations.
In practice, it turns out that nearly all of the places we were
interacting with the Keychain was _already_ within some sort of
non-critical Task or equivalent, especially the important parts like the
UI thread. So this complexity is not required.
The Keychain was moved from an `actor` class to a regular enum because
we don't need the serialization guarantees here -- since the calls are
blocking, there is no risk of Keychain operations happening out of
order.
Tested on macOS 15.
- Refactor Telemetry module to expose firezoneId and accountSlug for
easier access in the Adapter module
- Set accountSlug to WrappedSession.connect for hydrating the Rust
sentry context
The CI swift workflow needs to be updated to accommodate the macOS
standalone build. This required a decent amount of refactoring to make
the Apple build process more maintainable.
Unfortunately this PR ended up being a giant ball of yarn where pulling
on one thread tended to unravel things elsewhere, since building the
Apple artifacts involve multiple interconnected systems. Combined with
the slow iteration of running in CI, I wasn't able to split this PR into
easier to digest commits, so I've annotated the PR as much as I can to
explain what's changed.
The good news is that Apple release artifacts can now be easily built
from a developer's machine with simply
`scripts/build/macos-standalone.sh`. The only thing needed is the proper
provisioning profiles and signing certs installed.
Since this PR is so big already, I'll save the swift/apple/README.md
updates for another PR.
Standalone distribution requires using a different signing identity
(certificate), set of provisioning profiles, and (annoyingly) requires
the `-systemextension` suffix for our network extension capabilities.
This PR prepares the Xcode environment for building a Standalone app in
CI that will be notarized by matching certificates and provisioning
profiles in our Apple Developer account.
Unlike the App extension which runs as the user, the system extension
introduced in macOS client 1.4.0 runs as `root` and thus cannot read the
App Group container directory for the GUI process. However, both
processes can read and write to the shared Keychain, which is how we
pass the token between the two processes already.
This PR does two things:
1. Tries to read an existing `firezone-id` from the pre-1.4.0 App Group
container upon app launch. This needs to be done from the GUI process.
If found, it stores it in the Keychain.
1. Refactors the `firezone-id` to be stored in the Keychain instead of a
plaintext file going forward.
The Keychain API is also cleaned up and abstracted to be more ergonomic
to use for both Token and Firezone ID storage purposes.
- Installs the system extension on app launch instead of each time we
start the tunnel, as [recommended by
Apple](https://developer.apple.com/documentation/systemextensions/installing-system-extensions-and-drivers).
This will typically happen when the app is installed for the first time,
or upgraded / downgraded.
- Changes the completion handler functionality for observing the system
extension status to an observed property on the class. This allows us to
update the MenuBar based on the status of the installation, preventing
the user from attempting to sign in unless the system extension has been
installed.
~~This PR exposes a new, subtle issue - since we don't reinstall the
system extension on each startTunnel, the process stays running. This is
expected. However, now the logging handle needs to be maintained across
connlib sessions, similar to the Android tunnel lifetime.~~ Fixed in
#7460
Expect one or two more PRs to handle further edge cases with improved UX
as more testing with the release build and upgrade/downgrade workflows
are attempted.
To allow macOS users to rollback, it would be helpful to distribute a
standalone macOS app, similar to how we distribute the GUI client.
The first step in this process is to refactor the macOS client to use a
System Extension -based Network Extension rather than an App Extension
based one. This offers us the flexibility to distribute the macOS client
outside the Mac App Store in addition to via the store.
For this PR I focused on making the minimal set of changes necessary to
support this change. This PR intentionally doesn't update the CI
pipeline to notarize and attach a standalone bundle that will run ad-hoc
on other Macs. That will come in a subsequent PR.
One thing to note about System Extensions is that they're slightly more
finicky when it comes to getting the signing and packaging right. Thus,
the README.md is updated to account for the gotchas involved in
developing System Extensions locally.
Related: #7071.
This patch adds a notification for updates for macos clients when they
are on an old version.
This is how it looks:
<img width="497" alt="image"
src="https://github.com/user-attachments/assets/829044fd-e8bc-4b47-b64d-67b8ef72adb0">
The orange dot is shown regardless of the notification being dismissed.
If the notification is dismissed by the "Dismiss this version" button,
until there's no new version there won't be notifications.
Updates are check at the start of firezone and every 6 hours after. This
is saved in `UserDefaults`.
Permissions for notifications needs to be allowed so that it's show,
this should be done by the `requestAuthorization`
Also, when an update is available a new `Update available...` option
appears in the menu
<img width="230" alt="image"
src="https://github.com/user-attachments/assets/16d7fea8-3cf5-4711-9d42-5c49faffe6c8">
This option, same as the notification takes you to the appstore.
---------
Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>
Fixes#5500
Unfortunately showing the menubar menu _only_ on re-launch is
non-trivial due to the way "re-launches" are processed in macOS.
We can handle them with the `applicationDidBecomeActive` override in
`AppDelegate`, but then this will be triggered whenever we sign in, open
Settings, or open About window because we activate the app then as well
in order to bring the Window to the foreground.
There's no good to way to determine who asked us to activate either.
Instead, we show the Welcome window (FirstTimeView on macOS), and in
there is a new button to show the app menu to use as a fallback for
users who need an alternative way to open the menu with a busy menubar.
<img width="1012" alt="Screenshot 2024-06-23 at 5 14 57 PM"
src="https://github.com/firezone/firezone/assets/167144/1a7dde08-1e83-4dc8-9516-e0390f29c941">
~~Had to do another big round of ball-of-yarn untangling in order to
create two auth flows for iOS / macOS.~~
~~This is required because of App Store guidelines.~~
Edit: Appeal filed. There seems to be no good way to have a nice
browser-based auth experience without using ASWebAuthenticationSession.
The last resort would be to open a normal browser tab and push the
client's token over an unauthenticated websocket channel from the portal
to the client.
This PR is now just to clean up the SwiftUI mess.
---------
Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Tried to organize this PR into commits so that it's a bit easier to
review.
1. Involves simplifying the logic in Adapter.swift so that us mortals
can maintain it confidently:
- The `.stoppingTunnel`, `.stoppedTunnelTemporarily`, and
`.stoppingTunnelTemporarily` states have been removed.
- I also removed the `self.` prefix from local vars when it's not
necessary to use it, to be more consistent.
- `onTunnelReady` and `getSystemDefaultResolvers` has been removed, and
`onUpdateRoutes` wired up, along with cleanup necessary to support that.
2. Involves adding the `reconnect` and `set_dns` stubs in the FFI and
fixing the log filter so that we can log them (see #4182 )
3. Involves getting the path update handler working well on macOS using
`SystemConfiguration` to read DNS servers.
4. Involves getting the path update handler working well on iOS by
employing careful trickery to prevent path update cycles by detecting if
`path.gateways` has changed, and avoid setting new DNS if it hasn't.
Refs #4028Fixes#4297Fixes#3565Fixes#3429Fixes#4175Fixes#4176Fixes#4309
---------
Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Attempt to fix#2881.
I can't reproduce the exact issue anymore, but I'm guessing activating
the app causes the web view window to lose selectedness. So we don't do
that in the PR.
Also, this PR fixes the scenario where the app is quit while the web
view is shown -- we now close the webview window in that case.
Expect this to fix#2850 and #2928.
When the app detects that there are no tunnels configured, it shows a UI
with a "Grant VPN Permission" button. On clicking that, the OS prompt
asking to allow VPN is shown.
* Removes remaining traces of account ID. We don't have any external
users using the app _yet_ so I wanted to remove the tech debt completely
before we get the app to testers
* Adds logo to welcome screen
* Removes "CONNECTION" section from MainView
