ipn/ipnlocal: prevent duplicate profiles of the same user

Updates #713

Signed-off-by: Maisem Ali <maisem@tailscale.com>

ipn/ipnlocal/profiles.go

@@ -14,6 +14,7 @@ import (
 
 	"golang.org/x/exp/slices"
 	"tailscale.com/ipn"
+	"tailscale.com/tailcfg"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/strs"
@@ -69,6 +70,15 @@ func (pm *profileManager) SetCurrentUser(uid string) error {
 	return nil
 }
 
+func (pm *profileManager) findProfileByUserID(userID tailcfg.UserID) *ipn.LoginProfile {
+	for _, p := range pm.knownProfiles {
+		if p.UserProfile.ID == userID {
+			return p
+		}
+	}
+	return nil
+}
+
 func (pm *profileManager) findProfileByName(name string) *ipn.LoginProfile {
 	for _, p := range pm.knownProfiles {
 		if p.Name == name {
@@ -129,8 +139,14 @@ func (pm *profileManager) SetPrefs(prefsIn ipn.PrefsView) error {
 	wasNamedWithLoginName := cp.Name == cp.UserProfile.LoginName
 	if pm.isNewProfile {
 		pm.isNewProfile = false
+		// Check if we already have a profile for this user.
+		existing := pm.findProfileByUserID(ps.UserProfile.ID)
+		if existing != nil && existing.ID != "" {
+			cp = existing
+		} else {
 			cp.ID, cp.Key = newUnusedID(pm.knownProfiles)
 			cp.Name = ps.LoginName
+		}
 		cp.UserProfile = ps.UserProfile
 		cp.LocalUserID = pm.currentUserID
 	} else {
@@ -140,6 +156,7 @@ func (pm *profileManager) SetPrefs(prefsIn ipn.PrefsView) error {
 		cp.Name = ps.LoginName
 	}
 	pm.knownProfiles[cp.ID] = cp
+	pm.currentProfile = cp
 	if err := pm.writeKnownProfiles(); err != nil {
 		return err
 	}

ipn/ipnlocal/profiles_test.go

@@ -9,6 +9,8 @@ import (
 
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/store/mem"
+	"tailscale.com/tailcfg"
+	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/persist"
 )
@@ -53,15 +55,27 @@ func TestProfileManagement(t *testing.T) {
 				t.Fatal(err)
 			}
 			// Use Hostname as a proxy for all prefs.
-			if got.Hostname() != wantProfiles[p.Name].Hostname() {
-				t.Fatalf("Prefs for profile %q = %v; want %v", p, got.Pretty(), wantProfiles[p.Name].Pretty())
+			if !got.Equals(wantProfiles[p.Name]) {
+				t.Fatalf("Prefs for profile %q =\n got=%+v\nwant=%v", p, got.Pretty(), wantProfiles[p.Name].Pretty())
 			}
 		}
 	}
+	logins := make(map[string]tailcfg.UserID)
 	setPrefs := func(t *testing.T, loginName string) ipn.PrefsView {
+		t.Helper()
 		p := pm.CurrentPrefs().AsStruct()
+		id := logins[loginName]
+		if id.IsZero() {
+			id = tailcfg.UserID(len(logins) + 1)
+			logins[loginName] = id
+		}
 		p.Persist = &persist.Persist{
 			LoginName:      loginName,
+			PrivateNodeKey: key.NewNode(),
+			UserProfile: tailcfg.UserProfile{
+				ID:        id,
+				LoginName: loginName,
+			},
 		}
 		if err := pm.SetPrefs(p.View()); err != nil {
 			t.Fatal(err)
@@ -117,6 +131,18 @@ func TestProfileManagement(t *testing.T) {
 		t.Fatal(err)
 	}
 	checkProfiles(t)
+
+	t.Logf("Create new profile again")
+	pm.NewProfile()
+	wantCurProfile = ""
+	wantProfiles[""] = emptyPrefs
+	checkProfiles(t)
+
+	t.Logf("Login with the existing profile")
+	wantProfiles["user@2.example.com"] = setPrefs(t, "user@2.example.com")
+	delete(wantProfiles, "")
+	wantCurProfile = "user@2.example.com"
+	checkProfiles(t)
 }
 
 // TestProfileManagementWindows tests going into and out of Unattended mode on
@@ -143,11 +169,21 @@ func TestProfileManagementWindows(t *testing.T) {
 			t.Fatalf("CurrentPrefs = %+v; want %+v", p.Pretty(), wantProfiles[wantCurProfile].Pretty())
 		}
 	}
+	logins := make(map[string]tailcfg.UserID)
 	setPrefs := func(t *testing.T, loginName string, forceDaemon bool) ipn.PrefsView {
+		id := logins[loginName]
+		if id.IsZero() {
+			id = tailcfg.UserID(len(logins) + 1)
+			logins[loginName] = id
+		}
 		p := pm.CurrentPrefs().AsStruct()
 		p.ForceDaemon = forceDaemon
 		p.Persist = &persist.Persist{
 			LoginName: loginName,
+			UserProfile: tailcfg.UserProfile{
+				ID:        id,
+				LoginName: loginName,
+			},
 		}
 		if err := pm.SetPrefs(p.View()); err != nil {
 			t.Fatal(err)

ipn/prefs.go

@@ -696,13 +696,27 @@ type ProfileID string
 // LoginProfile represents a single login profile as managed
 // by the ProfileManager.
 type LoginProfile struct {
+	// ID is a unique identifier for this profile.
+	// It is assigned on creation and never changes.
+	// It may seem redundant to have both ID and UserProfile.ID
+	// but they are different things. UserProfile.ID may change
+	// over time (e.g. if a device is tagged).
 	ID ProfileID
+
+	// Name is the user-visible name of this profile.
+	// It is filled in from the UserProfile.LoginName field.
 	Name string
+
+	// Key is the StateKey under which the profile is stored.
+	// It is assigned once at profile creation time and never changes.
 	Key StateKey
 
+	// UserProfile is the server provided UserProfile for this profile.
+	// This is updated whenever the server provides a new UserProfile.
 	UserProfile tailcfg.UserProfile
 
 	// LocalUserID is the user ID of the user who created this profile.
 	// It is only relevant on Windows where we have a multi-user system.
+	// It is assigned once at profile creation time and never changes.
 	LocalUserID string
 }