Add one-shot migration

client/internal/profilemanager/migration.go

@@ -0,0 +1,191 @@
+package profilemanager
+
+import (
+	"fmt"
+	"os"
+	"os/user"
+	"path/filepath"
+	"strconv"
+	"strings"
+
+	log "github.com/sirupsen/logrus"
+
+	"github.com/netbirdio/netbird/client/internal/getent"
+	"github.com/netbirdio/netbird/client/internal/ipcauth"
+)
+
+// MigrateLegacyProfiles prepares the per-username layout for a daemon that
+// addresses profiles by ID and owner.
+//
+// Two things have to be settled before a directory name can stop carrying
+// meaning. Every profile needs an ID no other profile holds, since a legacy ID
+// is a display name two accounts can each have, and the profiles of the account
+// the machine last ran as need their owner recorded, which the active profile
+// state is the only lossless record of.
+//
+// The profiles.v1 directory is the marker and is created only once both are
+// done, so a run that fails leaves no marker and is retried on the next start.
+// Callers log a failure and carry on rather than refusing to start.
+func (s *ServiceManager) MigrateLegacyProfiles() error {
+	dest := s.profilesDirPath()
+	if _, err := os.Stat(dest); err == nil {
+		return nil
+	}
+
+	profiles, err := s.loadAllProfiles()
+	if err != nil {
+		return fmt.Errorf("load profiles: %w", err)
+	}
+
+	active, err := s.GetActiveProfileState()
+	if err != nil {
+		return fmt.Errorf("active profile state: %w", err)
+	}
+
+	if err := s.rekeyDuplicateIDs(profiles, active); err != nil {
+		return err
+	}
+
+	if err := s.stampActiveUserDir(profiles, active); err != nil {
+		return err
+	}
+
+	if err := os.MkdirAll(dest, 0700); err != nil {
+		return fmt.Errorf("create shared profile directory: %w", err)
+	}
+
+	log.Infof("profile migration complete, %s now marks it done", dest)
+	return nil
+}
+
+// rekeyDuplicateIDs gives every profile sharing an ID a fresh one, in place.
+// The file stays in its directory, only its name changes, so nothing that holds
+// a path to a sibling file is disturbed.
+func (s *ServiceManager) rekeyDuplicateIDs(profiles []Profile, active *ActiveProfileState) error {
+	groups := make(map[ID][]*Profile, len(profiles))
+	for i := range profiles {
+		p := &profiles[i]
+		if p.ID == defaultProfileName {
+			continue
+		}
+		groups[p.ID] = append(groups[p.ID], p)
+	}
+
+	activeDir := sanitizeProfileName(active.Username)
+	for id, group := range groups {
+		if len(group) < 2 {
+			continue
+		}
+
+		// Renaming the active profile without knowing which of the namesakes it
+		// is would leave the active state pointing at nothing. The recorded
+		// username is the only thing that tells them apart, so without it the
+		// safer move is to leave the group alone and keep resolving it the old
+		// way.
+		if id == active.ID && activeDir == "" {
+			log.Warnf("leaving %d profiles named %q as they are, the active profile state does not say which one is active", len(group), id)
+			continue
+		}
+
+		for _, p := range group {
+			wasActive := id == active.ID && filepath.Base(filepath.Dir(p.Path)) == activeDir
+
+			fresh, err := generateProfileID()
+			if err != nil {
+				return fmt.Errorf("generate profile ID: %w", err)
+			}
+			if err := rekeyProfile(p, fresh); err != nil {
+				return err
+			}
+
+			if wasActive {
+				active.ID = fresh
+				if err := s.SetActiveProfileState(active); err != nil {
+					return fmt.Errorf("repoint active profile: %w", err)
+				}
+			}
+		}
+	}
+
+	return nil
+}
+
+// rekeyProfile renames a profile and its sidecars to a fresh ID.
+func rekeyProfile(p *Profile, fresh ID) error {
+	// A legacy profile's display name is its filename, so it has to be in the
+	// file before the filename stops meaning anything. The loader already
+	// falls back to the stem, so writing p.Name is a no-op when the file
+	// carries a name of its own.
+	if err := writeProfileName(p.Path, p.Name); err != nil {
+		return fmt.Errorf("record display name of %s: %w", p.ID, err)
+	}
+
+	dir := filepath.Dir(p.Path)
+	target := filepath.Join(dir, fresh.String()+".json")
+	if err := os.Rename(p.Path, target); err != nil {
+		return fmt.Errorf("rekey %s: %w", p.ID, err)
+	}
+
+	for _, suffix := range []string{stateFileSuffix, prefsFileSuffix} {
+		src := filepath.Join(dir, p.ID.String()+suffix)
+		if _, err := os.Stat(src); err != nil {
+			continue
+		}
+		if err := os.Rename(src, filepath.Join(dir, fresh.String()+suffix)); err != nil {
+			log.Warnf("could not rename %s alongside its profile: %v", src, err)
+		}
+	}
+
+	log.Infof("profile %q in %s now has the unique ID %s", p.ID, dir, fresh)
+	p.ID, p.Path = fresh, target
+	return nil
+}
+
+// stampActiveUserDir records the owner of every unowned profile in the
+// directory of the account the active profile state names.
+//
+// That name is the one lossless input the old layout left behind. Resolving it
+// forward, from name to uid, avoids reversing a sanitized directory name, which
+// no amount of enumeration does reliably.
+func (s *ServiceManager) stampActiveUserDir(profiles []Profile, active *ActiveProfileState) error {
+	if active.Username == "" {
+		return nil
+	}
+
+	u, err := getent.LookupUser(active.Username)
+	if err != nil {
+		return fmt.Errorf("resolve %q: %w", active.Username, err)
+	}
+
+	principal, ok := principalForUser(u)
+	if !ok {
+		return fmt.Errorf("account %q has no usable id %q", active.Username, u.Uid)
+	}
+
+	dir := sanitizeProfileName(active.Username)
+	for i := range profiles {
+		p := &profiles[i]
+		if len(p.Owners) > 0 || p.LegacyUserDir != dir {
+			continue
+		}
+		if err := stampPrincipal(p.Path, principal); err != nil {
+			return fmt.Errorf("stamp %s: %w", p.ID, err)
+		}
+		log.Infof("recorded %s as the owner of %s, the directory it sits in is that account's", principal, p.Path)
+	}
+
+	return nil
+}
+
+// principalForUser turns a resolved account into an owner principal. os/user
+// reports a numeric id on Unix and a SID on Windows, which is what tells the
+// two kinds apart without a build tag.
+func principalForUser(u *user.User) (string, bool) {
+	if uid, err := strconv.ParseUint(u.Uid, 10, 32); err == nil {
+		return ipcauth.UIDPrincipal(uint32(uid)), true
+	}
+	if strings.HasPrefix(u.Uid, "S-") {
+		return ipcauth.SIDPrincipal(u.Uid), true
+	}
+	return "", false
+}

client/internal/profilemanager/migration_test.go

@@ -0,0 +1,121 @@
+package profilemanager
+
+import (
+	"os"
+	"os/user"
+	"path/filepath"
+	"runtime"
+	"strconv"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// currentUserPrincipal returns the running account's name and the owner
+// principal migration should record for it. Migration resolves a real account
+// through the host's user database, so there is nothing to stub.
+func currentUserPrincipal(t *testing.T) (string, string) {
+	t.Helper()
+	if runtime.GOOS == "windows" {
+		t.Skip("the windows path records a SID rather than a numeric uid")
+	}
+
+	u, err := user.Current()
+	require.NoError(t, err)
+	uid, err := strconv.ParseUint(u.Uid, 10, 32)
+	require.NoError(t, err)
+
+	return u.Username, "uid:" + strconv.FormatUint(uid, 10)
+}
+
+func TestMigrate_StampsTheActiveAccountsDirectory(t *testing.T) {
+	username, principal := currentUserPrincipal(t)
+
+	withLegacyLayout(t, func(sm *ServiceManager, configDir string) {
+		dir := sanitizeProfileName(username)
+		mine := writeLegacyProfile(t, configDir, dir, "work", nil)
+		theirs := writeLegacyProfile(t, configDir, "someone-else", "theirs", nil)
+		require.NoError(t, sm.SetActiveProfileState(&ActiveProfileState{ID: "work", Username: username}))
+
+		require.NoError(t, sm.MigrateLegacyProfiles())
+
+		assert.Equal(t, []string{principal}, readOwners(t, mine))
+		assert.Empty(t, readOwners(t, theirs),
+			"only the account the active state names has a directory we can attribute")
+		assert.DirExists(t, filepath.Join(configDir, DefaultProfilePathDir),
+			"the marker is written once both halves are done")
+	})
+}
+
+func TestMigrate_IsIdempotentAndSkipsOnceMarked(t *testing.T) {
+	username, principal := currentUserPrincipal(t)
+
+	withLegacyLayout(t, func(sm *ServiceManager, configDir string) {
+		path := writeLegacyProfile(t, configDir, sanitizeProfileName(username), "work", nil)
+		require.NoError(t, sm.SetActiveProfileState(&ActiveProfileState{ID: "work", Username: username}))
+
+		require.NoError(t, sm.MigrateLegacyProfiles())
+		require.NoError(t, sm.MigrateLegacyProfiles())
+		assert.Equal(t, []string{principal}, readOwners(t, path))
+
+		// A profile appearing after the marker is left to the per-caller claim.
+		late := writeLegacyProfile(t, configDir, sanitizeProfileName(username), "late", nil)
+		require.NoError(t, sm.MigrateLegacyProfiles())
+		assert.Empty(t, readOwners(t, late))
+	})
+}
+
+func TestMigrate_RekeysDuplicateIDsAndKeepsTheActiveOne(t *testing.T) {
+	username, principal := currentUserPrincipal(t)
+
+	withLegacyLayout(t, func(sm *ServiceManager, configDir string) {
+		dir := sanitizeProfileName(username)
+		writeLegacyProfile(t, configDir, dir, "work", nil)
+		writeLegacyProfile(t, configDir, "someone-else", "work", nil)
+		require.NoError(t, os.WriteFile(filepath.Join(configDir, dir, "work"+stateFileSuffix), []byte("{}"), 0600))
+		require.NoError(t, sm.SetActiveProfileState(&ActiveProfileState{ID: "work", Username: username}))
+
+		require.NoError(t, sm.MigrateLegacyProfiles())
+
+		profiles, err := sm.loadAllProfiles()
+		require.NoError(t, err)
+
+		ids := map[ID]int{}
+		for _, p := range profiles {
+			ids[p.ID]++
+			if p.ID != defaultProfileName {
+				assert.Equal(t, "work", p.Name, "the display name outlives the filename it came from")
+			}
+		}
+		for id, n := range ids {
+			assert.Equal(t, 1, n, "%s is still shared after migration", id)
+		}
+
+		state, err := sm.GetActiveProfileState()
+		require.NoError(t, err)
+		assert.NotEqual(t, ID("work"), state.ID, "the active profile follows its new ID")
+
+		active, err := sm.ActiveProfilePath(state)
+		require.NoError(t, err)
+		assert.Equal(t, dir, filepath.Base(filepath.Dir(active)),
+			"and still resolves inside the account that was running it")
+		assert.Equal(t, []string{principal}, readOwners(t, active))
+		assert.FileExists(t, filepath.Join(configDir, dir, state.ID.String()+stateFileSuffix),
+			"the state file follows the profile it belongs to")
+	})
+}
+
+func TestMigrate_UnresolvableAccountLeavesNoMarker(t *testing.T) {
+	withLegacyLayout(t, func(sm *ServiceManager, configDir string) {
+		path := writeLegacyProfile(t, configDir, "ghost", "work", nil)
+		require.NoError(t, sm.SetActiveProfileState(&ActiveProfileState{
+			ID: "work", Username: "no-such-account-here",
+		}))
+
+		require.Error(t, sm.MigrateLegacyProfiles())
+		assert.Empty(t, readOwners(t, path))
+		assert.NoDirExists(t, filepath.Join(configDir, DefaultProfilePathDir),
+			"an unfinished run leaves no marker, so the next start tries again")
+	})
+}

client/internal/profilemanager/prefs.go

@@ -11,7 +11,10 @@ import (
 	"github.com/netbirdio/netbird/util"
 )
 
-const prefsFileSuffix = ".prefs.json"
+const (
+	prefsFileSuffix = ".prefs.json"
+	stateFileSuffix = ".state.json"
+)
 
 var prefsMu sync.Mutex
 

client/internal/profilemanager/service.go

@@ -179,20 +179,26 @@ func (s *ServiceManager) ActiveProfilePath(a *ActiveProfileState) (string, error
 		}
 	}
 
-	switch len(matches) {
+	if len(matches) == 0 {
-	case 0:
 		// Nothing on disk under that ID, so the legacy layout is the only
 		// guess left for where the file would go.
 		return a.FilePath()
-	case 1:
+	}
+
+	if len(matches) == 1 {
 		return matches[0].Path, nil
 	}
 
-	// Two directories hold the same legacy ID, so the recorded hint says which
+	// Migration gives every profile an ID no other profile holds, so getting
-	// one the daemon activated. State written before the hint was a directory
+	// here means it has not run yet or did not finish. Until it does, the
-	// recorded the raw account name, which the legacy layout sanitized on its
+	// recorded account name is the only thing telling namesakes apart, and
-	// way to becoming a directory, so try it both ways.
+	// picking the wrong one would point the daemon at another user's config.
+	// State written before the field held a directory recorded the raw account
+	// name, which the old layout sanitized on its way to becoming one.
 	for _, want := range []string{a.Username, sanitizeProfileName(a.Username)} {
+		if want == "" {
+			continue
+		}
 		for _, p := range matches {
 			if filepath.Base(filepath.Dir(p.Path)) == want {
 				return p.Path, nil
@@ -200,8 +206,8 @@ func (s *ServiceManager) ActiveProfilePath(a *ActiveProfileState) (string, error
 		}
 	}
 
-	log.Warnf("active profile %q exists in %d directories and none of them is %q, using %s",
+	log.Warnf("%d profiles share the ID %q and none of them sits in %q, using %s",
-		a.ID, len(matches), a.Username, matches[0].Path)
+		len(matches), a.ID, a.Username, matches[0].Path)
 	return matches[0].Path, nil
 }
 
@@ -346,10 +352,6 @@ func (s *ServiceManager) SetActiveProfileState(a *ActiveProfileState) error {
 		return errors.New("invalid active profile state")
 	}
 
-	if a.ID != defaultProfileName && a.Username == "" {
-		return fmt.Errorf("username must be set for non-default profiles, got: %s", a.ID)
-	}
-
 	if a.ID != defaultProfileName && !IsValidProfileFilenameStem(a.ID) {
 		return fmt.Errorf("invalid profile ID: %q", a.ID)
 	}
@@ -440,20 +442,7 @@ func (s *ServiceManager) RenameProfile(id ID, userID ipcauth.Identity, newName s
 		return ErrProfileNotFound
 	}
 
-	data, err := os.ReadFile(target.Path)
+	return writeProfileName(target.Path, displayName)
-	if err != nil {
-		return err
-	}
-	var cfg Config
-	if err := json.Unmarshal(data, &cfg); err != nil {
-		return err
-	}
-	cfg.Name = displayName
-
-	if err := util.WriteJson(context.Background(), target.Path, cfg); err != nil {
-		return fmt.Errorf("failed to write profile name: %w", err)
-	}
-	return nil
 }
 
 // RemoveProfile deletes the profile identified by id. Callers must have
@@ -898,9 +887,35 @@ func readProfileOwners(path string) ([]ipcauth.Principal, error) {
 	return []ipcauth.Principal{principal}, nil
 }
 
-// StampOwner records owner as a profile's owner, replacing whoever is recorded
+// StampOwner records a caller as a profile's owner, replacing whoever is
-// now.
+// recorded now.
 func StampOwner(path string, owner ipcauth.Identity) error {
+	return stampPrincipal(path, ipcauth.OwnerPrincipalForIdentity(owner))
+}
+
+// stampPrincipal records an owner principal directly. Migration needs this: it
+// resolves an account name rather than a caller, and a name the kernel never
+// vouched for must not become an Identity on the way.
+func stampPrincipal(path, principal string) error {
+	return updateProfileConfig(path, func(cfg *Config) {
+		cfg.Owners = []string{principal}
+	})
+}
+
+// writeProfileName sets a profile's display name. Renaming does it on request,
+// migration does it to move a name out of a filename that is about to change.
+func writeProfileName(path, name string) error {
+	return updateProfileConfig(path, func(cfg *Config) {
+		cfg.Name = name
+	})
+}
+
+// updateProfileConfig reads a profile, applies mutate and writes it back.
+//
+// The whole config makes the round trip, which is what every writer here does,
+// so a field this version does not model is dropped. That only happens after a
+// downgrade, and a downgrade already drops the owners it cannot read.
+func updateProfileConfig(path string, mutate func(*Config)) error {
 	data, err := os.ReadFile(path)
 	if err != nil {
 		return err
@@ -910,10 +925,10 @@ func StampOwner(path string, owner ipcauth.Identity) error {
 	if err := json.Unmarshal(data, &cfg); err != nil {
 		return err
 	}
-	cfg.Owners = []string{ipcauth.OwnerPrincipalForIdentity(owner)}
+	mutate(&cfg)
 
 	if err := util.WriteJson(context.Background(), path, cfg); err != nil {
-		return fmt.Errorf("write profile owner: %w", err)
+		return fmt.Errorf("write profile %s: %w", path, err)
 	}
 	return nil
 }

client/internal/profilemanager/service_test.go

@@ -477,6 +477,36 @@ func TestClaimLegacyProfile_SkipsOneAlreadyOwnedInTheSameDirectory(t *testing.T)
 	})
 }
 
+func TestRenameProfile(t *testing.T) {
+	withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) {
+		created, err := sm.AddProfile("work", &userID)
+		require.NoError(t, err)
+
+		require.NoError(t, sm.RenameProfile(created.ID, userID, "weekend"))
+
+		got, err := sm.ResolveProfile(created.ID.String(), userID)
+		require.NoError(t, err)
+		assert.Equal(t, "weekend", got.Name, "the new name is on disk")
+		assert.Equal(t, created.ID, got.ID, "renaming does not re-key the profile")
+		assert.Equal(t, created.Path, got.Path)
+	})
+}
+
+func TestRenameProfile_NotTheCallersProfile(t *testing.T) {
+	withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) {
+		created, err := sm.AddProfile("work", &userID)
+		require.NoError(t, err)
+
+		stranger := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242})
+		require.Error(t, sm.RenameProfile(created.ID, stranger, "weekend"),
+			"a profile the caller cannot address is not theirs to rename")
+
+		got, err := sm.ResolveProfile(created.ID.String(), userID)
+		require.NoError(t, err)
+		assert.Equal(t, "work", got.Name)
+	})
+}
+
 func TestListProfiles_PrivilegedCallerDoesNotClaim(t *testing.T) {
 	withLegacyLayout(t, func(sm *ServiceManager, configDir string) {
 		path := writeLegacyProfile(t, configDir, "root", "work", nil)

client/server/server.go

@@ -288,6 +288,13 @@ func (s *Server) Start() error {
 		return err
 	}
 
+	// A half-migrated machine still runs, it just keeps resolving profiles the
+	// old way, so a failure here is logged and retried on the next start rather
+	// than kept from starting at all.
+	if err := s.profileManager.MigrateLegacyProfiles(); err != nil {
+		log.Errorf("profile migration did not finish, retrying on next start: %v", err)
+	}
+
 	activeProf, err := s.profileManager.GetActiveProfileState()
 	if err != nil {
 		return fmt.Errorf("failed to get active profile state: %w", err)