diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go index 95d4c564f..f14b955f9 100644 --- a/client/internal/profilemanager/config.go +++ b/client/internal/profilemanager/config.go @@ -276,7 +276,7 @@ func baseConfigDir() (string, error) { return os.UserConfigDir() } -func getConfigDirForUser(username string) (string, error) { +func getConfigDirForUserLegacy(username string) (string, error) { if ConfigDirOverride != "" { return ConfigDirOverride, nil } diff --git a/client/internal/profilemanager/prefs.go b/client/internal/profilemanager/prefs.go index 5613b0be3..e00ea51e0 100644 --- a/client/internal/profilemanager/prefs.go +++ b/client/internal/profilemanager/prefs.go @@ -29,7 +29,7 @@ func (s *ServiceManager) ProfilePrefs(id ID, username string) (*Prefs, error) { if id == defaultProfileName { return &Prefs{path: filepath.Join(filepath.Dir(DefaultConfigPath), id.String()+prefsFileSuffix)}, nil } - configDir, err := s.getConfigDir(username) + configDir, err := s.getConfigDirLegacy(username) if err != nil { return nil, fmt.Errorf("get config directory for user %s: %w", username, err) } diff --git a/client/internal/profilemanager/prefs_test.go b/client/internal/profilemanager/prefs_test.go index 8cca6e513..6d1b6022c 100644 --- a/client/internal/profilemanager/prefs_test.go +++ b/client/internal/profilemanager/prefs_test.go @@ -3,11 +3,14 @@ package profilemanager import ( "errors" "os" + "os/user" "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/client/internal/ipcauth" ) type testPrefsSection struct { @@ -15,9 +18,20 @@ type testPrefsSection struct { Dest string `json:"dest"` } +// currentUsername returns the account the prefs store keys its legacy config +// directory by. Profile ownership itself is carried by an ipcauth.Identity, but +// the on-disk layout is still per-username. +func currentUsername(t *testing.T) string { + t.Helper() + u, err := user.Current() + require.NoError(t, err) + return u.Username +} + func TestProfilePrefs_RoundTrip(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - created, err := sm.AddProfile("work", username, nil) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + username := currentUsername(t) + created, err := sm.AddProfile("work", &userID) require.NoError(t, err) prefs, err := sm.ProfilePrefs(created.ID, username) @@ -41,8 +55,9 @@ func TestProfilePrefs_RoundTrip(t *testing.T) { } func TestProfilePrefs_GetMissingNamespace(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - created, err := sm.AddProfile("work", username, nil) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + username := currentUsername(t) + created, err := sm.AddProfile("work", &userID) require.NoError(t, err) prefs, err := sm.ProfilePrefs(created.ID, username) @@ -56,8 +71,9 @@ func TestProfilePrefs_GetMissingNamespace(t *testing.T) { } func TestProfilePrefs_RemoveNamespace(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - created, err := sm.AddProfile("work", username, nil) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + username := currentUsername(t) + created, err := sm.AddProfile("work", &userID) require.NoError(t, err) prefs, err := sm.ProfilePrefs(created.ID, username) @@ -82,15 +98,17 @@ func TestProfilePrefs_RemoveNamespace(t *testing.T) { } func TestProfilePrefs_RejectsInvalidID(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + username := currentUsername(t) _, err := sm.ProfilePrefs("../escape", username) assert.Error(t, err) }) } func TestProfilePrefs_RejectsEmptyNamespace(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - created, err := sm.AddProfile("work", username, nil) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + username := currentUsername(t) + created, err := sm.AddProfile("work", &userID) require.NoError(t, err) prefs, err := sm.ProfilePrefs(created.ID, username) @@ -104,7 +122,8 @@ func TestProfilePrefs_RejectsEmptyNamespace(t *testing.T) { } func TestProfilePrefs_DefaultProfile(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + username := currentUsername(t) prefs, err := sm.ProfilePrefs(defaultProfileName, username) require.NoError(t, err) @@ -117,21 +136,22 @@ func TestProfilePrefs_DefaultProfile(t *testing.T) { } func TestRemoveProfile_DeletesPrefsFile(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - created, err := sm.AddProfile("work", username, nil) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + username := currentUsername(t) + created, err := sm.AddProfile("work", &userID) require.NoError(t, err) prefs, err := sm.ProfilePrefs(created.ID, username) require.NoError(t, err) require.NoError(t, prefs.Put("filedrop", testPrefsSection{Mode: 2})) - configDir, err := sm.getConfigDir(username) + configDir, err := sm.getConfigDirLegacy(username) require.NoError(t, err) prefsPath := filepath.Join(configDir, created.ID.String()+prefsFileSuffix) _, err = os.Stat(prefsPath) require.NoError(t, err) - require.NoError(t, sm.RemoveProfile(created.ID, username)) + require.NoError(t, sm.RemoveProfile(created.ID, userID)) _, err = os.Stat(prefsPath) assert.True(t, errors.Is(err, os.ErrNotExist), "prefs file should be removed") }) diff --git a/client/internal/profilemanager/profilemanager.go b/client/internal/profilemanager/profilemanager.go index 44f578ae0..6813a0d6d 100644 --- a/client/internal/profilemanager/profilemanager.go +++ b/client/internal/profilemanager/profilemanager.go @@ -34,6 +34,22 @@ type Profile struct { Owners []ipcauth.Principal } +// AccessibleBy reports whether a kernel-attested caller may address this +// profile. +func (p *Profile) AccessibleBy(id ipcauth.Identity) bool { + if !id.Known() { + return false + } + if ipcauth.IsPrivilegedCaller(id) { + return true + } + // TODO: decide on unowned behavior + if len(p.Owners) == 0 { + return false + } + return p.Owners[0].Matches(id) +} + func (p *Profile) FilePath() (string, error) { if p.Path != "" { return p.Path, nil @@ -60,7 +76,7 @@ func (p *Profile) FilePath() (string, error) { return "", fmt.Errorf("failed to get current user: %w", err) } - configDir, err := getConfigDirForUser(username.Username) + configDir, err := getConfigDirForUserLegacy(username.Username) if err != nil { return "", fmt.Errorf("failed to get config directory for user %s: %w", username.Username, err) } diff --git a/client/internal/profilemanager/profilemanager_test.go b/client/internal/profilemanager/profilemanager_test.go index 882a71d0a..811a00a6f 100644 --- a/client/internal/profilemanager/profilemanager_test.go +++ b/client/internal/profilemanager/profilemanager_test.go @@ -27,7 +27,10 @@ func withPatchedGlobals(t *testing.T, configDir string, testFunc func()) { DefaultConfigPath = filepath.Join(configDir, "default.json") ActiveProfileStatePath = filepath.Join(configDir, "active_profile.json") oldDefaultConfigPath = filepath.Join(configDir, "old_config.json") - ConfigDirOverride = configDir + // A subdirectory, mirroring production: loadAllProfiles only descends into + // directories under DefaultConfigPathDir, so profiles written straight into + // the config root would be invisible to it. + ConfigDirOverride = filepath.Join(configDir, DefaultProfilePathDir) // Clean up any files in the config dir to ensure isolation os.RemoveAll(configDir) os.MkdirAll(configDir, 0755) //nolint: errcheck diff --git a/client/internal/profilemanager/service.go b/client/internal/profilemanager/service.go index 68d1c23a1..6088ef44b 100644 --- a/client/internal/profilemanager/service.go +++ b/client/internal/profilemanager/service.go @@ -27,6 +27,8 @@ var ( DefaultConfigPath = "" ActiveProfileStatePath = "" + DefaultProfilePathDir = "profiles.v1" + ErrorOldDefaultConfigNotFound = errors.New("old default config not found") ) @@ -115,7 +117,7 @@ func (a *ActiveProfileState) FilePath() (string, error) { return "", fmt.Errorf("invalid profile ID: %q", a.ID) } - configDir, err := getConfigDirForUser(a.Username) + configDir, err := getConfigDirForUserLegacy(a.Username) if err != nil { return "", fmt.Errorf("failed to get config directory for user %s: %w", a.Username, err) } @@ -302,8 +304,8 @@ func (s *ServiceManager) DefaultProfilePath() string { // The returned Profile carries the freshly-generated ID so callers can // show it to the user (and so the gRPC AddProfileResponse can include // it). -func (s *ServiceManager) AddProfile(displayName string, username string, callerId *ipcauth.Identity) (*Profile, error) { - configDir, err := s.getConfigDir(username) +func (s *ServiceManager) AddProfile(displayName string, callerId *ipcauth.Identity) (*Profile, error) { + configDir, err := s.getConfigDir() if err != nil { return nil, fmt.Errorf("failed to get config directory: %w", err) } @@ -336,7 +338,7 @@ func (s *ServiceManager) AddProfile(displayName string, username string, callerI }, nil } -func (s *ServiceManager) RenameProfile(id ID, username string, newName string) error { +func (s *ServiceManager) RenameProfile(id ID, userID ipcauth.Identity, newName string) error { displayName, err := sanitizeDisplayName(newName) if err != nil { return fmt.Errorf("invalid profile name: %w", err) @@ -346,7 +348,7 @@ func (s *ServiceManager) RenameProfile(id ID, username string, newName string) e return fmt.Errorf("invalid profile ID: %q", id) } - profiles, err := s.loadAllProfiles(username) + profiles, err := s.loadAllProfilesForIdentity(userID) if err != nil { return fmt.Errorf("load profiles: %w", err) } @@ -381,7 +383,7 @@ func (s *ServiceManager) RenameProfile(id ID, username string, newName string) e // RemoveProfile deletes the profile identified by id. Callers must have // already resolved any user-supplied handle to a concrete ID via // ResolveProfile. -func (s *ServiceManager) RemoveProfile(id ID, username string) error { +func (s *ServiceManager) RemoveProfile(id ID, userID ipcauth.Identity) error { if id == defaultProfileName { defaultName := readProfileName(DefaultConfigPath) if defaultName == "" { @@ -393,7 +395,7 @@ func (s *ServiceManager) RemoveProfile(id ID, username string) error { return fmt.Errorf("invalid profile ID: %q", id) } - profiles, err := s.loadAllProfiles(username) + profiles, err := s.loadAllProfilesForIdentity(userID) if err != nil { return fmt.Errorf("load profiles: %w", err) } @@ -436,8 +438,8 @@ func (s *ServiceManager) RemoveProfile(id ID, username string) error { // ListProfiles returns every profile for the given user, including the // default profile, with IsActive flags set. -func (s *ServiceManager) ListProfiles(username string) ([]Profile, error) { - return s.loadAllProfiles(username) +func (s *ServiceManager) ListProfiles(userID ipcauth.Identity) ([]Profile, error) { + return s.loadAllProfilesForIdentity(userID) } // GetStatePath returns the path to the state file based on the operating system @@ -468,7 +470,7 @@ func (s *ServiceManager) GetStatePath() string { return defaultStatePath } - configDir, err := s.getConfigDir(activeProf.Username) + configDir, err := s.getConfigDirLegacy(activeProf.Username) if err != nil { log.Warnf("failed to get config directory for user %s: %v", activeProf.Username, err) return defaultStatePath @@ -477,13 +479,32 @@ func (s *ServiceManager) GetStatePath() string { return filepath.Join(configDir, activeProf.ID.String()+".state.json") } -// getConfigDir returns the profiles directory, using profilesDir if set, otherwise getConfigDirForUser -func (s *ServiceManager) getConfigDir(username string) (string, error) { +// getConfigDirLegacy returns the profiles directory, using profilesDir if set, otherwise getConfigDirForUser +func (s *ServiceManager) getConfigDirLegacy(username string) (string, error) { if s.profilesDir != "" { return s.profilesDir, nil } - return getConfigDirForUser(username) + return getConfigDirForUserLegacy(username) +} + +func (s *ServiceManager) getConfigDir() (string, error) { + if s.profilesDir != "" { + return s.profilesDir, nil + } + + if ConfigDirOverride != "" { + return ConfigDirOverride, nil + } + + configDir := filepath.Join(DefaultConfigPathDir, DefaultProfilePathDir) + if _, err := os.Stat(configDir); os.IsNotExist(err) { + if err := os.MkdirAll(configDir, 0700); err != nil { + return "", err + } + } + + return configDir, nil } // loadAllProfiles returns every profile visible to the daemon for the @@ -493,28 +514,44 @@ func (s *ServiceManager) getConfigDir(username string) (string, error) { // Each Profile is fully populated: ID is the filename stem, Name comes // from the JSON's "name" field (falling back to the filename stem when absent) // and Path is built from a basename read off disk. -func (s *ServiceManager) loadAllProfiles(username string) ([]Profile, error) { - activeID, activeIsDefault := s.activeProfileID() +func (s *ServiceManager) loadAllProfilesForIdentity(userID ipcauth.Identity) ([]Profile, error) { + allProfiles, err := s.loadAllProfiles() + if err != nil { + return nil, err + } + + accessible := make([]Profile, 0, len(allProfiles)) + for _, p := range allProfiles { + if p.AccessibleBy(userID) { + accessible = append(accessible, p) + } + } + + return accessible, nil +} + +func (s *ServiceManager) loadAllProfiles() ([]Profile, error) { + _, activeIsDefault := s.activeProfileID() defaultName := readProfileName(DefaultConfigPath) if defaultName == "" { defaultName = defaultProfileName } + // The default profile is not seeded with an owner: it starts unowned, and + // the first claim stamps it like any other profile. + defaultOwners, err := readProfileOwners(DefaultConfigPath) + if err != nil { + return nil, err + } profiles := []Profile{{ ID: defaultProfileName, Name: defaultName, Path: DefaultConfigPath, IsActive: activeIsDefault, - // TODO: determine how to seed default owners - Owners: []ipcauth.Principal{}, + Owners: defaultOwners, }} - configDir, err := s.getConfigDir(username) - if err != nil { - return nil, fmt.Errorf("get config directory: %w", err) - } - - entries, err := os.ReadDir(configDir) + configPathDir, err := os.ReadDir(DefaultConfigPathDir) if err != nil { if errors.Is(err, os.ErrNotExist) { return profiles, nil @@ -522,6 +559,38 @@ func (s *ServiceManager) loadAllProfiles(username string) ([]Profile, error) { return nil, fmt.Errorf("read profile directory: %w", err) } + var fileProfiles []Profile + for _, entry := range configPathDir { + if entry.IsDir() { + legacyUsernameProfiles, err := s.getProfilesFromDirectory(filepath.Join(DefaultConfigPathDir, entry.Name())) + if err != nil { + return nil, err + } + fileProfiles = append(fileProfiles, legacyUsernameProfiles...) + } + } + + sort.Slice(fileProfiles, func(i, j int) bool { + if fileProfiles[i].Name != fileProfiles[j].Name { + return fileProfiles[i].Name < fileProfiles[j].Name + } + // Sort tie-break on ID so duplicate names always render in the same order. + return fileProfiles[i].ID < fileProfiles[j].ID + }) + profiles = append(profiles, fileProfiles...) + return profiles, nil +} + +func (s *ServiceManager) getProfilesFromDirectory(configDir string) ([]Profile, error) { + activeID, _ := s.activeProfileID() + entries, err := os.ReadDir(configDir) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return []Profile{}, nil + } + return nil, fmt.Errorf("read profile directory: %w", err) + } + var fileProfiles []Profile for _, entry := range entries { if entry.IsDir() { @@ -550,7 +619,8 @@ func (s *ServiceManager) loadAllProfiles(username string) ([]Profile, error) { owners, err := readProfileOwners(path) if err != nil { - return nil, err + log.Warnf("reading profile owner failed for %s: %v", path, err) + continue } fileProfiles = append(fileProfiles, Profile{ ID: stem, @@ -560,16 +630,7 @@ func (s *ServiceManager) loadAllProfiles(username string) ([]Profile, error) { Owners: owners, }) } - - sort.Slice(fileProfiles, func(i, j int) bool { - if fileProfiles[i].Name != fileProfiles[j].Name { - return fileProfiles[i].Name < fileProfiles[j].Name - } - // Sort tie-break on ID so duplicate names always render in the same order. - return fileProfiles[i].ID < fileProfiles[j].ID - }) - profiles = append(profiles, fileProfiles...) - return profiles, nil + return fileProfiles, nil } // readProfileName parses just the "name" field from the profile Json. @@ -606,9 +667,10 @@ func readProfileOwners(path string) ([]ipcauth.Principal, error) { principal, ok := ipcauth.ParsePrincipal(meta.Owners[0]) if !ok { - // A malformed entry is ignored rather than trusted. - log.Warnf("ignoring unparseable owner %q in %s", meta.Owners[0], path) - return nil, nil + // An entry that cannot be parsed is not trusted, and it is not an + // absence of ownership either: the profile records an owner that cannot + // be matched against anyone. + return nil, fmt.Errorf("unparseable owner %q in %s", meta.Owners[0], path) } return []ipcauth.Principal{principal}, nil } @@ -648,12 +710,12 @@ func (s *ServiceManager) activeProfileID() (ID, bool) { // precedence is: exact ID match, then unique exact name, then unique ID // prefix. Ambiguous matches return *ErrAmbiguousHandle so callers can // surface the candidates. -func (s *ServiceManager) ResolveProfile(handle, username string) (*Profile, error) { +func (s *ServiceManager) ResolveProfile(handle string, userID ipcauth.Identity) (*Profile, error) { if handle == "" { return nil, fmt.Errorf("profile handle is empty") } - profiles, err := s.loadAllProfiles(username) + profiles, err := s.loadAllProfilesForIdentity(userID) if err != nil { return nil, err } diff --git a/client/internal/profilemanager/service_test.go b/client/internal/profilemanager/service_test.go index 09251119a..c7cc94949 100644 --- a/client/internal/profilemanager/service_test.go +++ b/client/internal/profilemanager/service_test.go @@ -6,18 +6,20 @@ import ( "os" "os/user" "path/filepath" + "strconv" "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/netbirdio/netbird/client/internal/ipcauth" "github.com/netbirdio/netbird/util" ) // withTestSM wires up patched globals + a clean config dir and returns a // fully initialized ServiceManager plus the username we are scoped to. -func withTestSM(t *testing.T, fn func(sm *ServiceManager, username string)) { +func withTestSM(t *testing.T, fn func(sm *ServiceManager, id ipcauth.Identity)) { t.Helper() withTempConfigDir(t, func(configDir string) { withPatchedGlobals(t, configDir, func() { @@ -25,17 +27,21 @@ func withTestSM(t *testing.T, fn func(sm *ServiceManager, username string)) { require.NoError(t, err) sm := &ServiceManager{} require.NoError(t, sm.CreateDefaultProfile()) - fn(sm, u.Username) + uid, err := strconv.ParseUint(u.Uid, 10, 32) + require.NoError(t, err) + userID := ipcauth.Identity{UID: uint32(uid)} + userID = ipcauth.KnownForTest(userID) + fn(sm, userID) }) }) } func TestServiceProfile_ExactID(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - created, err := sm.AddProfile("work", username, nil) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + created, err := sm.AddProfile("work", nil) require.NoError(t, err) - got, err := sm.ResolveProfile(created.ID.String(), username) + got, err := sm.ResolveProfile(created.ID.String(), userID) require.NoError(t, err) assert.Equal(t, created.ID, got.ID) assert.Equal(t, "work", got.Name) @@ -43,29 +49,31 @@ func TestServiceProfile_ExactID(t *testing.T) { } func TestServiceProfile_IDPrefix(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - created, err := sm.AddProfile("work", username, nil) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + created, err := sm.AddProfile("work", &userID) require.NoError(t, err) prefix := created.ID[:4] - got, err := sm.ResolveProfile(prefix.String(), username) + got, err := sm.ResolveProfile(prefix.String(), userID) require.NoError(t, err) assert.Equal(t, created.ID, got.ID) }) } func TestServiceProfile_AmbiguousPrefix(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { // Plant two profiles whose IDs share a known prefix by writing // the files directly, since generated IDs are random. - configDir, err := sm.getConfigDir(username) + user, err := user.Current() + require.NoError(t, err) + configDir, err := sm.getConfigDirLegacy(user.Username) require.NoError(t, err) for _, id := range []string{"abcd1111aaaa", "abcd2222bbbb"} { path := filepath.Join(configDir, id+".json") require.NoError(t, util.WriteJson(context.Background(), path, &Config{Name: id})) } - _, err = sm.ResolveProfile("abcd", username) + _, err = sm.ResolveProfile("abcd", userID) var amb *ErrAmbiguousHandle require.ErrorAs(t, err, &amb) assert.Equal(t, AmbiguityKindIDPrefix, amb.Kind) @@ -74,24 +82,24 @@ func TestServiceProfile_AmbiguousPrefix(t *testing.T) { } func TestServiceProfile_ExactNameUnique(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - _, err := sm.AddProfile("work", username, nil) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + _, err := sm.AddProfile("work", &userID) require.NoError(t, err) - got, err := sm.ResolveProfile("work", username) + got, err := sm.ResolveProfile("work", userID) require.NoError(t, err) assert.Equal(t, "work", got.Name) }) } func TestServiceProfile_AmbiguousName(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - _, err := sm.AddProfile("work", username, nil) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + _, err := sm.AddProfile("work", &userID) require.NoError(t, err) - _, err = sm.AddProfile("work", username, nil) + _, err = sm.AddProfile("work", &userID) require.NoError(t, err) - _, err = sm.ResolveProfile("work", username) + _, err = sm.ResolveProfile("work", userID) var amb *ErrAmbiguousHandle require.ErrorAs(t, err, &amb) assert.Equal(t, AmbiguityKindName, amb.Kind) @@ -100,15 +108,15 @@ func TestServiceProfile_AmbiguousName(t *testing.T) { } func TestServiceProfile_NotFound(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - _, err := sm.ResolveProfile("nope", username) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + _, err := sm.ResolveProfile("nope", userID) assert.ErrorIs(t, err, ErrProfileNotFound) }) } func TestServiceProfile_DefaultByExactID(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - got, err := sm.ResolveProfile(defaultProfileName, username) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + got, err := sm.ResolveProfile(defaultProfileName, userID) require.NoError(t, err) assert.Equal(t, defaultProfileName, got.ID.String()) }) @@ -117,13 +125,15 @@ func TestServiceProfile_DefaultByExactID(t *testing.T) { func TestServiceProfile_LegacyFilenameCoexists(t *testing.T) { // Legacy profiles stored as .json with no "name" JSON field // should still be discoverable by name and removable by name. - withTestSM(t, func(sm *ServiceManager, username string) { - configDir, err := sm.getConfigDir(username) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + user, err := user.Current() + require.NoError(t, err) + configDir, err := sm.getConfigDirLegacy(user.Username) require.NoError(t, err) path := filepath.Join(configDir, "legacy.json") require.NoError(t, util.WriteJson(context.Background(), path, &Config{})) - got, err := sm.ResolveProfile("legacy", username) + got, err := sm.ResolveProfile("legacy", userID) require.NoError(t, err) assert.Equal(t, "legacy", got.ID.String()) // Name falls back to the filename stem when JSON omits it. @@ -132,11 +142,11 @@ func TestServiceProfile_LegacyFilenameCoexists(t *testing.T) { } func TestAddProfile_AllowsDuplicateWithFlag(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - first, err := sm.AddProfile("work", username, nil) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + first, err := sm.AddProfile("work", &userID) require.NoError(t, err) - second, err := sm.AddProfile("work", username, nil) + second, err := sm.AddProfile("work", &userID) require.NoError(t, err) assert.NotEqual(t, first.ID, second.ID) assert.Equal(t, "work", second.Name) @@ -144,22 +154,22 @@ func TestAddProfile_AllowsDuplicateWithFlag(t *testing.T) { } func TestAddProfile_RejectsInvalidNames(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { cases := []string{ "", // empty "\x00\x01", // only control chars (becomes empty) strings.Repeat("a", maxProfileNameLen+1), // too long } for _, name := range cases { - _, err := sm.AddProfile(name, username, nil) + _, err := sm.AddProfile(name, &userID) assert.Error(t, err, "expected error for %q", name) } }) } func TestRemoveProfile_RejectsInvalidID(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - err := sm.RemoveProfile("../escape", username) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + err := sm.RemoveProfile("../escape", userID) assert.Error(t, err) }) } @@ -214,17 +224,120 @@ func TestIsValidProfileFilenameStem(t *testing.T) { } func TestRemoveProfile_DeletesStateFile(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, username string) { - created, err := sm.AddProfile("work", username, nil) + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + created, err := sm.AddProfile("work", &userID) require.NoError(t, err) - configDir, err := sm.getConfigDir(username) + user, err := user.Current() + require.NoError(t, err) + configDir, err := sm.getConfigDirLegacy(user.Username) require.NoError(t, err) statePath := filepath.Join(configDir, created.ID.String()+".state.json") require.NoError(t, os.WriteFile(statePath, []byte(`{"email":"a@b"}`), 0600)) - require.NoError(t, sm.RemoveProfile(created.ID, username)) + require.NoError(t, sm.RemoveProfile(created.ID, userID)) _, err = os.Stat(statePath) assert.True(t, errors.Is(err, os.ErrNotExist), "state file should be removed") }) } + +// profileIDs is the set of profile IDs in a listing, for membership assertions +// that do not care about the default profile always being present. +func profileIDs(profiles []Profile) []string { + ids := make([]string, 0, len(profiles)) + for _, p := range profiles { + ids = append(ids, p.ID.String()) + } + return ids +} + +func TestListProfiles_ScopedToOwner(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, _ ipcauth.Identity) { + // Two synthetic users rather than the current one: this process is its + // own daemon, and IsPrivilegedCaller delegates to a caller sharing an + // unprivileged daemon's identity, so the current user resolves + // unfiltered here. + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + bob := ipcauth.KnownForTest(ipcauth.Identity{UID: 4243}) + hers, err := sm.AddProfile("hers", &alice) + require.NoError(t, err) + his, err := sm.AddProfile("his", &bob) + require.NoError(t, err) + + got, err := sm.ListProfiles(alice) + require.NoError(t, err) + assert.Contains(t, profileIDs(got), hers.ID.String()) + assert.NotContains(t, profileIDs(got), his.ID.String(), + "another user's profile must not be listed") + }) +} + +func TestListProfiles_PrivilegedResolvesUnfiltered(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + other := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + mine, err := sm.AddProfile("mine", &userID) + require.NoError(t, err) + theirs, err := sm.AddProfile("theirs", &other) + require.NoError(t, err) + + root := ipcauth.KnownForTest(ipcauth.Identity{UID: 0}) + got, err := sm.ListProfiles(root) + require.NoError(t, err) + assert.Contains(t, profileIDs(got), mine.ID.String()) + assert.Contains(t, profileIDs(got), theirs.ID.String()) + assert.Contains(t, profileIDs(got), defaultProfileName) + }) +} + +func TestListProfiles_UnownedStaysOpenUntilClaimed(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + unowned, err := sm.AddProfile("unowned", nil) + require.NoError(t, err) + + other := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + got, err := sm.ListProfiles(other) + require.NoError(t, err) + assert.Contains(t, profileIDs(got), unowned.ID.String(), + "an unowned profile is addressable until it is claimed") + }) +} + +func TestListProfiles_UnreadableOwnersArePrivilegedOnly(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, _ ipcauth.Identity) { + configDir, err := sm.getConfigDir() + require.NoError(t, err) + require.NoError(t, os.MkdirAll(configDir, 0700)) + // An owner entry that parses as JSON but names no principal: the + // profile records an owner, so it is not unowned, but nothing can match + // it. + const tampered = "abcd1111aaaa" + path := filepath.Join(configDir, tampered+".json") + require.NoError(t, os.WriteFile(path, []byte(`{"Name":"tampered","Owners":["garbage"]}`), 0600)) + + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + got, err := sm.ListProfiles(alice) + require.NoError(t, err) + assert.NotContains(t, profileIDs(got), tampered, + "a profile whose owners cannot be read must not fall back to unowned") + + root := ipcauth.KnownForTest(ipcauth.Identity{UID: 0}) + got, err = sm.ListProfiles(root) + require.NoError(t, err) + assert.Contains(t, profileIDs(got), tampered) + }) +} + +func TestListProfiles_UnidentifiedCallerGetsNothing(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { + _, err := sm.AddProfile("mine", &userID) + require.NoError(t, err) + _, err = sm.AddProfile("unowned", nil) + require.NoError(t, err) + + // The zero Identity carries uid 0, so an unidentified caller must be + // refused before privilege is ever considered. + got, err := sm.ListProfiles(ipcauth.Identity{}) + require.NoError(t, err) + assert.Empty(t, got) + }) +} diff --git a/client/server/server.go b/client/server/server.go index 1c7e5e2ac..8ea3d241f 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -2337,7 +2337,12 @@ func (s *Server) RenameProfile(ctx context.Context, msg *proto.RenameProfileRequ return nil, err } - err = s.profileManager.RenameProfile(resolved.ID, msg.Username, msg.NewProfileName) + userID, ok := ipcauth.CallerIdentity(ctx) + if !ok { + return nil, gstatus.Error(codes.Unauthenticated, "caller identity could not be resolved") + } + + err = s.profileManager.RenameProfile(resolved.ID, userID, msg.NewProfileName) if err != nil { log.Errorf("failed to rename profile: %v", err) return nil, fmt.Errorf("failed to rename profile: %w", err) @@ -2432,7 +2437,12 @@ func (s *Server) ListProfiles(ctx context.Context, msg *proto.ListProfilesReques return nil, gstatus.Errorf(codes.InvalidArgument, "username must be provided") } - profiles, err := s.profileManager.ListProfiles(msg.Username) + userID, ok := ipcauth.CallerIdentity(ctx) + if !ok { + return nil, gstatus.Error(codes.Unauthenticated, "caller identity could not be resolved") + } + + profiles, err := s.profileManager.ListProfiles(userID) if err != nil { log.Errorf("failed to list profiles: %v", err) return nil, fmt.Errorf("failed to list profiles: %w", err) @@ -2465,10 +2475,15 @@ func (s *Server) GetActiveProfile(ctx context.Context, msg *proto.GetActiveProfi return nil, fmt.Errorf("failed to get active profile state: %w", err) } + userID, ok := ipcauth.CallerIdentity(ctx) + if !ok { + return nil, gstatus.Error(codes.Unauthenticated, "caller identity could not be resolved") + } + // Fallback to legacy name == ID displayName := activeProfile.ID.String() if activeProfile.ID != profilemanager.DefaultProfileName { - if profiles, lerr := s.profileManager.ListProfiles(activeProfile.Username); lerr == nil { + if profiles, lerr := s.profileManager.ListProfiles(userID); lerr == nil { for _, p := range profiles { if p.ID == activeProfile.ID { displayName = p.Name