Skip to content

Commit

Permalink
fix(cache): Do not change homedir for user that already exists (#399)
Browse files Browse the repository at this point in the history
We used to update every part of the user information in the cache but
this can create problems if the admin changes the homedir configuration
on the broker side, since it would update the homedir parsing for the
user resulting in PAM creating a new one for the user at login.

UDENG-3125
  • Loading branch information
denisonbarbosa committed Jun 27, 2024
2 parents 5132a3f + 215739a commit a44a3e4
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 2 deletions.
10 changes: 10 additions & 0 deletions internal/users/cache/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,15 @@ func TestUpdateUserEntry(t *testing.T) {
// These values don't matter. We just want to make sure they are the same as the ones provided by the manager.
LastPwdChange: -1, MaxPwdAge: -1, PwdWarnPeriod: -1, PwdInactivity: -1, MinPwdAge: -1, ExpirationDate: -1,
},
"user1-new-homedir": {
Name: "user1",
UID: 1111,
Gecos: "User1 gecos\nOn multiple lines",
Dir: "/new/home/user1",
Shell: "/bin/bash",
// These values don't matter. We just want to make sure they are the same as the ones provided by the manager.
LastPwdChange: -1, MaxPwdAge: -1, PwdWarnPeriod: -1, PwdInactivity: -1, MinPwdAge: -1, ExpirationDate: -1,
},
"user1-without-gecos": {
Name: "user1",
UID: 1111,
Expand Down Expand Up @@ -154,6 +163,7 @@ func TestUpdateUserEntry(t *testing.T) {

// User and Group renames
"Update user by changing attributes": {userCase: "user1-new-attributes", dbFile: "one_user_and_group"},
"Update user does not change homedir if it exists": {userCase: "user1-new-homedir", 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": {groupCases: []string{"newgroup1"}, dbFile: "one_user_and_group"},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ GroupByName:
GroupToUsers:
"11111": '{"GID":11111,"UIDs":[1111]}'
UserByID:
"1111": '{"Name":"newuser1","UID":1111,"GID":11111,"Gecos":"New user1 gecos","Dir":"/home/newuser1","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
"1111": '{"Name":"newuser1","UID":1111,"GID":11111,"Gecos":"New user1 gecos","Dir":"/home/user1","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
UserByName:
newuser1: '{"Name":"newuser1","UID":1111,"GID":11111,"Gecos":"New user1 gecos","Dir":"/home/newuser1","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
newuser1: '{"Name":"newuser1","UID":1111,"GID":11111,"Gecos":"New user1 gecos","Dir":"/home/user1","Shell":"/bin/dash","LastPwdChange":-1,"MaxPwdAge":-1,"PwdWarnPeriod":-1,"PwdInactivity":-1,"MinPwdAge":-1,"ExpirationDate":-1,"LastLogin":"ABCDETIME"}'
UserToBroker:
"1111": '"broker-id"'
UserToGroups:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
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":"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"}'
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":"ABCDETIME"}'
UserToBroker:
"1111": '"broker-id"'
UserToGroups:
"1111": '{"UID":1111,"GIDs":[11111]}'
6 changes: 6 additions & 0 deletions internal/users/cache/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ func updateUser(buckets map[string]bucketWithName, userContent userDB) {
_ = buckets[userByNameBucketName].Delete([]byte(existingUser.Name)) // No error as we are not in a RO transaction.
}

// Ensure that we use the same homedir as the one we have in cache.
if existingUser.Dir != "" && existingUser.Dir != userContent.Dir {
slog.Warn(fmt.Sprintf("User %q already has a homedir. The existing %q one will be kept instead of %q", userContent.Name, existingUser.Dir, userContent.Dir))
userContent.Dir = existingUser.Dir
}

// Update user buckets
updateBucket(buckets[userByIDBucketName], userContent.UID, userContent)
updateBucket(buckets[userByNameBucketName], userContent.Name, userContent)
Expand Down

0 comments on commit a44a3e4

Please sign in to comment.