Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore case when comparing the user name #511

Merged
merged 1 commit into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 5 additions & 10 deletions internal/brokers/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion internal/brokers/broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand All @@ -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"},
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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: <nil>
13 changes: 6 additions & 7 deletions internal/services/pam/pam_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down

This file was deleted.

This file was deleted.

1 change: 0 additions & 1 deletion pam/integration-tests/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down
1 change: 0 additions & 1 deletion pam/integration-tests/native_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down

This file was deleted.

Loading
Loading