refactor(apple): Consolidate configuration to host app by jamilbk · Pull Request #9196 · firezone/firezone

jamilbk · 2025-05-21T19:13:27Z

On Apple platforms, UserDefaults provides a convenient way to store and fetch simple plist-compatible data for your app. Unbeknownst to the author at the time of original implementation was the fact this these keys are already designed for managed configurations to "mask" any user-configured equivalents.

This means we no longer need to juggle two dicts in UserDefaults, and we can instead check which keys are forced via a simple method call.

Additionally, the implementation was simplified in the following ways:

The host app is the "source of truth" for app configuration now. The tunnel service receives setConfiguration which applies the current configuration, and saves it in order to start up again without the GUI connected. The obvious caveat here is that if the GUI isn't running, configuration such as internetResourceEnabled applied by the administrator won't take effect. This is considered an edge case for the time being since no customers have asked for this. Additionally, admins can be advised to ensure the Firezone GUI is running on the system at all times to prevent this.
Settings and ConfigurationManager are now able to be removed - these became redundant after consolidating configuration to the containing app.

Related: #4505

vercel · 2025-05-21T19:13:31Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
firezone	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 22, 2025 4:06pm

jamilbk · 2025-05-21T19:14:26Z

@thomaseizinger I'm still testing this PR, but it's ready for review. Let me know if you want to pair on it.

jamilbk · 2025-05-22T06:24:04Z

@thomaseizinger This is ready. Tested for about 2-3 hours, setting various keys manually and via the command line. Seems pretty solid but I will do another round of QA once I get the managed profiles set up with Kandji.

Copilot

Pull Request Overview

This PR consolidates configuration handling into the host app by removing the dual-dictionary approach in UserDefaults and eliminating the old ConfigurationManager and Settings classes. The tunnel service now uses a simplified TunnelConfiguration type for IPC, and UI and networking components reference the centralized Configuration singleton.

Centralize UserDefaults-backed config in new Configuration class and TunnelConfiguration for IPC
Update PacketTunnelProvider and IPCClient to load/save via TunnelConfiguration
Remove legacy ConfigurationManager/Settings, update UI views and store to use Configuration signals

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
PacketTunnelProvider.swift	Load/save `TunnelConfiguration`, migrate firezone-ID, new error
ConfigurationManager.swift	Deleted redundant legacy manager
UpdateNotification.swift	Refactored to use `Configuration`, removed reactive start logic
SettingsView.swift	Removed `SettingsViewModel` dep on `Settings`, use `Configuration`
SettingsViewModel.swift	New view model bound to `Configuration`
SessionView.swift	Switch to `Configuration.shared` for resource title
ResourceView.swift	Introduce `ToggleInternetResourceButton` with `Configuration`
MenuBar.swift	Inject `Configuration`, react to forced settings signals
Store.swift	Replace optional config with `Configuration`
WebAuthSession.swift	Pass `Configuration` into auth flow
Configuration.swift	New `Configuration` observable wrapper and `TunnelConfiguration`
IPCClient.swift	Remove old getConfiguration, use `TunnelConfiguration`

Comments suppressed due to low confidence (1)

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/SessionView.swift:83

This helper still reads from store.configuration instead of the new Configuration.shared forced logic, so it won’t reflect managed overrides correctly. Update it to use configuration.isInternetResourceForced and published values.

let status = store.configuration?.internetResourceEnabled == true ? StatusSymbol.enabled : StatusSymbol.disabled

Copilot · 2025-05-22T06:28:05Z

+      guard let apiURL = legacyConfiguration?["apiURL"] ?? tunnelConfiguration?.apiURL,
+            let logFilter = legacyConfiguration?["logFilter"] ?? tunnelConfiguration?.logFilter,


We dropped the previous default fallbacks (Configuration.defaultApiURL and Configuration.defaultLogFilter). If the stored values are ever missing, this guard will now fail instead of using sensible defaults—consider preserving the original defaults as a fallback.

Suggested change

guard let apiURL = legacyConfiguration?["apiURL"] ?? tunnelConfiguration?.apiURL,

let logFilter = legacyConfiguration?["logFilter"] ?? tunnelConfiguration?.logFilter,

guard let apiURL = legacyConfiguration?["apiURL"] ?? tunnelConfiguration?.apiURL ?? "https://default.api.url",

let logFilter = legacyConfiguration?["logFilter"] ?? tunnelConfiguration?.logFilter ?? "default",

Copilot · 2025-05-22T06:28:06Z

+      guard let apiURL = legacyConfiguration?["apiURL"] ?? tunnelConfiguration?.apiURL,
+            let logFilter = legacyConfiguration?["logFilter"] ?? tunnelConfiguration?.logFilter,
+            let accountSlug = legacyConfiguration?["accountSlug"] ?? tunnelConfiguration?.accountSlug
      else {
-        throw PacketTunnelProviderError.accountSlugIsInvalid
+        throw PacketTunnelProviderError.tunnelConfigurationIsInvalid
      }


The single tunnelConfigurationIsInvalid error case hides which specific value was missing (apiURL, logFilter, or accountSlug). Splitting into more descriptive error cases or including context in the error would aid debugging.

Copilot · 2025-05-22T06:28:06Z

+    let filename = "firezone-id"
+    let key = "firezoneId"


[nitpick] Hardcoded string keys like "firezone-id" and later "configurationCache" are duplicated; extracting them into shared constants would reduce typo risk and improve maintainability.

Suggested change

let filename = "firezone-id"

let key = "firezoneId"

let filename = Constants.firezoneIdFilename

let key = Constants.firezoneIdKey

Copilot · 2025-05-22T06:28:06Z

    self.store = store
-    _viewModel = StateObject(wrappedValue: SettingsViewModel(store: store))
+    self.configuration = configuration ?? Configuration.shared
+    _viewModel = StateObject(wrappedValue: SettingsViewModel())


The view model is always initialized with Configuration.shared, even if a custom configuration was injected into SettingsView. Consider passing self.configuration to the SettingsViewModel initializer to keep them in sync.

Suggested change

_viewModel = StateObject(wrappedValue: SettingsViewModel())

_viewModel = StateObject(wrappedValue: SettingsViewModel(configuration: self.configuration))

jamilbk · 2025-05-22T06:31:07Z

+public class Configuration: ObservableObject {
+  static let shared = Configuration()
+
+  @Published private(set) var publishedInternetResourceEnabled = false


We need separate Published properties here because:

computed properties cannot be published

the observers need the actual changed value - observing configuration.objectWillChange doesn't provide that

jamilbk · 2025-05-22T06:34:37Z

One thing that might clean this up a little bit is to define a base ConfigurationItem struct that each item inherits from, storing them all in a collection here in the parent. That would help reduce some of the repetitiveness here I think.

On second thought, we do deviate from pure similarity in cases like calling UserDefaults methods and updating published properties, so maybe it's not worth the cognitive overhead.

jamilbk

@thomaseizinger Will probably need this in main to test further so that I can have a release installer package to provision with Kandji.

jamilbk · 2025-05-22T08:40:04Z

Woohoo! It works - the keys are set by Kandji and cannot be changed:

thomaseizinger

LGTM

Opening a new PR because #9196 is stacked. - Fixes the disabling of the Reset button to properly handle cases where only some fields are overridden - Binds configuration updates to the SettingsViewModel so that fields are properly disable after app launch if the admin sets a configuration

Adds managed configuration support for Android in line with other platforms. Related: firezone#4505 Related: firezone#9203 Related: firezone#9196

Copilot AI review requested due to automatic review settings May 21, 2025 19:13

jamilbk requested review from thomaseizinger and removed request for Copilot May 21, 2025 19:13

jamilbk marked this pull request as draft May 21, 2025 19:13

jamilbk force-pushed the fix/apple/managed-configuration branch from dd498d0 to a969bd9 Compare May 21, 2025 20:34

vercel Bot deployed to Preview May 21, 2025 20:35 View deployment

Use standard keys for Apple configuration

b62343d

jamilbk force-pushed the fix/apple/managed-configuration branch from a969bd9 to b62343d Compare May 21, 2025 23:29

vercel Bot deployed to Preview May 21, 2025 23:30 View deployment

vercel Bot deployed to Preview May 22, 2025 00:24 View deployment

Handle internet resource configuration updates

7cab3ba

jamilbk force-pushed the fix/apple/managed-configuration branch from 25f84a9 to 7cab3ba Compare May 22, 2025 00:32

vercel Bot deployed to Preview May 22, 2025 00:33 View deployment

jamilbk added 2 commits May 21, 2025 21:32

only warn users when saving settings while signed in

a88f529

Fix bug preventing accountSlug from being saved

ed04056

vercel Bot deployed to Preview May 22, 2025 04:33 View deployment

jamilbk mentioned this pull request May 22, 2025

feat(apple): Add start on login functionality #9168

Merged

jamilbk marked this pull request as ready for review May 22, 2025 06:23

Copilot AI review requested due to automatic review settings May 22, 2025 06:23

vercel Bot deployed to Preview May 22, 2025 06:23 View deployment

Copilot AI reviewed May 22, 2025

View reviewed changes

Handle other keys changing and debounce

163a7da

jamilbk force-pushed the fix/apple/managed-configuration branch from 09c748d to 163a7da Compare May 22, 2025 06:41

vercel Bot deployed to Preview May 22, 2025 06:42 View deployment

jamilbk commented May 22, 2025

View reviewed changes

jamilbk mentioned this pull request May 22, 2025

feat(gui-client): add MDM config for Windows #9203

Merged

Revert introduced bug in UpdateNotification

c3acae9

vercel Bot deployed to Preview May 22, 2025 08:30 View deployment

jamilbk commented May 22, 2025

View reviewed changes

Comment thread swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift

Comment thread swift/apple/FirezoneKit/Sources/FirezoneKit/Views/ResourceView.swift Outdated

jamilbk mentioned this pull request May 22, 2025

fix(apple): Fix state of Settings when managed config exists #9206

Merged

thomaseizinger approved these changes May 22, 2025

View reviewed changes

Apply viewModel suggestion

921a3ec

jamilbk enabled auto-merge May 22, 2025 16:06

vercel Bot deployed to Preview May 22, 2025 16:06 View deployment

jamilbk added this pull request to the merge queue May 22, 2025

Merged via the queue into main with commit 6fd3493 May 22, 2025
111 checks passed

jamilbk deleted the fix/apple/managed-configuration branch May 22, 2025 16:30

jamilbk mentioned this pull request May 25, 2025

feat(android): Managed configuration #9227

Merged

		guard let apiURL = legacyConfiguration?["apiURL"] ?? tunnelConfiguration?.apiURL,
		let logFilter = legacyConfiguration?["logFilter"] ?? tunnelConfiguration?.logFilter,

	_viewModel = StateObject(wrappedValue: SettingsViewModel())
	_viewModel = StateObject(wrappedValue: SettingsViewModel(configuration: self.configuration))

Conversation

jamilbk commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel Bot commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jamilbk commented May 21, 2025

Uh oh!

jamilbk commented May 22, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jamilbk May 22, 2025

Choose a reason for hiding this comment

Uh oh!

jamilbk May 22, 2025

Choose a reason for hiding this comment

Uh oh!

jamilbk May 22, 2025

Choose a reason for hiding this comment

Uh oh!

jamilbk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jamilbk commented May 22, 2025

Uh oh!

thomaseizinger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jamilbk commented May 21, 2025 •

edited

Loading

vercel Bot commented May 21, 2025 •

edited

Loading