Skip to content

Commit

Permalink
Do not change homedir for user that already exists
Browse files Browse the repository at this point in the history
We used to update every part of the user information in cache, but this
can create problems if the admin changes the homedir configuration in
the broker side, since it would update the homedir parsing for the user
and, thus, "losing" access to the previous one.
  • Loading branch information
denisonbarbosa committed Jun 27, 2024
1 parent 941686c commit f47d69e
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 != "" {
slog.Warn(fmt.Sprintf("User %q already has a homedir. The existing %q one will be kept", userContent.Name, existingUser.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 f47d69e

Please sign in to comment.