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

brokers/pam: Support (error) messages on Next authentication result #815

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions .github/workflows/qa.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ jobs:
if: matrix.test == 'race'
env:
GO_TESTS_TIMEOUT: 35m
AUTHD_TESTS_SLEEP_MULTIPLIER: 3
run: |
go test -json -timeout ${GO_TESTS_TIMEOUT} -race ./... | \
gotestfmt --logfile "${AUTHD_TEST_ARTIFACTS_PATH}/gotestfmt.race.log"
Expand All @@ -258,6 +259,7 @@ jobs:
CGO_CFLAGS: "-O0 -g3 -fno-omit-frame-pointer"
G_DEBUG: "fatal-criticals"
GO_TESTS_TIMEOUT: 30m
AUTHD_TESTS_SLEEP_MULTIPLIER: 1.5
# Use these flags to give ASAN a better time to unwind the stack trace
GO_GC_FLAGS: -N -l
run: |
Expand Down
6 changes: 5 additions & 1 deletion examplebroker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -626,9 +626,13 @@ func (b *Broker) IsAuthenticated(ctx context.Context, sessionID, authenticationD

access, data = b.handleIsAuthenticated(ctx, sessionInfo, authData)
if access == auth.Granted && sessionInfo.currentAuthStep < sessionInfo.neededAuthSteps {
data = ""
if sessionInfo.pwdChange != noReset && sessionInfo.sessionMode == auth.SessionModeLogin {
data = fmt.Sprintf(`{"message": "Password reset, %d step(s) missing"}`,
sessionInfo.neededAuthSteps-sessionInfo.currentAuthStep)
}
sessionInfo.currentAuthStep++
access = auth.Next
data = ""
} else if access == auth.Retry {
sessionInfo.attemptsPerMode[sessionInfo.currentAuthMode]++
if sessionInfo.attemptsPerMode[sessionInfo.currentAuthMode] >= maxAttempts {
Expand Down
10 changes: 9 additions & 1 deletion internal/brokers/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,15 @@ func (b Broker) IsAuthenticated(ctx context.Context, sessionID, authenticationDa
return "", "", err
}

case auth.Cancelled, auth.Next:
case auth.Next:
if data == "{}" {
break
}
if _, err := unmarshalAndGetKey(data, "message"); err != nil {
return "", "", err
}

case auth.Cancelled:
if data != "{}" {
return "", "", fmt.Errorf("access mode %q should not return any data, got: %v", access, data)
}
Expand Down
3 changes: 2 additions & 1 deletion internal/brokers/broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ func TestIsAuthenticated(t *testing.T) {
"Denies_authentication_when_broker_times_out": {sessionID: "IA_timeout"},
"Adds_default_groups_even_if_broker_did_not_set_them": {sessionID: "IA_info_empty_groups"},
"No_error_when_auth.Next_and_no_data": {sessionID: "IA_next"},
"No_error_when_auth.Next_and_message": {sessionID: "IA_next_with_data"},
"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"},
Expand All @@ -231,7 +232,7 @@ func TestIsAuthenticated(t *testing.T) {
"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"},
"Error_when_broker_returns_data_on_auth.Next": {sessionID: "IA_next_with_data"},
"Error_when_broker_returns_invalid_data_on_auth.Next": {sessionID: "IA_next_with_invalid_data"},
"Error_when_broker_returns_data_on_auth.Cancelled": {sessionID: "IA_cancelled_with_data"},
"Error_when_broker_returns_no_data_on_auth.Denied": {sessionID: "IA_denied_without_data"},
"Error_when_broker_returns_no_data_on_auth.Retry": {sessionID: "IA_retry_without_data"},
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
FIRST CALL:
access:
data:
err: missing key "message" in returned message, got: {"msg": "there should not be a message here"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
FIRST CALL:
access: next
data: {"message": "It's fine to show a message here"}
err: <nil>
1 change: 1 addition & 0 deletions internal/testutils/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func SleepMultiplier() float64 {
if sleepMultiplier <= 0 {
panic("Negative or 0 sleep multiplier is not supported")
}
return
}

if IsAsan() {
Expand Down
6 changes: 5 additions & 1 deletion internal/testutils/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,11 @@ func (b *BrokerBusMock) IsAuthenticated(sessionID, authenticationData string) (a

case "IA_next_with_data":
access = authNext
data = `{"message": "there should not be a message here"}`
data = `{"message": "It's fine to show a message here"}`

case "IA_next_with_invalid_data":
access = authNext
data = `{"msg": "there should not be a message here"}`

case "IA_cancelled_with_data":
access = authCancelled
Expand Down
7 changes: 4 additions & 3 deletions pam/integration-tests/gdm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ func TestGdmModule(t *testing.T) {
},
wantUILayouts: []*authd.UILayout{&testPasswordUILayout, &testNewPasswordUILayout},
wantAuthResponses: []*authd.IAResponse{
{Access: auth.Next},
{Access: auth.Next, Msg: "Password reset, 1 step(s) missing"},
{Access: auth.Granted},
},
},
Expand Down Expand Up @@ -359,8 +359,8 @@ func TestGdmModule(t *testing.T) {
&testNewPasswordUILayout,
},
wantAuthResponses: []*authd.IAResponse{
{Access: auth.Next},
{Access: auth.Next},
{Access: auth.Next, Msg: "Password reset, 2 step(s) missing"},
{Access: auth.Next, Msg: "Password reset, 1 step(s) missing"},
{
Access: auth.Retry,
Msg: "The password fails the dictionary check - it is based on a dictionary word",
Expand Down Expand Up @@ -416,6 +416,7 @@ func TestGdmModule(t *testing.T) {
wantAuthResponses: []*authd.IAResponse{
{
Access: auth.Next,
Msg: "Password reset, 1 step(s) missing",
},
{
Access: auth.Retry,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ Gimme your password:
> ********
────────────────────────────────────────────────────────────────────────────────
> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK}
Gimme your password:
> ********
Password reset, 1 step(s) missing
────────────────────────────────────────────────────────────────────────────────
> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK}
Enter your new password (3 days until mandatory)

New password:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ Gimme your password:
> ********
────────────────────────────────────────────────────────────────────────────────
> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK}
Gimme your password:
> ********
Password reset, 1 step(s) missing
────────────────────────────────────────────────────────────────────────────────
> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK}
Enter your new password

New password:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,16 @@ Gimme your password:
> ********
────────────────────────────────────────────────────────────────────────────────
> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK}
Gimme your password:
> ********
Password reset, 2 step(s) missing
────────────────────────────────────────────────────────────────────────────────
> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK}
Plug your fido device and press with your thumb
────────────────────────────────────────────────────────────────────────────────
> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK}
Plug your fido device and press with your thumb
Password reset, 1 step(s) missing
────────────────────────────────────────────────────────────────────────────────
> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK}
Enter your new password (3 days until mandatory)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ Gimme your password:
> ********
────────────────────────────────────────────────────────────────────────────────
> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK}
Gimme your password:
> ********
Password reset, 1 step(s) missing
────────────────────────────────────────────────────────────────────────────────
> ./pam_authd login socket=${AUTHD_TESTS_CLI_AUTHENTICATE_TESTS_SOCK}
Enter your new password

New password:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Choose your provider:
Enter 'r' to cancel the request and go back to select the authentication method
Gimme your password:
>
Password reset, 1 step(s) missing
== Password reset ==
1. Proceed with password update
2. Skip
Expand All @@ -50,6 +51,7 @@ Choose your provider:
Enter 'r' to cancel the request and go back to select the authentication method
Gimme your password:
>
Password reset, 1 step(s) missing
== Password reset ==
1. Proceed with password update
2. Skip
Expand All @@ -67,6 +69,7 @@ Choose your provider:
Enter 'r' to cancel the request and go back to select the authentication method
Gimme your password:
>
Password reset, 1 step(s) missing
== Password reset ==
1. Proceed with password update
2. Skip
Expand All @@ -88,6 +91,7 @@ Choose your provider:
Enter 'r' to cancel the request and go back to select the authentication method
Gimme your password:
>
Password reset, 1 step(s) missing
== Password reset ==
1. Proceed with password update
2. Skip
Expand All @@ -111,6 +115,7 @@ Choose your provider:
Enter 'r' to cancel the request and go back to select the authentication method
Gimme your password:
>
Password reset, 1 step(s) missing
== Password reset ==
1. Proceed with password update
2. Skip
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Choose your provider:
Enter 'r' to cancel the request and go back to select the authentication method
Gimme your password:
>
Password reset, 1 step(s) missing
== Password reset ==
1. Proceed with password update
2. Skip
Expand All @@ -50,6 +51,7 @@ Choose your provider:
Enter 'r' to cancel the request and go back to select the authentication method
Gimme your password:
>
Password reset, 1 step(s) missing
== Password reset ==
1. Proceed with password update
2. Skip
Expand All @@ -67,6 +69,7 @@ Choose your provider:
Enter 'r' to cancel the request and go back to select the authentication method
Gimme your password:
>
Password reset, 1 step(s) missing
== Password reset ==
1. Proceed with password update
2. Skip
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Choose your provider:
Enter 'r' to cancel the request and go back to select the authentication method
Gimme your password:
>
Password reset, 1 step(s) missing
== Password reset ==
Enter 'r' to cancel the request and go back to choose the provider
Enter your new password:
Expand All @@ -48,6 +49,7 @@ Choose your provider:
Enter 'r' to cancel the request and go back to select the authentication method
Gimme your password:
>
Password reset, 1 step(s) missing
== Password reset ==
Enter 'r' to cancel the request and go back to choose the provider
Enter your new password:
Expand All @@ -65,6 +67,7 @@ Choose your provider:
Enter 'r' to cancel the request and go back to select the authentication method
Gimme your password:
>
Password reset, 1 step(s) missing
== Password reset ==
Enter 'r' to cancel the request and go back to choose the provider
Enter your new password:
Expand Down
Loading
Loading