From c2580151654976b9dcbf7831723d8eae09d58078 Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Fri, 25 Nov 2022 06:11:06 -0800 Subject: [PATCH] ipn/ipnlocal,ipnserver: rename {,Set}CurrentUser to {,Set}CurrentUserID Address comments from https://github.com/tailscale/tailscale/pull/6506#discussion_r1032454064 Signed-off-by: Maisem Ali --- ipn/ipnlocal/local.go | 18 ++++++++++++++---- ipn/ipnlocal/profiles.go | 8 ++++---- ipn/ipnlocal/profiles_test.go | 24 ++++++++++++------------ 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 23d4d87ed..d4ec241d2 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2039,7 +2039,7 @@ func (b *LocalBackend) InServerMode() bool { func (b *LocalBackend) CheckIPNConnectionAllowed(ci *ipnauth.ConnIdentity) error { b.mu.Lock() defer b.mu.Unlock() - serverModeUid := b.pm.CurrentUser() + serverModeUid := b.pm.CurrentUserID() if serverModeUid == "" { // Either this platform isn't a "multi-user" platform or we're not yet // running as one. @@ -2053,11 +2053,21 @@ func (b *LocalBackend) CheckIPNConnectionAllowed(ci *ipnauth.ConnIdentity) error return errors.New("empty user uid in connection identity") } if uid != serverModeUid { - return fmt.Errorf("Tailscale running in server mode (uid=%v); connection from %q not allowed", serverModeUid, uid) + return fmt.Errorf("Tailscale running in server mode (%q); connection from %q not allowed", b.tryLookupUserName(serverModeUid), b.tryLookupUserName(uid)) } return nil } +// tryLookupUserName tries to look up the username for the uid. +// It returns the username on success, or the UID on failure. +func (b *LocalBackend) tryLookupUserName(uid string) string { + u, err := ipnauth.LookupUserFromID(b.logf, uid) + if err != nil { + return uid + } + return u.Username +} + // Login implements Backend. // As of 2022-11-15, this is only exists for Android. func (b *LocalBackend) Login(token *tailcfg.Oauth2Token) { @@ -2221,11 +2231,11 @@ func (b *LocalBackend) shouldUploadServices() bool { // On non-multi-user systems, the uid should be set to empty string. func (b *LocalBackend) SetCurrentUserID(uid string) { b.mu.Lock() - if b.pm.CurrentUser() == uid { + if b.pm.CurrentUserID() == uid { b.mu.Unlock() return } - if err := b.pm.SetCurrentUser(uid); err != nil { + if err := b.pm.SetCurrentUserID(uid); err != nil { b.mu.Unlock() return } diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index e8107e16b..13e734d5e 100644 --- a/ipn/ipnlocal/profiles.go +++ b/ipn/ipnlocal/profiles.go @@ -40,15 +40,15 @@ type profileManager struct { isNewProfile bool } -// CurrentUser returns the current user ID. It is only non-empty on +// CurrentUserID returns the current user ID. It is only non-empty on // Windows where we have a multi-user system. -func (pm *profileManager) CurrentUser() string { +func (pm *profileManager) CurrentUserID() string { return pm.currentUserID } -// SetCurrentUser sets the current user ID. The uid is only non-empty +// SetCurrentUserID sets the current user ID. The uid is only non-empty // on Windows where we have a multi-user system. -func (pm *profileManager) SetCurrentUser(uid string) error { +func (pm *profileManager) SetCurrentUserID(uid string) error { if pm.currentUserID == uid { return nil } diff --git a/ipn/ipnlocal/profiles_test.go b/ipn/ipnlocal/profiles_test.go index 3bbfedfb1..9543b2421 100644 --- a/ipn/ipnlocal/profiles_test.go +++ b/ipn/ipnlocal/profiles_test.go @@ -44,7 +44,7 @@ func TestProfileCurrentUserSwitch(t *testing.T) { return p.View() } - pm.SetCurrentUser("user1") + pm.SetCurrentUserID("user1") newProfile(t, "user1") cp := pm.currentProfile pm.DeleteProfile(cp.ID) @@ -61,7 +61,7 @@ func TestProfileCurrentUserSwitch(t *testing.T) { if err != nil { t.Fatal(err) } - pm.SetCurrentUser("user1") + pm.SetCurrentUserID("user1") if pm.currentProfile == nil { t.Fatal("currentProfile is nil") } else if pm.currentProfile.ID != "" { @@ -112,18 +112,18 @@ func TestProfileList(t *testing.T) { } } - pm.SetCurrentUser("user1") + pm.SetCurrentUserID("user1") newProfile(t, "alice") newProfile(t, "bob") checkProfiles(t, "alice", "bob") - pm.SetCurrentUser("user2") + pm.SetCurrentUserID("user2") checkProfiles(t) newProfile(t, "carol") carol := pm.currentProfile checkProfiles(t, "carol") - pm.SetCurrentUser("user1") + pm.SetCurrentUserID("user1") checkProfiles(t, "alice", "bob") if lp := pm.findProfileByKey(carol.Key); lp != nil { t.Fatalf("found profile for user2 in user1's profile list") @@ -138,7 +138,7 @@ func TestProfileList(t *testing.T) { t.Fatalf("found profile for user2 in user1's profile list") } - pm.SetCurrentUser("user2") + pm.SetCurrentUserID("user2") checkProfiles(t, "carol") } @@ -342,7 +342,7 @@ func TestProfileManagementWindows(t *testing.T) { { t.Logf("Set user1 as logged in user") - if err := pm.SetCurrentUser("user1"); err != nil { + if err := pm.SetCurrentUserID("user1"); err != nil { t.Fatal(err) } checkProfiles(t) @@ -378,7 +378,7 @@ func TestProfileManagementWindows(t *testing.T) { { t.Logf("Set user1 as current user") - if err := pm.SetCurrentUser("user1"); err != nil { + if err := pm.SetCurrentUserID("user1"); err != nil { t.Fatal(err) } wantCurProfile = "test" @@ -388,8 +388,8 @@ func TestProfileManagementWindows(t *testing.T) { t.Logf("set unattended mode") wantProfiles["test"] = setPrefs(t, "test", true) } - if pm.CurrentUser() != "user1" { - t.Fatalf("CurrentUserID = %q; want %q", pm.CurrentUser(), "user1") + if pm.CurrentUserID() != "user1" { + t.Fatalf("CurrentUserID = %q; want %q", pm.CurrentUserID(), "user1") } // Recreate the profile manager to ensure that it starts with test profile. @@ -398,7 +398,7 @@ func TestProfileManagementWindows(t *testing.T) { t.Fatal(err) } checkProfiles(t) - if pm.CurrentUser() != "user1" { - t.Fatalf("CurrentUserID = %q; want %q", pm.CurrentUser(), "user1") + if pm.CurrentUserID() != "user1" { + t.Fatalf("CurrentUserID = %q; want %q", pm.CurrentUserID(), "user1") } }