Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions e2e-tests/tests/authctl_group_set_gid.robot
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
*** Settings ***
Resource ./resources/authd/utils.resource
Resource ./resources/authd/authd.resource
Resource ./resources/broker/broker.resource

# Test Tags robot:exit-on-failure

Test Setup utils.Test Setup ${snapshot}
Test Teardown utils.Test Teardown


*** Variables ***
${snapshot} %{BROKER}-installed
${username} %{E2E_USER}
${local_password} qwer1234
${new_gid} 60500


*** Test Cases ***
Test authctl group set-gid
[Documentation] Test that authctl group set-gid changes the GID of a remote group
... and updates the home directory ownership.

Log In

Open Terminal
Log In With Remote User Through CLI: QR Code ${username} ${local_password}
Check If User Was Added Properly ${username}
Log Out From Terminal Session
Close Focused Window

# No session termination needed here: unlike set-uid (which calls
# proc.CheckUserBusy), set-gid does not check for running processes.

${group_name} = SSH.Execute id -gn ${username}
Should Not Be Empty ${group_name}

${home_dir} = SSH.Execute getent passwd ${username} | cut -d: -f6
Should Not Be Empty ${home_dir}
SSH.Execute sudo -u ${username} touch ${home_dir}/test-file

${output} = SSH.Execute sudo authctl group set-gid ${group_name} ${new_gid} 2>&1
Should Contain ${output} GID of group '${group_name}' set to ${new_gid}.

${actual_gid} = SSH.Execute getent group ${group_name} | cut -d: -f3
Should Be Equal As Strings ${actual_gid} ${new_gid}

${reverse_lookup} = SSH.Execute getent group ${new_gid} | cut -d: -f1
Should Be Equal As Strings ${reverse_lookup} ${group_name}

${passwd_gid} = SSH.Execute getent passwd ${username} | cut -d: -f4
Should Be Equal As Strings ${passwd_gid} ${new_gid}

${home_gid} = SSH.Execute stat -c %g ${home_dir}
Should Be Equal As Strings ${home_gid} ${new_gid}
${file_gid} = SSH.Execute sudo stat -c %g ${home_dir}/test-file
Should Be Equal As Strings ${file_gid} ${new_gid}

Open Terminal
Log In With Remote User Through CLI: Local Password ${username} ${local_password}
${post_login_gid} = SSH.Execute getent passwd ${username} | cut -d: -f4
Should Be Equal As Strings ${post_login_gid} ${new_gid}
Log Out From Terminal Session
Close Focused Window
65 changes: 65 additions & 0 deletions e2e-tests/tests/authctl_user_set_uid.robot
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
*** Settings ***
Resource ./resources/authd/utils.resource
Resource ./resources/authd/authd.resource
Resource ./resources/broker/broker.resource

# Test Tags robot:exit-on-failure

Test Setup utils.Test Setup ${snapshot}
Test Teardown utils.Test Teardown


*** Variables ***
${snapshot} %{BROKER}-installed
${username} %{E2E_USER}
${local_password} qwer1234
${new_uid} 60500


*** Test Cases ***
Test authctl user set-uid
[Documentation] Test that authctl user set-uid changes the UID of a remote user
... and updates the home directory ownership.

Log In

Open Terminal
Log In With Remote User Through CLI: QR Code ${username} ${local_password}
Check If User Was Added Properly ${username}
Log Out From Terminal Session
Close Focused Window

${home_dir} = SSH.Execute getent passwd ${username} | cut -d: -f6
Should Not Be Empty ${home_dir}
SSH.Execute sudo -u ${username} touch ${home_dir}/test-file

# Terminate the remote user's session so that proc.CheckUserBusy (which
# rejects set-uid when any process runs under that UID) does not block the
# operation. Use loginctl to tear down the session gracefully, then poll
# until all processes have exited rather than relying on a hard sleep.
SSH.Execute sudo loginctl terminate-user ${username} || true
Wait Until Keyword Succeeds 30s 1s SSH.Execute test -z "$(pgrep -u ${username})"

${output} = SSH.Execute sudo authctl user set-uid ${username} ${new_uid} 2>&1
Should Contain ${output} UID of user '${username}' set to ${new_uid}.

${actual_uid} = SSH.Execute getent passwd ${username} | cut -d: -f3
Should Be Equal As Strings ${actual_uid} ${new_uid}

${reverse_lookup} = SSH.Execute getent passwd ${new_uid} | cut -d: -f1
Should Be Equal As Strings ${reverse_lookup} ${username}

${id_output} = SSH.Execute id ${username}
Should Contain ${id_output} uid=${new_uid}

${home_uid} = SSH.Execute stat -c %u ${home_dir}
Should Be Equal As Strings ${home_uid} ${new_uid}
${file_uid} = SSH.Execute sudo stat -c %u ${home_dir}/test-file
Should Be Equal As Strings ${file_uid} ${new_uid}

Open Terminal
Log In With Remote User Through CLI: Local Password ${username} ${local_password}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To check that the ownership of files is changed recursively, we could create a file (e.g. touch foo) above, in line 27, and then check the ownership of that file here. Same in the set-gid test.

${id_uid} = SSH.Execute id -u ${username}
Should Be Equal As Strings ${id_uid} ${new_uid}
Log Out From Terminal Session
Close Focused Window
11 changes: 9 additions & 2 deletions internal/users/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,15 @@ func (m *Manager) UpdateUser(u types.UserInfo) (err error) {
}
}

// User private Group GID is the same of the user UID.
userPrivateGroup.GID = &u.UID
// Preserve any admin-set GID for the UPG (e.g. via 'authctl group set-gid').
// On first login the UPG doesn't exist yet, so default to GID = UID.
if existingUPG, err := m.findGroup(*userPrivateGroup); err == nil {
userPrivateGroup.GID = &existingUPG.GID
} else if !errors.Is(err, db.NoDataFoundError{}) {
return err
} else {
userPrivateGroup.GID = &u.UID
}

var groupRows []db.GroupRow
var localGroups []string
Expand Down
49 changes: 49 additions & 0 deletions internal/users/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1342,6 +1342,55 @@ func TestUpdateUserAfterUnlock(t *testing.T) {
require.NoError(t, err, "UpdateUser should not fail")
}

// TestUpdateUser_PreservesUPGGIDAfterSetGroupID is a regression test: UpdateUser must not
// revert a UPG GID that was explicitly changed via 'authctl group set-gid'.
func TestUpdateUser_PreservesUPGGIDAfterSetGroupID(t *testing.T) {
t.Parallel()

const (
username = "testuser@example.com"
initUID = uint32(1111)
newGID = uint32(60500)
)

dbDir := t.TempDir()
m := newManagerForTests(t, dbDir, users.WithIDGenerator(&users.IDGeneratorMock{
UIDsToGenerate: []uint32{initUID},
}))

// First login: creates the user and its UPG with GID == UID.
userInfo := types.UserInfo{
Name: username,
Dir: "/home/" + username,
Shell: "/bin/bash",
}
err := m.UpdateUser(userInfo)
require.NoError(t, err)

user, err := m.DB().UserByName(username)
require.NoError(t, err)
group, err := m.DB().GroupByName(username)
require.NoError(t, err)
require.EqualValues(t, initUID, user.GID)
require.EqualValues(t, initUID, group.GID)

// Simulate 'authctl group set-gid'.
_, err = m.DB().SetGroupID(username, newGID)
require.NoError(t, err)

// Re-login with a changed attribute to force the full UpdateUser path.
userInfo.Shell = "/bin/zsh"
err = m.UpdateUser(userInfo)
require.NoError(t, err)

user, err = m.DB().UserByName(username)
require.NoError(t, err)
group, err = m.DB().GroupByName(username)
require.NoError(t, err)
require.EqualValues(t, newGID, user.GID, "users.gid must not revert to UID after re-login")
require.EqualValues(t, newGID, group.GID, "groups.gid must not revert to UID after re-login")
}

func requireErrorAssertions(t *testing.T, gotErr, wantErrType error, wantErr bool) {
t.Helper()

Expand Down
Loading