Skip to content

Commit

Permalink
Allowing Windows to find console users without requiring User.UiD (#1128
Browse files Browse the repository at this point in the history
)

* Allowing Windows to find console users without requiring User.UiD
  • Loading branch information
CJ Barrett authored Apr 13, 2023
1 parent 5b82a80 commit f401999
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 16 deletions.
20 changes: 14 additions & 6 deletions cmd/launcher/desktop.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "" {
Expand Down
11 changes: 4 additions & 7 deletions ee/consoleuser/consoleuser_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package consoleuser
import (
"context"
"fmt"
"os/user"
"path/filepath"

"github.com/shirou/gopsutil/process"
Expand Down Expand Up @@ -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
}
6 changes: 5 additions & 1 deletion ee/desktop/runner/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 9 additions & 2 deletions ee/localserver/request-id.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"net/http"
"os/user"
"runtime"
"time"

"github.com/go-kit/kit/log/level"
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit f401999

Please sign in to comment.