From ae97338203ca90c344c9c048b7f95a279cc37c3a Mon Sep 17 00:00:00 2001 From: Adrian Dombeck Date: Mon, 27 Jan 2025 21:09:11 +0100 Subject: [PATCH 1/6] Don't delete user from database if updating local groups fails It was broken in multiple ways: * It deletes the user even if it already existed in the database before this update. * It's not protected by a mutex, so a concurrent update could have successfully added the user, which is then incorrectly deleted here. In addition to that, now that we don't automatically delete empty groups, it still leaves the groups (including the user private group) which were added. Fixing all that would require effort which I'm not sure would be justified. I don't see any issues with users/groups from an authenticated but erroneous login attempt still being in the database, except that it might be a bit surprising to users. Therefore, with this commit we simply leave the updated user and groups as they are. --- .../cache.db | 25 ++++++++--- internal/users/manager.go | 2 +- internal/users/manager_test.go | 4 -- ..._updating_local_groups_remove_user_from_db | 11 ----- ...emove_user_from_db_even_if_already_existed | 45 ------------------- ..._user_from_db_without_touching_other_users | 45 ------------------- 6 files changed, 19 insertions(+), 113 deletions(-) delete mode 100644 internal/users/testdata/golden/TestUpdateUser/Error_when_updating_local_groups_remove_user_from_db delete mode 100644 internal/users/testdata/golden/TestUpdateUser/Error_when_updating_local_groups_remove_user_from_db_even_if_already_existed delete mode 100644 internal/users/testdata/golden/TestUpdateUser/Error_when_updating_local_groups_remove_user_from_db_without_touching_other_users diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db index 6aa4bc21a..b715be5fd 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db @@ -1,10 +1,21 @@ -GroupByID: {} -GroupByName: {} -GroupByUGID: {} -GroupToUsers: {} -UserByID: {} -UserByName: {} +GroupByID: + "1111": '{"Name":"TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file_separator_success_with_local_groups","GID":1111,"UGID":"TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file_separator_success_with_local_groups"}' + "2222": '{"Name":"group-success_with_local_groups","GID":2222,"UGID":"ugid-success_with_local_groups"}' +GroupByName: + TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file_separator_success_with_local_groups: '{"Name":"TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file_separator_success_with_local_groups","GID":1111,"UGID":"TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file_separator_success_with_local_groups"}' + group-success_with_local_groups: '{"Name":"group-success_with_local_groups","GID":2222,"UGID":"ugid-success_with_local_groups"}' +GroupByUGID: + TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file_separator_success_with_local_groups: '{"Name":"TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file_separator_success_with_local_groups","GID":1111,"UGID":"TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file_separator_success_with_local_groups"}' + ugid-success_with_local_groups: '{"Name":"group-success_with_local_groups","GID":2222,"UGID":"ugid-success_with_local_groups"}' +GroupToUsers: + "1111": '{"GID":1111,"UIDs":[1111]}' + "2222": '{"GID":2222,"UIDs":[1111]}' +UserByID: + "1111": '{"Name":"TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file_separator_success_with_local_groups","UID":1111,"GID":1111,"Gecos":"gecos for success_with_local_groups","Dir":"/home/success_with_local_groups","Shell":"/bin/sh/success_with_local_groups","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' +UserByName: + TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file_separator_success_with_local_groups: '{"Name":"TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file_separator_success_with_local_groups","UID":1111,"GID":1111,"Gecos":"gecos for success_with_local_groups","Dir":"/home/success_with_local_groups","Shell":"/bin/sh/success_with_local_groups","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' UserToBroker: {} -UserToGroups: {} +UserToGroups: + "1111": '{"UID":1111,"GIDs":[1111,2222]}' UserToLocalGroups: "1111": '["localgroup1","localgroup3"]' diff --git a/internal/users/manager.go b/internal/users/manager.go index 763239780..23cc39b34 100644 --- a/internal/users/manager.go +++ b/internal/users/manager.go @@ -216,7 +216,7 @@ func (m *Manager) UpdateUser(u types.UserInfo) (err error) { // Update local groups. if err := localentries.Update(u.Name, localGroups, oldLocalGroups); err != nil { - return errors.Join(err, m.cache.DeleteUser(uid)) + return err } if err = checkHomeDirOwnership(userDB.Dir, userDB.UID, userDB.GID); err != nil { diff --git a/internal/users/manager_test.go b/internal/users/manager_test.go index 987bbae37..b094918d9 100644 --- a/internal/users/manager_test.go +++ b/internal/users/manager_test.go @@ -168,10 +168,6 @@ func TestUpdateUser(t *testing.T) { "Error_if_user_exists_on_system": {userCase: "user-exists-on-system", wantErr: true, noOutput: true}, "Error_if_group_exists_on_system": {groupsCase: "group-exists-on-system", wantErr: true, noOutput: true}, - "Error_when_updating_local_groups_remove_user_from_db": {groupsCase: "mixed-groups-gpasswd-fail", localGroupsFile: "gpasswdfail_in_deleted_group.group", wantErr: true}, - "Error_when_updating_local_groups_remove_user_from_db_without_touching_other_users": {dbFile: "multiple_users_and_groups", groupsCase: "mixed-groups-gpasswd-fail", localGroupsFile: "gpasswdfail_in_deleted_group.group", wantErr: true}, - "Error_when_updating_local_groups_remove_user_from_db_even_if_already_existed": {userCase: "user2", dbFile: "multiple_users_and_groups", groupsCase: "mixed-groups-gpasswd-fail", localGroupsFile: "gpasswdfail_in_deleted_group.group", wantErr: true}, - "Error_on_invalid_entry": {groupsCase: "authd-group", dbFile: "invalid_entry_in_userToGroups", localGroupsFile: "users_in_groups.group", wantErr: true, noOutput: true}, } for name, tc := range tests { diff --git a/internal/users/testdata/golden/TestUpdateUser/Error_when_updating_local_groups_remove_user_from_db b/internal/users/testdata/golden/TestUpdateUser/Error_when_updating_local_groups_remove_user_from_db deleted file mode 100644 index aaa31b69b..000000000 --- a/internal/users/testdata/golden/TestUpdateUser/Error_when_updating_local_groups_remove_user_from_db +++ /dev/null @@ -1,11 +0,0 @@ -| - GroupByID: {} - GroupByName: {} - GroupByUGID: {} - GroupToUsers: {} - UserByID: {} - UserByName: {} - UserToBroker: {} - UserToGroups: {} - UserToLocalGroups: - "1111": '["gpasswdfail"]' diff --git a/internal/users/testdata/golden/TestUpdateUser/Error_when_updating_local_groups_remove_user_from_db_even_if_already_existed b/internal/users/testdata/golden/TestUpdateUser/Error_when_updating_local_groups_remove_user_from_db_even_if_already_existed deleted file mode 100644 index a3170e8eb..000000000 --- a/internal/users/testdata/golden/TestUpdateUser/Error_when_updating_local_groups_remove_user_from_db_even_if_already_existed +++ /dev/null @@ -1,45 +0,0 @@ -| - GroupByID: - "11111": '{"Name":"group1","GID":11111,"UGID":"12345678"}' - "22222": '{"Name":"group2","GID":22222,"UGID":"56781234"}' - "33333": '{"Name":"group3","GID":33333,"UGID":"34567812"}' - "44444": '{"Name":"group4","GID":44444,"UGID":"45678123"}' - "99999": '{"Name":"commongroup","GID":99999,"UGID":"87654321"}' - GroupByName: - commongroup: '{"Name":"commongroup","GID":99999,"UGID":"87654321"}' - group1: '{"Name":"group1","GID":11111,"UGID":"12345678"}' - group2: '{"Name":"group2","GID":22222,"UGID":"56781234"}' - group3: '{"Name":"group3","GID":33333,"UGID":"34567812"}' - group4: '{"Name":"group4","GID":44444,"UGID":"45678123"}' - GroupByUGID: - "12345678": '{"Name":"group1","GID":11111,"UGID":"12345678"}' - "34567812": '{"Name":"group3","GID":33333,"UGID":"34567812"}' - "45678123": '{"Name":"group4","GID":44444,"UGID":"45678123"}' - "56781234": '{"Name":"group2","GID":22222,"UGID":"56781234"}' - "87654321": '{"Name":"commongroup","GID":99999,"UGID":"87654321"}' - GroupToUsers: - "11111": '{"GID":11111,"UIDs":[1111]}' - "22222": '{"GID":22222,"UIDs":[2222]}' - "33333": '{"GID":33333,"UIDs":[3333]}' - "44444": '{"GID":33333,"UIDs":[4444]}' - "99999": '{"GID":99999,"UIDs":[2222,3333]}' - UserByID: - "1111": '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"AAAAATIME"}' - "2222": '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' - "3333": '{"Name":"user3","UID":3333,"GID":33333,"Gecos":"User3","Dir":"/home/user3","Shell":"/bin/zsh","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' - "4444": '{"Name":"userwithoutbroker","UID":4444,"GID":44444,"Gecos":"userwithoutbroker","Dir":"/home/userwithoutbroker","Shell":"/bin/sh","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' - UserByName: - user1: '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"AAAAATIME"}' - user2: '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' - user3: '{"Name":"user3","UID":3333,"GID":33333,"Gecos":"User3","Dir":"/home/user3","Shell":"/bin/zsh","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' - userwithoutbroker: '{"Name":"userwithoutbroker","UID":4444,"GID":44444,"Gecos":"userwithoutbroker","Dir":"/home/userwithoutbroker","Shell":"/bin/sh","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' - UserToBroker: - "1111": '"broker-id"' - "2222": '"broker-id"' - "3333": '"broker-id"' - UserToGroups: - "1111": '{"UID":1111,"GIDs":[11111]}' - "2222": '{"UID":2222,"GIDs":[22222,99999]}' - "3333": '{"UID":3333,"GIDs":[33333,99999]}' - "4444": '{"UID":4444,"GIDs":[44444]}' - UserToLocalGroups: {} diff --git a/internal/users/testdata/golden/TestUpdateUser/Error_when_updating_local_groups_remove_user_from_db_without_touching_other_users b/internal/users/testdata/golden/TestUpdateUser/Error_when_updating_local_groups_remove_user_from_db_without_touching_other_users deleted file mode 100644 index a3170e8eb..000000000 --- a/internal/users/testdata/golden/TestUpdateUser/Error_when_updating_local_groups_remove_user_from_db_without_touching_other_users +++ /dev/null @@ -1,45 +0,0 @@ -| - GroupByID: - "11111": '{"Name":"group1","GID":11111,"UGID":"12345678"}' - "22222": '{"Name":"group2","GID":22222,"UGID":"56781234"}' - "33333": '{"Name":"group3","GID":33333,"UGID":"34567812"}' - "44444": '{"Name":"group4","GID":44444,"UGID":"45678123"}' - "99999": '{"Name":"commongroup","GID":99999,"UGID":"87654321"}' - GroupByName: - commongroup: '{"Name":"commongroup","GID":99999,"UGID":"87654321"}' - group1: '{"Name":"group1","GID":11111,"UGID":"12345678"}' - group2: '{"Name":"group2","GID":22222,"UGID":"56781234"}' - group3: '{"Name":"group3","GID":33333,"UGID":"34567812"}' - group4: '{"Name":"group4","GID":44444,"UGID":"45678123"}' - GroupByUGID: - "12345678": '{"Name":"group1","GID":11111,"UGID":"12345678"}' - "34567812": '{"Name":"group3","GID":33333,"UGID":"34567812"}' - "45678123": '{"Name":"group4","GID":44444,"UGID":"45678123"}' - "56781234": '{"Name":"group2","GID":22222,"UGID":"56781234"}' - "87654321": '{"Name":"commongroup","GID":99999,"UGID":"87654321"}' - GroupToUsers: - "11111": '{"GID":11111,"UIDs":[1111]}' - "22222": '{"GID":22222,"UIDs":[2222]}' - "33333": '{"GID":33333,"UIDs":[3333]}' - "44444": '{"GID":33333,"UIDs":[4444]}' - "99999": '{"GID":99999,"UIDs":[2222,3333]}' - UserByID: - "1111": '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"AAAAATIME"}' - "2222": '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' - "3333": '{"Name":"user3","UID":3333,"GID":33333,"Gecos":"User3","Dir":"/home/user3","Shell":"/bin/zsh","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' - "4444": '{"Name":"userwithoutbroker","UID":4444,"GID":44444,"Gecos":"userwithoutbroker","Dir":"/home/userwithoutbroker","Shell":"/bin/sh","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' - UserByName: - user1: '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"AAAAATIME"}' - user2: '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' - user3: '{"Name":"user3","UID":3333,"GID":33333,"Gecos":"User3","Dir":"/home/user3","Shell":"/bin/zsh","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' - userwithoutbroker: '{"Name":"userwithoutbroker","UID":4444,"GID":44444,"Gecos":"userwithoutbroker","Dir":"/home/userwithoutbroker","Shell":"/bin/sh","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' - UserToBroker: - "1111": '"broker-id"' - "2222": '"broker-id"' - "3333": '"broker-id"' - UserToGroups: - "1111": '{"UID":1111,"GIDs":[11111]}' - "2222": '{"UID":2222,"GIDs":[22222,99999]}' - "3333": '{"UID":3333,"GIDs":[33333,99999]}' - "4444": '{"UID":4444,"GIDs":[44444]}' - UserToLocalGroups: {} From a71ad8b6fa7d5b5f8c8a68b403d85c5bac6b7341 Mon Sep 17 00:00:00 2001 From: Adrian Dombeck Date: Mon, 27 Jan 2025 15:39:06 +0100 Subject: [PATCH 2/6] Don't delete empty groups from database When a user logs in, we fetch the groups from the provider and update our database accordingly. When this causes a user to be removed from an authd group and that group doesn't have any other users in it, it's currently removed from our database. That's an issue for two reasons: 1. The next time a user who is a member of that group logs in, a new random GID is generated for that group, which means that any existing files owned by the group won't be accessible to members of the group anymore. 2. Whenever a another group is added, the random GID generated for that group can by chance be the same as the GID of the deleted group, allowing members access to existing files owned by the deleted group. With this commit, we don't delete empty groups but just keep them in the database, so that its GID is still reserved and reused the next time a user who is a member of that group logs in. --- internal/users/cache/delete.go | 28 ++----------------- .../TestDeleteUser/Delete_existing_user | 12 +++++--- ...e_existing_user_keeping_other_users_intact | 4 +++ .../Remove_group_from_user | 4 +++ ...UID_does_not_change_if_user_already_exists | 4 +++ 5 files changed, 22 insertions(+), 30 deletions(-) diff --git a/internal/users/cache/delete.go b/internal/users/cache/delete.go index cb672c967..e868521c6 100644 --- a/internal/users/cache/delete.go +++ b/internal/users/cache/delete.go @@ -41,34 +41,10 @@ func deleteUserFromGroup(buckets map[string]bucketWithName, uid, gid uint32) err } groupToUsers.UIDs = slices.DeleteFunc(groupToUsers.UIDs, func(id uint32) bool { return id == uid }) - if len(groupToUsers.UIDs) > 0 { - // Update the group entry with the new list of UIDs - updateBucket(buckets[groupToUsersBucketName], gid, groupToUsers) - return nil - } - // We now need to delete this group with no remaining user. - // We need the group.Name to delete from groupByName bucket. - group, err := getFromBucket[GroupDB](buckets[groupByIDBucketName], gid) - if err != nil { - return err - } + // Update the group entry with the new list of UIDs + updateBucket(buckets[groupToUsersBucketName], gid, groupToUsers) - gidKey := []byte(strconv.FormatUint(uint64(gid), 10)) - // Delete group - // Delete calls fail if the transaction is read only, so we should panic if this function is called in that context. - if err = buckets[groupToUsersBucketName].Delete(gidKey); err != nil { - panic(fmt.Sprintf("programming error: delete is not allowed in a RO transaction: %v", err)) - } - if err = buckets[groupByIDBucketName].Delete(gidKey); err != nil { - panic(fmt.Sprintf("programming error: delete is not allowed in a RO transaction: %v", err)) - } - if err = buckets[groupByNameBucketName].Delete([]byte(group.Name)); err != nil { - panic(fmt.Sprintf("programming error: delete is not allowed in a RO transaction: %v", err)) - } - if err = buckets[groupByUGIDBucketName].Delete([]byte(group.UGID)); err != nil { - panic(fmt.Sprintf("programming error: delete is not allowed in a RO transaction: %v", err)) - } return nil } diff --git a/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user b/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user index 79f78e127..4d53cbd4c 100644 --- a/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user +++ b/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user @@ -1,7 +1,11 @@ -GroupByID: {} -GroupByName: {} -GroupByUGID: {} -GroupToUsers: {} +GroupByID: + "11111": '{"Name":"group1","GID":11111,"UGID":"12345678"}' +GroupByName: + group1: '{"Name":"group1","GID":11111,"UGID":"12345678"}' +GroupByUGID: + "12345678": '{"Name":"group1","GID":11111,"UGID":"12345678"}' +GroupToUsers: + "11111": '{"GID":11111,"UIDs":[]}' UserByID: {} UserByName: {} UserToBroker: {} diff --git a/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user_keeping_other_users_intact b/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user_keeping_other_users_intact index d7195e86c..07d4ab7e0 100644 --- a/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user_keeping_other_users_intact +++ b/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user_keeping_other_users_intact @@ -1,19 +1,23 @@ GroupByID: + "11111": '{"Name":"group1","GID":11111,"UGID":"12345678"}' "22222": '{"Name":"group2","GID":22222,"UGID":"56781234"}' "33333": '{"Name":"group3","GID":33333,"UGID":"34567812"}' "44444": '{"Name":"group4","GID":44444,"UGID":"45678123"}' "99999": '{"Name":"commongroup","GID":99999,"UGID":"87654321"}' GroupByName: commongroup: '{"Name":"commongroup","GID":99999,"UGID":"87654321"}' + group1: '{"Name":"group1","GID":11111,"UGID":"12345678"}' group2: '{"Name":"group2","GID":22222,"UGID":"56781234"}' group3: '{"Name":"group3","GID":33333,"UGID":"34567812"}' group4: '{"Name":"group4","GID":44444,"UGID":"45678123"}' GroupByUGID: + "12345678": '{"Name":"group1","GID":11111,"UGID":"12345678"}' "34567812": '{"Name":"group3","GID":33333,"UGID":"34567812"}' "45678123": '{"Name":"group4","GID":44444,"UGID":"45678123"}' "56781234": '{"Name":"group2","GID":22222,"UGID":"56781234"}' "87654321": '{"Name":"commongroup","GID":99999,"UGID":"87654321"}' GroupToUsers: + "11111": '{"GID":11111,"UIDs":[]}' "22222": '{"GID":22222,"UIDs":[2222]}' "33333": '{"GID":33333,"UIDs":[3333]}' "44444": '{"GID":33333,"UIDs":[4444]}' diff --git a/internal/users/cache/testdata/golden/TestUpdateUserEntry/Remove_group_from_user b/internal/users/cache/testdata/golden/TestUpdateUserEntry/Remove_group_from_user index c708d8d1f..b48932105 100644 --- a/internal/users/cache/testdata/golden/TestUpdateUserEntry/Remove_group_from_user +++ b/internal/users/cache/testdata/golden/TestUpdateUserEntry/Remove_group_from_user @@ -1,10 +1,14 @@ GroupByID: + "11111": '{"Name":"group1","GID":11111,"UGID":"12345678"}' "22222": '{"Name":"group2","GID":22222,"UGID":"56781234"}' GroupByName: + group1: '{"Name":"group1","GID":11111,"UGID":"12345678"}' group2: '{"Name":"group2","GID":22222,"UGID":"56781234"}' GroupByUGID: + "12345678": '{"Name":"group1","GID":11111,"UGID":"12345678"}' "56781234": '{"Name":"group2","GID":22222,"UGID":"56781234"}' GroupToUsers: + "11111": '{"GID":11111,"UIDs":[]}' "22222": '{"GID":22222,"UIDs":[1111]}' UserByID: "1111": '{"Name":"user1","UID":1111,"GID":22222,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' diff --git a/internal/users/testdata/golden/TestUpdateUser/UID_does_not_change_if_user_already_exists b/internal/users/testdata/golden/TestUpdateUser/UID_does_not_change_if_user_already_exists index aca312d47..5e79e3ce1 100644 --- a/internal/users/testdata/golden/TestUpdateUser/UID_does_not_change_if_user_already_exists +++ b/internal/users/testdata/golden/TestUpdateUser/UID_does_not_change_if_user_already_exists @@ -1,12 +1,16 @@ | GroupByID: "11110": '{"Name":"user1","GID":11110,"UGID":"user1"}' + "11111": '{"Name":"group1","GID":11111,"UGID":"12345678"}' GroupByName: + group1: '{"Name":"group1","GID":11111,"UGID":"12345678"}' user1: '{"Name":"user1","GID":11110,"UGID":"user1"}' GroupByUGID: + "12345678": '{"Name":"group1","GID":11111,"UGID":"12345678"}' user1: '{"Name":"user1","GID":11110,"UGID":"user1"}' GroupToUsers: "11110": '{"GID":11110,"UIDs":[1111]}' + "11111": '{"GID":11111,"UIDs":[]}' UserByID: "1111": '{"Name":"user1","UID":1111,"GID":11110,"Gecos":"gecos for user1","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' UserByName: From f8fbfe7e1b530059a2c57c4717ad1bcd2c89886e Mon Sep 17 00:00:00 2001 From: Adrian Dombeck Date: Tue, 28 Jan 2025 11:10:42 +0100 Subject: [PATCH 3/6] multiple_users_and_groups.db: Make all users members of the commongroup ... as the name suggests. This allows us to test that removing user1 removes it from that group but doesn't remove other members. --- .../cache/testdata/golden/TestAllGroups/Get_multiple_groups | 2 ++ .../Delete_existing_user_keeping_other_users_intact | 4 ++-- .../golden/TestNew/New_with_already_existing_database | 6 +++--- .../TestUpdateUserEntry/Add_user_to_group_from_another_user | 4 ++-- .../Remove_user_from_a_group_still_part_from_another_user | 6 +++--- .../Update_only_user_even_if_we_have_multiple_of_them | 4 ++-- .../users/cache/testdata/multiple_users_and_groups.db.yaml | 6 +++--- .../golden/TestAllGroups/Successfully_get_all_groups | 2 ++ .../Successfully_create_manager_with_custom_config | 6 +++--- .../Successfully_create_manager_with_default_config | 6 +++--- .../Successfully_update_broker_for_user | 6 +++--- 11 files changed, 28 insertions(+), 24 deletions(-) diff --git a/internal/users/cache/testdata/golden/TestAllGroups/Get_multiple_groups b/internal/users/cache/testdata/golden/TestAllGroups/Get_multiple_groups index 4417e67bf..80d8ac679 100644 --- a/internal/users/cache/testdata/golden/TestAllGroups/Get_multiple_groups +++ b/internal/users/cache/testdata/golden/TestAllGroups/Get_multiple_groups @@ -22,5 +22,7 @@ gid: 99999 ugid: "87654321" users: + - user1 - user2 - user3 + - userwithoutbroker diff --git a/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user_keeping_other_users_intact b/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user_keeping_other_users_intact index 07d4ab7e0..317e47df2 100644 --- a/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user_keeping_other_users_intact +++ b/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user_keeping_other_users_intact @@ -21,7 +21,7 @@ GroupToUsers: "22222": '{"GID":22222,"UIDs":[2222]}' "33333": '{"GID":33333,"UIDs":[3333]}' "44444": '{"GID":33333,"UIDs":[4444]}' - "99999": '{"GID":99999,"UIDs":[2222,3333]}' + "99999": '{"GID":99999,"UIDs":[2222,3333,4444]}' UserByID: "2222": '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' "3333": '{"Name":"user3","UID":3333,"GID":33333,"Gecos":"User3","Dir":"/home/user3","Shell":"/bin/zsh","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' @@ -36,5 +36,5 @@ UserToBroker: UserToGroups: "2222": '{"UID":2222,"GIDs":[22222,99999]}' "3333": '{"UID":3333,"GIDs":[33333,99999]}' - "4444": '{"UID":4444,"GIDs":[44444]}' + "4444": '{"UID":4444,"GIDs":[44444,99999]}' UserToLocalGroups: {} diff --git a/internal/users/cache/testdata/golden/TestNew/New_with_already_existing_database b/internal/users/cache/testdata/golden/TestNew/New_with_already_existing_database index a0804f90f..02ed6a050 100644 --- a/internal/users/cache/testdata/golden/TestNew/New_with_already_existing_database +++ b/internal/users/cache/testdata/golden/TestNew/New_with_already_existing_database @@ -21,7 +21,7 @@ GroupToUsers: "22222": '{"GID":22222,"UIDs":[2222]}' "33333": '{"GID":33333,"UIDs":[3333]}' "44444": '{"GID":33333,"UIDs":[4444]}' - "99999": '{"GID":99999,"UIDs":[2222,3333]}' + "99999": '{"GID":99999,"UIDs":[1111,2222,3333,4444]}' UserByID: "1111": '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"AAAAATIME"}' "2222": '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' @@ -37,8 +37,8 @@ UserToBroker: "2222": '"broker-id"' "3333": '"broker-id"' UserToGroups: - "1111": '{"UID":1111,"GIDs":[11111]}' + "1111": '{"UID":1111,"GIDs":[11111,99999]}' "2222": '{"UID":2222,"GIDs":[22222,99999]}' "3333": '{"UID":3333,"GIDs":[33333,99999]}' - "4444": '{"UID":4444,"GIDs":[44444]}' + "4444": '{"UID":4444,"GIDs":[44444,99999]}' UserToLocalGroups: {} diff --git a/internal/users/cache/testdata/golden/TestUpdateUserEntry/Add_user_to_group_from_another_user b/internal/users/cache/testdata/golden/TestUpdateUserEntry/Add_user_to_group_from_another_user index 8f64d43ca..591f040de 100644 --- a/internal/users/cache/testdata/golden/TestUpdateUserEntry/Add_user_to_group_from_another_user +++ b/internal/users/cache/testdata/golden/TestUpdateUserEntry/Add_user_to_group_from_another_user @@ -21,7 +21,7 @@ GroupToUsers: "22222": '{"GID":22222,"UIDs":[2222,1111]}' "33333": '{"GID":33333,"UIDs":[3333]}' "44444": '{"GID":33333,"UIDs":[4444]}' - "99999": '{"GID":99999,"UIDs":[2222,3333]}' + "99999": '{"GID":99999,"UIDs":[2222,3333,4444]}' UserByID: "1111": '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' "2222": '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' @@ -40,6 +40,6 @@ UserToGroups: "1111": '{"UID":1111,"GIDs":[11111,22222]}' "2222": '{"UID":2222,"GIDs":[22222,99999]}' "3333": '{"UID":3333,"GIDs":[33333,99999]}' - "4444": '{"UID":4444,"GIDs":[44444]}' + "4444": '{"UID":4444,"GIDs":[44444,99999]}' UserToLocalGroups: "1111": "null" diff --git a/internal/users/cache/testdata/golden/TestUpdateUserEntry/Remove_user_from_a_group_still_part_from_another_user b/internal/users/cache/testdata/golden/TestUpdateUserEntry/Remove_user_from_a_group_still_part_from_another_user index 5c1ed624a..e225b69d6 100644 --- a/internal/users/cache/testdata/golden/TestUpdateUserEntry/Remove_user_from_a_group_still_part_from_another_user +++ b/internal/users/cache/testdata/golden/TestUpdateUserEntry/Remove_user_from_a_group_still_part_from_another_user @@ -21,7 +21,7 @@ GroupToUsers: "22222": '{"GID":22222,"UIDs":[2222]}' "33333": '{"GID":33333,"UIDs":[3333]}' "44444": '{"GID":33333,"UIDs":[4444]}' - "99999": '{"GID":99999,"UIDs":[2222]}' + "99999": '{"GID":99999,"UIDs":[1111,2222,4444]}' UserByID: "1111": '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"AAAAATIME"}' "2222": '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' @@ -37,9 +37,9 @@ UserToBroker: "2222": '"broker-id"' "3333": '"broker-id"' UserToGroups: - "1111": '{"UID":1111,"GIDs":[11111]}' + "1111": '{"UID":1111,"GIDs":[11111,99999]}' "2222": '{"UID":2222,"GIDs":[22222,99999]}' "3333": '{"UID":3333,"GIDs":[33333]}' - "4444": '{"UID":4444,"GIDs":[44444]}' + "4444": '{"UID":4444,"GIDs":[44444,99999]}' UserToLocalGroups: "3333": "null" diff --git a/internal/users/cache/testdata/golden/TestUpdateUserEntry/Update_only_user_even_if_we_have_multiple_of_them b/internal/users/cache/testdata/golden/TestUpdateUserEntry/Update_only_user_even_if_we_have_multiple_of_them index 29bea64a5..131c35613 100644 --- a/internal/users/cache/testdata/golden/TestUpdateUserEntry/Update_only_user_even_if_we_have_multiple_of_them +++ b/internal/users/cache/testdata/golden/TestUpdateUserEntry/Update_only_user_even_if_we_have_multiple_of_them @@ -21,7 +21,7 @@ GroupToUsers: "22222": '{"GID":22222,"UIDs":[2222]}' "33333": '{"GID":33333,"UIDs":[3333]}' "44444": '{"GID":33333,"UIDs":[4444]}' - "99999": '{"GID":99999,"UIDs":[2222,3333]}' + "99999": '{"GID":99999,"UIDs":[2222,3333,4444]}' UserByID: "1111": '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' "2222": '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' @@ -40,6 +40,6 @@ UserToGroups: "1111": '{"UID":1111,"GIDs":[11111]}' "2222": '{"UID":2222,"GIDs":[22222,99999]}' "3333": '{"UID":3333,"GIDs":[33333,99999]}' - "4444": '{"UID":4444,"GIDs":[44444]}' + "4444": '{"UID":4444,"GIDs":[44444,99999]}' UserToLocalGroups: "1111": "null" diff --git a/internal/users/cache/testdata/multiple_users_and_groups.db.yaml b/internal/users/cache/testdata/multiple_users_and_groups.db.yaml index 6416cb1ee..1d9429395 100644 --- a/internal/users/cache/testdata/multiple_users_and_groups.db.yaml +++ b/internal/users/cache/testdata/multiple_users_and_groups.db.yaml @@ -21,7 +21,7 @@ GroupToUsers: "22222": '{"GID":22222,"UIDs":[2222]}' "33333": '{"GID":33333,"UIDs":[3333]}' "44444": '{"GID":33333,"UIDs":[4444]}' - "99999": '{"GID":99999,"UIDs":[2222,3333]}' + "99999": '{"GID":99999,"UIDs":[1111,2222,3333,4444]}' UserByID: "1111": '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"AAAAATIME"}' "2222": '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' @@ -33,10 +33,10 @@ UserByName: user3: '{"Name":"user3","UID":3333,"GID":33333,"Gecos":"User3","Dir":"/home/user3","Shell":"/bin/zsh","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' userwithoutbroker: '{"Name":"userwithoutbroker","UID":4444,"GID":44444,"Gecos":"userwithoutbroker","Dir":"/home/userwithoutbroker","Shell":"/bin/sh","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' UserToGroups: - "1111": '{"UID":1111,"GIDs":[11111]}' + "1111": '{"UID":1111,"GIDs":[11111,99999]}' "2222": '{"UID":2222,"GIDs":[22222,99999]}' "3333": '{"UID":3333,"GIDs":[33333,99999]}' - "4444": '{"UID":4444,"GIDs":[44444]}' + "4444": '{"UID":4444,"GIDs":[44444,99999]}' UserToBroker: "1111": '"broker-id"' "2222": '"broker-id"' diff --git a/internal/users/testdata/golden/TestAllGroups/Successfully_get_all_groups b/internal/users/testdata/golden/TestAllGroups/Successfully_get_all_groups index d44e911cb..f91102994 100644 --- a/internal/users/testdata/golden/TestAllGroups/Successfully_get_all_groups +++ b/internal/users/testdata/golden/TestAllGroups/Successfully_get_all_groups @@ -21,6 +21,8 @@ - name: commongroup gid: 99999 users: + - user1 - user2 - user3 + - userwithoutbroker passwd: "" diff --git a/internal/users/testdata/golden/TestNewManager/Successfully_create_manager_with_custom_config b/internal/users/testdata/golden/TestNewManager/Successfully_create_manager_with_custom_config index a0804f90f..02ed6a050 100644 --- a/internal/users/testdata/golden/TestNewManager/Successfully_create_manager_with_custom_config +++ b/internal/users/testdata/golden/TestNewManager/Successfully_create_manager_with_custom_config @@ -21,7 +21,7 @@ GroupToUsers: "22222": '{"GID":22222,"UIDs":[2222]}' "33333": '{"GID":33333,"UIDs":[3333]}' "44444": '{"GID":33333,"UIDs":[4444]}' - "99999": '{"GID":99999,"UIDs":[2222,3333]}' + "99999": '{"GID":99999,"UIDs":[1111,2222,3333,4444]}' UserByID: "1111": '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"AAAAATIME"}' "2222": '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' @@ -37,8 +37,8 @@ UserToBroker: "2222": '"broker-id"' "3333": '"broker-id"' UserToGroups: - "1111": '{"UID":1111,"GIDs":[11111]}' + "1111": '{"UID":1111,"GIDs":[11111,99999]}' "2222": '{"UID":2222,"GIDs":[22222,99999]}' "3333": '{"UID":3333,"GIDs":[33333,99999]}' - "4444": '{"UID":4444,"GIDs":[44444]}' + "4444": '{"UID":4444,"GIDs":[44444,99999]}' UserToLocalGroups: {} diff --git a/internal/users/testdata/golden/TestNewManager/Successfully_create_manager_with_default_config b/internal/users/testdata/golden/TestNewManager/Successfully_create_manager_with_default_config index a0804f90f..02ed6a050 100644 --- a/internal/users/testdata/golden/TestNewManager/Successfully_create_manager_with_default_config +++ b/internal/users/testdata/golden/TestNewManager/Successfully_create_manager_with_default_config @@ -21,7 +21,7 @@ GroupToUsers: "22222": '{"GID":22222,"UIDs":[2222]}' "33333": '{"GID":33333,"UIDs":[3333]}' "44444": '{"GID":33333,"UIDs":[4444]}' - "99999": '{"GID":99999,"UIDs":[2222,3333]}' + "99999": '{"GID":99999,"UIDs":[1111,2222,3333,4444]}' UserByID: "1111": '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"AAAAATIME"}' "2222": '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' @@ -37,8 +37,8 @@ UserToBroker: "2222": '"broker-id"' "3333": '"broker-id"' UserToGroups: - "1111": '{"UID":1111,"GIDs":[11111]}' + "1111": '{"UID":1111,"GIDs":[11111,99999]}' "2222": '{"UID":2222,"GIDs":[22222,99999]}' "3333": '{"UID":3333,"GIDs":[33333,99999]}' - "4444": '{"UID":4444,"GIDs":[44444]}' + "4444": '{"UID":4444,"GIDs":[44444,99999]}' UserToLocalGroups: {} diff --git a/internal/users/testdata/golden/TestUpdateBrokerForUser/Successfully_update_broker_for_user b/internal/users/testdata/golden/TestUpdateBrokerForUser/Successfully_update_broker_for_user index 984fbd050..ee2d172e2 100644 --- a/internal/users/testdata/golden/TestUpdateBrokerForUser/Successfully_update_broker_for_user +++ b/internal/users/testdata/golden/TestUpdateBrokerForUser/Successfully_update_broker_for_user @@ -22,7 +22,7 @@ "22222": '{"GID":22222,"UIDs":[2222]}' "33333": '{"GID":33333,"UIDs":[3333]}' "44444": '{"GID":33333,"UIDs":[4444]}' - "99999": '{"GID":99999,"UIDs":[2222,3333]}' + "99999": '{"GID":99999,"UIDs":[1111,2222,3333,4444]}' UserByID: "1111": '{"Name":"user1","UID":1111,"GID":11111,"Gecos":"User1 gecos\nOn multiple lines","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"AAAAATIME"}' "2222": '{"Name":"user2","UID":2222,"GID":22222,"Gecos":"User2","Dir":"/home/user2","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"BBBBBTIME"}' @@ -38,8 +38,8 @@ "2222": '"broker-id"' "3333": '"broker-id"' UserToGroups: - "1111": '{"UID":1111,"GIDs":[11111]}' + "1111": '{"UID":1111,"GIDs":[11111,99999]}' "2222": '{"UID":2222,"GIDs":[22222,99999]}' "3333": '{"UID":3333,"GIDs":[33333,99999]}' - "4444": '{"UID":4444,"GIDs":[44444]}' + "4444": '{"UID":4444,"GIDs":[44444,99999]}' UserToLocalGroups: {} From 79bfb85de4740406bd421314d48caf768c1d5c65 Mon Sep 17 00:00:00 2001 From: Adrian Dombeck Date: Tue, 28 Jan 2025 11:17:29 +0100 Subject: [PATCH 4/6] Rename test case To make it clear that it tests keeping other group members intact. --- internal/users/cache/db_test.go | 4 ++-- ...> Deleting_existing_user_keeps_other_group_members_intact} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename internal/users/cache/testdata/golden/TestDeleteUser/{Delete_existing_user_keeping_other_users_intact => Deleting_existing_user_keeps_other_group_members_intact} (100%) diff --git a/internal/users/cache/db_test.go b/internal/users/cache/db_test.go index d5641ef35..ec1ee1e0f 100644 --- a/internal/users/cache/db_test.go +++ b/internal/users/cache/db_test.go @@ -484,8 +484,8 @@ func TestDeleteUser(t *testing.T) { wantErr bool wantErrType error }{ - "Delete_existing_user": {dbFile: "one_user_and_group"}, - "Delete_existing_user_keeping_other_users_intact": {dbFile: "multiple_users_and_groups"}, + "Delete_existing_user": {dbFile: "one_user_and_group"}, + "Deleting_existing_user_keeps_other_group_members_intact": {dbFile: "multiple_users_and_groups"}, "Error_on_missing_user": {wantErrType: cache.NoDataFoundError{}}, "Error_on_invalid_database_entry": {dbFile: "invalid_entry_in_userByID", wantErr: true}, diff --git a/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user_keeping_other_users_intact b/internal/users/cache/testdata/golden/TestDeleteUser/Deleting_existing_user_keeps_other_group_members_intact similarity index 100% rename from internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user_keeping_other_users_intact rename to internal/users/cache/testdata/golden/TestDeleteUser/Deleting_existing_user_keeps_other_group_members_intact From c6f2b4fab217d9e684251adbe777b5dec00b5374 Mon Sep 17 00:00:00 2001 From: Adrian Dombeck Date: Tue, 28 Jan 2025 11:19:05 +0100 Subject: [PATCH 5/6] Rename test case To make it clear that it tests keeping the group record when the last user of a group is deleted. --- internal/users/cache/db_test.go | 2 +- ...r => Deleting_last_user_from_a_group_keeps_the_group_record} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename internal/users/cache/testdata/golden/TestDeleteUser/{Delete_existing_user => Deleting_last_user_from_a_group_keeps_the_group_record} (100%) diff --git a/internal/users/cache/db_test.go b/internal/users/cache/db_test.go index ec1ee1e0f..45adb8cc9 100644 --- a/internal/users/cache/db_test.go +++ b/internal/users/cache/db_test.go @@ -484,7 +484,7 @@ func TestDeleteUser(t *testing.T) { wantErr bool wantErrType error }{ - "Delete_existing_user": {dbFile: "one_user_and_group"}, + "Deleting_last_user_from_a_group_keeps_the_group_record": {dbFile: "one_user_and_group"}, "Deleting_existing_user_keeps_other_group_members_intact": {dbFile: "multiple_users_and_groups"}, "Error_on_missing_user": {wantErrType: cache.NoDataFoundError{}}, diff --git a/internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user b/internal/users/cache/testdata/golden/TestDeleteUser/Deleting_last_user_from_a_group_keeps_the_group_record similarity index 100% rename from internal/users/cache/testdata/golden/TestDeleteUser/Delete_existing_user rename to internal/users/cache/testdata/golden/TestDeleteUser/Deleting_last_user_from_a_group_keeps_the_group_record From eac7af1ce2576790669faab9d956275bf2a61f2a Mon Sep 17 00:00:00 2001 From: Adrian Dombeck Date: Tue, 28 Jan 2025 11:23:05 +0100 Subject: [PATCH 6/6] Also test removing last user from a group in TestUpdateUser --- internal/users/manager_test.go | 1 + ...t_user_from_a_group_keeps_the_group_record | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 internal/users/testdata/golden/TestUpdateUser/Removing_last_user_from_a_group_keeps_the_group_record diff --git a/internal/users/manager_test.go b/internal/users/manager_test.go index b094918d9..4bde9c2f7 100644 --- a/internal/users/manager_test.go +++ b/internal/users/manager_test.go @@ -160,6 +160,7 @@ func TestUpdateUser(t *testing.T) { "UID_does_not_change_if_user_already_exists": {userCase: "same-name-different-uid", dbFile: "one_user_and_group", wantSameUID: true}, "GID_does_not_change_if_group_with_same_UGID_exists": {groupsCase: "different-name-same-ugid", dbFile: "one_user_and_group"}, "GID_does_not_change_if_group_with_same_name_and_empty_UGID_exists": {groupsCase: "authd-group", dbFile: "group-with-empty-UGID"}, + "Removing_last_user_from_a_group_keeps_the_group_record": {groupsCase: "no-groups", dbFile: "one_user_and_group"}, "Error_if_user_has_no_username": {userCase: "nameless", wantErr: true, noOutput: true}, "Error_if_group_has_no_name": {groupsCase: "nameless-group", wantErr: true, noOutput: true}, diff --git a/internal/users/testdata/golden/TestUpdateUser/Removing_last_user_from_a_group_keeps_the_group_record b/internal/users/testdata/golden/TestUpdateUser/Removing_last_user_from_a_group_keeps_the_group_record new file mode 100644 index 000000000..5e79e3ce1 --- /dev/null +++ b/internal/users/testdata/golden/TestUpdateUser/Removing_last_user_from_a_group_keeps_the_group_record @@ -0,0 +1,23 @@ +| + GroupByID: + "11110": '{"Name":"user1","GID":11110,"UGID":"user1"}' + "11111": '{"Name":"group1","GID":11111,"UGID":"12345678"}' + GroupByName: + group1: '{"Name":"group1","GID":11111,"UGID":"12345678"}' + user1: '{"Name":"user1","GID":11110,"UGID":"user1"}' + GroupByUGID: + "12345678": '{"Name":"group1","GID":11111,"UGID":"12345678"}' + user1: '{"Name":"user1","GID":11110,"UGID":"user1"}' + GroupToUsers: + "11110": '{"GID":11110,"UIDs":[1111]}' + "11111": '{"GID":11111,"UIDs":[]}' + UserByID: + "1111": '{"Name":"user1","UID":1111,"GID":11110,"Gecos":"gecos for user1","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":11110,"Gecos":"gecos for user1","Dir":"/home/user1","Shell":"/bin/bash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}' + UserToBroker: + "1111": '"broker-id"' + UserToGroups: + "1111": '{"UID":1111,"GIDs":[11110]}' + UserToLocalGroups: + "1111": "null"