feature,ipn/ipnlocal: add profileManager.StateChangeHook

We update profileManager to allow registering a single state (profile+prefs) change hook.
This is to invert the dependency between the profileManager and the LocalBackend, so that
instead of LocalBackend asking profileManager for the state, we can have profileManager
call LocalBackend when the state changes.

We also update feature.Hook with a new (*feature.Hook).GetOk method to avoid calling both
IsSet and Get.

Updates tailscale/corp#28014
Updates #12614

Signed-off-by: Nick Khyl <nickk@tailscale.com>

ipn/ipnlocal/profiles_test.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"os/user"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -609,3 +610,535 @@ func TestDefaultPrefs(t *testing.T) {
 		t.Errorf("defaultPrefs is %s, want %s; defaultPrefs should only modify WantRunning and LoggedOut, all other defaults should be in ipn.NewPrefs.", p2.Pretty(), p1.Pretty())
 	}
 }
+
+// mutPrefsFn is a function that mutates the prefs.
+// Deserialization pre‑populates prefs with default (non‑zero) values.
+// After saving prefs and reading them back, we may not get exactly what we set.
+// For this reason, tests apply changes through a helper that mutates
+// [ipn.NewPrefs] instead of hard‑coding expected values in each case.
+type mutPrefsFn func(*ipn.Prefs)
+
+type profileState struct {
+	*ipn.LoginProfile
+	mutPrefs mutPrefsFn
+}
+
+func (s *profileState) prefs() ipn.PrefsView {
+	prefs := ipn.NewPrefs() // apply changes to the default prefs
+	s.mutPrefs(prefs)
+	return prefs.View()
+}
+
+type profileStateChange struct {
+	*ipn.LoginProfile
+	mutPrefs mutPrefsFn
+	sameNode bool
+}
+
+func wantProfileChange(state profileState) profileStateChange {
+	return profileStateChange{
+		LoginProfile: state.LoginProfile,
+		mutPrefs:     state.mutPrefs,
+		sameNode:     false,
+	}
+}
+
+func wantPrefsChange(state profileState) profileStateChange {
+	return profileStateChange{
+		LoginProfile: state.LoginProfile,
+		mutPrefs:     state.mutPrefs,
+		sameNode:     true,
+	}
+}
+
+func makeDefaultPrefs(p *ipn.Prefs) { *p = *defaultPrefs.AsStruct() }
+
+func makeKnownProfileState(id int, nameSuffix string, uid ipn.WindowsUserID, mutPrefs mutPrefsFn) profileState {
+	lowerNameSuffix := strings.ToLower(nameSuffix)
+	nid := "node-" + tailcfg.StableNodeID(lowerNameSuffix)
+	up := tailcfg.UserProfile{
+		ID:          tailcfg.UserID(id),
+		LoginName:   fmt.Sprintf("user-%s@example.com", lowerNameSuffix),
+		DisplayName: "User " + nameSuffix,
+	}
+	return profileState{
+		LoginProfile: &ipn.LoginProfile{
+			LocalUserID: uid,
+			Name:        up.LoginName,
+			ID:          ipn.ProfileID(fmt.Sprintf("%04X", id)),
+			Key:         "profile-" + ipn.StateKey(nameSuffix),
+			NodeID:      nid,
+			UserProfile: up,
+		},
+		mutPrefs: func(p *ipn.Prefs) {
+			p.Hostname = "Hostname-" + nameSuffix
+			if mutPrefs != nil {
+				mutPrefs(p) // apply any additional changes
+			}
+			p.Persist = &persist.Persist{NodeID: nid, UserProfile: up}
+		},
+	}
+}
+
+func TestProfileStateChangeCallback(t *testing.T) {
+	t.Parallel()
+
+	// A few well-known profiles to use in tests.
+	emptyProfile := profileState{
+		LoginProfile: &ipn.LoginProfile{},
+		mutPrefs:     makeDefaultPrefs,
+	}
+	profile0000 := profileState{
+		LoginProfile: &ipn.LoginProfile{ID: "0000", Key: "profile-0000"},
+		mutPrefs:     makeDefaultPrefs,
+	}
+	profileA := makeKnownProfileState(0xA, "A", "", nil)
+	profileB := makeKnownProfileState(0xB, "B", "", nil)
+	profileC := makeKnownProfileState(0xC, "C", "", nil)
+
+	aliceUserID := ipn.WindowsUserID("S-1-5-21-1-2-3-4")
+	aliceEmptyProfile := profileState{
+		LoginProfile: &ipn.LoginProfile{LocalUserID: aliceUserID},
+		mutPrefs:     makeDefaultPrefs,
+	}
+	bobUserID := ipn.WindowsUserID("S-1-5-21-3-4-5-6")
+	bobEmptyProfile := profileState{
+		LoginProfile: &ipn.LoginProfile{LocalUserID: bobUserID},
+		mutPrefs:     makeDefaultPrefs,
+	}
+	bobKnownProfile := makeKnownProfileState(0xB0B, "Bob", bobUserID, nil)
+
+	tests := []struct {
+		name          string
+		initial       *profileState         // if non-nil, this is the initial profile and prefs to start wit
+		knownProfiles []profileState        // known profiles we can switch to
+		action        func(*profileManager) // action to take on the profile manager
+		wantChanges   []profileStateChange  // expected state changes
+	}{
+		{
+			name: "no-changes",
+			action: func(*profileManager) {
+				// do nothing
+			},
+			wantChanges: nil,
+		},
+		{
+			name: "no-initial/new-profile",
+			action: func(pm *profileManager) {
+				// The profile manager is new and started with a new empty profile.
+				// This should not trigger a state change callback.
+				pm.SwitchToNewProfile()
+			},
+			wantChanges: nil,
+		},
+		{
+			name: "no-initial/new-profile-for-user",
+			action: func(pm *profileManager) {
+				// But switching to a new profile for a specific user should trigger
+				// a state change callback.
+				pm.SwitchToNewProfileForUser(aliceUserID)
+			},
+			wantChanges: []profileStateChange{
+				// We want a new empty profile (owned by the specified user)
+				// and the default prefs.
+				wantProfileChange(aliceEmptyProfile),
+			},
+		},
+		{
+			name:    "with-initial/new-profile",
+			initial: &profile0000,
+			action: func(pm *profileManager) {
+				// And so does switching to a new profile when the initial profile
+				// is non-empty.
+				pm.SwitchToNewProfile()
+			},
+			wantChanges: []profileStateChange{
+				// We want a new empty profile and the default prefs.
+				wantProfileChange(emptyProfile),
+			},
+		},
+		{
+			name:    "with-initial/new-profile/twice",
+			initial: &profile0000,
+			action: func(pm *profileManager) {
+				// If we switch to a new profile twice, we should only get one state change.
+				pm.SwitchToNewProfile()
+				pm.SwitchToNewProfile()
+			},
+			wantChanges: []profileStateChange{
+				// We want a new empty profile and the default prefs.
+				wantProfileChange(emptyProfile),
+			},
+		},
+		{
+			name:    "with-initial/new-profile-for-user/twice",
+			initial: &profile0000,
+			action: func(pm *profileManager) {
+				// Unless we switch to a new profile for a specific user,
+				// in which case we should get a state change twice.
+				pm.SwitchToNewProfileForUser(aliceUserID)
+				pm.SwitchToNewProfileForUser(aliceUserID) // no change here
+				pm.SwitchToNewProfileForUser(bobUserID)
+			},
+			wantChanges: []profileStateChange{
+				// Both profiles are empty, but they are owned by different users.
+				wantProfileChange(aliceEmptyProfile),
+				wantProfileChange(bobEmptyProfile),
+			},
+		},
+		{
+			name:    "with-initial/new-profile/twice/with-prefs-change",
+			initial: &profile0000,
+			action: func(pm *profileManager) {
+				// Or unless we switch to a new profile, change the prefs,
+				// then switch to a new profile again. Since the current
+				// profile is not empty after the prefs change, we should
+				// get state changes for all three actions.
+				pm.SwitchToNewProfile()
+				p := pm.CurrentPrefs().AsStruct()
+				p.WantRunning = true
+				pm.SetPrefs(p.View(), ipn.NetworkProfile{})
+				pm.SwitchToNewProfile()
+			},
+			wantChanges: []profileStateChange{
+				wantProfileChange(emptyProfile), // new empty profile
+				wantPrefsChange(profileState{ // prefs change, same profile
+					LoginProfile: &ipn.LoginProfile{},
+					mutPrefs: func(p *ipn.Prefs) {
+						*p = *defaultPrefs.AsStruct()
+						p.WantRunning = true
+					},
+				}),
+				wantProfileChange(emptyProfile), // new empty profile again
+			},
+		},
+		{
+			name:          "switch-to-profile/by-id",
+			knownProfiles: []profileState{profileA, profileB, profileC},
+			action: func(pm *profileManager) {
+				// Switching to a known profile by ID should trigger a state change callback.
+				pm.SwitchToProfileByID(profileB.ID)
+			},
+			wantChanges: []profileStateChange{
+				wantProfileChange(profileB),
+			},
+		},
+		{
+			name:          "switch-to-profile/by-id/non-existent",
+			knownProfiles: []profileState{profileA, profileC}, // no profileB
+			action: func(pm *profileManager) {
+				// Switching to a non-existent profile should fail and not trigger a state change callback.
+				pm.SwitchToProfileByID(profileB.ID)
+			},
+			wantChanges: []profileStateChange{},
+		},
+		{
+			name:          "switch-to-profile/by-id/twice-same",
+			knownProfiles: []profileState{profileA, profileB, profileC},
+			action: func(pm *profileManager) {
+				// But only for the first switch.
+				// The second switch to the same profile should not trigger a state change callback.
+				pm.SwitchToProfileByID(profileB.ID)
+				pm.SwitchToProfileByID(profileB.ID)
+			},
+			wantChanges: []profileStateChange{
+				wantProfileChange(profileB),
+			},
+		},
+		{
+			name:          "switch-to-profile/by-id/many",
+			knownProfiles: []profileState{profileA, profileB, profileC},
+			action: func(pm *profileManager) {
+				// Same idea, but with multiple switches.
+				pm.SwitchToProfileByID(profileB.ID) // switch to Profile-B
+				pm.SwitchToProfileByID(profileB.ID) // then to Profile-B again (no change)
+				pm.SwitchToProfileByID(profileC.ID) // then to Profile-C (change)
+				pm.SwitchToProfileByID(profileA.ID) // then to Profile-A (change)
+				pm.SwitchToProfileByID(profileB.ID) // then to Profile-B (change)
+			},
+			wantChanges: []profileStateChange{
+				wantProfileChange(profileB),
+				wantProfileChange(profileC),
+				wantProfileChange(profileA),
+				wantProfileChange(profileB),
+			},
+		},
+		{
+			name:          "switch-to-profile/by-view",
+			knownProfiles: []profileState{profileA, profileB, profileC},
+			action: func(pm *profileManager) {
+				// Switching to a known profile by an [ipn.LoginProfileView]
+				// should also trigger a state change callback.
+				pm.SwitchToProfile(profileB.View())
+			},
+			wantChanges: []profileStateChange{
+				wantProfileChange(profileB),
+			},
+		},
+		{
+			name:    "switch-to-profile/by-view/empty",
+			initial: &profile0000,
+			action: func(pm *profileManager) {
+				// SwitchToProfile supports switching to an empty profile.
+				emptyProfile := &ipn.LoginProfile{}
+				pm.SwitchToProfile(emptyProfile.View())
+			},
+			wantChanges: []profileStateChange{
+				wantProfileChange(emptyProfile),
+			},
+		},
+		{
+			name:          "switch-to-profile/by-view/non-existent",
+			knownProfiles: []profileState{profileA, profileC},
+			action: func(pm *profileManager) {
+				// Switching to a an unknown profile by an [ipn.LoginProfileView]
+				// should fail and not trigger a state change callback.
+				pm.SwitchToProfile(profileB.View())
+			},
+			wantChanges: []profileStateChange{},
+		},
+		{
+			name:    "switch-to-profile/by-view/empty-for-user",
+			initial: &profile0000,
+			action: func(pm *profileManager) {
+				// And switching to an empty profile for a specific user also works.
+				pm.SwitchToProfile(bobEmptyProfile.View())
+			},
+			wantChanges: []profileStateChange{
+				wantProfileChange(bobEmptyProfile),
+			},
+		},
+		{
+			name:    "switch-to-profile/by-view/invalid",
+			initial: &profile0000,
+			action: func(pm *profileManager) {
+				// Switching to an invalid profile should create and switch
+				// to a new empty profile.
+				pm.SwitchToProfile(ipn.LoginProfileView{})
+			},
+			wantChanges: []profileStateChange{
+				wantProfileChange(emptyProfile),
+			},
+		},
+		{
+			name:          "delete-profile/current",
+			initial:       &profileA, // profileA is the current profile
+			knownProfiles: []profileState{profileA, profileB, profileC},
+			action: func(pm *profileManager) {
+				// Deleting the current profile should switch to a new empty profile.
+				pm.DeleteProfile(profileA.ID)
+			},
+			wantChanges: []profileStateChange{
+				wantProfileChange(emptyProfile),
+			},
+		},
+		{
+			name:          "delete-profile/current-with-user",
+			initial:       &bobKnownProfile,
+			knownProfiles: []profileState{profileA, profileB, profileC, bobKnownProfile},
+			action: func(pm *profileManager) {
+				// Similarly, deleting the current profile for a specific user should switch
+				// to a new empty profile for that user (at least while the "current user"
+				// is still a thing on Windows).
+				pm.DeleteProfile(bobKnownProfile.ID)
+			},
+			wantChanges: []profileStateChange{
+				wantProfileChange(bobEmptyProfile),
+			},
+		},
+		{
+			name:          "delete-profile/non-current",
+			initial:       &profileA, // profileA is the current profile
+			knownProfiles: []profileState{profileA, profileB, profileC},
+			action: func(pm *profileManager) {
+				// But deleting a non-current profile should not trigger a state change callback.
+				pm.DeleteProfile(profileB.ID)
+			},
+			wantChanges: []profileStateChange{},
+		},
+		{
+			name:    "set-prefs/new-profile",
+			initial: &emptyProfile, // the current profile is empty
+			action: func(pm *profileManager) {
+				// The current profile is new and empty, but we can still set p.
+				// This should trigger a state change callback.
+				p := pm.CurrentPrefs().AsStruct()
+				p.WantRunning = true
+				p.Hostname = "New-Hostname"
+				pm.SetPrefs(p.View(), ipn.NetworkProfile{})
+			},
+			wantChanges: []profileStateChange{
+				// Still an empty profile, but with new prefs.
+				wantPrefsChange(profileState{
+					LoginProfile: emptyProfile.LoginProfile,
+					mutPrefs: func(p *ipn.Prefs) {
+						*p = *emptyProfile.prefs().AsStruct()
+						p.WantRunning = true
+						p.Hostname = "New-Hostname"
+					},
+				}),
+			},
+		},
+		{
+			name:          "set-prefs/current-profile",
+			initial:       &profileA, // profileA is the current profile
+			knownProfiles: []profileState{profileA, profileB, profileC},
+			action: func(pm *profileManager) {
+				p := pm.CurrentPrefs().AsStruct()
+				p.WantRunning = true
+				p.Hostname = "New-Hostname"
+				pm.SetPrefs(p.View(), ipn.NetworkProfile{})
+			},
+			wantChanges: []profileStateChange{
+				wantPrefsChange(profileState{
+					LoginProfile: profileA.LoginProfile, // same profile
+					mutPrefs: func(p *ipn.Prefs) { // but with new prefs
+						*p = *profileA.prefs().AsStruct()
+						p.WantRunning = true
+						p.Hostname = "New-Hostname"
+					},
+				}),
+			},
+		},
+		{
+			name:          "set-prefs/current-profile/profile-name",
+			initial:       &profileA, // profileA is the current profile
+			knownProfiles: []profileState{profileA, profileB, profileC},
+			action: func(pm *profileManager) {
+				p := pm.CurrentPrefs().AsStruct()
+				p.ProfileName = "This is User A"
+				pm.SetPrefs(p.View(), ipn.NetworkProfile{})
+			},
+			wantChanges: []profileStateChange{
+				// Still the same profile, but with a new profile name
+				// populated from the prefs. The prefs are also updated.
+				wantPrefsChange(profileState{
+					LoginProfile: func() *ipn.LoginProfile {
+						p := profileA.Clone()
+						p.Name = "This is User A"
+						return p
+					}(),
+					mutPrefs: func(p *ipn.Prefs) {
+						*p = *profileA.prefs().AsStruct()
+						p.ProfileName = "This is User A"
+					},
+				}),
+			},
+		},
+		{
+			name:          "set-prefs/implicit-switch/from-new",
+			initial:       &emptyProfile, // a new, empty profile
+			knownProfiles: []profileState{profileA, profileB, profileC},
+			action: func(pm *profileManager) {
+				// The user attempted to add a new profile but actually logged in as the same
+				// node/user as profileB. When [LocalBackend.SetControlClientStatus] calls
+				// [profileManager.SetPrefs] with the [persist.Persist] for profileB, we
+				// implicitly switch to that profile instead of creating a duplicate for the
+				// same node/user.
+				//
+				// TODO(nickkhyl): currently, [LocalBackend.SetControlClientStatus] uses the p
+				// of the current profile, not those of the profile we switch to. This is all wrong
+				// and should be fixed. But for now, we just test that the state change callback
+				// is called with the new profile and p.
+				p := pm.CurrentPrefs().AsStruct()
+				p.Persist = profileB.prefs().Persist().AsStruct()
+				p.WantRunning = true
+				p.LoggedOut = false
+				pm.SetPrefs(p.View(), ipn.NetworkProfile{})
+			},
+			wantChanges: []profileStateChange{
+				// Calling [profileManager.SetPrefs] like this is effectively a profile switch
+				// rather than a prefs change.
+				wantProfileChange(profileState{
+					LoginProfile: profileB.LoginProfile,
+					mutPrefs: func(p *ipn.Prefs) {
+						*p = *emptyProfile.prefs().AsStruct()
+						p.Persist = profileB.prefs().Persist().AsStruct()
+						p.WantRunning = true
+						p.LoggedOut = false
+					},
+				}),
+			},
+		},
+		{
+			name:          "set-prefs/implicit-switch/from-other",
+			initial:       &profileA, // profileA is the current profile
+			knownProfiles: []profileState{profileA, profileB, profileC},
+			action: func(pm *profileManager) {
+				// Same idea, but the current profile is profileA rather than a new empty profile.
+				// Note: this is all wrong. See the comment above and [profileManager.SetPrefs].
+				p := pm.CurrentPrefs().AsStruct()
+				p.Persist = profileB.prefs().Persist().AsStruct()
+				p.WantRunning = true
+				p.LoggedOut = false
+				pm.SetPrefs(p.View(), ipn.NetworkProfile{})
+			},
+			wantChanges: []profileStateChange{
+				wantProfileChange(profileState{
+					LoginProfile: profileB.LoginProfile,
+					mutPrefs: func(p *ipn.Prefs) {
+						*p = *profileA.prefs().AsStruct()
+						p.Persist = profileB.prefs().Persist().AsStruct()
+						p.WantRunning = true
+						p.LoggedOut = false
+					},
+				}),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			store := new(mem.Store)
+			pm, err := newProfileManagerWithGOOS(store, logger.Discard, new(health.Tracker), "linux")
+			if err != nil {
+				t.Fatalf("newProfileManagerWithGOOS: %v", err)
+			}
+			for _, p := range tt.knownProfiles {
+				pm.writePrefsToStore(p.Key, p.prefs())
+				pm.knownProfiles[p.ID] = p.View()
+			}
+			if err := pm.writeKnownProfiles(); err != nil {
+				t.Fatalf("writeKnownProfiles: %v", err)
+			}
+
+			if tt.initial != nil {
+				pm.currentUserID = tt.initial.LocalUserID
+				pm.currentProfile = tt.initial.View()
+				pm.prefs = tt.initial.prefs()
+			}
+
+			type stateChange struct {
+				Profile  *ipn.LoginProfile
+				Prefs    *ipn.Prefs
+				SameNode bool
+			}
+			wantChanges := make([]stateChange, 0, len(tt.wantChanges))
+			for _, w := range tt.wantChanges {
+				wantPrefs := ipn.NewPrefs()
+				w.mutPrefs(wantPrefs) // apply changes to the default prefs
+				wantChanges = append(wantChanges, stateChange{
+					Profile:  w.LoginProfile,
+					Prefs:    wantPrefs,
+					SameNode: w.sameNode,
+				})
+			}
+
+			gotChanges := make([]stateChange, 0, len(tt.wantChanges))
+			pm.StateChangeHook = func(profile ipn.LoginProfileView, prefs ipn.PrefsView, sameNode bool) {
+				gotChanges = append(gotChanges, stateChange{
+					Profile:  profile.AsStruct(),
+					Prefs:    prefs.AsStruct(),
+					SameNode: sameNode,
+				})
+			}
+
+			tt.action(pm)
+
+			if diff := cmp.Diff(wantChanges, gotChanges, defaultCmpOpts...); diff != "" {
+				t.Errorf("StateChange callbacks: (-want +got): %v", diff)
+			}
+		})
+	}
+}