mirrors
/
tailscale
mirror of https://github.com/tailscale/tailscale.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

ipn/ipnlocal: fix profile duplication 

We would only look for duplicate profiles when a new login
occurred but when using `--force-reauth` we could switch
users which would end up with duplicate profiles.

Updates #7726

Signed-off-by: Maisem Ali <maisem@tailscale.com>
This commit is contained in:
Maisem Ali 2023-08-04 13:22:33 -06:00 committed by Maisem Ali
parent 500b9579d5
commit 3e255d76e1
3 changed files with 67 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

125
ipn/ipnlocal/profiles.go
View File

@ -16,9 +16,9 @@ import (
"golang.org/x/exp/slices"
"tailscale.com/envknob"
"tailscale.com/ipn"
"tailscale.com/tailcfg"
"tailscale.com/types/logger"
"tailscale.com/util/clientmetric"
"tailscale.com/util/cmpx"
"tailscale.com/util/winutil"
)
@ -33,15 +33,9 @@ type profileManager struct {
logf logger.Logf
currentUserID ipn.WindowsUserID
knownProfiles map[ipn.ProfileID]*ipn.LoginProfile
currentProfile *ipn.LoginProfile // always non-nil
prefs ipn.PrefsView // always Valid.
// isNewProfile is a sentinel value that indicates that the
// current profile is new and has not been saved to disk yet.
// It is reset to false after a call to SetPrefs with a filled
// in LoginName.
isNewProfile bool
knownProfiles map[ipn.ProfileID]*ipn.LoginProfile // always non-nil
currentProfile *ipn.LoginProfile // always non-nil
prefs ipn.PrefsView // always Valid.
}
func (pm *profileManager) dlogf(format string, args ...any) {
@ -107,40 +101,45 @@ func (pm *profileManager) SetCurrentUserID(uid ipn.WindowsUserID) error {
}
pm.currentProfile = prof
pm.prefs = prefs
pm.isNewProfile = false
return nil
}
// allProfiles returns all profiles that belong to the currentUserID.
// The returned profiles are sorted by Name.
func (pm *profileManager) allProfiles() (out []*ipn.LoginProfile) {
for _, p := range pm.knownProfiles {
if p.LocalUserID == pm.currentUserID {
out = append(out, p)
}
}
slices.SortFunc(out, func(a, b *ipn.LoginProfile) int {
return cmpx.Compare(a.Name, b.Name)
})
return out
}
// matchingProfiles returns all profiles that match the given predicate and
// belong to the currentUserID.
// The returned profiles are sorted by Name.
func (pm *profileManager) matchingProfiles(f func(*ipn.LoginProfile) bool) (out []*ipn.LoginProfile) {
for _, p := range pm.knownProfiles {
if p.LocalUserID == pm.currentUserID && f(p) {
all := pm.allProfiles()
out = all[:0]
for _, p := range all {
if f(p) {
out = append(out, p)
}
}
return out
}
// findProfilesByNodeID returns all profiles that have the provided nodeID and
// belong to the same control server.
func (pm *profileManager) findProfilesByNodeID(controlURL string, nodeID tailcfg.StableNodeID) []*ipn.LoginProfile {
if nodeID.IsZero() {
return nil
}
// findMatchinProfiles returns all profiles that represent the same node/user as
// prefs.
// The returned profiles are sorted by Name.
func (pm *profileManager) findMatchingProfiles(prefs *ipn.Prefs) []*ipn.LoginProfile {
return pm.matchingProfiles(func(p *ipn.LoginProfile) bool {
return p.NodeID == nodeID && p.ControlURL == controlURL
})
}
// findProfilesByUserID returns all profiles that have the provided userID and
// belong to the same control server.
func (pm *profileManager) findProfilesByUserID(controlURL string, userID tailcfg.UserID) []*ipn.LoginProfile {
if userID.IsZero() {
return nil
}
return pm.matchingProfiles(func(p *ipn.LoginProfile) bool {
return p.UserProfile.ID == userID && p.ControlURL == controlURL
return p.ControlURL == prefs.ControlURL &&
(p.UserProfile.ID == prefs.Persist.UserProfile.ID ||
p.NodeID == prefs.Persist.NodeID)
})
}
@ -206,40 +205,47 @@ func init() {
// It also saves the prefs to the StateStore. It stores a copy of the
// provided prefs, which may be accessed via CurrentPrefs.
func (pm *profileManager) SetPrefs(prefsIn ipn.PrefsView) error {
prefs := prefsIn.AsStruct().View()
newPersist := prefs.Persist().AsStruct()
prefs := prefsIn.AsStruct()
newPersist := prefs.Persist
if newPersist == nil || newPersist.NodeID == "" || newPersist.UserProfile.LoginName == "" {
return pm.setPrefsLocked(prefs)
// We don't know anything about this profile, so ignore it for now.
return pm.setPrefsLocked(prefs.View())
}
up := newPersist.UserProfile
if up.DisplayName == "" {
up.DisplayName = up.LoginName
}
cp := pm.currentProfile
if pm.isNewProfile {
pm.isNewProfile = false
// Check if we already have a profile for this user.
existing := pm.findProfilesByUserID(prefs.ControlURL(), newPersist.UserProfile.ID)
// Also check if we have a profile with the same NodeID.
existing = append(existing, pm.findProfilesByNodeID(prefs.ControlURL(), newPersist.NodeID)...)
if len(existing) == 0 {
cp.ID, cp.Key = newUnusedID(pm.knownProfiles)
} else {
// Only one profile per user/nodeID should exist.
for _, p := range existing[1:] {
// Best effort cleanup.
pm.DeleteProfile(p.ID)
// Check if we already have an existing profile that matches the user/node.
if existing := pm.findMatchingProfiles(prefs); len(existing) > 0 {
// We already have a profile for this user/node we should reuse it. Also
// cleanup any other duplicate profiles.
cp = existing[0]
existing = existing[1:]
for _, p := range existing {
// Clear the state.
if err := pm.store.WriteState(p.Key, nil); err != nil {
// We couldn't delete the state, so keep the profile around.
continue
}
cp = existing[0]
// Remove the profile, knownProfiles will be persisted below.
delete(pm.knownProfiles, p.ID)
}
} else if cp.ID == "" {
// We didn't have an existing profile, so create a new one.
cp.ID, cp.Key = newUnusedID(pm.knownProfiles)
cp.LocalUserID = pm.currentUserID
} else {
// This means that there was a force-reauth as a new node that
// we haven't seen before.
}
if prefs.ProfileName() != "" {
cp.Name = prefs.ProfileName()
if prefs.ProfileName != "" {
cp.Name = prefs.ProfileName
} else {
cp.Name = up.LoginName
}
cp.ControlURL = prefs.ControlURL()
cp.ControlURL = prefs.ControlURL
cp.UserProfile = newPersist.UserProfile
cp.NodeID = newPersist.NodeID
pm.knownProfiles[cp.ID] = cp
@ -250,7 +256,7 @@ func (pm *profileManager) SetPrefs(prefsIn ipn.PrefsView) error {
if err := pm.setAsUserSelectedProfileLocked(); err != nil {
return err
}
if err := pm.setPrefsLocked(prefs); err != nil {
if err := pm.setPrefsLocked(prefs.View()); err != nil {
return err
}
return nil
@ -273,7 +279,7 @@ func newUnusedID(knownProfiles map[ipn.ProfileID]*ipn.LoginProfile) (ipn.Profile
// is not new.
func (pm *profileManager) setPrefsLocked(clonedPrefs ipn.PrefsView) error {
pm.prefs = clonedPrefs
if pm.isNewProfile {
if pm.currentProfile.ID == "" {
return nil
}
if err := pm.writePrefsToStore(pm.currentProfile.Key, pm.prefs); err != nil {
@ -295,12 +301,9 @@ func (pm *profileManager) writePrefsToStore(key ipn.StateKey, prefs ipn.PrefsVie
// Profiles returns the list of known profiles.
func (pm *profileManager) Profiles() []ipn.LoginProfile {
profiles := pm.matchingProfiles(func(*ipn.LoginProfile) bool { return true })
slices.SortFunc(profiles, func(a, b *ipn.LoginProfile) int {
return strings.Compare(a.Name, b.Name)
})
out := make([]ipn.LoginProfile, 0, len(profiles))
for _, p := range profiles {
allProfiles := pm.allProfiles()
out := make([]ipn.LoginProfile, 0, len(allProfiles))
for _, p := range allProfiles {
out = append(out, *p)
}
return out
@ -328,7 +331,6 @@ func (pm *profileManager) SwitchProfile(id ipn.ProfileID) error {
}
pm.prefs = prefs
pm.currentProfile = kp
pm.isNewProfile = false
return pm.setAsUserSelectedProfileLocked()
}
@ -380,7 +382,7 @@ var errProfileNotFound = errors.New("profile not found")
func (pm *profileManager) DeleteProfile(id ipn.ProfileID) error {
metricDeleteProfile.Add(1)
if id == "" && pm.isNewProfile {
if id == "" {
// Deleting the in-memory only new profile, just create a new one.
pm.NewProfile()
return nil
@ -431,7 +433,6 @@ func (pm *profileManager) NewProfile() {
metricNewProfile.Add(1)
pm.prefs = defaultPrefs
pm.isNewProfile = true
pm.currentProfile = &ipn.LoginProfile{}
}

4
ipn/ipnlocal/profiles_notwindows.go
View File

@ -16,9 +16,7 @@ import (
func (pm *profileManager) loadLegacyPrefs() (string, ipn.PrefsView, error) {
k := ipn.LegacyGlobalDaemonStateKey
switch {
case runtime.GOOS == "ios":
k = "ipn-go-bridge"
case version.IsSandboxedMacOS():
case runtime.GOOS == "ios", version.IsSandboxedMacOS():
k = "ipn-go-bridge"
case runtime.GOOS == "android":
k = "ipn-android"

15
ipn/ipnlocal/profiles_test.go
View File

@ -203,11 +203,8 @@ func TestProfileDupe(t *testing.T) {
{reauth, user1Node1},
},
profs: []*persist.Persist{
// BUG: This is incorrect, and should be:
// user1Node1,
// user2Node2,
user1Node1,
user1Node1,
user2Node2,
},
},
{
@ -218,11 +215,8 @@ func TestProfileDupe(t *testing.T) {
{reauth, user2Node1},
},
profs: []*persist.Persist{
// BUG: This is incorrect, and should be:
// user2Node1,
// user3Node3,
user1Node1,
user2Node1,
user3Node3,
},
},
{
@ -233,11 +227,8 @@ func TestProfileDupe(t *testing.T) {
{reauth, user1Node2},
},
profs: []*persist.Persist{
// BUG: This is incorrect, and should be:
// user1Node2,
// user3Node3,
user1Node1,
user1Node2,
user3Node3,
},
},
{