Skip to content

Commit 1b5ae45

Browse files
committed
os/user: User.GroupIds shouldn't error on users with no groups
On Windows, the User.GroupIds currently errors out if the user has no groups. This is incorrect, as the user may not be a member of any groups as demonstrated by the new TestGroupIdsTestUser test. Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest,gotip-windows-arm64 Change-Id: I436aa6214f2b98ef98dfb6064caec3d682b3f3d2 Reviewed-on: https://go-review.googlesource.com/c/go/+/606675 Reviewed-by: Michael Pratt <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Alex Brainman <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
1 parent af86efb commit 1b5ae45

File tree

2 files changed

+34
-14
lines changed

2 files changed

+34
-14
lines changed

src/os/user/lookup_windows.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,17 +151,13 @@ func listGroupsForUsernameAndDomain(username, domain string) ([]string, error) {
151151
// NetUserGetLocalGroups() would return a list of LocalGroupUserInfo0
152152
// elements which hold the names of local groups where the user participates.
153153
// The list does not follow any sorting order.
154-
//
155-
// If no groups can be found for this user, NetUserGetLocalGroups() should
156-
// always return the SID of a single group called "None", which
157-
// also happens to be the primary group for the local user.
158154
err = windows.NetUserGetLocalGroups(nil, q, 0, windows.LG_INCLUDE_INDIRECT, &p0, windows.MAX_PREFERRED_LENGTH, &entriesRead, &totalEntries)
159155
if err != nil {
160156
return nil, err
161157
}
162158
defer syscall.NetApiBufferFree(p0)
163159
if entriesRead == 0 {
164-
return nil, fmt.Errorf("listGroupsForUsernameAndDomain: NetUserGetLocalGroups() returned an empty list for domain: %s, username: %s", domain, username)
160+
return nil, nil
165161
}
166162
entries := (*[1024]windows.LocalGroupUserInfo0)(unsafe.Pointer(p0))[:entriesRead:entriesRead]
167163
var sids []string

src/os/user/user_windows_test.go

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,13 @@ import (
2323
// windowsTestAcount creates a test user and returns a token for that user.
2424
// If the user already exists, it will be deleted and recreated.
2525
// The caller is responsible for closing the token.
26-
func windowsTestAcount(t *testing.T) syscall.Token {
26+
func windowsTestAcount(t *testing.T) (syscall.Token, *User) {
27+
const testUserName = "GoStdTestUser01"
2728
var password [33]byte
2829
rand.Read(password[:])
2930
// Add special chars to ensure it satisfies password requirements.
3031
pwd := base64.StdEncoding.EncodeToString(password[:]) + "_-As@!%*(1)4#2"
31-
name, err := syscall.UTF16PtrFromString("GoStdTestUser01")
32+
name, err := syscall.UTF16PtrFromString(testUserName)
3233
if err != nil {
3334
t.Fatal(err)
3435
}
@@ -57,6 +58,13 @@ func windowsTestAcount(t *testing.T) syscall.Token {
5758
} else if err != nil {
5859
t.Fatal(err)
5960
}
61+
t.Cleanup(func() {
62+
if err = windows.NetUserDel(nil, name); err != nil {
63+
if !errors.Is(err, windows.NERR_UserNotFound) {
64+
t.Fatal(err)
65+
}
66+
}
67+
})
6068
domain, err := syscall.UTF16PtrFromString(".")
6169
if err != nil {
6270
t.Fatal(err)
@@ -69,13 +77,12 @@ func windowsTestAcount(t *testing.T) syscall.Token {
6977
}
7078
t.Cleanup(func() {
7179
token.Close()
72-
if err = windows.NetUserDel(nil, name); err != nil {
73-
if !errors.Is(err, windows.NERR_UserNotFound) {
74-
t.Fatal(err)
75-
}
76-
}
7780
})
78-
return token
81+
usr, err := Lookup(testUserName)
82+
if err != nil {
83+
t.Fatal(err)
84+
}
85+
return token, usr
7986
}
8087

8188
func TestImpersonatedSelf(t *testing.T) {
@@ -127,7 +134,7 @@ func TestImpersonated(t *testing.T) {
127134
}
128135

129136
// Create a test user and log in as that user.
130-
token := windowsTestAcount(t)
137+
token, _ := windowsTestAcount(t)
131138

132139
// Impersonate the test user.
133140
if err = windows.ImpersonateLoggedOnUser(token); err != nil {
@@ -178,3 +185,20 @@ func TestCurrentNetapi32(t *testing.T) {
178185
t.Fatalf("%v\n%s", err, out)
179186
}
180187
}
188+
189+
func TestGroupIdsTestUser(t *testing.T) {
190+
// Create a test user and log in as that user.
191+
_, user := windowsTestAcount(t)
192+
193+
gids, err := user.GroupIds()
194+
if err != nil {
195+
t.Fatal(err)
196+
}
197+
198+
if err != nil {
199+
t.Fatalf("%+v.GroupIds(): %v", user, err)
200+
}
201+
if !containsID(gids, user.Gid) {
202+
t.Errorf("%+v.GroupIds() = %v; does not contain user GID %s", user, gids, user.Gid)
203+
}
204+
}

0 commit comments

Comments
 (0)