From f401999dddf5eea6bfde9206cf7393c0f3948494 Mon Sep 17 00:00:00 2001 From: CJ Barrett Date: Thu, 13 Apr 2023 16:14:19 -0400 Subject: [PATCH] Allowing Windows to find console users without requiring User.UiD (#1128) * Allowing Windows to find console users without requiring User.UiD --- cmd/launcher/desktop.go | 20 ++++++++++++++------ ee/consoleuser/consoleuser_windows.go | 11 ++++------- ee/desktop/runner/runner_test.go | 6 +++++- ee/localserver/request-id.go | 11 +++++++++-- 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/cmd/launcher/desktop.go b/cmd/launcher/desktop.go index 09095304c..62f5a12ba 100644 --- a/cmd/launcher/desktop.go +++ b/cmd/launcher/desktop.go @@ -68,18 +68,26 @@ func runDesktop(args []string) error { return fmt.Errorf("parsing flags: %w", err) } - user, err := user.Current() - if err != nil { - return fmt.Errorf("getting current user: %w", err) - } - // set up logging logger := logutil.NewServerLogger(*fldebug) logger = log.With(logger, "subprocess", "desktop", "pid", os.Getpid(), - "uid", user.Uid, ) + + // Try to get the current user, so we can use the UID for logging. Not a fatal error if we can't, though + user, err := user.Current() + if err != nil { + level.Debug(logger).Log( + "msg", "error getting current user", + "err", err, + ) + } else { + logger = log.With(logger, + "uid", user.Uid, + ) + } + level.Info(logger).Log("msg", "starting") if *flsocketpath == "" { diff --git a/ee/consoleuser/consoleuser_windows.go b/ee/consoleuser/consoleuser_windows.go index 287882524..266bb266e 100644 --- a/ee/consoleuser/consoleuser_windows.go +++ b/ee/consoleuser/consoleuser_windows.go @@ -6,7 +6,6 @@ package consoleuser import ( "context" "fmt" - "os/user" "path/filepath" "github.com/shirou/gopsutil/process" @@ -94,10 +93,8 @@ func processOwnerUid(ctx context.Context, proc *process.Process) (string, error) return "", fmt.Errorf("getting process username (for pid %d): %w", proc.Pid, err) } - user, err := user.Lookup(username) - if err != nil { - return "", fmt.Errorf("looking up username %s: %w", username, err) - } - - return user.Uid, nil + // Looking up the proper UID (which on Windows, is a SID) seems to be problematic and + // can fail for reasons we don't quite understand. We just need something to uniquely + // identify the user, so on Windows we use the username instead of numeric UID. + return username, nil } diff --git a/ee/desktop/runner/runner_test.go b/ee/desktop/runner/runner_test.go index 49b81ad12..365ffc20b 100644 --- a/ee/desktop/runner/runner_test.go +++ b/ee/desktop/runner/runner_test.go @@ -144,7 +144,11 @@ func TestDesktopUserProcessRunner_Execute(t *testing.T) { if tt.cleanShutdown || (os.Getenv("CI") == "true" && runtime.GOOS == "linux") { assert.Len(t, r.uidProcs, 0, "unexpected process: logs: %s", logBytes.String()) } else { - assert.Contains(t, r.uidProcs, user.Uid) + if runtime.GOOS == "windows" { + assert.Contains(t, r.uidProcs, user.Username) + } else { + assert.Contains(t, r.uidProcs, user.Uid) + } assert.Len(t, r.uidProcs, 1) if len(tt.logContains) > 0 { diff --git a/ee/localserver/request-id.go b/ee/localserver/request-id.go index eba06dd17..eb9472763 100644 --- a/ee/localserver/request-id.go +++ b/ee/localserver/request-id.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "os/user" + "runtime" "time" "github.com/go-kit/kit/log/level" @@ -107,12 +108,18 @@ func consoleUsers() ([]*user.User, error) { } for _, uid := range uids { - user, err := user.LookupId(uid) + var err error + var u *user.User + if runtime.GOOS == "windows" { + u, err = user.Lookup(uid) + } else { + u, err = user.LookupId(uid) + } if err != nil { return err } - users = append(users, user) + users = append(users, u) } return nil }, maxDuration, 250*time.Millisecond)