From c127159b1461efe5065f25aa49e2962f45993067 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 22:13:33 +0000 Subject: [PATCH 1/7] Initial plan From 51722b9a1b41f093a950ed72a0ec69ddb60a2f2f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 22:28:38 +0000 Subject: [PATCH 2/7] Add username validation restricting to Debian/Ubuntu allowed characters - Add ValidateUsername() function in internal/users/types/ using regex ^[a-z_][-a-z0-9_]*[$]?$ following shadow-utils/useradd rules - Call ValidateUsername() in validateUserInfo() in brokers/broker.go to validate system usernames returned by brokers before they are stored - Update test mock broker to return valid system usernames (remove @example.com suffix, add ia_info_invalid_username case) - Add TestValidateUsername unit test with valid and invalid username cases - Add Error_when_broker_returns_userinfo_with_invalid_username test cases to both broker and PAM service tests - Update all golden files to reflect new valid username format Agent-Logs-Url: https://github.com/canonical/authd/sessions/0deae640-34db-4f1f-bcb1-e06aa4f75083 Co-authored-by: adombeck <18482300+adombeck@users.noreply.github.com> --- internal/brokers/broker.go | 3 ++ internal/brokers/broker_test.go | 1 + ...ult_groups_even_if_broker_did_not_set_them | 2 +- ...ker_returns_userinfo_with_invalid_username | 4 ++ ...enticated_a_second_time_without_cancelling | 2 +- ...n_broker_returns_userinfo_with_empty_gecos | 2 +- ...eturns_userinfo_with_group_with_empty_UGID | 2 +- ...returns_userinfo_with_mismatching_username | 2 +- .../Successfully_authenticate | 2 +- ...y_authenticate_after_cancelling_first_call | 2 +- .../Successfully_pre-check_user | 12 +++--- internal/services/pam/pam_test.go | 1 + .../TestIDGeneration/Generate_ID/cache.db | 16 ++++---- .../IsAuthenticated | 2 +- .../cache.db | 16 ++++---- .../IsAuthenticated | 4 ++ .../cache.db | 4 ++ .../cache.db | 16 ++++---- .../Error_when_user_is_locked/IsAuthenticated | 2 +- .../Successfully_authenticate/cache.db | 16 ++++---- .../cache.db | 16 ++++---- .../cache.db | 16 ++++---- .../cache.db | 16 ++++---- .../Update_existing_DB_on_success/cache.db | 17 ++++---- .../Update_local_groups.group | 4 +- .../Update_local_groups/cache.db | 16 ++++---- .../Precheck_user_if_not_in_db | 8 ++-- ...ases_in_username_has_same_id_as_lower_case | 8 ++-- internal/testutils/broker.go | 6 ++- internal/users/types/userinfo.go | 22 +++++++++- internal/users/types/userinfo_test.go | 41 +++++++++++++++++++ 31 files changed, 179 insertions(+), 102 deletions(-) create mode 100644 internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_userinfo_with_invalid_username create mode 100644 internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/IsAuthenticated create mode 100644 internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/cache.db diff --git a/internal/brokers/broker.go b/internal/brokers/broker.go index eddc7da7ff..b88b04ffd0 100644 --- a/internal/brokers/broker.go +++ b/internal/brokers/broker.go @@ -355,6 +355,9 @@ func validateUserInfo(uInfo types.UserInfo) (err error) { if uInfo.Name == "" { return errors.New("empty username") } + if err := types.ValidateUsername(uInfo.Name); err != nil { + return err + } // Validate home and shell directories if !filepath.IsAbs(filepath.Clean(uInfo.Dir)) { diff --git a/internal/brokers/broker_test.go b/internal/brokers/broker_test.go index 7abd54ff03..988bd823f3 100644 --- a/internal/brokers/broker_test.go +++ b/internal/brokers/broker_test.go @@ -229,6 +229,7 @@ func TestIsAuthenticated(t *testing.T) { "Error_when_broker_returns_invalid_access": {sessionID: "ia_invalid_access"}, "Error_when_broker_returns_invalid_userinfo": {sessionID: "ia_invalid_userinfo"}, "Error_when_broker_returns_userinfo_with_empty_username": {sessionID: "ia_info_empty_user_name"}, + "Error_when_broker_returns_userinfo_with_invalid_username": {sessionID: "ia_info_invalid_username"}, "Error_when_broker_returns_userinfo_with_empty_group_name": {sessionID: "ia_info_empty_group_name"}, "Error_when_broker_returns_userinfo_with_invalid_homedir": {sessionID: "ia_info_invalid_home"}, "Error_when_broker_returns_userinfo_with_invalid_shell": {sessionID: "ia_info_invalid_shell"}, diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/Adds_default_groups_even_if_broker_did_not_set_them b/internal/brokers/testdata/golden/TestIsAuthenticated/Adds_default_groups_even_if_broker_did_not_set_them index 6f5d818a13..28d4e9b787 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/Adds_default_groups_even_if_broker_did_not_set_them +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/Adds_default_groups_even_if_broker_did_not_set_them @@ -1,4 +1,4 @@ FIRST CALL: access: granted - data: {"Name":"ia_info_empty_groups@example.com","UID":0,"Gecos":"gecos for ia_info_empty_groups@example.com","Dir":"/home/ia_info_empty_groups@example.com","Shell":"/bin/sh/ia_info_empty_groups@example.com","Groups":[]} + data: {"Name":"ia_info_empty_groups","UID":0,"Gecos":"gecos for ia_info_empty_groups","Dir":"/home/ia_info_empty_groups","Shell":"/bin/sh/ia_info_empty_groups","Groups":[]} err: diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_userinfo_with_invalid_username b/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_userinfo_with_invalid_username new file mode 100644 index 0000000000..2634edf519 --- /dev/null +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_userinfo_with_invalid_username @@ -0,0 +1,4 @@ +FIRST CALL: + access: + data: + err: provided userinfo is invalid: username "user@invalid-name" is not valid: it must match ^[a-z_][-a-z0-9_]*[$]?$ diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_calling_IsAuthenticated_a_second_time_without_cancelling b/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_calling_IsAuthenticated_a_second_time_without_cancelling index 85cd38ebae..6d978cf6b6 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_calling_IsAuthenticated_a_second_time_without_cancelling +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_calling_IsAuthenticated_a_second_time_without_cancelling @@ -1,6 +1,6 @@ FIRST CALL: access: granted - data: {"Name":"ia_second_call@example.com","UID":0,"Gecos":"gecos for ia_second_call@example.com","Dir":"/home/ia_second_call@example.com","Shell":"/bin/sh/ia_second_call@example.com","Groups":[{"Name":"group-ia_second_call@example.com","GID":null,"UGID":"ugid-ia_second_call@example.com"}]} + data: {"Name":"ia_second_call","UID":0,"Gecos":"gecos for ia_second_call","Dir":"/home/ia_second_call","Shell":"/bin/sh/ia_second_call","Groups":[{"Name":"group-ia_second_call","GID":null,"UGID":"ugid-ia_second_call"}]} err: SECOND CALL: access: diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_empty_gecos b/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_empty_gecos index 4da9b1bc19..bd158bc47c 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_empty_gecos +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_empty_gecos @@ -1,4 +1,4 @@ FIRST CALL: access: granted - data: {"Name":"ia_info_empty_gecos@example.com","UID":0,"Gecos":"","Dir":"/home/ia_info_empty_gecos@example.com","Shell":"/bin/sh/ia_info_empty_gecos@example.com","Groups":[{"Name":"group-ia_info_empty_gecos@example.com","GID":null,"UGID":"ugid-ia_info_empty_gecos@example.com"}]} + data: {"Name":"ia_info_empty_gecos","UID":0,"Gecos":"","Dir":"/home/ia_info_empty_gecos","Shell":"/bin/sh/ia_info_empty_gecos","Groups":[{"Name":"group-ia_info_empty_gecos","GID":null,"UGID":"ugid-ia_info_empty_gecos"}]} err: diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_group_with_empty_UGID b/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_group_with_empty_UGID index e3343d5e7a..5a452be25a 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_group_with_empty_UGID +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_group_with_empty_UGID @@ -1,4 +1,4 @@ FIRST CALL: access: granted - data: {"Name":"ia_info_empty_ugid@example.com","UID":0,"Gecos":"gecos for ia_info_empty_ugid@example.com","Dir":"/home/ia_info_empty_ugid@example.com","Shell":"/bin/sh/ia_info_empty_ugid@example.com","Groups":[{"Name":"group-ia_info_empty_ugid@example.com","GID":null,"UGID":""}]} + data: {"Name":"ia_info_empty_ugid","UID":0,"Gecos":"gecos for ia_info_empty_ugid","Dir":"/home/ia_info_empty_ugid","Shell":"/bin/sh/ia_info_empty_ugid","Groups":[{"Name":"group-ia_info_empty_ugid","GID":null,"UGID":""}]} err: diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_mismatching_username b/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_mismatching_username index 6883f59433..8d6b55633b 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_mismatching_username +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_mismatching_username @@ -1,4 +1,4 @@ FIRST CALL: access: granted - data: {"Name":"different_username@example.com","UID":0,"Gecos":"gecos for ia_info_mismatching_user_name@example.com","Dir":"/home/ia_info_mismatching_user_name@example.com","Shell":"/bin/sh/ia_info_mismatching_user_name@example.com","Groups":[{"Name":"group-ia_info_mismatching_user_name@example.com","GID":null,"UGID":"ugid-ia_info_mismatching_user_name@example.com"}]} + data: {"Name":"different_username","UID":0,"Gecos":"gecos for ia_info_mismatching_user_name","Dir":"/home/ia_info_mismatching_user_name","Shell":"/bin/sh/ia_info_mismatching_user_name","Groups":[{"Name":"group-ia_info_mismatching_user_name","GID":null,"UGID":"ugid-ia_info_mismatching_user_name"}]} err: diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate b/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate index e2d4f94097..2079266ded 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate @@ -1,4 +1,4 @@ FIRST CALL: access: granted - data: {"Name":"success@example.com","UID":0,"Gecos":"gecos for success@example.com","Dir":"/home/success@example.com","Shell":"/bin/sh/success@example.com","Groups":[{"Name":"group-success@example.com","GID":null,"UGID":"ugid-success@example.com"}]} + data: {"Name":"success","UID":0,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","Groups":[{"Name":"group-success","GID":null,"UGID":"ugid-success"}]} err: diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate_after_cancelling_first_call b/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate_after_cancelling_first_call index 84a6f76fcc..6ff7aaa546 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate_after_cancelling_first_call +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate_after_cancelling_first_call @@ -4,5 +4,5 @@ FIRST CALL: err: SECOND CALL: access: granted - data: {"Name":"ia_second_call@example.com","UID":0,"Gecos":"gecos for ia_second_call@example.com","Dir":"/home/ia_second_call@example.com","Shell":"/bin/sh/ia_second_call@example.com","Groups":[{"Name":"group-ia_second_call@example.com","GID":null,"UGID":"ugid-ia_second_call@example.com"}]} + data: {"Name":"ia_second_call","UID":0,"Gecos":"gecos for ia_second_call","Dir":"/home/ia_second_call","Shell":"/bin/sh/ia_second_call","Groups":[{"Name":"group-ia_second_call","GID":null,"UGID":"ugid-ia_second_call"}]} err: diff --git a/internal/brokers/testdata/golden/TestUserPreCheck/Successfully_pre-check_user b/internal/brokers/testdata/golden/TestUserPreCheck/Successfully_pre-check_user index 5b8ab8cb9a..f30e55c2f6 100644 --- a/internal/brokers/testdata/golden/TestUserPreCheck/Successfully_pre-check_user +++ b/internal/brokers/testdata/golden/TestUserPreCheck/Successfully_pre-check_user @@ -1,9 +1,9 @@ { - "name": "user-pre-check@example.com", + "name": "user-pre-check", "uuid": "", - "gecos": "gecos for user-pre-check@example.com", - "dir": "/home/user-pre-check@example.com", - "shell": "/bin/sh/user-pre-check@example.com", - "avatar": "avatar for user-pre-check@example.com", - "groups": [ {"name": "group-user-pre-check@example.com", "ugid": "ugid-user-pre-check@example.com"} ] + "gecos": "gecos for user-pre-check", + "dir": "/home/user-pre-check", + "shell": "/bin/sh/user-pre-check", + "avatar": "avatar for user-pre-check", + "groups": [ {"name": "group-user-pre-check", "ugid": "ugid-user-pre-check"} ] } \ No newline at end of file diff --git a/internal/services/pam/pam_test.go b/internal/services/pam/pam_test.go index 77beba8367..ff212ecfba 100644 --- a/internal/services/pam/pam_test.go +++ b/internal/services/pam/pam_test.go @@ -456,6 +456,7 @@ func TestIsAuthenticated(t *testing.T) { "Error_when_broker_returns_invalid_access": {username: "ia_invalid_access@example.com"}, "Error_when_broker_returns_invalid_data": {username: "ia_invalid_data@example.com"}, "Error_when_broker_returns_invalid_userinfo": {username: "ia_invalid_userinfo@example.com"}, + "Error_when_broker_returns_invalid_username": {username: "ia_info_invalid_username@example.com"}, "Error_when_calling_second_time_without_cancelling": {username: "ia_second_call@example.com", secondCall: true}, // local group error diff --git a/internal/services/pam/testdata/golden/TestIDGeneration/Generate_ID/cache.db b/internal/services/pam/testdata/golden/TestIDGeneration/Generate_ID/cache.db index 46987965c5..1b7385e9c4 100644 --- a/internal/services/pam/testdata/golden/TestIDGeneration/Generate_ID/cache.db +++ b/internal/services/pam/testdata/golden/TestIDGeneration/Generate_ID/cache.db @@ -1,17 +1,17 @@ users: - - name: success@example.com + - name: success uid: 1111 gid: 1111 - gecos: gecos for success@example.com - dir: /home/success@example.com - shell: /bin/sh/success@example.com + gecos: gecos for success + dir: /home/success + shell: /bin/sh/success groups: - - name: success@example.com + - name: success gid: 1111 - ugid: success@example.com - - name: group-success@example.com + ugid: success + - name: group-success gid: 22222 - ugid: ugid-success@example.com + ugid: ugid-success users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/IsAuthenticated b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/IsAuthenticated index 8ced0f180c..87401b003c 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/IsAuthenticated +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/IsAuthenticated @@ -1,4 +1,4 @@ FIRST CALL: access: msg: - err: failed to update user "success_with_local_groups@example.com": could not update local groups for user "success_with_local_groups@example.com": could not fetch existing local group: open : no such file or directory + err: failed to update user "success_with_local_groups": could not update local groups for user "success_with_local_groups": could not fetch existing local group: open : no such file or directory diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db index c6f6c0a54a..b40a404e43 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db @@ -1,17 +1,17 @@ users: - - name: success_with_local_groups@example.com + - name: success_with_local_groups uid: 1111 gid: 1111 - gecos: gecos for success_with_local_groups@example.com - dir: /home/success_with_local_groups@example.com - shell: /bin/sh/success_with_local_groups@example.com + gecos: gecos for success_with_local_groups + dir: /home/success_with_local_groups + shell: /bin/sh/success_with_local_groups groups: - - name: success_with_local_groups@example.com + - name: success_with_local_groups gid: 1111 - ugid: success_with_local_groups@example.com - - name: group-success_with_local_groups@example.com + ugid: success_with_local_groups + - name: group-success_with_local_groups gid: 22222 - ugid: ugid-success_with_local_groups@example.com + ugid: ugid-success_with_local_groups users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/IsAuthenticated b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/IsAuthenticated new file mode 100644 index 0000000000..b3707bad92 --- /dev/null +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/IsAuthenticated @@ -0,0 +1,4 @@ +FIRST CALL: + access: + msg: + err: provided userinfo is invalid: username "user@invalid-name" is not valid: it must match ^[a-z_][-a-z0-9_]*[$]?$ diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/cache.db new file mode 100644 index 0000000000..03de3ff950 --- /dev/null +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/cache.db @@ -0,0 +1,4 @@ +users: [] +groups: [] +users_to_groups: [] +schema_version: 2 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_calling_second_time_without_cancelling/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_calling_second_time_without_cancelling/cache.db index da22195b18..508cdf0c31 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_calling_second_time_without_cancelling/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_calling_second_time_without_cancelling/cache.db @@ -1,17 +1,17 @@ users: - - name: ia_second_call@example.com + - name: ia_second_call uid: 1111 gid: 1111 - gecos: gecos for ia_second_call@example.com - dir: /home/ia_second_call@example.com - shell: /bin/sh/ia_second_call@example.com + gecos: gecos for ia_second_call + dir: /home/ia_second_call + shell: /bin/sh/ia_second_call groups: - - name: ia_second_call@example.com + - name: ia_second_call gid: 1111 - ugid: ia_second_call@example.com - - name: group-ia_second_call@example.com + ugid: ia_second_call + - name: group-ia_second_call gid: 22222 - ugid: ugid-ia_second_call@example.com + ugid: ugid-ia_second_call users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/IsAuthenticated b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/IsAuthenticated index 10b91a2a57..40d8af08f3 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/IsAuthenticated +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/IsAuthenticated @@ -1,4 +1,4 @@ FIRST CALL: access: msg: - err: permission denied: user locked@example.com is locked + err: failed to update user "locked": UID for user "locked" already in use by a different user "locked@example.com" diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate/cache.db index 46987965c5..1b7385e9c4 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate/cache.db @@ -1,17 +1,17 @@ users: - - name: success@example.com + - name: success uid: 1111 gid: 1111 - gecos: gecos for success@example.com - dir: /home/success@example.com - shell: /bin/sh/success@example.com + gecos: gecos for success + dir: /home/success + shell: /bin/sh/success groups: - - name: success@example.com + - name: success gid: 1111 - ugid: success@example.com - - name: group-success@example.com + ugid: success + - name: group-success gid: 22222 - ugid: ugid-success@example.com + ugid: ugid-success users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_if_first_call_is_canceled/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_if_first_call_is_canceled/cache.db index da22195b18..508cdf0c31 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_if_first_call_is_canceled/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_if_first_call_is_canceled/cache.db @@ -1,17 +1,17 @@ users: - - name: ia_second_call@example.com + - name: ia_second_call uid: 1111 gid: 1111 - gecos: gecos for ia_second_call@example.com - dir: /home/ia_second_call@example.com - shell: /bin/sh/ia_second_call@example.com + gecos: gecos for ia_second_call + dir: /home/ia_second_call + shell: /bin/sh/ia_second_call groups: - - name: ia_second_call@example.com + - name: ia_second_call gid: 1111 - ugid: ia_second_call@example.com - - name: group-ia_second_call@example.com + ugid: ia_second_call + - name: group-ia_second_call gid: 22222 - ugid: ugid-ia_second_call@example.com + ugid: ugid-ia_second_call users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_user_with_uppercase/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_user_with_uppercase/cache.db index 46987965c5..1b7385e9c4 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_user_with_uppercase/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_user_with_uppercase/cache.db @@ -1,17 +1,17 @@ users: - - name: success@example.com + - name: success uid: 1111 gid: 1111 - gecos: gecos for success@example.com - dir: /home/success@example.com - shell: /bin/sh/success@example.com + gecos: gecos for success + dir: /home/success + shell: /bin/sh/success groups: - - name: success@example.com + - name: success gid: 1111 - ugid: success@example.com - - name: group-success@example.com + ugid: success + - name: group-success gid: 22222 - ugid: ugid-success@example.com + ugid: ugid-success users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_with_groups_with_uppercase/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_with_groups_with_uppercase/cache.db index 587e2a9701..dcfd145ed8 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_with_groups_with_uppercase/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_with_groups_with_uppercase/cache.db @@ -1,17 +1,17 @@ users: - - name: success_with_uppercase_groups@example.com + - name: success_with_uppercase_groups uid: 1111 gid: 1111 - gecos: gecos for success_with_uppercase_groups@example.com - dir: /home/success_with_uppercase_groups@example.com - shell: /bin/sh/success_with_uppercase_groups@example.com + gecos: gecos for success_with_uppercase_groups + dir: /home/success_with_uppercase_groups + shell: /bin/sh/success_with_uppercase_groups groups: - - name: success_with_uppercase_groups@example.com + - name: success_with_uppercase_groups gid: 1111 - ugid: success_with_uppercase_groups@example.com - - name: group-success_with_uppercase_groups@example.com + ugid: success_with_uppercase_groups + - name: group-success_with_uppercase_groups gid: 22222 - ugid: ugid-success_with_uppercase_groups@example.com + ugid: ugid-success_with_uppercase_groups - name: group1 gid: 33333 ugid: "12345678" diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_existing_DB_on_success/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_existing_DB_on_success/cache.db index 8929a8fa3b..25f872c7f0 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_existing_DB_on_success/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_existing_DB_on_success/cache.db @@ -1,10 +1,10 @@ users: - - name: success@example.com + - name: success uid: 1111 gid: 1111 - gecos: gecos for success@example.com - dir: /home/success@example.com - shell: /bin/sh/success@example.com + gecos: gecos for success + dir: /home/success + shell: /bin/sh/success - name: otheruser@example.com uid: 77777 gid: 88888 @@ -13,12 +13,9 @@ users: shell: /bin/sh/otheruser broker_id: broker-id groups: - - name: success@example.com + - name: success gid: 1111 - ugid: success@example.com - - name: group-success@example.com - gid: 22222 - ugid: ugid-success@example.com + ugid: success - name: group-success gid: 88888 ugid: ugid-success @@ -26,7 +23,7 @@ users_to_groups: - uid: 1111 gid: 1111 - uid: 1111 - gid: 22222 + gid: 88888 - uid: 77777 gid: 88888 schema_version: 2 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group index 1658d2ffbe..b35774b297 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group @@ -1,6 +1,6 @@ -localgroup1:x:41:otheruser@example.com,success_with_local_groups@example.com,otheruser2@example.com +localgroup1:x:41:otheruser@example.com,success_with_local_groups@example.com,otheruser2@example.com,success_with_local_groups localgroup2:x:42:success_with_local_groups@example.com -localgroup3:x:43:otheruser2@example.com,success_with_local_groups@example.com +localgroup3:x:43:otheruser2@example.com,success_with_local_groups localgroup4:x:44:otheruser2@example.com cloudgroup1:x:9998:otheruser3@example.com cloudgroup2:x:9999:otheruser4@example.com diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups/cache.db index c6f6c0a54a..b40a404e43 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups/cache.db @@ -1,17 +1,17 @@ users: - - name: success_with_local_groups@example.com + - name: success_with_local_groups uid: 1111 gid: 1111 - gecos: gecos for success_with_local_groups@example.com - dir: /home/success_with_local_groups@example.com - shell: /bin/sh/success_with_local_groups@example.com + gecos: gecos for success_with_local_groups + dir: /home/success_with_local_groups + shell: /bin/sh/success_with_local_groups groups: - - name: success_with_local_groups@example.com + - name: success_with_local_groups gid: 1111 - ugid: success_with_local_groups@example.com - - name: group-success_with_local_groups@example.com + ugid: success_with_local_groups + - name: group-success_with_local_groups gid: 22222 - ugid: ugid-success_with_local_groups@example.com + ugid: ugid-success_with_local_groups users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/user/testdata/golden/TestGetUserByName/Precheck_user_if_not_in_db b/internal/services/user/testdata/golden/TestGetUserByName/Precheck_user_if_not_in_db index f048f3065a..ce06b0424b 100644 --- a/internal/services/user/testdata/golden/TestGetUserByName/Precheck_user_if_not_in_db +++ b/internal/services/user/testdata/golden/TestGetUserByName/Precheck_user_if_not_in_db @@ -1,6 +1,6 @@ -name: user-pre-check@example.com +name: user-pre-check uid: 1234 gid: 1234 -gecos: gecos for user-pre-check@example.com -homedir: /home/user-pre-check@example.com -shell: /bin/sh/user-pre-check@example.com +gecos: gecos for user-pre-check +homedir: /home/user-pre-check +shell: /bin/sh/user-pre-check diff --git a/internal/services/user/testdata/golden/TestGetUserByName/Prechecked_user_with_upper_cases_in_username_has_same_id_as_lower_case b/internal/services/user/testdata/golden/TestGetUserByName/Prechecked_user_with_upper_cases_in_username_has_same_id_as_lower_case index f048f3065a..ce06b0424b 100644 --- a/internal/services/user/testdata/golden/TestGetUserByName/Prechecked_user_with_upper_cases_in_username_has_same_id_as_lower_case +++ b/internal/services/user/testdata/golden/TestGetUserByName/Prechecked_user_with_upper_cases_in_username_has_same_id_as_lower_case @@ -1,6 +1,6 @@ -name: user-pre-check@example.com +name: user-pre-check uid: 1234 gid: 1234 -gecos: gecos for user-pre-check@example.com -homedir: /home/user-pre-check@example.com -shell: /bin/sh/user-pre-check@example.com +gecos: gecos for user-pre-check +homedir: /home/user-pre-check +shell: /bin/sh/user-pre-check diff --git a/internal/testutils/broker.go b/internal/testutils/broker.go index 3ef9eb3d26..f85d3db065 100644 --- a/internal/testutils/broker.go +++ b/internal/testutils/broker.go @@ -373,7 +373,7 @@ func userInfoFromName(sessionID string, extraGroups []groupJSONInfo) string { // Default values parsedID := parseSessionID(sessionID) - name := parsedID + "@example.com" + name := parsedID group := "group-" + name home := "/home/" + name shell := "/bin/sh/" + name @@ -384,7 +384,9 @@ func userInfoFromName(sessionID string, extraGroups []groupJSONInfo) string { case "ia_info_empty_user_name": name = "" case "ia_info_mismatching_user_name": - name = "different_username@example.com" + name = "different_username" + case "ia_info_invalid_username": + name = "user@invalid-name" case "ia_info_empty_group_name": group = "" case "ia_info_empty_ugid": diff --git a/internal/users/types/userinfo.go b/internal/users/types/userinfo.go index 54e44136ef..5472d3eefc 100644 --- a/internal/users/types/userinfo.go +++ b/internal/users/types/userinfo.go @@ -1,6 +1,26 @@ package types -import "github.com/canonical/authd/internal/sliceutils" +import ( + "fmt" + "regexp" + + "github.com/canonical/authd/internal/sliceutils" +) + +// usernameRegexp is the regexp that a valid username must match. +// It follows the Debian/Ubuntu username policy as defined by shadow-utils/useradd rules. +var usernameRegexp = regexp.MustCompile(`^[a-z_][-a-z0-9_]*[$]?$`) + +// ValidateUsername checks if the given username is valid. +// Valid usernames follow the Debian/Ubuntu naming convention: they start with a lowercase letter or +// underscore, followed by lowercase letters, digits, hyphens, or underscores, with an optional +// trailing dollar sign. +func ValidateUsername(name string) error { + if !usernameRegexp.MatchString(name) { + return fmt.Errorf("username %q is not valid: it must match %s", name, usernameRegexp) + } + return nil +} // Equals checks that two users are equal. func (u UserInfo) Equals(other UserInfo) bool { diff --git a/internal/users/types/userinfo_test.go b/internal/users/types/userinfo_test.go index 2e1e2703b1..42812bfc5b 100644 --- a/internal/users/types/userinfo_test.go +++ b/internal/users/types/userinfo_test.go @@ -136,3 +136,44 @@ func TestUserInfoEquals(t *testing.T) { }) } } + +func TestValidateUsername(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + username string + wantErr bool + }{ + // Valid usernames + "Valid_lowercase_name": {username: "user"}, + "Valid_name_starting_with_underscore": {username: "_user"}, + "Valid_name_with_hyphen": {username: "user-name"}, + "Valid_name_with_underscore": {username: "user_name"}, + "Valid_name_with_digits": {username: "user1"}, + "Valid_name_with_trailing_dollar_sign": {username: "user$"}, + "Valid_single_character_name": {username: "a"}, + "Valid_name_with_mixed_allowed_chars": {username: "a-b_c0"}, + + // Invalid usernames + "Error_on_empty_username": {username: "", wantErr: true}, + "Error_on_uppercase_character": {username: "User", wantErr: true}, + "Error_on_email_style_name": {username: "user@example.com", wantErr: true}, + "Error_on_name_starting_with_digit": {username: "1user", wantErr: true}, + "Error_on_name_starting_with_hyphen": {username: "-user", wantErr: true}, + "Error_on_name_with_dot": {username: "user.name", wantErr: true}, + "Error_on_name_with_dollar_not_at_end": {username: "user$name", wantErr: true}, + "Error_on_name_with_space": {username: "user name", wantErr: true}, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + err := types.ValidateUsername(tc.username) + if tc.wantErr { + require.Error(t, err, "ValidateUsername should return an error for %q, but did not", tc.username) + return + } + require.NoError(t, err, "ValidateUsername should not return an error for %q, but did", tc.username) + }) + } +} From 6b53ffa9bae65af0b2673721b1a250c6ec479aeb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 22:32:06 +0000 Subject: [PATCH 3/7] Fix test data files to use valid usernames and update golden files - Update valid.group to use 'success_with_local_groups' instead of 'success_with_local_groups@example.com' to prevent duplicate entries - Update cache-with-locked-user.db to use 'locked' instead of 'locked@example.com' so the locked user check works correctly - Update golden files to reflect correct test behavior Agent-Logs-Url: https://github.com/canonical/authd/sessions/0deae640-34db-4f1f-bcb1-e06aa4f75083 Co-authored-by: adombeck <18482300+adombeck@users.noreply.github.com> --- .../testdata/TestIsAuthenticated/cache-with-locked-user.db | 4 ++-- .../services/pam/testdata/TestIsAuthenticated/valid.group | 4 ++-- .../Error_when_user_is_locked/IsAuthenticated | 2 +- .../TestIsAuthenticated/Error_when_user_is_locked/cache.db | 4 ++-- .../golden/TestIsAuthenticated/Update_local_groups.group | 4 ++-- .../TestIsAuthenticated/Update_local_groups.group.backup | 4 ++-- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/internal/services/pam/testdata/TestIsAuthenticated/cache-with-locked-user.db b/internal/services/pam/testdata/TestIsAuthenticated/cache-with-locked-user.db index 05fe2dd42d..066473f8a7 100644 --- a/internal/services/pam/testdata/TestIsAuthenticated/cache-with-locked-user.db +++ b/internal/services/pam/testdata/TestIsAuthenticated/cache-with-locked-user.db @@ -1,9 +1,9 @@ users: - - name: locked@example.com + - name: locked uid: 1111 gid: 11111 gecos: gecos for other user - dir: /home/locked@example.com + dir: /home/locked shell: /bin/bash broker_id: broker-id locked: true diff --git a/internal/services/pam/testdata/TestIsAuthenticated/valid.group b/internal/services/pam/testdata/TestIsAuthenticated/valid.group index 2ebaf20f75..af78dd4c1d 100644 --- a/internal/services/pam/testdata/TestIsAuthenticated/valid.group +++ b/internal/services/pam/testdata/TestIsAuthenticated/valid.group @@ -1,5 +1,5 @@ -localgroup1:x:41:otheruser@example.com,success_with_local_groups@example.com,otheruser2@example.com -localgroup2:x:42:success_with_local_groups@example.com +localgroup1:x:41:otheruser@example.com,success_with_local_groups,otheruser2@example.com +localgroup2:x:42:success_with_local_groups localgroup3:x:43:otheruser2@example.com localgroup4:x:44:otheruser2@example.com cloudgroup1:x:9998:otheruser3@example.com diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/IsAuthenticated b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/IsAuthenticated index 40d8af08f3..1eb4862c34 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/IsAuthenticated +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/IsAuthenticated @@ -1,4 +1,4 @@ FIRST CALL: access: msg: - err: failed to update user "locked": UID for user "locked" already in use by a different user "locked@example.com" + err: permission denied: user locked is locked diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/cache.db index c4de41699a..f882759b24 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/cache.db @@ -1,9 +1,9 @@ users: - - name: locked@example.com + - name: locked uid: 1111 gid: 11111 gecos: gecos for other user - dir: /home/locked@example.com + dir: /home/locked shell: /bin/bash broker_id: broker-id locked: true diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group index b35774b297..fa75206fc9 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group @@ -1,5 +1,5 @@ -localgroup1:x:41:otheruser@example.com,success_with_local_groups@example.com,otheruser2@example.com,success_with_local_groups -localgroup2:x:42:success_with_local_groups@example.com +localgroup1:x:41:otheruser@example.com,success_with_local_groups,otheruser2@example.com +localgroup2:x:42:success_with_local_groups localgroup3:x:43:otheruser2@example.com,success_with_local_groups localgroup4:x:44:otheruser2@example.com cloudgroup1:x:9998:otheruser3@example.com diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup index 2ebaf20f75..af78dd4c1d 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup @@ -1,5 +1,5 @@ -localgroup1:x:41:otheruser@example.com,success_with_local_groups@example.com,otheruser2@example.com -localgroup2:x:42:success_with_local_groups@example.com +localgroup1:x:41:otheruser@example.com,success_with_local_groups,otheruser2@example.com +localgroup2:x:42:success_with_local_groups localgroup3:x:43:otheruser2@example.com localgroup4:x:44:otheruser2@example.com cloudgroup1:x:9998:otheruser3@example.com From d3a021541e7e28f66c0dcc2e9483255f78dadeac Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 22:32:21 +0000 Subject: [PATCH 4/7] Remove accidentally committed backup file Agent-Logs-Url: https://github.com/canonical/authd/sessions/0deae640-34db-4f1f-bcb1-e06aa4f75083 Co-authored-by: adombeck <18482300+adombeck@users.noreply.github.com> --- .../TestIsAuthenticated/Update_local_groups.group.backup | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup deleted file mode 100644 index af78dd4c1d..0000000000 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup +++ /dev/null @@ -1,6 +0,0 @@ -localgroup1:x:41:otheruser@example.com,success_with_local_groups,otheruser2@example.com -localgroup2:x:42:success_with_local_groups -localgroup3:x:43:otheruser2@example.com -localgroup4:x:44:otheruser2@example.com -cloudgroup1:x:9998:otheruser3@example.com -cloudgroup2:x:9999:otheruser4@example.com From 395b917d7b7f66f470913523d5abbef57e575811 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 22:34:16 +0000 Subject: [PATCH 5/7] Update test group files to use valid usernames for consistency Remove @example.com from group member names in test data files to be consistent with the new username validation rules Agent-Logs-Url: https://github.com/canonical/authd/sessions/0deae640-34db-4f1f-bcb1-e06aa4f75083 Co-authored-by: adombeck <18482300+adombeck@users.noreply.github.com> --- .../pam/testdata/TestIsAuthenticated/valid.group | 10 +++++----- .../TestIsAuthenticated/Update_local_groups.group | 10 +++++----- .../Update_local_groups.group.backup | 6 ++++++ 3 files changed, 16 insertions(+), 10 deletions(-) create mode 100644 internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup diff --git a/internal/services/pam/testdata/TestIsAuthenticated/valid.group b/internal/services/pam/testdata/TestIsAuthenticated/valid.group index af78dd4c1d..f3dce64ae9 100644 --- a/internal/services/pam/testdata/TestIsAuthenticated/valid.group +++ b/internal/services/pam/testdata/TestIsAuthenticated/valid.group @@ -1,6 +1,6 @@ -localgroup1:x:41:otheruser@example.com,success_with_local_groups,otheruser2@example.com +localgroup1:x:41:otheruser,success_with_local_groups,otheruser2 localgroup2:x:42:success_with_local_groups -localgroup3:x:43:otheruser2@example.com -localgroup4:x:44:otheruser2@example.com -cloudgroup1:x:9998:otheruser3@example.com -cloudgroup2:x:9999:otheruser4@example.com +localgroup3:x:43:otheruser2 +localgroup4:x:44:otheruser2 +cloudgroup1:x:9998:otheruser3 +cloudgroup2:x:9999:otheruser4 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group index fa75206fc9..8f8934fc6a 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group @@ -1,6 +1,6 @@ -localgroup1:x:41:otheruser@example.com,success_with_local_groups,otheruser2@example.com +localgroup1:x:41:otheruser,success_with_local_groups,otheruser2 localgroup2:x:42:success_with_local_groups -localgroup3:x:43:otheruser2@example.com,success_with_local_groups -localgroup4:x:44:otheruser2@example.com -cloudgroup1:x:9998:otheruser3@example.com -cloudgroup2:x:9999:otheruser4@example.com +localgroup3:x:43:otheruser2,success_with_local_groups +localgroup4:x:44:otheruser2 +cloudgroup1:x:9998:otheruser3 +cloudgroup2:x:9999:otheruser4 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup new file mode 100644 index 0000000000..f3dce64ae9 --- /dev/null +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup @@ -0,0 +1,6 @@ +localgroup1:x:41:otheruser,success_with_local_groups,otheruser2 +localgroup2:x:42:success_with_local_groups +localgroup3:x:43:otheruser2 +localgroup4:x:44:otheruser2 +cloudgroup1:x:9998:otheruser3 +cloudgroup2:x:9999:otheruser4 From ee16c0ad1f4ac2f4c7e39c93ab7d1dab503e056f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 22:41:23 +0000 Subject: [PATCH 6/7] Allow @ and . in usernames for email-style cloud identity provider names - Update regex to ^[a-z_][-a-z0-9_.@]*[$]?$ to allow @ and . chars which are needed for email-style usernames (e.g. user@example.com) used by cloud identity providers like MS Entra ID and Google IAM - Revert test mock broker to use @example.com email-style usernames - Update ia_info_invalid_username to use -invalid_username (starts with hyphen) as the clearly invalid test case instead - Revert test data files (valid.group, cache-with-locked-user.db) - Update test unit cases: email-style names are valid, not invalid - Regenerate all affected golden files Agent-Logs-Url: https://github.com/canonical/authd/sessions/aab36fdb-b65d-458f-9c58-8bee7bb06733 Co-authored-by: 3v1n0 <345675+3v1n0@users.noreply.github.com> --- ...fault_groups_even_if_broker_did_not_set_them | 2 +- ...roker_returns_userinfo_with_invalid_username | 2 +- ...thenticated_a_second_time_without_cancelling | 2 +- ...hen_broker_returns_userinfo_with_empty_gecos | 2 +- ..._returns_userinfo_with_group_with_empty_UGID | 2 +- ...r_returns_userinfo_with_mismatching_username | 2 +- .../Successfully_authenticate | 2 +- ...lly_authenticate_after_cancelling_first_call | 2 +- .../Successfully_pre-check_user | 12 ++++++------ .../cache-with-locked-user.db | 4 ++-- .../testdata/TestIsAuthenticated/valid.group | 12 ++++++------ .../TestIDGeneration/Generate_ID/cache.db | 16 ++++++++-------- .../IsAuthenticated | 2 +- .../cache.db | 16 ++++++++-------- .../IsAuthenticated | 2 +- .../cache.db | 16 ++++++++-------- .../Error_when_user_is_locked/IsAuthenticated | 2 +- .../Error_when_user_is_locked/cache.db | 4 ++-- .../Successfully_authenticate/cache.db | 16 ++++++++-------- .../cache.db | 16 ++++++++-------- .../cache.db | 16 ++++++++-------- .../cache.db | 16 ++++++++-------- .../Update_existing_DB_on_success/cache.db | 17 ++++++++++------- .../Update_local_groups.group | 12 ++++++------ .../Update_local_groups.group.backup | 12 ++++++------ .../Update_local_groups/cache.db | 16 ++++++++-------- .../Precheck_user_if_not_in_db | 8 ++++---- ..._cases_in_username_has_same_id_as_lower_case | 8 ++++---- internal/testutils/broker.go | 6 +++--- internal/users/types/userinfo.go | 11 +++++++---- internal/users/types/userinfo_test.go | 6 ++++-- 31 files changed, 135 insertions(+), 127 deletions(-) diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/Adds_default_groups_even_if_broker_did_not_set_them b/internal/brokers/testdata/golden/TestIsAuthenticated/Adds_default_groups_even_if_broker_did_not_set_them index 28d4e9b787..6f5d818a13 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/Adds_default_groups_even_if_broker_did_not_set_them +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/Adds_default_groups_even_if_broker_did_not_set_them @@ -1,4 +1,4 @@ FIRST CALL: access: granted - data: {"Name":"ia_info_empty_groups","UID":0,"Gecos":"gecos for ia_info_empty_groups","Dir":"/home/ia_info_empty_groups","Shell":"/bin/sh/ia_info_empty_groups","Groups":[]} + data: {"Name":"ia_info_empty_groups@example.com","UID":0,"Gecos":"gecos for ia_info_empty_groups@example.com","Dir":"/home/ia_info_empty_groups@example.com","Shell":"/bin/sh/ia_info_empty_groups@example.com","Groups":[]} err: diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_userinfo_with_invalid_username b/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_userinfo_with_invalid_username index 2634edf519..6e50a8e529 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_userinfo_with_invalid_username +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_userinfo_with_invalid_username @@ -1,4 +1,4 @@ FIRST CALL: access: data: - err: provided userinfo is invalid: username "user@invalid-name" is not valid: it must match ^[a-z_][-a-z0-9_]*[$]?$ + err: provided userinfo is invalid: username "-invalid_username" is not valid: it must match ^[a-z_][-a-z0-9_.@]*[$]?$ diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_calling_IsAuthenticated_a_second_time_without_cancelling b/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_calling_IsAuthenticated_a_second_time_without_cancelling index 6d978cf6b6..85cd38ebae 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_calling_IsAuthenticated_a_second_time_without_cancelling +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/Error_when_calling_IsAuthenticated_a_second_time_without_cancelling @@ -1,6 +1,6 @@ FIRST CALL: access: granted - data: {"Name":"ia_second_call","UID":0,"Gecos":"gecos for ia_second_call","Dir":"/home/ia_second_call","Shell":"/bin/sh/ia_second_call","Groups":[{"Name":"group-ia_second_call","GID":null,"UGID":"ugid-ia_second_call"}]} + data: {"Name":"ia_second_call@example.com","UID":0,"Gecos":"gecos for ia_second_call@example.com","Dir":"/home/ia_second_call@example.com","Shell":"/bin/sh/ia_second_call@example.com","Groups":[{"Name":"group-ia_second_call@example.com","GID":null,"UGID":"ugid-ia_second_call@example.com"}]} err: SECOND CALL: access: diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_empty_gecos b/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_empty_gecos index bd158bc47c..4da9b1bc19 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_empty_gecos +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_empty_gecos @@ -1,4 +1,4 @@ FIRST CALL: access: granted - data: {"Name":"ia_info_empty_gecos","UID":0,"Gecos":"","Dir":"/home/ia_info_empty_gecos","Shell":"/bin/sh/ia_info_empty_gecos","Groups":[{"Name":"group-ia_info_empty_gecos","GID":null,"UGID":"ugid-ia_info_empty_gecos"}]} + data: {"Name":"ia_info_empty_gecos@example.com","UID":0,"Gecos":"","Dir":"/home/ia_info_empty_gecos@example.com","Shell":"/bin/sh/ia_info_empty_gecos@example.com","Groups":[{"Name":"group-ia_info_empty_gecos@example.com","GID":null,"UGID":"ugid-ia_info_empty_gecos@example.com"}]} err: diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_group_with_empty_UGID b/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_group_with_empty_UGID index 5a452be25a..e3343d5e7a 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_group_with_empty_UGID +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_group_with_empty_UGID @@ -1,4 +1,4 @@ FIRST CALL: access: granted - data: {"Name":"ia_info_empty_ugid","UID":0,"Gecos":"gecos for ia_info_empty_ugid","Dir":"/home/ia_info_empty_ugid","Shell":"/bin/sh/ia_info_empty_ugid","Groups":[{"Name":"group-ia_info_empty_ugid","GID":null,"UGID":""}]} + data: {"Name":"ia_info_empty_ugid@example.com","UID":0,"Gecos":"gecos for ia_info_empty_ugid@example.com","Dir":"/home/ia_info_empty_ugid@example.com","Shell":"/bin/sh/ia_info_empty_ugid@example.com","Groups":[{"Name":"group-ia_info_empty_ugid@example.com","GID":null,"UGID":""}]} err: diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_mismatching_username b/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_mismatching_username index 8d6b55633b..6883f59433 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_mismatching_username +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/No_error_when_broker_returns_userinfo_with_mismatching_username @@ -1,4 +1,4 @@ FIRST CALL: access: granted - data: {"Name":"different_username","UID":0,"Gecos":"gecos for ia_info_mismatching_user_name","Dir":"/home/ia_info_mismatching_user_name","Shell":"/bin/sh/ia_info_mismatching_user_name","Groups":[{"Name":"group-ia_info_mismatching_user_name","GID":null,"UGID":"ugid-ia_info_mismatching_user_name"}]} + data: {"Name":"different_username@example.com","UID":0,"Gecos":"gecos for ia_info_mismatching_user_name@example.com","Dir":"/home/ia_info_mismatching_user_name@example.com","Shell":"/bin/sh/ia_info_mismatching_user_name@example.com","Groups":[{"Name":"group-ia_info_mismatching_user_name@example.com","GID":null,"UGID":"ugid-ia_info_mismatching_user_name@example.com"}]} err: diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate b/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate index 2079266ded..e2d4f94097 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate @@ -1,4 +1,4 @@ FIRST CALL: access: granted - data: {"Name":"success","UID":0,"Gecos":"gecos for success","Dir":"/home/success","Shell":"/bin/sh/success","Groups":[{"Name":"group-success","GID":null,"UGID":"ugid-success"}]} + data: {"Name":"success@example.com","UID":0,"Gecos":"gecos for success@example.com","Dir":"/home/success@example.com","Shell":"/bin/sh/success@example.com","Groups":[{"Name":"group-success@example.com","GID":null,"UGID":"ugid-success@example.com"}]} err: diff --git a/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate_after_cancelling_first_call b/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate_after_cancelling_first_call index 6ff7aaa546..84a6f76fcc 100644 --- a/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate_after_cancelling_first_call +++ b/internal/brokers/testdata/golden/TestIsAuthenticated/Successfully_authenticate_after_cancelling_first_call @@ -4,5 +4,5 @@ FIRST CALL: err: SECOND CALL: access: granted - data: {"Name":"ia_second_call","UID":0,"Gecos":"gecos for ia_second_call","Dir":"/home/ia_second_call","Shell":"/bin/sh/ia_second_call","Groups":[{"Name":"group-ia_second_call","GID":null,"UGID":"ugid-ia_second_call"}]} + data: {"Name":"ia_second_call@example.com","UID":0,"Gecos":"gecos for ia_second_call@example.com","Dir":"/home/ia_second_call@example.com","Shell":"/bin/sh/ia_second_call@example.com","Groups":[{"Name":"group-ia_second_call@example.com","GID":null,"UGID":"ugid-ia_second_call@example.com"}]} err: diff --git a/internal/brokers/testdata/golden/TestUserPreCheck/Successfully_pre-check_user b/internal/brokers/testdata/golden/TestUserPreCheck/Successfully_pre-check_user index f30e55c2f6..5b8ab8cb9a 100644 --- a/internal/brokers/testdata/golden/TestUserPreCheck/Successfully_pre-check_user +++ b/internal/brokers/testdata/golden/TestUserPreCheck/Successfully_pre-check_user @@ -1,9 +1,9 @@ { - "name": "user-pre-check", + "name": "user-pre-check@example.com", "uuid": "", - "gecos": "gecos for user-pre-check", - "dir": "/home/user-pre-check", - "shell": "/bin/sh/user-pre-check", - "avatar": "avatar for user-pre-check", - "groups": [ {"name": "group-user-pre-check", "ugid": "ugid-user-pre-check"} ] + "gecos": "gecos for user-pre-check@example.com", + "dir": "/home/user-pre-check@example.com", + "shell": "/bin/sh/user-pre-check@example.com", + "avatar": "avatar for user-pre-check@example.com", + "groups": [ {"name": "group-user-pre-check@example.com", "ugid": "ugid-user-pre-check@example.com"} ] } \ No newline at end of file diff --git a/internal/services/pam/testdata/TestIsAuthenticated/cache-with-locked-user.db b/internal/services/pam/testdata/TestIsAuthenticated/cache-with-locked-user.db index 066473f8a7..05fe2dd42d 100644 --- a/internal/services/pam/testdata/TestIsAuthenticated/cache-with-locked-user.db +++ b/internal/services/pam/testdata/TestIsAuthenticated/cache-with-locked-user.db @@ -1,9 +1,9 @@ users: - - name: locked + - name: locked@example.com uid: 1111 gid: 11111 gecos: gecos for other user - dir: /home/locked + dir: /home/locked@example.com shell: /bin/bash broker_id: broker-id locked: true diff --git a/internal/services/pam/testdata/TestIsAuthenticated/valid.group b/internal/services/pam/testdata/TestIsAuthenticated/valid.group index f3dce64ae9..2ebaf20f75 100644 --- a/internal/services/pam/testdata/TestIsAuthenticated/valid.group +++ b/internal/services/pam/testdata/TestIsAuthenticated/valid.group @@ -1,6 +1,6 @@ -localgroup1:x:41:otheruser,success_with_local_groups,otheruser2 -localgroup2:x:42:success_with_local_groups -localgroup3:x:43:otheruser2 -localgroup4:x:44:otheruser2 -cloudgroup1:x:9998:otheruser3 -cloudgroup2:x:9999:otheruser4 +localgroup1:x:41:otheruser@example.com,success_with_local_groups@example.com,otheruser2@example.com +localgroup2:x:42:success_with_local_groups@example.com +localgroup3:x:43:otheruser2@example.com +localgroup4:x:44:otheruser2@example.com +cloudgroup1:x:9998:otheruser3@example.com +cloudgroup2:x:9999:otheruser4@example.com diff --git a/internal/services/pam/testdata/golden/TestIDGeneration/Generate_ID/cache.db b/internal/services/pam/testdata/golden/TestIDGeneration/Generate_ID/cache.db index 1b7385e9c4..46987965c5 100644 --- a/internal/services/pam/testdata/golden/TestIDGeneration/Generate_ID/cache.db +++ b/internal/services/pam/testdata/golden/TestIDGeneration/Generate_ID/cache.db @@ -1,17 +1,17 @@ users: - - name: success + - name: success@example.com uid: 1111 gid: 1111 - gecos: gecos for success - dir: /home/success - shell: /bin/sh/success + gecos: gecos for success@example.com + dir: /home/success@example.com + shell: /bin/sh/success@example.com groups: - - name: success + - name: success@example.com gid: 1111 - ugid: success - - name: group-success + ugid: success@example.com + - name: group-success@example.com gid: 22222 - ugid: ugid-success + ugid: ugid-success@example.com users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/IsAuthenticated b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/IsAuthenticated index 87401b003c..8ced0f180c 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/IsAuthenticated +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/IsAuthenticated @@ -1,4 +1,4 @@ FIRST CALL: access: msg: - err: failed to update user "success_with_local_groups": could not update local groups for user "success_with_local_groups": could not fetch existing local group: open : no such file or directory + err: failed to update user "success_with_local_groups@example.com": could not update local groups for user "success_with_local_groups@example.com": could not fetch existing local group: open : no such file or directory diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db index b40a404e43..c6f6c0a54a 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_on_updating_local_groups_with_unexisting_file/cache.db @@ -1,17 +1,17 @@ users: - - name: success_with_local_groups + - name: success_with_local_groups@example.com uid: 1111 gid: 1111 - gecos: gecos for success_with_local_groups - dir: /home/success_with_local_groups - shell: /bin/sh/success_with_local_groups + gecos: gecos for success_with_local_groups@example.com + dir: /home/success_with_local_groups@example.com + shell: /bin/sh/success_with_local_groups@example.com groups: - - name: success_with_local_groups + - name: success_with_local_groups@example.com gid: 1111 - ugid: success_with_local_groups - - name: group-success_with_local_groups + ugid: success_with_local_groups@example.com + - name: group-success_with_local_groups@example.com gid: 22222 - ugid: ugid-success_with_local_groups + ugid: ugid-success_with_local_groups@example.com users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/IsAuthenticated b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/IsAuthenticated index b3707bad92..717f814085 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/IsAuthenticated +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_broker_returns_invalid_username/IsAuthenticated @@ -1,4 +1,4 @@ FIRST CALL: access: msg: - err: provided userinfo is invalid: username "user@invalid-name" is not valid: it must match ^[a-z_][-a-z0-9_]*[$]?$ + err: provided userinfo is invalid: username "-invalid_username" is not valid: it must match ^[a-z_][-a-z0-9_.@]*[$]?$ diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_calling_second_time_without_cancelling/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_calling_second_time_without_cancelling/cache.db index 508cdf0c31..da22195b18 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_calling_second_time_without_cancelling/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_calling_second_time_without_cancelling/cache.db @@ -1,17 +1,17 @@ users: - - name: ia_second_call + - name: ia_second_call@example.com uid: 1111 gid: 1111 - gecos: gecos for ia_second_call - dir: /home/ia_second_call - shell: /bin/sh/ia_second_call + gecos: gecos for ia_second_call@example.com + dir: /home/ia_second_call@example.com + shell: /bin/sh/ia_second_call@example.com groups: - - name: ia_second_call + - name: ia_second_call@example.com gid: 1111 - ugid: ia_second_call - - name: group-ia_second_call + ugid: ia_second_call@example.com + - name: group-ia_second_call@example.com gid: 22222 - ugid: ugid-ia_second_call + ugid: ugid-ia_second_call@example.com users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/IsAuthenticated b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/IsAuthenticated index 1eb4862c34..10b91a2a57 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/IsAuthenticated +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/IsAuthenticated @@ -1,4 +1,4 @@ FIRST CALL: access: msg: - err: permission denied: user locked is locked + err: permission denied: user locked@example.com is locked diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/cache.db index f882759b24..c4de41699a 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Error_when_user_is_locked/cache.db @@ -1,9 +1,9 @@ users: - - name: locked + - name: locked@example.com uid: 1111 gid: 11111 gecos: gecos for other user - dir: /home/locked + dir: /home/locked@example.com shell: /bin/bash broker_id: broker-id locked: true diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate/cache.db index 1b7385e9c4..46987965c5 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate/cache.db @@ -1,17 +1,17 @@ users: - - name: success + - name: success@example.com uid: 1111 gid: 1111 - gecos: gecos for success - dir: /home/success - shell: /bin/sh/success + gecos: gecos for success@example.com + dir: /home/success@example.com + shell: /bin/sh/success@example.com groups: - - name: success + - name: success@example.com gid: 1111 - ugid: success - - name: group-success + ugid: success@example.com + - name: group-success@example.com gid: 22222 - ugid: ugid-success + ugid: ugid-success@example.com users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_if_first_call_is_canceled/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_if_first_call_is_canceled/cache.db index 508cdf0c31..da22195b18 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_if_first_call_is_canceled/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_if_first_call_is_canceled/cache.db @@ -1,17 +1,17 @@ users: - - name: ia_second_call + - name: ia_second_call@example.com uid: 1111 gid: 1111 - gecos: gecos for ia_second_call - dir: /home/ia_second_call - shell: /bin/sh/ia_second_call + gecos: gecos for ia_second_call@example.com + dir: /home/ia_second_call@example.com + shell: /bin/sh/ia_second_call@example.com groups: - - name: ia_second_call + - name: ia_second_call@example.com gid: 1111 - ugid: ia_second_call - - name: group-ia_second_call + ugid: ia_second_call@example.com + - name: group-ia_second_call@example.com gid: 22222 - ugid: ugid-ia_second_call + ugid: ugid-ia_second_call@example.com users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_user_with_uppercase/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_user_with_uppercase/cache.db index 1b7385e9c4..46987965c5 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_user_with_uppercase/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_user_with_uppercase/cache.db @@ -1,17 +1,17 @@ users: - - name: success + - name: success@example.com uid: 1111 gid: 1111 - gecos: gecos for success - dir: /home/success - shell: /bin/sh/success + gecos: gecos for success@example.com + dir: /home/success@example.com + shell: /bin/sh/success@example.com groups: - - name: success + - name: success@example.com gid: 1111 - ugid: success - - name: group-success + ugid: success@example.com + - name: group-success@example.com gid: 22222 - ugid: ugid-success + ugid: ugid-success@example.com users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_with_groups_with_uppercase/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_with_groups_with_uppercase/cache.db index dcfd145ed8..587e2a9701 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_with_groups_with_uppercase/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Successfully_authenticate_with_groups_with_uppercase/cache.db @@ -1,17 +1,17 @@ users: - - name: success_with_uppercase_groups + - name: success_with_uppercase_groups@example.com uid: 1111 gid: 1111 - gecos: gecos for success_with_uppercase_groups - dir: /home/success_with_uppercase_groups - shell: /bin/sh/success_with_uppercase_groups + gecos: gecos for success_with_uppercase_groups@example.com + dir: /home/success_with_uppercase_groups@example.com + shell: /bin/sh/success_with_uppercase_groups@example.com groups: - - name: success_with_uppercase_groups + - name: success_with_uppercase_groups@example.com gid: 1111 - ugid: success_with_uppercase_groups - - name: group-success_with_uppercase_groups + ugid: success_with_uppercase_groups@example.com + - name: group-success_with_uppercase_groups@example.com gid: 22222 - ugid: ugid-success_with_uppercase_groups + ugid: ugid-success_with_uppercase_groups@example.com - name: group1 gid: 33333 ugid: "12345678" diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_existing_DB_on_success/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_existing_DB_on_success/cache.db index 25f872c7f0..8929a8fa3b 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_existing_DB_on_success/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_existing_DB_on_success/cache.db @@ -1,10 +1,10 @@ users: - - name: success + - name: success@example.com uid: 1111 gid: 1111 - gecos: gecos for success - dir: /home/success - shell: /bin/sh/success + gecos: gecos for success@example.com + dir: /home/success@example.com + shell: /bin/sh/success@example.com - name: otheruser@example.com uid: 77777 gid: 88888 @@ -13,9 +13,12 @@ users: shell: /bin/sh/otheruser broker_id: broker-id groups: - - name: success + - name: success@example.com gid: 1111 - ugid: success + ugid: success@example.com + - name: group-success@example.com + gid: 22222 + ugid: ugid-success@example.com - name: group-success gid: 88888 ugid: ugid-success @@ -23,7 +26,7 @@ users_to_groups: - uid: 1111 gid: 1111 - uid: 1111 - gid: 88888 + gid: 22222 - uid: 77777 gid: 88888 schema_version: 2 diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group index 8f8934fc6a..1658d2ffbe 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group @@ -1,6 +1,6 @@ -localgroup1:x:41:otheruser,success_with_local_groups,otheruser2 -localgroup2:x:42:success_with_local_groups -localgroup3:x:43:otheruser2,success_with_local_groups -localgroup4:x:44:otheruser2 -cloudgroup1:x:9998:otheruser3 -cloudgroup2:x:9999:otheruser4 +localgroup1:x:41:otheruser@example.com,success_with_local_groups@example.com,otheruser2@example.com +localgroup2:x:42:success_with_local_groups@example.com +localgroup3:x:43:otheruser2@example.com,success_with_local_groups@example.com +localgroup4:x:44:otheruser2@example.com +cloudgroup1:x:9998:otheruser3@example.com +cloudgroup2:x:9999:otheruser4@example.com diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup index f3dce64ae9..2ebaf20f75 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups.group.backup @@ -1,6 +1,6 @@ -localgroup1:x:41:otheruser,success_with_local_groups,otheruser2 -localgroup2:x:42:success_with_local_groups -localgroup3:x:43:otheruser2 -localgroup4:x:44:otheruser2 -cloudgroup1:x:9998:otheruser3 -cloudgroup2:x:9999:otheruser4 +localgroup1:x:41:otheruser@example.com,success_with_local_groups@example.com,otheruser2@example.com +localgroup2:x:42:success_with_local_groups@example.com +localgroup3:x:43:otheruser2@example.com +localgroup4:x:44:otheruser2@example.com +cloudgroup1:x:9998:otheruser3@example.com +cloudgroup2:x:9999:otheruser4@example.com diff --git a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups/cache.db b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups/cache.db index b40a404e43..c6f6c0a54a 100644 --- a/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups/cache.db +++ b/internal/services/pam/testdata/golden/TestIsAuthenticated/Update_local_groups/cache.db @@ -1,17 +1,17 @@ users: - - name: success_with_local_groups + - name: success_with_local_groups@example.com uid: 1111 gid: 1111 - gecos: gecos for success_with_local_groups - dir: /home/success_with_local_groups - shell: /bin/sh/success_with_local_groups + gecos: gecos for success_with_local_groups@example.com + dir: /home/success_with_local_groups@example.com + shell: /bin/sh/success_with_local_groups@example.com groups: - - name: success_with_local_groups + - name: success_with_local_groups@example.com gid: 1111 - ugid: success_with_local_groups - - name: group-success_with_local_groups + ugid: success_with_local_groups@example.com + - name: group-success_with_local_groups@example.com gid: 22222 - ugid: ugid-success_with_local_groups + ugid: ugid-success_with_local_groups@example.com users_to_groups: - uid: 1111 gid: 1111 diff --git a/internal/services/user/testdata/golden/TestGetUserByName/Precheck_user_if_not_in_db b/internal/services/user/testdata/golden/TestGetUserByName/Precheck_user_if_not_in_db index ce06b0424b..f048f3065a 100644 --- a/internal/services/user/testdata/golden/TestGetUserByName/Precheck_user_if_not_in_db +++ b/internal/services/user/testdata/golden/TestGetUserByName/Precheck_user_if_not_in_db @@ -1,6 +1,6 @@ -name: user-pre-check +name: user-pre-check@example.com uid: 1234 gid: 1234 -gecos: gecos for user-pre-check -homedir: /home/user-pre-check -shell: /bin/sh/user-pre-check +gecos: gecos for user-pre-check@example.com +homedir: /home/user-pre-check@example.com +shell: /bin/sh/user-pre-check@example.com diff --git a/internal/services/user/testdata/golden/TestGetUserByName/Prechecked_user_with_upper_cases_in_username_has_same_id_as_lower_case b/internal/services/user/testdata/golden/TestGetUserByName/Prechecked_user_with_upper_cases_in_username_has_same_id_as_lower_case index ce06b0424b..f048f3065a 100644 --- a/internal/services/user/testdata/golden/TestGetUserByName/Prechecked_user_with_upper_cases_in_username_has_same_id_as_lower_case +++ b/internal/services/user/testdata/golden/TestGetUserByName/Prechecked_user_with_upper_cases_in_username_has_same_id_as_lower_case @@ -1,6 +1,6 @@ -name: user-pre-check +name: user-pre-check@example.com uid: 1234 gid: 1234 -gecos: gecos for user-pre-check -homedir: /home/user-pre-check -shell: /bin/sh/user-pre-check +gecos: gecos for user-pre-check@example.com +homedir: /home/user-pre-check@example.com +shell: /bin/sh/user-pre-check@example.com diff --git a/internal/testutils/broker.go b/internal/testutils/broker.go index f85d3db065..8be941a983 100644 --- a/internal/testutils/broker.go +++ b/internal/testutils/broker.go @@ -373,7 +373,7 @@ func userInfoFromName(sessionID string, extraGroups []groupJSONInfo) string { // Default values parsedID := parseSessionID(sessionID) - name := parsedID + name := parsedID + "@example.com" group := "group-" + name home := "/home/" + name shell := "/bin/sh/" + name @@ -384,9 +384,9 @@ func userInfoFromName(sessionID string, extraGroups []groupJSONInfo) string { case "ia_info_empty_user_name": name = "" case "ia_info_mismatching_user_name": - name = "different_username" + name = "different_username@example.com" case "ia_info_invalid_username": - name = "user@invalid-name" + name = "-invalid_username" case "ia_info_empty_group_name": group = "" case "ia_info_empty_ugid": diff --git a/internal/users/types/userinfo.go b/internal/users/types/userinfo.go index 5472d3eefc..f9afc8d080 100644 --- a/internal/users/types/userinfo.go +++ b/internal/users/types/userinfo.go @@ -8,13 +8,16 @@ import ( ) // usernameRegexp is the regexp that a valid username must match. -// It follows the Debian/Ubuntu username policy as defined by shadow-utils/useradd rules. -var usernameRegexp = regexp.MustCompile(`^[a-z_][-a-z0-9_]*[$]?$`) +// It follows the Debian/Ubuntu username policy as defined by shadow-utils/useradd rules, +// extended to allow '@' and '.' characters for cloud identity provider email-style usernames +// (e.g. user@example.com). +var usernameRegexp = regexp.MustCompile(`^[a-z_][-a-z0-9_.@]*[$]?$`) // ValidateUsername checks if the given username is valid. // Valid usernames follow the Debian/Ubuntu naming convention: they start with a lowercase letter or -// underscore, followed by lowercase letters, digits, hyphens, or underscores, with an optional -// trailing dollar sign. +// underscore, followed by lowercase letters, digits, hyphens, underscores, dots, or '@', with an +// optional trailing dollar sign. The '@' and '.' characters are also permitted to support +// email-style usernames used by cloud identity providers. func ValidateUsername(name string) error { if !usernameRegexp.MatchString(name) { return fmt.Errorf("username %q is not valid: it must match %s", name, usernameRegexp) diff --git a/internal/users/types/userinfo_test.go b/internal/users/types/userinfo_test.go index 42812bfc5b..9d1eb474c6 100644 --- a/internal/users/types/userinfo_test.go +++ b/internal/users/types/userinfo_test.go @@ -153,14 +153,16 @@ func TestValidateUsername(t *testing.T) { "Valid_name_with_trailing_dollar_sign": {username: "user$"}, "Valid_single_character_name": {username: "a"}, "Valid_name_with_mixed_allowed_chars": {username: "a-b_c0"}, + "Valid_email_style_name": {username: "user@example.com"}, + "Valid_email_style_name_with_subdomain": {username: "user@sub.example.com"}, + "Valid_name_with_dot": {username: "first.last"}, // Invalid usernames "Error_on_empty_username": {username: "", wantErr: true}, "Error_on_uppercase_character": {username: "User", wantErr: true}, - "Error_on_email_style_name": {username: "user@example.com", wantErr: true}, + "Error_on_uppercase_email": {username: "User@example.com", wantErr: true}, "Error_on_name_starting_with_digit": {username: "1user", wantErr: true}, "Error_on_name_starting_with_hyphen": {username: "-user", wantErr: true}, - "Error_on_name_with_dot": {username: "user.name", wantErr: true}, "Error_on_name_with_dollar_not_at_end": {username: "user$name", wantErr: true}, "Error_on_name_with_space": {username: "user name", wantErr: true}, } From b8eb81b09479c1158400bc4c73053a1b9122a7a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 01:48:09 +0000 Subject: [PATCH 7/7] Add injection/path-traversal character tests to TestValidateUsername Add explicit error test cases for characters that could cause injection or path traversal issues: /, \, ', ", `, ;, &, |, null byte, newline, tab, :, !, (, ), <, >. All are correctly rejected by the allowlist regex. Agent-Logs-Url: https://github.com/canonical/authd/sessions/28fe1c37-8afa-4239-a969-08d92f5a94f5 Co-authored-by: 3v1n0 <345675+3v1n0@users.noreply.github.com> --- internal/users/types/userinfo_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal/users/types/userinfo_test.go b/internal/users/types/userinfo_test.go index 9d1eb474c6..561261f9a7 100644 --- a/internal/users/types/userinfo_test.go +++ b/internal/users/types/userinfo_test.go @@ -165,6 +165,25 @@ func TestValidateUsername(t *testing.T) { "Error_on_name_starting_with_hyphen": {username: "-user", wantErr: true}, "Error_on_name_with_dollar_not_at_end": {username: "user$name", wantErr: true}, "Error_on_name_with_space": {username: "user name", wantErr: true}, + + // Injection / path traversal characters must be rejected + "Error_on_name_with_slash": {username: "user/name", wantErr: true}, + "Error_on_name_with_backslash": {username: `user\name`, wantErr: true}, + "Error_on_name_with_single_quote": {username: "user'name", wantErr: true}, + "Error_on_name_with_double_quote": {username: `user"name`, wantErr: true}, + "Error_on_name_with_backtick": {username: "user`name", wantErr: true}, + "Error_on_name_with_semicolon": {username: "user;name", wantErr: true}, + "Error_on_name_with_ampersand": {username: "user&name", wantErr: true}, + "Error_on_name_with_pipe": {username: "user|name", wantErr: true}, + "Error_on_name_with_null_byte": {username: "user\x00name", wantErr: true}, + "Error_on_name_with_newline": {username: "user\nname", wantErr: true}, + "Error_on_name_with_tab": {username: "user\tname", wantErr: true}, + "Error_on_name_with_colon": {username: "user:name", wantErr: true}, + "Error_on_name_with_exclamation": {username: "user!name", wantErr: true}, + "Error_on_name_with_open_paren": {username: "user(name", wantErr: true}, + "Error_on_name_with_close_paren": {username: "user)name", wantErr: true}, + "Error_on_name_with_less_than": {username: "username", wantErr: true}, } for name, tc := range tests { t.Run(name, func(t *testing.T) {