From 8b5060faf5a3f8e36aaa39324c86ac49a5343b03 Mon Sep 17 00:00:00 2001 From: Will Hannah Date: Tue, 30 Jun 2026 09:01:32 -0400 Subject: [PATCH] ipn/ipnlocal: sort profiles by date created when possible (#20223) This adds a Created field to LoginProfile to normalize the sort order of login profiles presented in the various client GUIs. The default sort order for existing profiles remains unchanged and continues to be based on Name. Newly added profiles will be stamped at creation time and returned at the top of the list of unstamped profiles, sorted by creation date in descending order. The rationale is to ensure that all clients present the user's profile list in the same order, regardless of newly added accounts, name changes, or nickname overrides. The Mac client was recently updated to remove various custom profile sorting behaviors (https://github.com/tailscale/corp/pull/43847). iOS, Android, and Windows do not currently perform GUI-level sorting, so this change should propagate to them seamlessly. updates tailscale/corp#43843 Signed-off-by: Will Hannah --- ipn/ipn_clone.go | 2 + ipn/ipn_view.go | 8 +++ ipn/ipnlocal/profiles.go | 15 ++++- ipn/ipnlocal/profiles_test.go | 106 ++++++++++++++++++++++++++++++++++ ipn/prefs.go | 7 +++ 5 files changed, 137 insertions(+), 1 deletion(-) diff --git a/ipn/ipn_clone.go b/ipn/ipn_clone.go index 773167a0d..e7c262c1b 100644 --- a/ipn/ipn_clone.go +++ b/ipn/ipn_clone.go @@ -8,6 +8,7 @@ package ipn import ( "maps" "net/netip" + "time" "tailscale.com/drive" "tailscale.com/tailcfg" @@ -38,6 +39,7 @@ var _LoginProfileCloneNeedsRegeneration = LoginProfile(struct { NodeID tailcfg.StableNodeID LocalUserID WindowsUserID ControlURL string + Created time.Time }{}) // Clone makes a deep copy of Prefs. diff --git a/ipn/ipn_view.go b/ipn/ipn_view.go index 31c3dff2f..9fb5fbb8a 100644 --- a/ipn/ipn_view.go +++ b/ipn/ipn_view.go @@ -9,6 +9,7 @@ import ( jsonv1 "encoding/json" "errors" "net/netip" + "time" jsonv2 "github.com/go-json-experiment/json" "github.com/go-json-experiment/json/jsontext" @@ -131,6 +132,12 @@ func (v LoginProfileView) LocalUserID() WindowsUserID { return v.ж.LocalUserID // into. func (v LoginProfileView) ControlURL() string { return v.ж.ControlURL } +// Created is when this profile was first added to this client. It is +// stamped once at profile creation and never changes. It is used to sort +// the profile list with newest first; profiles created before this field +// existed have a zero value and sort after all stamped profiles. +func (v LoginProfileView) Created() time.Time { return v.ж.Created } + // A compilation failure here means this code must be regenerated, with the command at the top of this file. var _LoginProfileViewNeedsRegeneration = LoginProfile(struct { ID ProfileID @@ -141,6 +148,7 @@ var _LoginProfileViewNeedsRegeneration = LoginProfile(struct { NodeID tailcfg.StableNodeID LocalUserID WindowsUserID ControlURL string + Created time.Time }{}) // View returns a read-only view of Prefs. diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index 4e073e5c9..5619f0639 100644 --- a/ipn/ipnlocal/profiles.go +++ b/ipn/ipnlocal/profiles.go @@ -21,6 +21,7 @@ import ( "tailscale.com/ipn" "tailscale.com/ipn/ipnext" "tailscale.com/tailcfg" + "tailscale.com/tstime" "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/types/persist" @@ -64,6 +65,10 @@ type profileManager struct { // Override for key.NewEmptyHardwareAttestationKey used for testing. newEmptyHardwareAttestationKey func() (key.HardwareAttestationKey, error) + + // clock supplies the current time when stamping LoginProfile.Created. + // Tests substitute a fake clock to make creation timestamps deterministic. + clock tstime.DefaultClock } // SetExtensionHost sets the [ExtensionHost] for the [profileManager]. @@ -250,7 +255,14 @@ func (pm *profileManager) allProfilesFor(uid ipn.WindowsUserID) []ipn.LoginProfi } } slices.SortFunc(out, func(a, b ipn.LoginProfileView) int { - return cmp.Compare(a.Name(), b.Name()) + // Legacy (zero Created) first, then stamped oldest-first. + if c := a.Created().Compare(b.Created()); c != 0 { + return c + } + if c := cmp.Compare(a.Name(), b.Name()); c != 0 { + return c + } + return cmp.Compare(a.NetworkProfile().DomainName, b.NetworkProfile().DomainName) }) return out } @@ -413,6 +425,7 @@ func (pm *profileManager) setProfilePrefs(lp *ipn.LoginProfile, prefsIn ipn.Pref if persist := prefsIn.Persist(); persist.Valid() && persist.NodeID() != "" && persist.UserProfile().LoginName() != "" { // Generate an ID and [ipn.StateKey] now that we have the node info. lp.ID, lp.Key = newUnusedID(pm.knownProfiles) + lp.Created = pm.clock.Now() } // Set the current user as the profile owner, unless the current user ID does diff --git a/ipn/ipnlocal/profiles_test.go b/ipn/ipnlocal/profiles_test.go index 7b6ce1cf5..8ac22a656 100644 --- a/ipn/ipnlocal/profiles_test.go +++ b/ipn/ipnlocal/profiles_test.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -19,6 +20,8 @@ import ( "tailscale.com/ipn" "tailscale.com/ipn/store/mem" "tailscale.com/tailcfg" + "tailscale.com/tstest" + "tailscale.com/tstime" "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/types/persist" @@ -1229,3 +1232,106 @@ func TestDeleteProfileClearsState(t *testing.T) { t.Fatalf("ReadState after delete: got err %v, want ErrStateNotExist", err) } } + +// TestProfileSortOrder verifies that allProfilesFor sorts zero-Created +// (legacy) profiles before stamped profiles, with stamped profiles +// ordered oldest-first so the most recently added one lands at the +// bottom. Name then DomainName break ties within either group. +func TestProfileSortOrder(t *testing.T) { + pm, err := newProfileManagerWithGOOS(new(mem.Store), logger.Discard, health.NewTracker(eventbustest.NewBus(t)), "linux") + if err != nil { + t.Fatal(err) + } + + base := time.Date(2026, 6, 23, 12, 0, 0, 0, time.UTC) + add := func(id ipn.ProfileID, name, domain string, created time.Time) { + pm.knownProfiles[id] = (&ipn.LoginProfile{ + ID: id, + Name: name, + NetworkProfile: ipn.NetworkProfile{DomainName: domain}, + Created: created, + }).View() + } + + add("a", "alice", "example.com", time.Time{}) + add("a2", "alice", "acme.com", time.Time{}) // legacy ties alice on Name; DomainName breaks it + add("b", "bob", "example.com", time.Time{}) + add("c", "carol", "example.com", base) + add("d", "dave", "example.com", base.Add(time.Minute)) + add("e", "eve", "example.com", base.Add(time.Minute)) // ties dave on Created; Name breaks it + add("e2", "eve", "acme.com", base.Add(time.Minute)) // ties eve on Created+Name; DomainName breaks it + + got := pm.allProfilesFor("") + type want struct{ name, domain string } + wantOrder := []want{ + // Legacy first, Name asc then DomainName asc. + {"alice", "acme.com"}, + {"alice", "example.com"}, + {"bob", "example.com"}, + // Stamped, oldest first. + {"carol", "example.com"}, + {"dave", "example.com"}, + {"eve", "acme.com"}, + {"eve", "example.com"}, + } + if len(got) != len(wantOrder) { + t.Fatalf("got %d profiles, want %d", len(got), len(wantOrder)) + } + for i, w := range wantOrder { + if got[i].Name() != w.name || got[i].NetworkProfile().DomainName != w.domain { + t.Errorf("position %d: got (%q,%q), want (%q,%q) (full: %v)", + i, got[i].Name(), got[i].NetworkProfile().DomainName, w.name, w.domain, profileNames(got)) + } + } +} + +func profileNames(ps []ipn.LoginProfileView) []string { + out := make([]string, len(ps)) + for i, p := range ps { + out[i] = p.Name() + } + return out +} + +// TestProfileCreatedStamped verifies that Created is set on profile creation +// from the injected clock, and is preserved across an update even when the +// clock advances. +func TestProfileCreatedStamped(t *testing.T) { + pm, err := newProfileManagerWithGOOS(new(mem.Store), logger.Discard, health.NewTracker(eventbustest.NewBus(t)), "linux") + if err != nil { + t.Fatal(err) + } + start := time.Date(2026, 6, 23, 12, 0, 0, 0, time.UTC) + clock := tstest.NewClock(tstest.ClockOpts{Start: start}) + pm.clock = tstime.DefaultClock{Clock: clock} + + pm.SwitchToNewProfile() + p := pm.CurrentPrefs().AsStruct() + p.Persist = &persist.Persist{ + NodeID: "n1", + PrivateNodeKey: key.NewNode(), + UserProfile: tailcfg.UserProfile{ + ID: 1, + LoginName: "alice@example.com", + }, + } + if err := pm.SetPrefs(p.View(), ipn.NetworkProfile{}); err != nil { + t.Fatal(err) + } + + if got := pm.currentProfile.Created(); !got.Equal(start) { + t.Errorf("Created = %v after create, want %v", got, start) + } + + // Advancing the clock and then performing an unrelated update (e.g. + // ProfileName change) must not move Created. + clock.Advance(time.Hour) + p = pm.CurrentPrefs().AsStruct() + p.ProfileName = "Alice" + if err := pm.SetPrefs(p.View(), ipn.NetworkProfile{}); err != nil { + t.Fatal(err) + } + if got := pm.currentProfile.Created(); !got.Equal(start) { + t.Errorf("Created changed across update: got %v, want %v", got, start) + } +} diff --git a/ipn/prefs.go b/ipn/prefs.go index 01c8bc5f8..6ac421eb0 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -17,6 +17,7 @@ import ( "runtime" "slices" "strings" + "time" "tailscale.com/atomicfile" "tailscale.com/drive" @@ -1091,6 +1092,12 @@ type LoginProfile struct { // ControlURL is the URL of the control server that this profile is logged // into. ControlURL string + + // Created is when this profile was first added to this client. It is + // stamped once at profile creation and never changes. It is used to sort + // the profile list with newest first; profiles created before this field + // existed have a zero value and sort after all stamped profiles. + Created time.Time `json:",omitzero"` } // Equals reports whether p and p2 are equal.