diff --git a/internal/brokers/broker.go b/internal/brokers/broker.go index d748e2f2d..6794dfefd 100644 --- a/internal/brokers/broker.go +++ b/internal/brokers/broker.go @@ -194,11 +194,7 @@ func (b Broker) IsAuthenticated(ctx context.Context, sessionID, authenticationDa return "", "", err } - b.ongoingUserRequestsMu.Lock() - selectedUsername := b.ongoingUserRequests[sessionID] - b.ongoingUserRequestsMu.Unlock() - - u, err := validateUserInfoAndGenerateIDs(selectedUsername, info) + u, err := validateUserInfoAndGenerateIDs(info) if err != nil { return "", "", err } @@ -362,16 +358,15 @@ func unmarshalUserInfo(rawMsg json.RawMessage) (userInfo, error) { } // validateUserInfoAndGenerateIDs checks if the specified userinfo is valid and generates the UID and GIDs. -func validateUserInfoAndGenerateIDs(selectedUsername string, uInfo userInfo) (user users.UserInfo, err error) { +func validateUserInfoAndGenerateIDs(uInfo userInfo) (user users.UserInfo, err error) { defer decorate.OnError(&err, "provided userinfo is invalid") - // Validate username + // Validate username. We don't want to check here if it matches the username from the request, because it's the + // broker's responsibility to do that and we don't know which usernames the provider considers equal, for example if + // they are case-sensitive or not. if uInfo.Name == "" { return users.UserInfo{}, fmt.Errorf("empty username") } - if uInfo.Name != selectedUsername { - return users.UserInfo{}, fmt.Errorf("username %q does not match the selected username %q", uInfo.Name, selectedUsername) - } // 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 2001d0410..ea2c04024 100644 --- a/internal/brokers/broker_test.go +++ b/internal/brokers/broker_test.go @@ -219,6 +219,7 @@ func TestIsAuthenticated(t *testing.T) { "No error when auth.Next and no data": {sessionID: "IA_next"}, "No error when broker returns userinfo with empty gecos": {sessionID: "IA_info_empty_gecos"}, "No error when broker returns userinfo with group with empty UGID": {sessionID: "IA_info_empty_ugid"}, + "No error when broker returns userinfo with mismatching username": {sessionID: "IA_info_mismatching_user_name"}, // broker errors "Error when authenticating": {sessionID: "IA_error"}, @@ -227,7 +228,6 @@ 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 mismatching username": {sessionID: "IA_info_mismatching_user_name"}, "Error when broker returns userinfo with empty group name": {sessionID: "IA_info_empty_group_name"}, "Error when broker returns userinfo with empty UUID": {sessionID: "IA_info_empty_uuid"}, "Error when broker returns userinfo with invalid homedir": {sessionID: "IA_info_invalid_home"}, diff --git a/internal/brokers/testdata/TestIsAuthenticated/golden/error_when_broker_returns_userinfo_with_mismatching_username b/internal/brokers/testdata/TestIsAuthenticated/golden/error_when_broker_returns_userinfo_with_mismatching_username deleted file mode 100644 index 7cc91f047..000000000 --- a/internal/brokers/testdata/TestIsAuthenticated/golden/error_when_broker_returns_userinfo_with_mismatching_username +++ /dev/null @@ -1,4 +0,0 @@ -FIRST CALL: - access: - data: - err: provided userinfo is invalid: username "different_username" does not match the selected username "TestIsAuthenticated/Error_when_broker_returns_userinfo_with_mismatching_username_separator_IA_info_mismatching_user_name" diff --git a/internal/brokers/testdata/TestIsAuthenticated/golden/no_error_when_broker_returns_userinfo_with_mismatching_username b/internal/brokers/testdata/TestIsAuthenticated/golden/no_error_when_broker_returns_userinfo_with_mismatching_username new file mode 100644 index 000000000..db7e8fc74 --- /dev/null +++ b/internal/brokers/testdata/TestIsAuthenticated/golden/no_error_when_broker_returns_userinfo_with_mismatching_username @@ -0,0 +1,4 @@ +FIRST CALL: + access: granted + data: {"Name":"different_username","UID":83626,"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":"different_username","GID":83626},{"Name":"group-IA_info_mismatching_user_name","GID":92849}]} + err: diff --git a/internal/services/pam/pam_test.go b/internal/services/pam/pam_test.go index ed0f3dcc7..53c58e275 100644 --- a/internal/services/pam/pam_test.go +++ b/internal/services/pam/pam_test.go @@ -437,13 +437,12 @@ func TestIsAuthenticated(t *testing.T) { "Error when there is no broker": {sessionID: "invalid-session"}, // broker errors - "Error when authenticating": {username: "IA_error"}, - "Error on empty data even if granted": {username: "IA_empty_data"}, - "Error when broker returns invalid access": {username: "IA_invalid_access"}, - "Error when broker returns invalid data": {username: "IA_invalid_data"}, - "Error when broker returns invalid userinfo": {username: "IA_invalid_userinfo"}, - "Error when broker returns username different than the one selected": {username: "IA_info_mismatching_user_name"}, - "Error when calling second time without cancelling": {username: "IA_second_call", secondCall: true}, + "Error when authenticating": {username: "IA_error"}, + "Error on empty data even if granted": {username: "IA_empty_data"}, + "Error when broker returns invalid access": {username: "IA_invalid_access"}, + "Error when broker returns invalid data": {username: "IA_invalid_data"}, + "Error when broker returns invalid userinfo": {username: "IA_invalid_userinfo"}, + "Error when calling second time without cancelling": {username: "IA_second_call", secondCall: true}, // local group error "Error on updating local groups with unexisting file": {username: "success_with_local_groups", localGroupsFile: "does_not_exists.group"}, diff --git a/internal/services/pam/testdata/TestIsAuthenticated/golden/error_when_broker_returns_username_different_than_the_one_selected/IsAuthenticated b/internal/services/pam/testdata/TestIsAuthenticated/golden/error_when_broker_returns_username_different_than_the_one_selected/IsAuthenticated deleted file mode 100644 index 9fb045b2f..000000000 --- a/internal/services/pam/testdata/TestIsAuthenticated/golden/error_when_broker_returns_username_different_than_the_one_selected/IsAuthenticated +++ /dev/null @@ -1,4 +0,0 @@ -FIRST CALL: - access: - msg: - err: can't check authentication: provided userinfo is invalid: username "different_username" does not match the selected username "TestIsAuthenticated/Error_when_broker_returns_username_different_than_the_one_selected_separator_IA_info_mismatching_user_name" diff --git a/internal/services/pam/testdata/TestIsAuthenticated/golden/error_when_broker_returns_username_different_than_the_one_selected/cache.db b/internal/services/pam/testdata/TestIsAuthenticated/golden/error_when_broker_returns_username_different_than_the_one_selected/cache.db deleted file mode 100644 index 0f4b4f51b..000000000 --- a/internal/services/pam/testdata/TestIsAuthenticated/golden/error_when_broker_returns_username_different_than_the_one_selected/cache.db +++ /dev/null @@ -1,7 +0,0 @@ -GroupByID: {} -GroupByName: {} -GroupToUsers: {} -UserByID: {} -UserByName: {} -UserToBroker: {} -UserToGroups: {} diff --git a/pam/integration-tests/cli_test.go b/pam/integration-tests/cli_test.go index 747e760bf..04ebbb315 100644 --- a/pam/integration-tests/cli_test.go +++ b/pam/integration-tests/cli_test.go @@ -65,7 +65,6 @@ func TestCLIAuthenticate(t *testing.T) { "Deny authentication if max attempts reached": {tape: "max_attempts"}, "Deny authentication if user does not exist": {tape: "unexistent_user"}, - "Deny authentication if usernames dont match": {tape: "mismatch_username"}, "Deny authentication if newpassword does not match required criteria": {tape: "bad_password"}, "Exit authd if local broker is selected": {tape: "local_broker"}, diff --git a/pam/integration-tests/native_test.go b/pam/integration-tests/native_test.go index 93bb6ec8e..3e48a9a54 100644 --- a/pam/integration-tests/native_test.go +++ b/pam/integration-tests/native_test.go @@ -66,7 +66,6 @@ func TestNativeAuthenticate(t *testing.T) { "Deny authentication if max attempts reached": {tape: "max_attempts"}, "Deny authentication if user does not exist": {tape: "unexistent_user"}, - "Deny authentication if usernames dont match": {tape: "mismatch_username"}, "Deny authentication if newpassword does not match required criteria": {tape: "bad_password"}, "Exit authd if local broker is selected": {tape: "local_broker"}, diff --git a/pam/integration-tests/testdata/TestCLIAuthenticate/golden/deny_authentication_if_usernames_dont_match b/pam/integration-tests/testdata/TestCLIAuthenticate/golden/deny_authentication_if_usernames_dont_match deleted file mode 100644 index 43f54dd3c..000000000 --- a/pam/integration-tests/testdata/TestCLIAuthenticate/golden/deny_authentication_if_usernames_dont_match +++ /dev/null @@ -1,198 +0,0 @@ -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} -Username: user name - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} -Username: user-mismatching-name - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} - Select your provider - -> 1. local - 2. ExampleBroker - - - - - - - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} -Gimme your password -> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} -Gimme your password -PAM Error Message: authentication status failure: can't check authentication: provided userinfo -is invalid: username "mismatching-username" does not match the selected username "user-mismatchi -ng-name" -PAM Authenticate() for user "user-mismatching-name" exited with error (PAM exit code: 4): System - error -PAM Info Message: acct=incomplete -PAM AcctMgmt() exited with error (PAM exit code: 25): The return value should be ignored by PAM -dispatch -> - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} -Gimme your password -PAM Error Message: authentication status failure: can't check authentication: provided userinfo -is invalid: username "mismatching-username" does not match the selected username "user-mismatchi -ng-name" -PAM Authenticate() for user "user-mismatching-name" exited with error (PAM exit code: 4): System - error -PAM Info Message: acct=incomplete -PAM AcctMgmt() exited with error (PAM exit code: 25): The return value should be ignored by PAM -dispatch -> - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── diff --git a/pam/integration-tests/testdata/TestNativeAuthenticate/golden/deny_authentication_if_usernames_do_not_match b/pam/integration-tests/testdata/TestNativeAuthenticate/golden/deny_authentication_if_usernames_do_not_match deleted file mode 100644 index 603ff011a..000000000 --- a/pam/integration-tests/testdata/TestNativeAuthenticate/golden/deny_authentication_if_usernames_do_not_match +++ /dev/null @@ -1,198 +0,0 @@ -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} force_native_client=true -Username: - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} force_native_client=true -Username: user-mismatching-name - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} force_native_client=true -Username: user-mismatching-name -== Broker selection (use 'r' to go back) == -1 - local -2 - ExampleBroker -Select broker: - - - - - - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} force_native_client=true -Username: user-mismatching-name -== Broker selection (use 'r' to go back) == -1 - local -2 - ExampleBroker -Select broker: 2 -Leave the input field empty or insert 'r' to cancel the request and go back -Gimme your password: - - - - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} force_native_client=true -Username: user-mismatching-name -== Broker selection (use 'r' to go back) == -1 - local -2 - ExampleBroker -Select broker: 2 -Leave the input field empty or insert 'r' to cancel the request and go back -Gimme your password: -authentication status failure: rpc error: code = Unknown desc = can't check authentication: prov -ided userinfo is invalid: username "mismatching-username" does not match the selected username " -user-mismatching-name" -PAM Authenticate() for user "user-mismatching-name" exited with error (PAM exit code: 4): System - error -acct=incomplete -PAM AcctMgmt() exited with error (PAM exit code: 25): The return value should be ignored by PAM -dispatch -> - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} force_native_client=true -Username: user-mismatching-name -== Broker selection (use 'r' to go back) == -1 - local -2 - ExampleBroker -Select broker: 2 -Leave the input field empty or insert 'r' to cancel the request and go back -Gimme your password: -authentication status failure: rpc error: code = Unknown desc = can't check authentication: prov -ided userinfo is invalid: username "mismatching-username" does not match the selected username " -user-mismatching-name" -PAM Authenticate() for user "user-mismatching-name" exited with error (PAM exit code: 4): System - error -acct=incomplete -PAM AcctMgmt() exited with error (PAM exit code: 25): The return value should be ignored by PAM -dispatch -> - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── diff --git a/pam/integration-tests/testdata/TestNativeAuthenticate/golden/deny_authentication_if_usernames_dont_match b/pam/integration-tests/testdata/TestNativeAuthenticate/golden/deny_authentication_if_usernames_dont_match deleted file mode 100644 index a826fbc2d..000000000 --- a/pam/integration-tests/testdata/TestNativeAuthenticate/golden/deny_authentication_if_usernames_dont_match +++ /dev/null @@ -1,198 +0,0 @@ -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} force_native_client=true -Username: - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} force_native_client=true -Username: user-mismatching-name - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} force_native_client=true -Username: user-mismatching-name -== Broker selection (use 'r' to go back) == -1 - local -2 - ExampleBroker -Select broker: - - - - - - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} force_native_client=true -Username: user-mismatching-name -== Broker selection (use 'r' to go back) == -1 - local -2 - ExampleBroker -Select broker: 2 -Insert 'r' to cancel the request and go back -Gimme your password: - - - - - - - - - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} force_native_client=true -Username: user-mismatching-name -== Broker selection (use 'r' to go back) == -1 - local -2 - ExampleBroker -Select broker: 2 -Insert 'r' to cancel the request and go back -Gimme your password: -authentication status failure: can't check authentication: provided userinfo is invalid: usernam -e "mismatching-username" does not match the selected username "user-mismatching-name" -PAM Authenticate() for user "user-mismatching-name" exited with error (PAM exit code: 4): System - error -acct=incomplete -PAM AcctMgmt() exited with error (PAM exit code: 25): The return value should be ignored by PAM -dispatch -> - - - - - - - - - - - - - - - - -──────────────────────────────────────────────────────────────────────────────── -> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK} force_native_client=true -Username: user-mismatching-name -== Broker selection (use 'r' to go back) == -1 - local -2 - ExampleBroker -Select broker: 2 -Insert 'r' to cancel the request and go back -Gimme your password: -authentication status failure: can't check authentication: provided userinfo is invalid: usernam -e "mismatching-username" does not match the selected username "user-mismatching-name" -PAM Authenticate() for user "user-mismatching-name" exited with error (PAM exit code: 4): System - error -acct=incomplete -PAM AcctMgmt() exited with error (PAM exit code: 25): The return value should be ignored by PAM -dispatch -> - - - - - - - - - - - - - - - - -────────────────────────────────────────────────────────────────────────────────