Skip to content

Commit

Permalink
Don't delete empty groups from database
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
adombeck committed Jan 27, 2025
1 parent 955bdeb commit 54e1f1a
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 30 deletions.
28 changes: 2 additions & 26 deletions internal/users/cache/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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: {}
Expand Down
Original file line number Diff line number Diff line change
@@ -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]}'
Expand Down
Original file line number Diff line number Diff line change
@@ -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"}'
Expand Down
Original file line number Diff line number Diff line change
@@ -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:
Expand Down

0 comments on commit 54e1f1a

Please sign in to comment.