WIP: Allow short usernames#1501
Draft
denisonbarbosa wants to merge 4 commits intomainfrom
Draft
Conversation
We used to check this on the PAM service, but the user manager should be responsible for this kind of control.
In order to support shortened usernames authentication, we need to make sure we still know what the fully qualified username looks like. This way, we can make sure we still can communicate effectively with the broker. Since up until now we only allowed users to authenticate with their fully qualified username, we can safely populate this column with the usernames of the existing users.
This adds a configuration setting to authd that allow users to authenticate using a short username rather than the FQDN'd one. This affects both user and homedir creation, but won't change existing users.
Since we require the user to provide their full username on the first authentication, we need a way to update the PAM_USER after the username is shortened, otherwise other helper modules (i.e. mkhomedir) will fail to run. This is marked as a WIP as it needs more discussion and we should probably figure out a better way to deal with this issue, as these changes have a clear side effect on the final message that we present to the user.
Contributor
There was a problem hiding this comment.
Pull request overview
This draft PR adds an opt-in configuration path for treating “short” usernames (without a domain suffix) as the canonical username within authd, while still tracking the broker-provided fully-qualified username for correlation and compatibility.
Changes:
- Add
use_short_usernamesconfiguration and plumb it through PAM service + integration tests. - Extend the users DB schema/rows to include
full_usernameand add lookups by full username. - Update PAM success handling so the authenticated (potentially shortened) username is propagated back into the PAM transaction (
pam.User), and add/refresh extensive golden + tape coverage.
Reviewed changes
Copilot reviewed 207 out of 207 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pam/pam.go | Stop printing PAM success messages; set PAM username from success payload. |
| pam/integration-tests/testdata/tapes/ssh/short_username.tape | New SSH tape for short-username success. |
| pam/integration-tests/testdata/tapes/ssh/short_username_not_allowed.tape | New SSH tape for short-username denial. |
| pam/integration-tests/testdata/tapes/ssh/short_username_existing.tape | New SSH tape for existing short-username user. |
| pam/integration-tests/testdata/tapes/native/short_username.tape | Update native tape to match new short-username flow. |
| pam/integration-tests/testdata/tapes/native/short_username_not_allowed.tape | New native tape for short-username denial. |
| pam/integration-tests/testdata/tapes/native/short_username_existing.tape | New native tape for existing short-username user. |
| pam/integration-tests/testdata/tapes/cli/short_username.tape | New CLI tape for short-username success. |
| pam/integration-tests/testdata/tapes/cli/short_username_not_allowed.tape | New CLI tape for short-username denial. |
| pam/integration-tests/testdata/tapes/cli/short_username_existing.tape | New CLI tape for existing short-username user. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Remember_last_successful_broker_and_mode_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Remember_last_successful_broker_and_mode | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Prevent_user_from_switching_username_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Prevent_user_from_switching_username | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Deny_authentication_if_newpassword_does_not_match_required_criteria_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Deny_authentication_if_newpassword_does_not_match_required_criteria | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_with_short_username_with_shared_sshd | New golden for short-username SSH auth. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_with_short_username | New golden for short-username SSH auth. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_with_qr_code_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_with_qr_code | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_with_mfa_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_with_mfa_and_reset_same_password_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_with_mfa_and_reset_same_password | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_with_mfa_and_reset_password_while_enforcing_policy_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_with_mfa_and_reset_password_while_enforcing_policy | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_with_mfa | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_with_form_mode_with_button_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_with_form_mode_with_button | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_switching_auth_mode | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_successfully_with_upper_case_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_successfully_with_upper_case | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_successfully_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_successfully_if_already_registered_with_upper_case_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_successfully_if_already_registered_with_upper_case | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_successfully_if_already_registered_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_successfully_if_already_registered | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_successfully_and_enters_shell_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_successfully_and_enters_shell | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_successfully | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_locks_and_unlocks_it_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_locks_and_unlocks_it | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_and_reset_password_with_case_insensitive_user_selection | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_and_reset_password_while_enforcing_policy_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_and_reset_password_while_enforcing_policy | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_and_offer_password_reset_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_and_offer_password_reset | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_and_add_it_to_local_group_with_shared_sshd | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_and_add_it_to_local_group | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_user_and_accept_password_reset | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_existing_user_with_short_username_with_shared_sshd | New golden for existing short-username SSH auth. |
| pam/integration-tests/testdata/golden/TestSSHAuthenticate/Authenticate_existing_user_with_short_username | New golden for existing short-username SSH auth. |
| pam/integration-tests/testdata/golden/TestNativeChangeAuthTok/Retry_if_new_password_is_same_of_previous | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeChangeAuthTok/Retry_if_new_password_is_rejected_by_broker | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeChangeAuthTok/Change_password_successfully_and_authenticate_with_new_one_with_single_broker_and_password_only_supported_method | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeChangeAuthTok/Change_password_successfully_and_authenticate_with_new_one_with_different_case | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeChangeAuthTok/Change_password_successfully_and_authenticate_with_new_one | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeChangeAuthTok/Change_passwd_after_MFA_auth | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Remember_last_successful_broker_and_mode | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Prevent_preset_user_from_switching_username | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Deny_authentication_if_short_username_is_used_but_not_allowed | New golden for short-username denial. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Deny_authentication_if_short_username_is_allowed_but_user_does_not_exist | New golden for short-username “first auth must be full” denial. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Deny_authentication_if_newpassword_does_not_match_required_criteria | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_with_warnings_on_unsupported_arguments | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_with_short_username | Golden update to new short-username behavior. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_with_qr_code_in_ssh | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_with_qr_code_in_screen | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_with_qr_code_in_a_TTY_session | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_with_qr_code_in_a_TTY | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_with_qr_code | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_with_mfa_and_reset_same_password | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_with_mfa_and_reset_password_while_enforcing_policy | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_with_mfa | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_with_form_mode_with_button_two_supported_methods | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_with_form_mode_with_button_in_polkit | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_with_form_mode_with_button | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_switching_username | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_switching_auth_mode | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_successfully_with_user_selection | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_successfully_with_upper_case | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_successfully_with_password_only_supported_method_in_polkit | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_successfully_with_password_only_supported_method | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_successfully_with_invalid_connection_timeout | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_successfully_using_upper_case_with_user_selection | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_successfully | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_on_ssh_service_with_custom_name_and_connection_env | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_on_ssh_service_with_custom_name_and_auth_info_env | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_on_ssh_service | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_and_reset_password_with_case_insensitive_user_selection | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_and_reset_password_while_enforcing_policy | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_and_offer_password_reset | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_and_add_it_to_local_group | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_user_and_accept_password_reset | Golden update for new PAM output. |
| pam/integration-tests/testdata/golden/TestNativeAuthenticate/Authenticate_existing_user_with_short_username | New golden for existing short-username native auth. |
| pam/integration-tests/testdata/golden/TestCLIAuthenticate/Deny_authentication_if_short_username_is_used_but_not_allowed | New golden for CLI short-username denial. |
| pam/integration-tests/testdata/golden/TestCLIAuthenticate/Deny_authentication_if_short_username_is_allowed_but_user_does_not_exist | New golden for CLI “first auth must be full” denial. |
| pam/integration-tests/testdata/golden/TestCLIAuthenticate/Authenticate_user_with_short_username | New golden for CLI short-username success. |
| pam/integration-tests/testdata/golden/TestCLIAuthenticate/Authenticate_existing_user_with_short_username | New golden for existing short-username CLI auth. |
| pam/integration-tests/testdata/db/db_with_short_username.db.yaml | New seeded DB for existing short-username users. |
| pam/integration-tests/ssh_test.go | Add short-username SSH test cases and daemon/sshd plumbing. |
| pam/integration-tests/native_test.go | Add short-username native test cases and daemon plumbing. |
| pam/integration-tests/helpers_test.go | Add helper to prepare an existing DB fixture. |
| pam/integration-tests/cli_test.go | Add short-username CLI test cases and daemon plumbing. |
| internal/users/testdata/golden/TestUpdateUser/UID_does_not_change_if_user_already_exists | Golden updated for full_username field. |
| internal/users/testdata/golden/TestUpdateUser/Successfully_update_user_using_shortened_username | New golden for short-username user update. |
| internal/users/testdata/golden/TestUpdateUser/Successfully_update_user_updating_local_groups_with_changes | Golden updated for full_username field. |
| internal/users/testdata/golden/TestUpdateUser/Successfully_update_user_updating_local_groups | Golden updated for full_username field. |
| internal/users/testdata/golden/TestUpdateUser/Successfully_update_user | Golden updated for full_username field. |
| internal/users/testdata/golden/TestUpdateUser/Removing_last_user_from_a_group_keeps_the_group_record | Golden updated for full_username field. |
| internal/users/testdata/golden/TestUpdateUser/GID_does_not_change_if_group_with_same_UGID_exists | Golden updated for full_username field. |
| internal/users/testdata/golden/TestUpdateUser/GID_does_not_change_if_group_with_same_name_and_empty_UGID_exists | Golden updated for full_username field. |
| internal/users/testdata/golden/TestUpdateUser/Allow_login_with_existing_group_on_system | Golden updated for full_username field. |
| internal/users/testdata/golden/TestUpdateBrokerForUser/Successfully_update_broker_for_user | Golden updated for full_username field. |
| internal/users/testdata/golden/TestUnlockUser/Successfully_enable_user | Golden updated for full_username field. |
| internal/users/testdata/golden/TestSetUserID/Warning_if_user_already_has_given_UID/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetUserID/Warning_if_home_directory_is_owned_by_other_user/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetUserID/Warning_if_home_directory_cannot_be_accessed/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetUserID/Successfully_set_UID/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetUserID/Successfully_set_UID_when_home_directory_does_not_exist/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetUserID/Successfully_set_UID_if_ID_is_already_in_use_as_GID_of_system_user/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetUserID/Successfully_set_UID_if_ID_is_already_in_use_as_GID_of_authd_user/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetGroupID/Warning_if_home_directory_is_owned_by_other_group/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetGroupID/Warning_if_home_directory_cannot_be_accessed/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetGroupID/Warning_if_group_already_has_given_GID/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetGroupID/Successfully_set_GID/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetGroupID/Successfully_set_GID_when_home_directory_does_not_exist/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetGroupID/Successfully_set_GID_when_group_is_not_primary_group_of_any_user/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetGroupID/Successfully_set_GID_if_ID_is_already_in_use_as_UID_of_system_user/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetGroupID/Successfully_set_GID_if_ID_is_already_in_use_as_UID_of_authd_user/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestSetGroupID/Primary_groups_of_multiple_users_are_updated/db | Golden DB updated for full_username field. |
| internal/users/testdata/golden/TestNewManager/Warns_creating_manager_with_partially_invalid_UID_ranges | Golden updated for full_username field. |
| internal/users/testdata/golden/TestNewManager/Warns_creating_manager_with_partially_invalid_GID_ranges | Golden updated for full_username field. |
| internal/users/testdata/golden/TestNewManager/Successfully_create_manager_with_UID_range_next_to_systemd_dynamic_users | Golden updated for full_username field. |
| internal/users/testdata/golden/TestNewManager/Successfully_create_manager_with_GID_range_next_to_systemd_dynamic_groups | Golden updated for full_username field. |
| internal/users/testdata/golden/TestNewManager/Successfully_create_manager_with_default_config | Golden updated for full_username field. |
| internal/users/testdata/golden/TestNewManager/Successfully_create_manager_with_custom_config | Golden updated for full_username field. |
| internal/users/testdata/golden/TestLockUser/Successfully_lock_user | Golden updated for full_username field. |
| internal/users/manager.go | Add short-username config, full_username handling, and new manager APIs. |
| internal/users/manager_test.go | Update tests to require domains unless short usernames enabled; add new cases. |
| internal/users/manager_bwrap_test.go | Update helper usage for new manager constructor signature. |
| internal/users/defs.go | Add UserIsLockedError type to propagate lock errors. |
| internal/users/db/users.go | Add full_username to user rows, queries, and add lookup by full username. |
| internal/users/db/update.go | Allow broker updates by name or full_username. |
| internal/users/db/sql/create_schema.sql | Add full_username column to schema. |
| internal/users/db/migration.go | Add helper to backfill full_username values. |
| internal/users/db/db.go | Backfill full_username after opening DB. |
| internal/users/db/testdata/TestMigrationToLowercaseUserAndGroupNames/one_users_multiple_groups_with_uppercase.sql | Update test SQL schema/data for full_username. |
| internal/users/db/testdata/TestMigrationToLowercaseUserAndGroupNames/one_users_multiple_groups_fully_uppercase.sql | Update test SQL schema/data for full_username. |
| internal/users/db/testdata/TestMigrationAddLockedColumnToUsersTable/one_user_and_group_without_locked_column.sql | Update test SQL schema/data for full_username. |
| internal/users/db/testdata/TestLoadSchemaV2WithIntUGID/one_user_and_group_v2.sql | Update test SQL schema for full_username. |
| internal/users/db/testdata/golden/TestUserByName/Get_existing_user | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUserByID/Get_existing_user | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Update_user_does_not_change_shell_if_it_exists | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Update_user_does_not_change_homedir_if_it_exists | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Update_user_by_renaming_a_group | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Update_user_by_removing_optional_gecos_field_if_not_set | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Update_user_by_changing_attributes | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Update_user_by_adding_a_new_local_group | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Update_user_by_adding_a_new_group | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Update_user_by_adding_a_new_default_group | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Update_only_user_even_if_we_have_multiple_of_them | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Remove_user_from_a_group_still_part_from_another_user | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Remove_group_from_user | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Insert_new_user_without_optional_gecos_field | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Insert_new_user | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestUpdateUserEntry/Add_user_to_group_from_another_user | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestSetUserID/Set_user_id_for_existing_user | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestSetGroupID/Set_group_id_for_existing_group | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestNew/New_with_already_existing_database | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestMigrationToLowercaseUserAndGroupNamesWithSymlinkedPreviousBackup/db | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestMigrationToLowercaseUserAndGroupNamesWithSymlinkedGroupFile/db | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestMigrationToLowercaseUserAndGroupNamesWithPreviousBackup/db | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestMigrationToLowercaseUserAndGroupNamesWithBackupFailure/db | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestMigrationToLowercaseUserAndGroupNamesAlreadyUpdated/db | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestMigrationToLowercaseUserAndGroupNames/db | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestMigrationAddLockedColumnToUsersTable | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestDeleteUser/Deleting_existing_user_keeps_other_group_members_intact | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestAllUsers/Get_one_user | Golden updated for full_username field. |
| internal/users/db/testdata/golden/TestAllUsers/Get_multiple_users | Golden updated for full_username field. |
| internal/services/pam/pam.go | Enforce/configure short usernames; return authenticated username to PAM via IAResponse.Msg. |
| internal/services/pam/pam_test.go | Add SelectBroker short-username test cases. |
| internal/services/pam/testdata/TestSelectBroker/one-user-one-group.db | New DB fixture including full_username. |
| internal/services/pam/testdata/golden/TestSetDefaultBrokerForUser/Username_is_case_insensitive/cache.db | Golden updated for full_username field. |
| internal/services/pam/testdata/golden/TestSetDefaultBrokerForUser/Update_default_broker_for_existing_user_with_a_broker/cache.db | Golden updated for full_username field. |
| internal/services/pam/testdata/golden/TestSetDefaultBrokerForUser/Set_default_broker_for_existing_user_with_no_broker/cache.db | Golden updated for full_username field. |
| internal/services/pam/testdata/golden/TestSelectBroker/Successfully_select_a_broker_with_short_username | New golden for short-username broker selection. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups/IsAuthenticated | Golden updated for new IAResponse.Msg behavior. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups/cache.db | Golden updated for full_username field. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Update_existing_DB_on_success/IsAuthenticated | Golden updated for new IAResponse.Msg behavior. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Update_existing_DB_on_success/cache.db | Golden updated for full_username field. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate/IsAuthenticated | Golden updated for new IAResponse.Msg behavior. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate/cache.db | Golden updated for full_username field. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_with_groups_with_uppercase/IsAuthenticated | Golden updated for new IAResponse.Msg behavior. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_with_groups_with_uppercase/cache.db | Golden updated for full_username field. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_user_with_uppercase/IsAuthenticated | Golden updated for new IAResponse.Msg behavior. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_user_with_uppercase/cache.db | Golden updated for full_username field. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_if_first_call_is_canceled/IsAuthenticated | Golden updated for new IAResponse.Msg behavior. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_if_first_call_is_canceled/cache.db | Golden updated for full_username field. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_after_calling_second_time_without_cancelling/cache.db | Golden updated for full_username field. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/cache.db | Golden updated for full_username field. |
| internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db | Golden updated for full_username field. |
| internal/services/pam/testdata/golden/TestIDGeneration/Generate_ID/cache.db | Golden updated for full_username field. |
| internal/services/user/testdata/golden/TestUnlockUser/Successfully_unlock_user_with_uppercase | Golden updated for full_username field. |
| internal/services/user/testdata/golden/TestUnlockUser/Successfully_unlock_user | Golden updated for full_username field. |
| internal/services/user/testdata/golden/TestLockUser/Successfully_lock_user_with_uppercase | Golden updated for full_username field. |
| internal/services/user/testdata/golden/TestLockUser/Successfully_lock_user | Golden updated for full_username field. |
| internal/testutils/daemon.go | Add use_short_usernames to generated test config + daemon options. |
| cmd/authd/daemon/testdata/golden/TestMaybeMigrateBBoltToSQLite/Migration_if_bbolt_exists_and_sqlite_does_not_exist/db | Golden updated for full_username field. |
Comments suppressed due to low confidence (2)
internal/users/db/sql/create_schema.sql:13
full_usernameis used as a lookup key (UserByFullUsername) and in updates (WHERE ... OR full_username = ?), but the schema does not enforce uniqueness. Without a unique index/constraint onusers.full_username, duplicates can exist (from manual edits, migrations, or bugs) and the lookup/update behavior becomes ambiguous. Consider adding a UNIQUE index onfull_username(allowing empty only if you never store empty) and handling migration conflicts explicitly.
internal/users/db/update.go:180UpdateBrokerForUserupdates rows wherename = ? OR full_username = ?. Iffull_usernameis not guaranteed unique, this can update multiple users’broker_idin one call, androwsAffected > 1is currently treated as success. Consider enforcing uniqueness onfull_usernameand/or first resolving the intended user row (by name vs full_username) and updating by UID to ensure exactly one row is modified.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+101
to
109
| // Existing databases might not have the full_username column, so we need to fill it after opening the database | ||
| // and applying migrations. | ||
| if err = m.fillFullUsernameColumns(); err != nil { | ||
| return nil, fmt.Errorf("failed to update full_username column: %w", err) | ||
| } | ||
|
|
||
| return m, nil | ||
| } | ||
|
|
Comment on lines
+198
to
+206
|
|
||
| func (m *Manager) fillFullUsernameColumns() error { | ||
| // Since we didn't have the full_username column before, we need to update the database right after opening it, | ||
| // to ensure that we don't have any user with an empty full_username field. | ||
| query := `UPDATE users SET full_username = name WHERE full_username IS NULL OR full_username = ''` | ||
| _, err := m.db.Exec(query) | ||
|
|
||
| return err | ||
| } |
Comment on lines
+215
to
+225
| // Even if the user doesn't exist in the database, we still want to check if a user with the same full username | ||
| // exists. | ||
| fullUsernameUser, err := m.db.UserByFullUsername(fullUsername) | ||
| if err != nil && !errors.Is(err, NoDataFoundError{}) { | ||
| return err | ||
| } | ||
| if fullUsernameUser.Name != "" && fullUsernameUser.FullUsername != "" && | ||
| fullUsernameUser.FullUsername != fullUsername { | ||
| log.Errorf(context.Background(), "Full username %q for user %q is already taken", fullUsername, u.Name) | ||
| return fmt.Errorf("full username %q is already taken", fullUsername) | ||
| } |
Comment on lines
+1004
to
+1011
|
|
||
| // We need to also format the self-named group | ||
| for _, g := range u.Groups { | ||
| if g.Name == u.Name { | ||
| g.Name = shortenedName | ||
| break | ||
| } | ||
| } |
Comment on lines
+162
to
+167
| fullUsername := u.Name | ||
| if m.config.UseShortUsernames { | ||
| log.Debug(context.TODO(), "Formatting user to use short-format") | ||
| u, fullUsername = formatShortUsername(u) | ||
| } | ||
|
|
3v1n0
reviewed
May 7, 2026
| func prepareExistingDB(t *testing.T, existingDB string) string { | ||
| t.Helper() | ||
|
|
||
| dbDir, err := os.MkdirTemp(t.TempDir(), "existing-db-path") |
Contributor
There was a problem hiding this comment.
Can't we rely on per-test t.TempDir?
3v1n0
reviewed
May 7, 2026
| PamUser: examplebroker.UserIntegrationUnexistent, | ||
| }, | ||
| }, | ||
| "Deny_authentication_if_user_does_not_exist_and_matches_cancel_key": { |
Contributor
There was a problem hiding this comment.
I remember we discussed about this, but I think we should remove this in a commit clarifying why the test does not apply anymore.
3v1n0
reviewed
May 7, 2026
| if !tc.skipRunnerCheck { | ||
| // If we have short usernames enabled, we need to trim the domain part for the check, as pam won't have it. | ||
| if tc.useShortUsernames { | ||
| tc.clientOptions.PamUser = strings.Split(tc.clientOptions.PamUser, "@")[0] |
Contributor
There was a problem hiding this comment.
Suggested change
| tc.clientOptions.PamUser = strings.Split(tc.clientOptions.PamUser, "@")[0] | |
| tc.clientOptions.PamUser, _, _ = strings.Cut(tc.clientOptions.PamUser, "@") |
3v1n0
reviewed
May 7, 2026
|
|
||
| // Existing databases might not have the full_username column, so we need to fill it after opening the database | ||
| // and applying migrations. | ||
| if err = m.fillFullUsernameColumns(); err != nil { |
Contributor
There was a problem hiding this comment.
Can't this be part of the migrations code we have?
3v1n0
reviewed
May 7, 2026
Comment on lines
+1005
to
+1011
| // We need to also format the self-named group | ||
| for _, g := range u.Groups { | ||
| if g.Name == u.Name { | ||
| g.Name = shortenedName | ||
| break | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Not sure if it's relevant but IMHO we could also just preserve both groups here.
3v1n0
reviewed
May 7, 2026
| break | ||
| } | ||
| } | ||
| u.Dir = strings.ReplaceAll(u.Dir, u.Name, shortenedName) |
Contributor
There was a problem hiding this comment.
Mh, I would replace only the suffix here, or in general use the same code to build home directories that we have when setting them up.
Member
Author
There was a problem hiding this comment.
The issue here is that the homedir is assembled at the broker, which shouldn't have any knowledge of how authd is handling the user creation. This could change once we investigate further the idea of using "custom claims" for the token, but, for now, that's how we decided to approach this.
3v1n0
reviewed
May 7, 2026
Comment on lines
-44
to
+45
| "Successfully_create_manager_with_custom_config": {uidMin: 10000, uidMax: 20000, gidMin: 10000, gidMax: 20000}, | ||
| "Successfully_create_manager_with_custom_config": {uidMin: 10000, uidMax: 20000, gidMin: 10000, gidMax: 20000, allowShortUsernames: true}, |
3v1n0
reviewed
May 7, 2026
Contributor
There was a problem hiding this comment.
Wondering if we could do something similar to TestConcurrentUserUpdate with the new config enabled and mixing both users with full qualified name and not.
3v1n0
reviewed
May 7, 2026
| switch rs := retStatus.(type) { | ||
| case adapter.PamSuccess: | ||
| style = pam.TextInfo | ||
| return // No need to show a message for success status |
Contributor
There was a problem hiding this comment.
Mhmh, why? If the broker wants to inform an user about some kind of authentication state it should be able to.
So if we send something here, it's up to lower levels of the stack to handle it IMHO.
Member
Author
There was a problem hiding this comment.
I agree. This is something that was done before, as a PoC for updating the PAM_USER and to check if updating PAM_USER would work. This PR is likely to change a lot once we implement subs (unique IDs assigned to users by the IdP) as the way to identify remote users.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces a feature that allows authd to be configured to accept users authenticating using shortened usernames (i.e., user.name) rather than their fully qualified name (i.e., user.name@email.com). This will also affect how the users will be created in the system (e.g., username, homedir, etc.).
We decided to hold the release of this feature for a little bit while we deal with another planned work that will directly affect these changes, but we'll leave the PR as a draft so that the work in progress can be tracked.