diff --git a/client/internal/profilemanager/service_test.go b/client/internal/profilemanager/service_test.go index dda9a5363..4eca3f714 100644 --- a/client/internal/profilemanager/service_test.go +++ b/client/internal/profilemanager/service_test.go @@ -291,7 +291,7 @@ func TestListProfiles_PrivilegedResolvesUnfiltered(t *testing.T) { }) } -func TestListProfiles_UnownedOutsideALegacyDirIsOpen(t *testing.T) { +func TestListProfiles_OnlyTheDefaultFailsOpenWhenUnowned(t *testing.T) { withTestSM(t, func(sm *ServiceManager, _ ipcauth.Identity) { unowned, err := sm.AddProfile("unowned", nil) require.NoError(t, err) @@ -299,10 +299,16 @@ func TestListProfiles_UnownedOutsideALegacyDirIsOpen(t *testing.T) { alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) got, err := sm.ListProfiles(alice) require.NoError(t, err) - assert.Contains(t, profileIDs(got), unowned.ID.String(), - "a profile that never had an owner stays usable until someone claims it") assert.Contains(t, profileIDs(got), defaultProfileName, "a fresh install has to be usable before anything is claimed") + assert.NotContains(t, profileIDs(got), unowned.ID.String(), + "every other profile needs an owner before anyone can address it") + + root := ipcauth.KnownForTest(ipcauth.Identity{UID: 0}) + got, err = sm.ListProfiles(root) + require.NoError(t, err) + assert.Contains(t, profileIDs(got), unowned.ID.String(), + "root still reaches it, which is how it gets assigned") nobody, err := sm.ListProfiles(ipcauth.Identity{}) require.NoError(t, err) @@ -507,6 +513,21 @@ func TestRenameProfile_NotTheCallersProfile(t *testing.T) { }) } +func TestResolveProfile_ClaimsOnTheWayThrough(t *testing.T) { + withLegacyLayout(t, func(sm *ServiceManager, configDir string) { + path := writeLegacyProfile(t, configDir, "alice", "work", nil) + stubLegacyDir(t, "alice") + + // Resolution is what switching a profile goes through, so the claim has + // to land here and not only when something lists profiles. + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + got, err := sm.ResolveProfile("work", alice) + require.NoError(t, err) + assert.Equal(t, path, got.Path) + assert.Equal(t, []string{"uid:4242"}, readOwners(t, path)) + }) +} + func TestListProfiles_PrivilegedCallerDoesNotClaim(t *testing.T) { withLegacyLayout(t, func(sm *ServiceManager, configDir string) { path := writeLegacyProfile(t, configDir, "root", "work", nil) diff --git a/client/server/server.go b/client/server/server.go index 295d3231a..d92399693 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -2837,17 +2837,14 @@ func (s *Server) SessionHolder() (ipcauth.Principal, bool) { // OwnsProfile reports whether the profile the handle resolves to answers to // this identity. // -// 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. +// This triggers stamping of legacy profiles, and reloads the current config +// if the handle is the active profile. func (s *Server) OwnsProfile(id ipcauth.Identity, handle string) bool { - if handle == "" { - act, err := s.profileManager.GetActiveProfileState() - if err != nil { - log.Warnf("failed to get active profile: %v", err) - } + act, err := s.profileManager.GetActiveProfileState() + if err != nil { + log.Warnf("failed to get active profile: %v", err) + } + if act != nil { handle = act.ID.String() } resolved, err := s.resolveProfileHandle(handle, id) @@ -2855,6 +2852,16 @@ func (s *Server) OwnsProfile(id ipcauth.Identity, handle string) bool { log.Errorf("failed to resolve profile %q: %v", handle, err) return false } + // resolveProfileHandle might stamp legacy profile owners and if + if act != nil { + config, _, err := s.getConfig(act) + if err != nil { + log.Errorf("failed to get active profile config: %v", err) + } + s.mutex.Lock() + s.config = config + s.mutex.Unlock() + } return resolved.AccessibleBy(id) }