feat(brokers/cache): Change user info validations to require at least one remote group

internal/brokers/broker.go

@@ -336,6 +336,14 @@ func validateUserInfoAndGenerateIDs(brokerName string, rawMsg json.RawMessage) (
 	}
 	uInfo.UID = generateID(brokerName + uInfo.UUID)
 
+	// User must be a part of at least one group.
+	if len(uInfo.Groups) == 0 {
+		return users.UserInfo{}, fmt.Errorf("empty groups")
+	}
+	// The default group for the user is the default and it must have a UGID.
+	if uInfo.Groups[0].UGID == "" {
+		return users.UserInfo{}, fmt.Errorf("default group has empty UGID")
+	}
 	// Validate UGIDs and generate GIDs
 	for _, g := range uInfo.Groups {
 		if g.Name == "" {

internal/brokers/broker_test.go

@@ -214,12 +214,11 @@ func TestIsAuthenticated(t *testing.T) {
 
 		cancelFirstCall bool
 	}{
-		"Successfully authenticate":                             {sessionID: "success"},
-		"Successfully authenticate after cancelling first call": {sessionID: "IA_second_call", secondCall: true},
-		"Denies authentication when broker times out":           {sessionID: "IA_timeout"},
-		"No error when auth.Next and no data":                   {sessionID: "IA_next"},
-
-		"No UGID means local group": {sessionID: "IA_info_missing_ugid"},
+		"Successfully authenticate":                              {sessionID: "success"},
+		"Successfully authenticate after cancelling first call":  {sessionID: "IA_second_call", secondCall: true},
+		"Denies authentication when broker times out":            {sessionID: "IA_timeout"},
+		"No error when auth.Next and no data":                    {sessionID: "IA_next"},
+		"No error when broker returns userinfo with empty gecos": {sessionID: "IA_info_missing_gecos"},
 
 		// broker errors
 		"Error when authenticating":                                           {sessionID: "IA_error"},
@@ -228,7 +227,9 @@ func TestIsAuthenticated(t *testing.T) {
 		"Error when broker returns invalid access":                            {sessionID: "IA_invalid_access"},
 		"Error when broker returns invalid userinfo":                          {sessionID: "IA_invalid_userinfo"},
 		"Error when broker returns userinfo with empty username":              {sessionID: "IA_info_missing_user_name"},
+		"Error when broker returns userinfo with no groups":                   {sessionID: "IA_info_missing_groups"},
 		"Error when broker returns userinfo with empty group name":            {sessionID: "IA_info_missing_group_name"},
+		"Error when broker returns userinfo with first group with empty UGID": {sessionID: "IA_info_missing_ugid"},
 		"Error when broker returns userinfo with empty UUID":                  {sessionID: "IA_info_missing_uuid"},
 		"Error when broker returns userinfo with invalid homedir":             {sessionID: "IA_info_invalid_home"},
 		"Error when broker returns userinfo with invalid shell":               {sessionID: "IA_info_invalid_shell"},

internal/brokers/testdata/TestIsAuthenticated/golden/error_when_broker_returns_userinfo_with_empty_username

@@ -4,7 +4,7 @@ FIRST CALL:
 	err: invalid user information provided by the broker ({
 		"name": "",
 		"uuid": "uuid-IA_info_missing_user_name",
-		"gecos": "gecos for ",
+		"gecos": "gecos for IA_info_missing_user_name",
 		"dir": "/home/IA_info_missing_user_name",
 		"shell": "/bin/sh/IA_info_missing_user_name",
 		"avatar": "avatar for ",

internal/brokers/testdata/TestIsAuthenticated/golden/error_when_broker_returns_userinfo_with_empty_ugid → internal/brokers/testdata/TestIsAuthenticated/golden/error_when_broker_returns_userinfo_with_first_group_with_empty_ugid

@@ -9,4 +9,4 @@ FIRST CALL:
 		"shell": "/bin/sh/IA_info_missing_ugid",
 		"avatar": "avatar for IA_info_missing_ugid",
 		"groups": [ {"name": "group-IA_info_missing_ugid", "ugid": ""} ]
-	}): group "group-IA_info_missing_ugid" has empty UGID
+	}): default group has empty UGID

internal/brokers/testdata/TestIsAuthenticated/golden/error_when_broker_returns_userinfo_with_no_groups

@@ -0,0 +1,12 @@
+FIRST CALL:
+	access: 
+	data: 
+	err: invalid user information provided by the broker ({
+		"name": "IA_info_missing_groups",
+		"uuid": "uuid-IA_info_missing_groups",
+		"gecos": "gecos for IA_info_missing_groups",
+		"dir": "/home/IA_info_missing_groups",
+		"shell": "/bin/sh/IA_info_missing_groups",
+		"avatar": "avatar for IA_info_missing_groups",
+		"groups": [  ]
+	}): empty groups

internal/brokers/testdata/TestIsAuthenticated/golden/no_error_when_broker_returns_userinfo_with_empty_gecos

@@ -0,0 +1,4 @@
+FIRST CALL:
+	access: granted
+	data: {"Name":"IA_info_missing_gecos","UID":67151,"Gecos":"","Dir":"/home/IA_info_missing_gecos","Shell":"/bin/sh/IA_info_missing_gecos","Groups":[{"Name":"group-IA_info_missing_gecos","GID":66857}]}
+	err: <nil>

internal/cache/db_test.go

@@ -179,6 +179,25 @@ func TestUpdateFromUserInfo(t *testing.T) {
 				{Name: "newgroup1", GID: ptrValue(11111)},
 			},
 		},
+		"user1-with-only-local-group": {
+			Name:  "user1",
+			UID:   1111,
+			Gecos: "User1 gecos\nOn multiple lines",
+			Dir:   "/home/user1",
+			Shell: "/bin/bash",
+			Groups: []users.GroupInfo{
+				{Name: "local-group"},
+			},
+		},
+		"user1-without-gecos": {
+			Name:  "user1",
+			UID:   1111,
+			Dir:   "/home/user1",
+			Shell: "/bin/bash",
+			Groups: []users.GroupInfo{
+				{Name: "group1", GID: ptrValue(11111)},
+			},
+		},
 		"user1-without-groups": {
 			Name:  "user1",
 			UID:   1111,
@@ -249,18 +268,19 @@ func TestUpdateFromUserInfo(t *testing.T) {
 		wantErr     bool
 	}{
 		// New user
-		"Insert new user":                 {userCase: "user1"},
-		"Update last login time for user": {userCase: "user1", dbFile: "one_user_and_group"},
+		"Insert new user":                              {userCase: "user1"},
+		"Update last login time for user":              {userCase: "user1", dbFile: "one_user_and_group"},
+		"Insert new user without optional gecos field": {userCase: "user1-without-gecos"},
 
 		// User and Group renames
-		"Update user by changing attributes":  {userCase: "user1-new-attributes", dbFile: "one_user_and_group"},
-		"Update group by changing attributes": {userCase: "group1-new-attributes", dbFile: "one_user_and_group"},
+		"Update user by changing attributes":                      {userCase: "user1-new-attributes", dbFile: "one_user_and_group"},
+		"Update user by removing optional gecos field if not set": {userCase: "user1-without-gecos", dbFile: "one_user_and_group"},
+		"Update group by changing attributes":                     {userCase: "group1-new-attributes", dbFile: "one_user_and_group"},
 
 		// Group updates
-		"Update user and keep existing groups without specifying them": {userCase: "user1-without-groups", dbFile: "one_user_and_group"},
-		"Update user by adding a new group":                            {userCase: "user1-with-new-group", dbFile: "one_user_and_group"},
-		"Update user by adding a new default group":                    {userCase: "user1-with-new-default-group", dbFile: "one_user_and_group"},
-		"Remove group from user":                                       {userCase: "user1-with-only-new-group", dbFile: "one_user_and_group"},
+		"Update user by adding a new group":         {userCase: "user1-with-new-group", dbFile: "one_user_and_group"},
+		"Update user by adding a new default group": {userCase: "user1-with-new-default-group", dbFile: "one_user_and_group"},
+		"Remove group from user":                    {userCase: "user1-with-only-new-group", dbFile: "one_user_and_group"},
 
 		// Multi users handling
 		"Update only user even if we have multiple of them":     {userCase: "user1", dbFile: "multiple_users_and_groups"},
@@ -271,17 +291,16 @@ func TestUpdateFromUserInfo(t *testing.T) {
 		"Local groups are filtered": {userCase: "user1-with-local-group"},
 
 		// Allowed inconsistent cases
-		"Invalid value entry in groupByID but user restating group recreates entries":       {userCase: "user1", dbFile: "invalid_entry_in_groupByID"},
-		"Invalid value entry in userByID recreates entries":                                 {userCase: "user1", dbFile: "invalid_entry_in_userByID"},
-		"Invalid value entry in groupByName recreates entries":                              {userCase: "user1", dbFile: "invalid_entry_in_groupByName"},
-		"Invalid value entry in groupByName recreates entries even without restating group": {userCase: "user1-without-groups", dbFile: "invalid_entry_in_groupByName"},
-		"Invalid value entry in userByName recreates entries":                               {userCase: "user1", dbFile: "invalid_entry_in_userByName"},
-		"Invalid value entries in other user and groups don't impact current request":       {userCase: "user1", dbFile: "invalid_entries_but_user_and_group1"},
+		"Invalid value entry in groupByID but user restating group recreates entries": {userCase: "user1", dbFile: "invalid_entry_in_groupByID"},
+		"Invalid value entry in userByID recreates entries":                           {userCase: "user1", dbFile: "invalid_entry_in_userByID"},
+		"Invalid value entry in groupByName recreates entries":                        {userCase: "user1", dbFile: "invalid_entry_in_groupByName"},
+		"Invalid value entry in userByName recreates entries":                         {userCase: "user1", dbFile: "invalid_entry_in_userByName"},
+		"Invalid value entries in other user and groups don't impact current request": {userCase: "user1", dbFile: "invalid_entries_but_user_and_group1"},
 
 		// Error cases
-		"Error on new user without any groups":                                                     {userCase: "user1-without-groups", wantErr: true},
+		"Error on user without any groups":                                                         {userCase: "user1-without-groups", wantErr: true},
+		"Error on user with only local group":                                                      {userCase: "user1-with-only-local-group", wantErr: true},
 		"Error on invalid value entry in userToGroups clear database":                              {userCase: "user1", dbFile: "invalid_entry_in_userToGroups", wantErr: true, wantClearDB: true},
-		"Error on invalid value entry in groupByID with user not restating groups clear database":  {userCase: "user1-without-groups", dbFile: "invalid_entry_in_groupByID", wantErr: true, wantClearDB: true},
 		"Error on invalid value entry in groupToUsers clear database":                              {userCase: "user1", dbFile: "invalid_entry_in_groupToUsers", wantErr: true, wantClearDB: true},
 		"Error on invalid value entry in groupToUsers for user dropping from group clear database": {userCase: "user1", dbFile: "invalid_entry_in_groupToUsers_secondary_group", wantErr: true, wantClearDB: true},
 		"Error on invalid value entry in groupByID for user dropping from group clear database":    {userCase: "user1", dbFile: "invalid_entry_in_groupByID_secondary_group", wantErr: true, wantClearDB: true},

internal/cache/getusers.go

@@ -13,7 +13,7 @@ type UserPasswdShadow struct {
 	Name  string
 	UID   int
 	GID   int
-	Gecos string
+	Gecos string // Gecos is an optional field. It can be empty.
 	Dir   string
 	Shell string
 

internal/cache/testdata/TestUpdateFromUserInfo/golden/insert_new_user_without_optional_gecos_field

@@ -0,0 +1,13 @@
+GroupByID:
+    "11111": '{"Name":"group1","GID":11111}'
+GroupByName:
+    group1: '{"Name":"group1","GID":11111}'
+GroupToUsers:
+    "11111": '{"GID":11111,"UIDs":[1111]}'
+UserByID:
+    "1111": '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
+UserByName:
+    user1: '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
+UserToBroker: {}
+UserToGroups:
+    "1111": '{"UID":1111,"GIDs":[11111]}'

internal/cache/testdata/TestUpdateFromUserInfo/golden/update_user_by_removing_optional_gecos_field_if_not_set

@@ -0,0 +1,13 @@
+GroupByID:
+    "11111": '{"Name":"group1","GID":11111}'
+GroupByName:
+    group1: '{"Name":"group1","GID":11111}'
+GroupToUsers:
+    "11111": '{"GID":11111,"UIDs":[1111]}'
+UserByID:
+    "1111": '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
+UserByName:
+    user1: '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
+UserToBroker: {}
+UserToGroups:
+    "1111": '{"UID":1111,"GIDs":[11111]}'

internal/cache/update.go

@@ -16,15 +16,17 @@ import (
 // UpdateFromUserInfo inserts or updates user and group buckets from the user information.
 func (c *Cache) UpdateFromUserInfo(u users.UserInfo) error {
 	// create bucket contents dynamically
-	gid := -1
-	if len(u.Groups) > 0 && u.Groups[0].GID != nil {
-		gid = *u.Groups[0].GID
+	if len(u.Groups) == 0 {
+		return fmt.Errorf("no group provided for user %s (%v)", u.Name, u.UID)
+	}
+	if u.Groups[0].GID == nil {
+		return fmt.Errorf("no gid provided for default group %q", u.Groups[0].Name)
 	}
 	userDB := userDB{
 		UserPasswdShadow: UserPasswdShadow{
 			Name:           u.Name,
 			UID:            u.UID,
-			GID:            gid,
+			GID:            *u.Groups[0].GID,
 			Gecos:          u.Gecos,
 			Dir:            u.Dir,
 			Shell:          u.Shell,
@@ -66,26 +68,6 @@ func (c *Cache) UpdateFromUserInfo(u users.UserInfo) error {
 			return err
 		}
 
-		// No groups were specified for this request.
-		if userDB.GID == -1 {
-			if len(previousGroupsForCurrentUser.GIDs) == 0 {
-				return fmt.Errorf("no group provided for user %v (%v) and no previous record found", userDB.Name, userDB.UID)
-			}
-
-			for _, gid := range previousGroupsForCurrentUser.GIDs {
-				g, err := getFromBucket[groupDB](buckets[groupByIDBucketName], gid)
-				if err != nil {
-					c.requestClearDatabase()
-					return err
-				}
-				groupContents = append(groupContents, groupDB{
-					Name: g.Name,
-					GID:  g.GID,
-				})
-			}
-			userDB.GID = groupContents[0].GID
-		}
-
 		/* 1. Handle user update */
 		updateUser(buckets, userDB)
 

internal/testutils/broker.go

@@ -333,6 +333,7 @@ func userInfoFromName(parsedID string, extraGroups []users.GroupInfo) string {
 	group := "group-" + parsedID
 	home := "/home/" + parsedID
 	shell := "/bin/sh/" + parsedID
+	gecos := "gecos for " + parsedID
 	uuid := "uuid-" + parsedID
 	ugid := "ugid-" + parsedID
 
@@ -345,6 +346,10 @@ func userInfoFromName(parsedID string, extraGroups []users.GroupInfo) string {
 		uuid = ""
 	case "IA_info_missing_ugid":
 		ugid = ""
+	case "IA_info_missing_gecos":
+		gecos = ""
+	case "IA_info_missing_groups":
+		group = "-"
 	case "IA_info_invalid_home":
 		home = "this is not a homedir"
 	case "IA_info_invalid_shell":
@@ -367,21 +372,25 @@ func userInfoFromName(parsedID string, extraGroups []users.GroupInfo) string {
 			UGID: ugid,
 		})
 	}
+	if group == "-" {
+		groups = []groupJSONInfo{}
+	}
 
 	user := struct {
 		Name   string
 		UUID   string
 		Home   string
 		Shell  string
 		Groups []groupJSONInfo
-	}{Name: name, UUID: uuid, Home: home, Shell: shell, Groups: groups}
+		Gecos  string
+	}{Name: name, UUID: uuid, Home: home, Shell: shell, Groups: groups, Gecos: gecos}
 
 	// only used for tests, we can ignore the template execution error as the returned data will be failing.
 	var buf bytes.Buffer
 	_ = template.Must(template.New("").Parse(`{
 		"name": "{{.Name}}",
 		"uuid": "{{.UUID}}",
-		"gecos": "gecos for {{.Name}}",
+		"gecos": "{{.Gecos}}",
 		"dir": "{{.Home}}",
 		"shell": "{{.Shell}}",
 		"avatar": "avatar for {{.Name}}",