Bug 2034845 - Fix broken Access Connector on policy modification by jporter-dev · Pull Request #809 · mozilla/enterprise-firefox

jporter-dev · 2026-04-28T19:15:35Z

Description

This PR fixes an issue that occurs when the AccessConnector policy is modified in-place (not added/removed). This results in a broken access connector due to the #inclusionList not being updated when MatchPatterns changes, and the connection not being re-initialized with the new server when the serverlist changes (host/port in policy)

Bugzilla: Bug-2034845

IPProtectionServerlist.sys.mjs: fix PrefServerList pref observer registration (was referencing instance instead of class), skip in maybeFetchList() when pref value is unchanged
IPPChannelFilter.sys.mjs: register/unregister pref observer to refresh #inclusionSet when changed
IPPProxyManager.sys.mjs: add reconnect method for reconnecting to the recommended server from the updated list
IPPAutoStart.sys.mjs: add logic to #handleListChanged to reconnect on live changes

Dependencies / Related Issues

Related: Bug 2034591 - Enterprise: make sure to properly react to policy changes for Access Connectors #788

Testing

Manual testing performed

Steps to verify changes:

Enable AccessConnector policy
Launch browser and navigate to a matching URL
Observe active AccessConnector state
Remove or change matching URL from MatchPatterns
- This updates the icon live without requiring new navigation or refresh
Observe inactive AccessConnector state
Repeat above for host or port
- Serverlist changes require new navigation to update
- Previously new navigation used the old server
- Changing to an invalid port will result in broken proxy, changing back should fix it

Expected result:

AccessConnector shows active/inactive properly when modifying policy

lissyx · 2026-05-05T12:21:08Z

@jporter-dev Now that we have #825 there is no reason not to rebase and add proper testing to ensure this does not regress

lissyx · 2026-05-05T12:32:54Z

+      if (this.autoStart) {
+        const state = lazy.IPPProxyManager.state;
+        if (state === lazy.IPPProxyStates.ACTIVE) {
+          lazy.IPPProxyManager.switch();
+          return;
+        }
+        if (state === lazy.IPPProxyStates.ERROR) {
+          await lazy.IPPProxyManager.reset();
+          lazy.IPPProxyManager.start(
+            false,
+            PrivateBrowsingUtils.permanentPrivateBrowsing
+          );
+          return;
+        }
+        if (state === lazy.IPPProxyStates.READY) {
+          lazy.IPPProxyManager.start(
+            false,
+            PrivateBrowsingUtils.permanentPrivateBrowsing
+          );
+          return;
+        }
+      }


Is this really the right place? Can we get feedback from people working on IPProtection for this?

I've pinged @strseb for some feedback

The autostart component's job is to "connect once when ready, asap".

So the way i'd read that code is, if autostart is enabled and something changes force-start no matter the current state.
If that always-on is the desired behavior, i'd probably move this into it's own little helper component. And swap out this helper component with the ipp-always-on in IPProtectionActivator.

Otherwise, given the bug-problem text, it sounds like just applying this while active should be sufficient. In that case ProxyManager should observe the serverlist and call switch() while in active.

@strseb I've extracted the logic into a enterprise/IPPAlwaysOn.sys.mjs component that gets loaded in alongside the other helpers in a similar fashion to your IPPEnterpriseAuthProvider PR.

Let me know what you think! If it's the right approach then I can prepare this for upstream

lissyx

LGTM; please verify with a peer of IPProtection for the #handleListChanged implementation if that is the right place.

strseb

Overall looks great! Just a few comments :)

strseb · 2026-05-19T11:17:18Z

  IPPAutoRestoreHelper,
  ...IPPAutoStartHelpers,
+  ...(AppConstants.MOZ_ENTERPRISE ? lazy.IPPAlwaysOnHelpers : []),


Wondering whether you actually need the AutoRestore (on session restore starts vpn when it was on) or the IPPAutostartHelpers (for the start on boot pref) on enterprise. Otherwise less objects that can start/stop are preferable :)

... AppConstants.MOZ_ENTERPRISE ? lazy.IPPAlwaysOnHelpers : [IPPAutoRestoreHelper,...IPPAutoStartHelpers ]

I may need to augment IPPAlwaysOn a bit more if removing those to handle the scenarios those handle, but it makes sense to rework it this way to avoid start/stop conflicts. I'll take a look at removing those and making sure IPPAlwaysOn has parity with them

strseb · 2026-05-19T11:21:22Z

+# ignores the pref-based server list set up in add_setup, so hasList stays false
+# and proxy activation tests time out.
+["test_IPPAlwaysOn.js"]
+run-if = ["enterprise"]


Thats a good point. Maybe we should have a head_enterpise.js for enterprise tests, so add_setup correctly creates the enviroment.

Good call, I'll fix this up properly in the Phab PR.

strseb · 2026-05-19T11:27:15Z

+/**
+ * Registers the channel filter at startup to prevent data leaks before the
+ * proxy connection is fully established in always-on mode. Mirrors
+ * IPPEarlyStartupFilter from IPPAutoStart but uses the AlwaysOn policy check.
+ */
+class IPPAlwaysOnEarlyStartupFilter {
+  #alwaysOnAtStartup = false;
+
+  constructor() {
+    this.handleEvent = this.#handleEvent.bind(this);
+  }


Wondering, this is pretty much the same as IPPEarlyStartupFilter - only with a diffrent condition.

WDYT if we do

class IPPEarlyStartupFilter{ constructor( shouldActivate:Function<bool>){...} init() { if(!this.shouldActivate()) return; ... } } export IPPAutostartFilter = new IPPEarlyStartupFilter(()=>IPPAutoStart.autoStart) export IPPAlwaysOnEarlyStartupFilter = new IPPEarlyStartupFilter(()=>IPPAlwaysOn.alwaysOnEnabled))

…ive inclusion.match_patterns pref changes

…s for host and port updates

…ver list changes

…rt become invalid and terminate VPN connection

…o constructor

…licy removal

…lter

jporter-dev self-assigned this Apr 28, 2026

jporter-dev added bug Something isn't working branch:main PR that should be merged on enterprise-main branch labels Apr 28, 2026

jporter-dev force-pushed the enterprise-bug2034845-access-connector-live-policy-modification branch 4 times, most recently from 94a8a20 to 47d71ac Compare April 29, 2026 22:15

jporter-dev marked this pull request as ready for review April 30, 2026 12:51

jporter-dev force-pushed the enterprise-bug2034845-access-connector-live-policy-modification branch from 47d71ac to 8436997 Compare April 30, 2026 13:05

lissyx self-requested a review May 4, 2026 13:41