From 8253bfb99f21663c08cfa3eaaf6497e8199bf0c1 Mon Sep 17 00:00:00 2001 From: "Theodor S. Midtlien" Date: Thu, 10 Sep 2026 15:39:38 +0200 Subject: [PATCH] (WIP) Migrate active_profile --- client/cmd/up_daemon_test.go | 14 +- client/internal/profilemanager/service.go | 170 ++++++++-- .../internal/profilemanager/service_test.go | 24 +- client/mobile/profile_manager.go | 28 +- client/server/login_gate_test.go | 3 + client/server/login_overrides_test.go | 3 +- client/server/logout_gate_test.go | 68 ++-- client/server/server.go | 303 ++++++++++-------- client/server/server_jwt_test.go | 7 +- client/server/setconfig_mdm_test.go | 11 + 10 files changed, 423 insertions(+), 208 deletions(-) diff --git a/client/cmd/up_daemon_test.go b/client/cmd/up_daemon_test.go index f61f11000..fe8d27ed7 100644 --- a/client/cmd/up_daemon_test.go +++ b/client/cmd/up_daemon_test.go @@ -4,10 +4,12 @@ import ( "context" "os" "os/user" + "path/filepath" "testing" "time" "github.com/netbirdio/netbird/client/internal" + "github.com/netbirdio/netbird/client/internal/ipcauth" "github.com/netbirdio/netbird/client/internal/profilemanager" ) @@ -18,8 +20,11 @@ func TestUpDaemon(t *testing.T) { tempDir := t.TempDir() origDefaultProfileDir := profilemanager.DefaultConfigPathDir origActiveProfileStatePath := profilemanager.ActiveProfileStatePath + origDefaultConfigPath := profilemanager.DefaultConfigPath profilemanager.DefaultConfigPathDir = tempDir profilemanager.ActiveProfileStatePath = tempDir + "/active_profile.json" + // Without this the loader reads the real /var/lib/netbird/default.json. + profilemanager.DefaultConfigPath = filepath.Join(tempDir, "default.json") profilemanager.ConfigDirOverride = tempDir currUser, err := user.Current() @@ -28,8 +33,14 @@ func TestUpDaemon(t *testing.T) { return } + identity, err := ipcauth.CurrentProcessIdentity() + if err != nil { + t.Fatalf("failed to read this process's identity: %v", err) + return + } + sm := profilemanager.ServiceManager{} - created, err := sm.AddProfile("test1", currUser.Username, nil) + created, err := sm.AddProfile("test1", &identity) if err != nil { t.Fatalf("failed to add profile: %v", err) return @@ -47,6 +58,7 @@ func TestUpDaemon(t *testing.T) { t.Cleanup(func() { profilemanager.DefaultConfigPathDir = origDefaultProfileDir profilemanager.ActiveProfileStatePath = origActiveProfileStatePath + profilemanager.DefaultConfigPath = origDefaultConfigPath profilemanager.ConfigDirOverride = "" }) diff --git a/client/internal/profilemanager/service.go b/client/internal/profilemanager/service.go index 6088ef44b..9371967b7 100644 --- a/client/internal/profilemanager/service.go +++ b/client/internal/profilemanager/service.go @@ -100,10 +100,21 @@ type ActiveProfileState struct { // before the ID-based config files. Legacy values were profile names, which // were also the legacy filename stems, so they still resolve to the correct // file on disk. - ID ID `json:"name"` + ID ID `json:"name"` + + // Username records which per-username directory a pre-migration profile's + // file lives in. It is a hint for reconstructing that path, not a statement + // about who owns the profile: ownership lives in the profile's own JSON, as + // typed principals. Profiles in the shared directory leave it empty, and + // the field goes away once no per-username directory is left. Username string `json:"username"` } +// FilePath rebuilds the profile's path from the per-username layout. +// +// Prefer ServiceManager.ActiveProfilePath: this reconstruction only holds for a +// profile that predates the ID-keyed layout, since a profile created after it +// lives in the shared directory instead, under no username at all. func (a *ActiveProfileState) FilePath() (string, error) { if a.ID == "" { return "", fmt.Errorf("active profile ID is empty") @@ -129,6 +140,68 @@ type ServiceManager struct { profilesDir string // If set, overrides ConfigDirOverride for profile operations } +// ActiveProfilePath returns the config file of the profile the active-profile +// state points at. +// +// The path is looked up through the loader rather than rebuilt from the +// recorded username, because a profile's directory is no longer a function of +// who owns it: profiles created since the ID-keyed layout share one directory, +// and only pre-migration ones sit under a per-username one. The username +// survives as a tiebreaker for the single case that still needs one, a legacy +// ID being a display name that two users can each hold. +// +// A state that points at a profile with no file yet still yields the path that +// file would have, so a caller reads "not created yet" from a stat rather than +// from an error. +func (s *ServiceManager) ActiveProfilePath(a *ActiveProfileState) (string, error) { + if a == nil || a.ID == "" { + return "", fmt.Errorf("active profile ID is empty") + } + if a.ID == defaultProfileName { + return DefaultConfigPath, nil + } + if !IsValidProfileFilenameStem(a.ID) { + return "", fmt.Errorf("invalid profile ID: %q", a.ID) + } + + profiles, err := s.loadAllProfiles() + if err != nil { + return "", fmt.Errorf("load profiles: %w", err) + } + + var matches []Profile + for _, p := range profiles { + if p.ID == a.ID { + matches = append(matches, p) + } + } + + switch len(matches) { + 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: + return matches[0].Path, nil + } + + // Two directories hold the same legacy ID, so the recorded hint says which + // one the daemon activated. State written before the hint was a directory + // recorded the raw account name, which the legacy layout sanitized on its + // way to becoming a directory, so try it both ways. + for _, want := range []string{a.Username, sanitizeProfileName(a.Username)} { + for _, p := range matches { + if filepath.Base(filepath.Dir(p.Path)) == want { + return p.Path, nil + } + } + } + + log.Warnf("active profile %q exists in %d directories and none of them is %q, using %s", + a.ID, len(matches), a.Username, matches[0].Path) + return matches[0].Path, nil +} + func NewServiceManager(defaultConfigPath string) *ServiceManager { if defaultConfigPath != "" { DefaultConfigPath = defaultConfigPath @@ -470,13 +543,13 @@ func (s *ServiceManager) GetStatePath() string { return defaultStatePath } - configDir, err := s.getConfigDirLegacy(activeProf.Username) + configPath, err := s.ActiveProfilePath(activeProf) if err != nil { - log.Warnf("failed to get config directory for user %s: %v", activeProf.Username, err) + log.Warnf("failed to resolve the active profile's path: %v", err) return defaultStatePath } - return filepath.Join(configDir, activeProf.ID.String()+".state.json") + return filepath.Join(filepath.Dir(configPath), activeProf.ID.String()+".state.json") } // getConfigDirLegacy returns the profiles directory, using profilesDir if set, otherwise getConfigDirForUser @@ -489,15 +562,7 @@ func (s *ServiceManager) getConfigDirLegacy(username string) (string, error) { } func (s *ServiceManager) getConfigDir() (string, error) { - if s.profilesDir != "" { - return s.profilesDir, nil - } - - if ConfigDirOverride != "" { - return ConfigDirOverride, nil - } - - configDir := filepath.Join(DefaultConfigPathDir, DefaultProfilePathDir) + configDir := s.profilesDirPath() if _, err := os.Stat(configDir); os.IsNotExist(err) { if err := os.MkdirAll(configDir, 0700); err != nil { return "", err @@ -507,6 +572,20 @@ func (s *ServiceManager) getConfigDir() (string, error) { return configDir, nil } +// profilesDirPath returns the directory new profiles are written to without +// creating it, so a read path can name it without leaving a directory behind. +func (s *ServiceManager) profilesDirPath() string { + if s.profilesDir != "" { + return s.profilesDir + } + + if ConfigDirOverride != "" { + return ConfigDirOverride + } + + return filepath.Join(DefaultConfigPathDir, DefaultProfilePathDir) +} + // loadAllProfiles returns every profile visible to the daemon for the // given user, including the default profile. The returned slice is sorted // by ID for a stable display order. @@ -538,36 +617,59 @@ func (s *ServiceManager) loadAllProfiles() ([]Profile, error) { } // The default profile is not seeded with an owner: it starts unowned, and - // the first claim stamps it like any other profile. + // the first claim stamps it like any other profile. A file that is not + // there yet is unowned rather than unreadable, since the daemon writes it + // on first run and every listing before that would otherwise fail. + var profiles []Profile defaultOwners, err := readProfileOwners(DefaultConfigPath) - if err != nil { - return nil, err + switch { + case err == nil, errors.Is(err, os.ErrNotExist): + profiles = append(profiles, Profile{ + ID: defaultProfileName, + Name: defaultName, + Path: DefaultConfigPath, + IsActive: activeIsDefault, + Owners: defaultOwners, + }) + default: + // Same rule as a discovered profile whose owners cannot be read: leave + // it out rather than treat it as unowned, and leave it out rather than + // fail, so one unreadable file does not take every other profile with + // it. + log.Warnf("leaving the default profile out of the listing, its owners could not be read: %v", err) } - profiles := []Profile{{ - ID: defaultProfileName, - Name: defaultName, - Path: DefaultConfigPath, - IsActive: activeIsDefault, - Owners: defaultOwners, - }} + + // The directory new profiles go to, plus every per-username directory left + // from before the ID-keyed layout. The first is not necessarily under + // DefaultConfigPathDir: a ServiceManager can be pointed at a directory of + // its own, which is what the mobile bindings do. + dirs := []string{s.profilesDirPath()} configPathDir, err := os.ReadDir(DefaultConfigPathDir) - if err != nil { - if errors.Is(err, os.ErrNotExist) { - return profiles, nil - } + if err != nil && !errors.Is(err, os.ErrNotExist) { return nil, fmt.Errorf("read profile directory: %w", err) } + for _, entry := range configPathDir { + if entry.IsDir() { + dirs = append(dirs, filepath.Join(DefaultConfigPathDir, entry.Name())) + } + } 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...) + scanned := make(map[string]bool, len(dirs)) + for _, dir := range dirs { + // The profiles directory is usually one of the subdirectories above, + // so without this a profile would be listed twice. + if scanned[dir] { + continue } + scanned[dir] = true + + dirProfiles, err := s.getProfilesFromDirectory(dir) + if err != nil { + return nil, err + } + fileProfiles = append(fileProfiles, dirProfiles...) } sort.Slice(fileProfiles, func(i, j int) bool { diff --git a/client/internal/profilemanager/service_test.go b/client/internal/profilemanager/service_test.go index c7cc94949..4d6b5f28d 100644 --- a/client/internal/profilemanager/service_test.go +++ b/client/internal/profilemanager/service_test.go @@ -289,20 +289,25 @@ func TestListProfiles_PrivilegedResolvesUnfiltered(t *testing.T) { }) } -func TestListProfiles_UnownedStaysOpenUntilClaimed(t *testing.T) { - withTestSM(t, func(sm *ServiceManager, userID ipcauth.Identity) { +func TestListProfiles_UnownedIsPrivilegedOnly(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, _ ipcauth.Identity) { unowned, err := sm.AddProfile("unowned", nil) require.NoError(t, err) - other := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) - got, err := sm.ListProfiles(other) + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + got, err := sm.ListProfiles(alice) require.NoError(t, err) - assert.Contains(t, profileIDs(got), unowned.ID.String(), - "an unowned profile is addressable until it is claimed") + assert.NotContains(t, profileIDs(got), unowned.ID.String(), + "an unowned profile is not addressable until it is claimed") + + root := ipcauth.KnownForTest(ipcauth.Identity{UID: 0}) + got, err = sm.ListProfiles(root) + require.NoError(t, err) + assert.Contains(t, profileIDs(got), unowned.ID.String()) }) } -func TestListProfiles_UnreadableOwnersArePrivilegedOnly(t *testing.T) { +func TestListProfiles_UnreadableOwnersAreSkipped(t *testing.T) { withTestSM(t, func(sm *ServiceManager, _ ipcauth.Identity) { configDir, err := sm.getConfigDir() require.NoError(t, err) @@ -320,10 +325,13 @@ func TestListProfiles_UnreadableOwnersArePrivilegedOnly(t *testing.T) { assert.NotContains(t, profileIDs(got), tampered, "a profile whose owners cannot be read must not fall back to unowned") + // Not even a privileged caller: the loader drops the profile before + // ownership is ever consulted, so corrupting the owner list hides the + // profile rather than unlocking it. root := ipcauth.KnownForTest(ipcauth.Identity{UID: 0}) got, err = sm.ListProfiles(root) require.NoError(t, err) - assert.Contains(t, profileIDs(got), tampered) + assert.NotContains(t, profileIDs(got), tampered) }) } diff --git a/client/mobile/profile_manager.go b/client/mobile/profile_manager.go index 66a8ba2dc..8cb721c8d 100644 --- a/client/mobile/profile_manager.go +++ b/client/mobile/profile_manager.go @@ -11,6 +11,7 @@ import ( log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/client/internal/ipcauth" "github.com/netbirdio/netbird/client/internal/profilemanager" "github.com/netbirdio/netbird/client/mdm" ) @@ -57,8 +58,12 @@ type Profile struct { // All profile identity is ID-based; the human-readable name lives inside the // profile config's Name field. type ProfileManager struct { - configDir string - username string + configDir string + username string + // identity scopes profile ownership. There is no IPC hop on mobile: the + // manager runs inside the app, so the owner of a profile is the app process + // itself, and the device has a single user anyway. + identity ipcauth.Identity serviceMgr *profilemanager.ServiceManager mdmLoader *mdm.Loader } @@ -82,9 +87,18 @@ func NewProfileManager(configDir, username string) *ProfileManager { profilesDir := filepath.Join(configDir, profilesSubdir) serviceMgr := profilemanager.NewServiceManagerWithProfilesDir(defaultConfigPath, profilesDir) + // A failed read leaves the zero Identity, which is not Known and therefore + // owns nothing: profile access fails closed rather than falling back to + // something permissive. + identity, err := ipcauth.CurrentProcessIdentity() + if err != nil { + log.Errorf("failed to read this process's identity, profiles will be inaccessible: %v", err) + } + return &ProfileManager{ configDir: configDir, username: username, + identity: identity, serviceMgr: serviceMgr, } } @@ -92,7 +106,7 @@ func NewProfileManager(configDir, username string) *ProfileManager { // ListProfiles returns all available profiles, including the default profile, // with their active status set. func (pm *ProfileManager) ListProfiles() ([]Profile, error) { - internalProfiles, err := pm.serviceMgr.ListProfiles(pm.username) + internalProfiles, err := pm.serviceMgr.ListProfiles(pm.identity) if err != nil { return nil, fmt.Errorf("list profiles: %w", err) } @@ -118,7 +132,7 @@ func (pm *ProfileManager) GetActiveProfile() (*Profile, error) { return nil, fmt.Errorf("get active profile: %w", err) } - prof, err := pm.serviceMgr.ResolveProfile(activeState.ID.String(), pm.username) + prof, err := pm.serviceMgr.ResolveProfile(activeState.ID.String(), pm.identity) if err != nil { return nil, fmt.Errorf("resolve active profile %q: %w", activeState.ID, err) } @@ -153,7 +167,7 @@ func (pm *ProfileManager) AddProfile(displayName string) (*Profile, error) { if err := pm.checkProfilesAllowed(); err != nil { return nil, err } - profile, err := pm.serviceMgr.AddProfile(displayName, pm.username, nil) + profile, err := pm.serviceMgr.AddProfile(displayName, &pm.identity) if err != nil { return nil, fmt.Errorf("add profile: %w", err) } @@ -168,7 +182,7 @@ func (pm *ProfileManager) RenameProfile(id string, newName string) error { if err := pm.checkProfilesAllowed(); err != nil { return err } - if err := pm.serviceMgr.RenameProfile(profilemanager.ID(id), pm.username, newName); err != nil { + if err := pm.serviceMgr.RenameProfile(profilemanager.ID(id), pm.identity, newName); err != nil { return fmt.Errorf("rename profile: %w", err) } @@ -222,7 +236,7 @@ func (pm *ProfileManager) RemoveProfile(id string) error { return err } - if err := pm.serviceMgr.RemoveProfile(profilemanager.ID(id), pm.username); err != nil { + if err := pm.serviceMgr.RemoveProfile(profilemanager.ID(id), pm.identity); err != nil { return fmt.Errorf("remove profile: %w", err) } diff --git a/client/server/login_gate_test.go b/client/server/login_gate_test.go index de62a8180..cff894ac7 100644 --- a/client/server/login_gate_test.go +++ b/client/server/login_gate_test.go @@ -31,6 +31,7 @@ func TestLogin_RefusedChangeLeavesTheProfileAlone(t *testing.T) { ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, target+".json"), ManagementURL: "https://api.netbird.io:443", ServerSSHAllowed: boolPtr(true), + Owner: testProfileOwner(), }) require.NoError(t, err) @@ -63,6 +64,7 @@ func TestLogin_ChangeThatBecomesPrivilegedMidRequestHasNoSideEffects(t *testing. ConfigPath: targetPath, ManagementURL: "https://api.netbird.io:443", ServerSSHAllowed: boolPtr(false), + Owner: testProfileOwner(), }) require.NoError(t, err) @@ -110,6 +112,7 @@ func TestLogin_RefusedChangeLeavesAnInProgressLoginAlone(t *testing.T) { ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, target+".json"), ManagementURL: "https://api.netbird.io:443", ServerSSHAllowed: boolPtr(true), + Owner: testProfileOwner(), }) require.NoError(t, err) diff --git a/client/server/login_overrides_test.go b/client/server/login_overrides_test.go index 5a2298764..59a15f5e4 100644 --- a/client/server/login_overrides_test.go +++ b/client/server/login_overrides_test.go @@ -80,7 +80,8 @@ func TestPersistLoginOverrides(t *testing.T) { require.NoError(t, err, "seed config") activeProf := &profilemanager.ActiveProfileState{ID: "default"} - err = persistLoginOverrides(activeProf, tt.newMgmtURL, tt.newPSK) + srv := &Server{profileManager: profilemanager.NewServiceManager("")} + err = srv.persistLoginOverrides(activeProf, tt.newMgmtURL, tt.newPSK) require.NoError(t, err, "persistLoginOverrides") cfg, err := profilemanager.ReadConfig(profilemanager.DefaultConfigPath) diff --git a/client/server/logout_gate_test.go b/client/server/logout_gate_test.go index 2d84d1b6a..4c9da943e 100644 --- a/client/server/logout_gate_test.go +++ b/client/server/logout_gate_test.go @@ -2,6 +2,7 @@ package server import ( "context" + "os" "path/filepath" "testing" "time" @@ -69,6 +70,7 @@ func TestLogout_OtherProfileStaysGatedWhenProfilesDisabled(t *testing.T) { _, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{ ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, other+".json"), ManagementURL: unreachableManagementURL, + Owner: testProfileOwner(), }) require.NoError(t, err) @@ -86,28 +88,18 @@ func TestLogout_OtherProfileStaysGatedWhenProfilesDisabled(t *testing.T) { // A legacy profile ID is a display name, so two users can hold the same ID in // their own profile directories. Matching on the ID alone would let one user's -// logout pass the gate against the other user's active profile, so the username -// is part of the comparison. +// logout pass the gate against the other user's active profile, so the config +// file, not the ID, decides which profile is the active one. func TestLogout_ForeignUserProfileStaysGatedWhenProfilesDisabled(t *testing.T) { s, _, _, username, _ := setupServerWithProfile(t) s.rootCtx = internal.CtxInitState(context.Background()) - // A legacy-style profile whose ID is its filename stem, and an active state - // claiming that same ID for a different user. shared := "shared-legacy-name" - _, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{ - ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, shared+".json"), - ManagementURL: unreachableManagementURL, - }) - require.NoError(t, err) - require.NoError(t, s.profileManager.SetActiveProfileState(&profilemanager.ActiveProfileState{ - ID: profilemanager.ID(shared), - Username: "someone-else", - })) + plantNamesakeProfiles(t, s, shared) s.profilesDisabled = true - _, err = s.Logout(userCtx(), &proto.LogoutRequest{ + _, err := s.Logout(userCtx(), &proto.LogoutRequest{ ProfileName: &shared, Username: &username, }) @@ -117,6 +109,35 @@ func TestLogout_ForeignUserProfileStaysGatedWhenProfilesDisabled(t *testing.T) { "another user's profile must not pass the gate on an ID match alone: %v", err) } +// plantNamesakeProfiles creates two profiles that share one legacy ID: the +// caller's own, and another user's in that user's legacy profile directory, +// which is the one made active. Only the caller's copy carries an owner, so +// that is the one a handle resolves to, while the active profile stays the +// other file. +func plantNamesakeProfiles(t *testing.T, s *Server, id string) { + t.Helper() + + foreignDir := filepath.Join(profilemanager.DefaultConfigPathDir, "someone-else") + require.NoError(t, os.MkdirAll(foreignDir, 0700)) + _, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{ + ConfigPath: filepath.Join(foreignDir, id+".json"), + ManagementURL: unreachableManagementURL, + }) + require.NoError(t, err) + + _, err = profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{ + ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, id+".json"), + ManagementURL: unreachableManagementURL, + Owner: testProfileOwner(), + }) + require.NoError(t, err) + + require.NoError(t, s.profileManager.SetActiveProfileState(&profilemanager.ActiveProfileState{ + ID: profilemanager.ID(id), + Username: "someone-else", + })) +} + // Deregistering a namesake profile must not go out with the running config. // logoutFromProfile reuses the connected client's config when the target is the // active profile, and on an ID-only match a shared legacy ID made it reuse it @@ -135,15 +156,7 @@ func TestLogout_ForeignUserProfileDoesNotUseTheRunningConfig(t *testing.T) { s.connectClient = newDummyConnectClient(context.Background()) shared := "shared-legacy-name" - _, err = profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{ - ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, shared+".json"), - ManagementURL: unreachableManagementURL, - }) - require.NoError(t, err) - require.NoError(t, s.profileManager.SetActiveProfileState(&profilemanager.ActiveProfileState{ - ID: profilemanager.ID(shared), - Username: "someone-else", - })) + plantNamesakeProfiles(t, s, shared) // Bounded so the deregistration the fixed path attempts fails on the dial // rather than sitting in gRPC backoff for the whole test timeout. @@ -165,18 +178,21 @@ func TestLogout_ForeignUserProfileDoesNotUseTheRunningConfig(t *testing.T) { // guardedConfigMu, which the logout path does not hold, so a login that landed // meanwhile must keep its connection. func TestCleanupAfterProfileLogout_FollowsTheCurrentActiveProfile(t *testing.T) { - s, _, activeProfile, username, _ := setupServerWithProfile(t) + s, _, activeProfile, _, cfgPath := setupServerWithProfile(t) s.rootCtx = internal.CtxInitState(context.Background()) state := internal.CtxGetState(s.rootCtx) - s.cleanupAfterProfileLogout("some-other-profile", username) + s.cleanupAfterProfileLogout(&profilemanager.Profile{ID: "some-other-profile"}) status, err := state.Status() require.NoError(t, err) require.NotEqual(t, internal.StatusNeedsLogin, status, "logging out of a profile that is not active must not ask for a new login") - s.cleanupAfterProfileLogout(profilemanager.ID(activeProfile), username) + s.cleanupAfterProfileLogout(&profilemanager.Profile{ + ID: profilemanager.ID(activeProfile), + Path: cfgPath, + }) status, err = state.Status() require.NoError(t, err) require.Equal(t, internal.StatusNeedsLogin, status, diff --git a/client/server/server.go b/client/server/server.go index 8ea3d241f..df64ed2d9 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -7,7 +7,7 @@ import ( "fmt" "os" "os/exec" - "os/user" + "path/filepath" "runtime" "strconv" "sync" @@ -24,7 +24,6 @@ import ( "github.com/netbirdio/netbird/client/internal/auth" "github.com/netbirdio/netbird/client/internal/expose" - "github.com/netbirdio/netbird/client/internal/getent" "github.com/netbirdio/netbird/client/internal/ipcauth" "github.com/prometheus/client_golang/prometheus" @@ -515,7 +514,12 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques return nil, err } - stored, err := s.storedProfileConfig(msg.ProfileName, msg.Username) + callerID, err := callerIdentity(callerCtx) + if err != nil { + return nil, err + } + + stored, err := s.storedProfileConfig(msg.ProfileName, callerID) if err != nil { return nil, err } @@ -523,7 +527,7 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques return nil, err } - config, err := s.setConfigInputFromRequest(msg) + config, err := s.setConfigInputFromRequest(msg, callerID) if err != nil { return nil, err } @@ -535,7 +539,7 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques } if activeProf, err := s.profileManager.GetActiveProfileState(); err == nil { - if activePath, err := activeProf.FilePath(); err == nil && activePath == config.ConfigPath { + if activePath, err := s.profileManager.ActiveProfilePath(activeProf); err == nil && activePath == config.ConfigPath { s.localMetrics.Reconcile(updatedConf.LocalMetricsEnabled, updatedConf.LocalMetricsAddress) } } @@ -553,10 +557,10 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques // field is its own optional case. Returns the resolved ConfigInput // and a non-nil error only when the active profile file path cannot // be determined. -func (s *Server) setConfigInputFromRequest(msg *proto.SetConfigRequest) (profilemanager.ConfigInput, error) { +func (s *Server) setConfigInputFromRequest(msg *proto.SetConfigRequest, callerID ipcauth.Identity) (profilemanager.ConfigInput, error) { var config profilemanager.ConfigInput - resolved, err := s.resolveProfileHandle(msg.ProfileName, msg.Username) + resolved, err := s.resolveProfileHandle(msg.ProfileName, callerID) if err != nil { log.Errorf("failed to resolve profile %q: %v", msg.ProfileName, err) return config, err @@ -659,6 +663,11 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro } } + callerID, err := callerIdentity(callerCtx) + if err != nil { + return nil, err + } + activeProf, err := s.profileManager.GetActiveProfileState() if err != nil { log.Errorf("failed to get active profile state: %v", err) @@ -670,7 +679,7 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro // refused login neither switches the profile nor cancels a login already in // progress, and it reads the profile the request targets, which is the one the // switch below would activate. - stored, err := s.storedLoginConfig(activeProf, msg) + stored, err := s.storedLoginConfig(activeProf, msg, callerID) if err != nil { return nil, err } @@ -703,7 +712,7 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro return nil, err } - log.Infof("active profile: %s for %s", activeProf.ID, activeProf.Username) + log.Infof("active profile: %s", activeProf.ID) s.mutex.Lock() @@ -1064,6 +1073,12 @@ func (s *Server) Up(callerCtx context.Context, msg *proto.UpRequest) (*proto.UpR return nil, fmt.Errorf("config is not defined, please call login command first") } + callerID, err := callerIdentity(callerCtx) + if err != nil { + s.mutex.Unlock() + return nil, err + } + activeProf, err := s.profileManager.GetActiveProfileState() if err != nil { s.mutex.Unlock() @@ -1072,7 +1087,7 @@ func (s *Server) Up(callerCtx context.Context, msg *proto.UpRequest) (*proto.UpR } if msg != nil && msg.ProfileName != nil { - if _, err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil { + if _, err := s.switchProfileIfNeeded(*msg.ProfileName, callerID, activeProf); err != nil { s.mutex.Unlock() log.Errorf("failed to switch profile: %v", err) return nil, err @@ -1086,7 +1101,7 @@ func (s *Server) Up(callerCtx context.Context, msg *proto.UpRequest) (*proto.UpR return nil, fmt.Errorf("failed to get active profile state: %w", err) } - log.Infof("active profile: %s for %s", activeProf.ID, activeProf.Username) + log.Infof("active profile: %s", activeProf.ID) config, _, err := s.getConfig(activeProf) if err != nil { @@ -1138,8 +1153,8 @@ func (s *Server) waitForUp(callerCtx context.Context) (*proto.UpResponse, error) // targets, so a privileged-change decision can be made against the values the // profile currently holds. A profile that has no config file yet yields nil, // which every caller must read as "nothing enabled yet". -func (s *Server) storedProfileConfig(handle, username string) (*profilemanager.Config, error) { - resolved, err := s.resolveProfileHandle(handle, username) +func (s *Server) storedProfileConfig(handle string, callerID ipcauth.Identity) (*profilemanager.Config, error) { + resolved, err := s.resolveProfileHandle(handle, callerID) if err != nil { return nil, err } @@ -1155,23 +1170,18 @@ func (s *Server) storedProfileConfig(handle, username string) (*profilemanager.C // storedLoginConfig loads the on-disk config of the profile a login request // targets: the one it names, or the active one when it names none. Used to decide // a privileged change before the request is allowed to switch profiles. -func (s *Server) storedLoginConfig(activeProf *profilemanager.ActiveProfileState, msg *proto.LoginRequest) (*profilemanager.Config, error) { +func (s *Server) storedLoginConfig(activeProf *profilemanager.ActiveProfileState, msg *proto.LoginRequest, callerID ipcauth.Identity) (*profilemanager.Config, error) { if msg.ProfileName == nil { - cfgPath, err := activeProf.FilePath() + cfgPath, err := s.profileManager.ActiveProfilePath(activeProf) if err != nil { return nil, fmt.Errorf("active profile file path: %w", err) } return s.storedConfigAtPath(cfgPath) } - // Mirrors switchProfileIfNeeded: the default profile resolves without a - // username, so this reads the same profile the switch would activate. - handle := *msg.ProfileName - username := "" - if handle != profilemanager.DefaultProfileName { - username = msg.GetUsername() - } - return s.storedProfileConfig(handle, username) + // Mirrors switchProfileIfNeeded, so this reads the very profile the switch + // would activate. + return s.storedProfileConfig(*msg.ProfileName, callerID) } // storedConfigAtPath reads a profile config file, yielding nil when it does not @@ -1191,11 +1201,26 @@ func (s *Server) storedConfigAtPath(path string) (*profilemanager.Config, error) return cfg, nil } +// callerIdentity returns the kernel-authenticated identity of the RPC caller. +// +// Every profile-addressing RPC scopes itself with this rather than with the +// username its request carries: that field is whatever the client chose to +// send, so scoping by it lets any local caller address another user's profile. +// The username fields on the wire are kept for compatibility and ignored. +func callerIdentity(ctx context.Context) (ipcauth.Identity, error) { + id, ok := ipcauth.CallerIdentity(ctx) + if !ok { + return ipcauth.Identity{}, gstatus.Error(codes.Unauthenticated, "caller identity could not be verified on the daemon control channel") + } + return id, nil +} + // resolveProfileHandle resolves a wire-level profile handle (display -// name, ID, or unique ID prefix) to a concrete profile. Returns gRPC -// status errors so handlers can return them directly. -func (s *Server) resolveProfileHandle(handle, username string) (*profilemanager.Profile, error) { - p, err := s.profileManager.ResolveProfile(handle, username) +// name, ID, or unique ID prefix) to a concrete profile owned by, or open to, +// the calling identity. Returns gRPC status errors so handlers can return them +// directly. +func (s *Server) resolveProfileHandle(handle string, callerID ipcauth.Identity) (*profilemanager.Profile, error) { + p, err := s.profileManager.ResolveProfile(handle, callerID) if err == nil { return p, nil } @@ -1212,36 +1237,28 @@ func (s *Server) resolveProfileHandle(handle, username string) (*profilemanager. // switchProfileIfNeeded resolves the user-supplied handle, updates the // active profile state if it differs from the current one, and returns // the resolved profile so callers can include its ID in RPC responses. -func (s *Server) switchProfileIfNeeded(handle string, userName *string, activeProf *profilemanager.ActiveProfileState) (*profilemanager.Profile, error) { - if handle != profilemanager.DefaultProfileName && (userName == nil || *userName == "") { - log.Errorf("profile name is set to %s, but username is not provided", handle) - return nil, fmt.Errorf("profile name is set to %s, but username is not provided", handle) - } - - var username string - if handle != profilemanager.DefaultProfileName { - username = *userName - } - - resolved, err := s.resolveProfileHandle(handle, username) +func (s *Server) switchProfileIfNeeded(handle string, callerID ipcauth.Identity, activeProf *profilemanager.ActiveProfileState) (*profilemanager.Profile, error) { + resolved, err := s.resolveProfileHandle(handle, callerID) if err != nil { return nil, err } - if resolved.ID != activeProf.ID || username != activeProf.Username { - if s.checkProfilesDisabled() { - log.Errorf("profiles are disabled, you cannot use this feature without profiles enabled") - return nil, gstatus.Errorf(codes.Unavailable, errProfilesDisabled) - } + if s.isActiveProfile(activeProf, resolved) { + return resolved, nil + } - log.Infof("switching to profile %s (%s) for user %s", resolved.Name, resolved.ID, username) - if err := s.profileManager.SetActiveProfileState(&profilemanager.ActiveProfileState{ - ID: resolved.ID, - Username: username, - }); err != nil { - log.Errorf("failed to set active profile state: %v", err) - return nil, fmt.Errorf("failed to set active profile state: %w", err) - } + if s.checkProfilesDisabled() { + log.Errorf("profiles are disabled, you cannot use this feature without profiles enabled") + return nil, gstatus.Errorf(codes.Unavailable, errProfilesDisabled) + } + + log.Infof("switching to profile %s (%s) for %s", resolved.Name, resolved.ID, callerID) + if err := s.profileManager.SetActiveProfileState(&profilemanager.ActiveProfileState{ + ID: resolved.ID, + Username: legacyDirHint(resolved), + }); err != nil { + log.Errorf("failed to set active profile state: %v", err) + return nil, fmt.Errorf("failed to set active profile state: %w", err) } return resolved, nil @@ -1252,6 +1269,11 @@ func (s *Server) SwitchProfile(callerCtx context.Context, msg *proto.SwitchProfi s.mutex.Lock() defer s.mutex.Unlock() + callerID, err := callerIdentity(callerCtx) + if err != nil { + return nil, err + } + activeProf, err := s.profileManager.GetActiveProfileState() if err != nil { log.Errorf("failed to get active profile state: %v", err) @@ -1259,7 +1281,7 @@ func (s *Server) SwitchProfile(callerCtx context.Context, msg *proto.SwitchProfi } if msg != nil && msg.ProfileName != nil { - if _, err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil { + if _, err := s.switchProfileIfNeeded(*msg.ProfileName, callerID, activeProf); err != nil { log.Errorf("failed to switch profile: %v", err) return nil, err } @@ -1395,12 +1417,12 @@ func (s *Server) Logout(ctx context.Context, msg *proto.LogoutRequest) (*proto.L } func (s *Server) handleProfileLogout(ctx context.Context, msg *proto.LogoutRequest) (*proto.LogoutResponse, error) { - if msg.Username == nil || *msg.Username == "" { - return nil, gstatus.Errorf(codes.InvalidArgument, "username must be provided when profile name is specified") + callerID, err := callerIdentity(ctx) + if err != nil { + return nil, err } - username := *msg.Username - resolved, err := s.resolveProfileHandle(*msg.ProfileName, username) + resolved, err := s.resolveProfileHandle(*msg.ProfileName, callerID) if err != nil { return nil, err } @@ -1410,11 +1432,11 @@ func (s *Server) handleProfileLogout(ctx context.Context, msg *proto.LogoutReque return nil, gstatus.Errorf(codes.FailedPrecondition, "failed to get active profile state: %v", err) } - if err := s.validateProfileLogout(resolved.ID, isActiveProfile(activeProf, resolved.ID, username)); err != nil { + if err := s.validateProfileLogout(resolved.ID, s.isActiveProfile(activeProf, resolved)); err != nil { return nil, err } - if err := s.logoutFromProfile(ctx, resolved, username); err != nil { + if err := s.logoutFromProfile(ctx, resolved); err != nil { log.Errorf("failed to logout from profile %s: %v", resolved.ID, err) // A refused deregistration is already a status error carrying the reason // and the command to run; rewrapping it as Internal would flatten both @@ -1425,7 +1447,7 @@ func (s *Server) handleProfileLogout(ctx context.Context, msg *proto.LogoutReque return nil, gstatus.Errorf(codes.Internal, "logout: %v", err) } - s.cleanupAfterProfileLogout(resolved.ID, username) + s.cleanupAfterProfileLogout(resolved) return &proto.LogoutResponse{}, nil } @@ -1436,14 +1458,14 @@ func (s *Server) handleProfileLogout(ctx context.Context, msg *proto.LogoutReque // check: Login switches profiles under guardedConfigMu, which this path does not // hold, so a login that landed meanwhile must not have its fresh connection // dropped by a logout that targeted the profile it replaced. -func (s *Server) cleanupAfterProfileLogout(id profilemanager.ID, username string) { +func (s *Server) cleanupAfterProfileLogout(profile *profilemanager.Profile) { activeProf, err := s.profileManager.GetActiveProfileState() if err != nil { - log.Errorf("failed to get active profile state after logout from profile %s: %v", id, err) + log.Errorf("failed to get active profile state after logout from profile %s: %v", profile.ID, err) return } - if !isActiveProfile(activeProf, id, username) { + if !s.isActiveProfile(activeProf, profile) { return } @@ -1489,7 +1511,7 @@ func (s *Server) handleActiveProfileLogout(ctx context.Context) (*proto.LogoutRe // getConfig reads config file and returns Config and whether the config file already existed. Errors out if it does not exist func (s *Server) getConfig(activeProf *profilemanager.ActiveProfileState) (*profilemanager.Config, bool, error) { - cfgPath, err := activeProf.FilePath() + cfgPath, err := s.profileManager.ActiveProfilePath(activeProf) if err != nil { return nil, false, fmt.Errorf("failed to get active profile file path: %w", err) } @@ -1533,27 +1555,51 @@ func (s *Server) validateProfileLogout(id profilemanager.ID, isActive bool) erro return nil } -// isActiveProfile reports whether id is the profile the daemon runs for -// username. The username is part of the comparison because legacy profile IDs -// are display names, which two users can both hold; the default profile is -// shared by every user and carries no username. -func isActiveProfile(activeProf *profilemanager.ActiveProfileState, id profilemanager.ID, username string) bool { - if activeProf == nil || activeProf.ID != id { +// isActiveProfile reports whether profile is the one the daemon runs. +// +// The comparison ends on the config file rather than on the ID, because a +// legacy profile ID is a display name that two users can each hold: the path is +// what tells alice's `work` from bob's. It is not the caller's username, which +// says nothing about which profile the daemon activated. +func (s *Server) isActiveProfile(activeProf *profilemanager.ActiveProfileState, profile *profilemanager.Profile) bool { + if activeProf == nil || profile == nil || activeProf.ID != profile.ID { return false } + if profile.ID == profilemanager.DefaultProfileName { + return true + } - return id == profilemanager.DefaultProfileName || activeProf.Username == username + activePath, err := s.profileManager.ActiveProfilePath(activeProf) + if err != nil { + log.Warnf("cannot resolve the active profile's path, treating %s as not active: %v", profile.ID, err) + return false + } + return activePath == profile.Path +} + +// legacyDirHint records which per-username directory a pre-migration profile's +// file sits in, which is all the active-profile state still reads its username +// for. A profile in the shared directory needs no hint: its ID is unique. +func legacyDirHint(profile *profilemanager.Profile) string { + if profile.Path == "" || profile.ID == profilemanager.DefaultProfileName { + return "" + } + dir := filepath.Base(filepath.Dir(profile.Path)) + if dir == profilemanager.DefaultProfilePathDir { + return "" + } + return dir } // logoutFromProfile deregisters profile, reusing the running config when -// profile is the one the daemon is connected with. The username takes part in -// that decision for the same reason it does in the logout gate: a legacy -// profile ID is a display name two users can share, and sending the running -// config for a namesake would deregister the active peer instead of the +// profile is the one the daemon is connected with. That decision is made on the +// profile's file rather than on its ID, for the same reason the logout gate is: +// a legacy profile ID is a display name two users can share, and sending the +// running config for a namesake would deregister the active peer instead of the // requested one. -func (s *Server) logoutFromProfile(ctx context.Context, profile *profilemanager.Profile, username string) error { +func (s *Server) logoutFromProfile(ctx context.Context, profile *profilemanager.Profile) error { activeProf, err := s.profileManager.GetActiveProfileState() - if err == nil && isActiveProfile(activeProf, profile.ID, username) && s.connectClient != nil { + if err == nil && s.isActiveProfile(activeProf, profile) && s.connectClient != nil { return s.sendLogoutRequest(ctx) } @@ -2188,7 +2234,12 @@ func (s *Server) GetConfig(ctx context.Context, req *proto.GetConfigRequest) (*p return nil, ctx.Err() } - resolved, err := s.resolveProfileHandle(req.ProfileName, req.Username) + callerID, err := callerIdentity(ctx) + if err != nil { + return nil, err + } + + resolved, err := s.resolveProfileHandle(req.ProfileName, callerID) if err != nil { log.Errorf("failed to resolve profile %q: %v", req.ProfileName, err) return nil, err @@ -2301,15 +2352,16 @@ func (s *Server) AddProfile(ctx context.Context, msg *proto.AddProfileRequest) ( return nil, gstatus.Errorf(codes.Unavailable, errProfilesDisabled) } - if msg.ProfileName == "" || msg.Username == "" { - return nil, gstatus.Errorf(codes.InvalidArgument, "profile name and username must be provided") + if msg.ProfileName == "" { + return nil, gstatus.Errorf(codes.InvalidArgument, "profile name must be provided") } - callerId, ok := ipcauth.CallerIdentity(ctx) - if !ok { - return nil, fmt.Errorf("failed to get identity from context") + callerID, err := callerIdentity(ctx) + if err != nil { + return nil, err } - created, err := s.profileManager.AddProfile(msg.ProfileName, msg.Username, &callerId) + + created, err := s.profileManager.AddProfile(msg.ProfileName, &callerID) if err != nil { log.Errorf("failed to create profile: %v", err) return nil, fmt.Errorf("failed to create profile: %w", err) @@ -2328,21 +2380,21 @@ func (s *Server) RenameProfile(ctx context.Context, msg *proto.RenameProfileRequ return nil, gstatus.Errorf(codes.Unavailable, errProfilesDisabled) } - if msg.Handle == "" || msg.Username == "" || msg.NewProfileName == "" { - return nil, gstatus.Errorf(codes.InvalidArgument, "profile name, username and new profile name must be provided") + if msg.Handle == "" || msg.NewProfileName == "" { + return nil, gstatus.Errorf(codes.InvalidArgument, "profile name and new profile name must be provided") } - resolved, err := s.resolveProfileHandle(msg.Handle, msg.Username) + callerID, err := callerIdentity(ctx) if err != nil { return nil, err } - userID, ok := ipcauth.CallerIdentity(ctx) - if !ok { - return nil, gstatus.Error(codes.Unauthenticated, "caller identity could not be resolved") + resolved, err := s.resolveProfileHandle(msg.Handle, callerID) + if err != nil { + return nil, err } - err = s.profileManager.RenameProfile(resolved.ID, userID, msg.NewProfileName) + err = s.profileManager.RenameProfile(resolved.ID, callerID, msg.NewProfileName) if err != nil { log.Errorf("failed to rename profile: %v", err) return nil, fmt.Errorf("failed to rename profile: %w", err) @@ -2366,19 +2418,24 @@ func (s *Server) RemoveProfile(ctx context.Context, msg *proto.RemoveProfileRequ return nil, gstatus.Errorf(codes.InvalidArgument, "profile name must be provided") } - resolved, err := s.resolveProfileHandle(msg.ProfileName, msg.Username) + callerID, err := callerIdentity(ctx) if err != nil { return nil, err } - if err := s.logoutFromProfile(ctx, resolved, msg.Username); err != nil { + resolved, err := s.resolveProfileHandle(msg.ProfileName, callerID) + if err != nil { + return nil, err + } + + if err := s.logoutFromProfile(ctx, resolved); err != nil { // Deregistration is best-effort here: the local profile is removed // either way, so an unprivileged caller leaves the peer registered on // the management server rather than being blocked from removing it. log.Warnf("removing profile %s locally without deregistering it: %v", resolved.ID, err) } - if err := s.profileManager.RemoveProfile(resolved.ID, msg.Username); err != nil { + if err := s.profileManager.RemoveProfile(resolved.ID, callerID); err != nil { log.Errorf("failed to remove profile: %v", err) return nil, fmt.Errorf("failed to remove profile: %w", err) } @@ -2433,16 +2490,12 @@ func (s *Server) ListProfiles(ctx context.Context, msg *proto.ListProfilesReques s.mutex.Lock() defer s.mutex.Unlock() - if msg.Username == "" { - return nil, gstatus.Errorf(codes.InvalidArgument, "username must be provided") + callerID, err := callerIdentity(ctx) + if err != nil { + return nil, err } - 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) + profiles, err := s.profileManager.ListProfiles(callerID) if err != nil { log.Errorf("failed to list profiles: %v", err) return nil, fmt.Errorf("failed to list profiles: %w", err) @@ -2701,10 +2754,15 @@ func (s *Server) authorizeAndPrepareLogin(callerCtx context.Context, msg *proto. afterLoginPreCheck() } + callerID, err := callerIdentity(callerCtx) + if err != nil { + return nil, nil, err + } + s.guardedConfigMu.Lock() defer s.guardedConfigMu.Unlock() - stored, err := s.storedLoginConfig(activeProf, msg) + stored, err := s.storedLoginConfig(activeProf, msg, callerID) if err != nil { return nil, nil, err } @@ -2728,7 +2786,7 @@ func (s *Server) authorizeAndPrepareLogin(callerCtx context.Context, msg *proto. } if msg.ProfileName != nil { - if _, err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil { + if _, err := s.switchProfileIfNeeded(*msg.ProfileName, callerID, activeProf); err != nil { return nil, nil, fmt.Errorf("switch profile: %w", err) } } @@ -2738,7 +2796,7 @@ func (s *Server) authorizeAndPrepareLogin(callerCtx context.Context, msg *proto. return nil, nil, fmt.Errorf("active profile state: %w", err) } - if err := persistLoginOverrides(activeProf, msg.ManagementUrl, msg.OptionalPreSharedKey); err != nil { + if err := s.persistLoginOverrides(activeProf, msg.ManagementUrl, msg.OptionalPreSharedKey); err != nil { return nil, nil, fmt.Errorf("persist login overrides: %w", err) } @@ -2769,37 +2827,24 @@ func (s *Server) SessionHolder() (ipcauth.Principal, bool) { return principal, true } -// OwnsProfile check if the Identity mathces the owner in the profile resolved -// from the handle. +// OwnsProfile reports whether the profile the handle resolves to answers to +// this identity. // -// Note: username is resolved locally and through LDAP/AD so lookup might fail -// or be delayed. +// The identity is handed to the loader directly. There is deliberately no +// uid-to-username lookup on the way: the loader no longer keys profiles by +// directory, and putting NSS on the authorization path would make every +// decision wait on a resolver that can be slow, or absent, and would deny +// every caller whenever it times out. func (s *Server) OwnsProfile(id ipcauth.Identity, handle string) bool { - var username *user.User - var lookupErr error - if id.IsWindows() { - username, lookupErr = getent.LookupUserID(id.SID) - } else { - username, lookupErr = getent.LookupUserID(strconv.FormatUint(uint64(id.UID), 10)) - } - if lookupErr != nil { - log.Errorf("failed to lookup user by Identity %v: %v", id, lookupErr) - return false - } - resolved, err := s.resolveProfileHandle(handle, username.Name) + resolved, err := s.resolveProfileHandle(handle, id) if err != nil { log.Errorf("failed to resolve profile %q: %v", handle, err) return false } - if len(resolved.Owners) < 1 { - // TODO: define unowned behavior - return true - } - owner := resolved.Owners[0] - return owner.Matches(id) + return resolved.AccessibleBy(id) } -func persistLoginOverrides(activeProf *profilemanager.ActiveProfileState, managementURL string, preSharedKey *string) error { +func (s *Server) persistLoginOverrides(activeProf *profilemanager.ActiveProfileState, managementURL string, preSharedKey *string) error { if preSharedKey != nil && *preSharedKey == "" { preSharedKey = nil } @@ -2807,7 +2852,7 @@ func persistLoginOverrides(activeProf *profilemanager.ActiveProfileState, manage return nil } - cfgPath, err := activeProf.FilePath() + cfgPath, err := s.profileManager.ActiveProfilePath(activeProf) if err != nil { return fmt.Errorf("active profile file path: %w", err) } diff --git a/client/server/server_jwt_test.go b/client/server/server_jwt_test.go index 3fec5598a..1907ebf77 100644 --- a/client/server/server_jwt_test.go +++ b/client/server/server_jwt_test.go @@ -102,17 +102,20 @@ func TestSwitchProfile_ClearsJWTCache(t *testing.T) { // switchProfileIfNeeded rather than the no-op path a nil request takes. const target = "second" username := "tester" + owner := unprivilegedIdentity() _, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{ ConfigPath: filepath.Join(profilemanager.DefaultConfigPathDir, target+".json"), ManagementURL: "https://api.netbird.io:443", + Owner: &owner, }) require.NoError(t, err) - owner := unprivilegedIdentity() s.jwtCache.store("token", owner, testTTL, s.jwtCache.currentGeneration()) name := target - _, err = s.SwitchProfile(ctx, &proto.SwitchProfileRequest{ProfileName: &name, Username: &username}) + // The handler scopes the switch to the caller's identity, which a real + // caller gets from the daemon's transport credentials. + _, err = s.SwitchProfile(ctxWithIdentity(owner), &proto.SwitchProfileRequest{ProfileName: &name, Username: &username}) require.NoError(t, err) active, err := s.profileManager.GetActiveProfileState() diff --git a/client/server/setconfig_mdm_test.go b/client/server/setconfig_mdm_test.go index a392af6d3..e2e7240b5 100644 --- a/client/server/setconfig_mdm_test.go +++ b/client/server/setconfig_mdm_test.go @@ -11,6 +11,7 @@ import ( "google.golang.org/grpc/codes" gstatus "google.golang.org/grpc/status" + "github.com/netbirdio/netbird/client/internal/ipcauth" "github.com/netbirdio/netbird/client/internal/profilemanager" "github.com/netbirdio/netbird/client/mdm" "github.com/netbirdio/netbird/client/proto" @@ -83,6 +84,7 @@ func setupServerWithProfile(t *testing.T) (s *Server, ctx context.Context, profN _, err = profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{ ConfigPath: cfgPath, ManagementURL: "https://api.netbird.io:443", + Owner: testProfileOwner(), }) require.NoError(t, err) @@ -101,6 +103,15 @@ func setupServerWithProfile(t *testing.T) (s *Server, ctx context.Context, profN return s, ctx, profName, currUser.Username, cfgPath } +// testProfileOwner is the identity the unprivileged test contexts carry, so a +// fixture profile can be owned by the very caller that drives the handler. +// Without an owner the profile is unowned, which the loader hides from every +// unprivileged caller. +func testProfileOwner() *ipcauth.Identity { + id := unprivilegedIdentity() + return &id +} + // extractViolation pulls the MDMManagedFieldsViolation detail from a // FailedPrecondition error. Fails the test if absent or malformed. func extractViolation(t *testing.T, err error) *proto.MDMManagedFieldsViolation {