From 8e0e46e01d48cc5277ab697280797474f40aa3e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 21:00:05 +0200 Subject: [PATCH 01/14] pam/gdm/extension: Do not write debug output twice We already write a similar output in conversation, and that's enough --- pam/internal/gdm/extension.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pam/internal/gdm/extension.go b/pam/internal/gdm/extension.go index 0963a7ff3..f593c5814 100644 --- a/pam/internal/gdm/extension.go +++ b/pam/internal/gdm/extension.go @@ -8,14 +8,12 @@ package gdm import "C" import ( - "context" "encoding/json" "errors" "fmt" "unsafe" "github.com/msteinert/pam/v2" - "github.com/ubuntu/authd/internal/log" ) const ( @@ -147,7 +145,6 @@ func NewBinaryJSONProtoRequest(data []byte) (*pam.BinaryConvRequest, error) { if err != nil { return nil, err } - log.Debugf(context.TODO(), "Sending to gdm %s", string(data)) return pam.NewBinaryConvRequest(request.encode(), func(ptr pam.BinaryPointer) { (*jsonProtoMessage)(ptr).release() }), nil } From 023c6ce121d96a87209472c9017f4722fd94fcb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 21:43:46 +0200 Subject: [PATCH 02/14] pam/gdmmodel: Add test for basic qrcode authentication --- pam/internal/adapter/gdmmodel_test.go | 73 +++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/pam/internal/adapter/gdmmodel_test.go b/pam/internal/adapter/gdmmodel_test.go index f27a534d7..cd9b4bc65 100644 --- a/pam/internal/adapter/gdmmodel_test.go +++ b/pam/internal/adapter/gdmmodel_test.go @@ -922,6 +922,79 @@ func TestGdmModel(t *testing.T) { }, wantExitStatus: PamSuccess{BrokerID: firstBrokerInfo.Id}, }, + "Authenticated with qrcode after auth selection stage from client after client-side broker and auth mode selection": { + supportedLayouts: []*authd.UILayout{ + pam_test.FormUILayout(), + pam_test.QrCodeUILayout(), + }, + clientOptions: append(slices.Clone(singleBrokerClientOptions), + pam_test.WithUILayout("qrcode", "Hello QR!", pam_test.QrCodeUILayout()), + pam_test.WithIsAuthenticatedWantWait(time.Millisecond*500), + ), + gdmEvents: []*gdm.EventData{ + gdm_test.SelectUserEvent("gdm-selected-user-broker-and-auth-mode"), + }, + messages: []tea.Msg{ + gdmTestWaitForStage{ + stage: pam_proto.Stage_brokerSelection, + events: []*gdm.EventData{ + gdm_test.SelectBrokerEvent(firstBrokerInfo.Id), + }, + }, + gdmTestWaitForStage{ + stage: pam_proto.Stage_challenge, + events: []*gdm.EventData{ + gdm_test.ChangeStageEvent(pam_proto.Stage_authModeSelection), + }, + commands: []tea.Cmd{ + sendEvent(gdmTestWaitForStage{ + stage: pam_proto.Stage_authModeSelection, + events: []*gdm.EventData{ + gdm_test.AuthModeSelectedEvent("qrcode"), + }, + commands: []tea.Cmd{ + sendEvent(gdmTestSendAuthDataWhenReady{&authd.IARequest_AuthenticationData_Wait{ + Wait: "true", + }}), + }, + }), + }, + }, + }, + wantUsername: "gdm-selected-user-broker-and-auth-mode", + wantSelectedBroker: firstBrokerInfo.Id, + wantGdmRequests: []gdm.RequestType{ + gdm.RequestType_uiLayoutCapabilities, + gdm.RequestType_changeStage, // -> broker Selection + gdm.RequestType_changeStage, // -> authMode Selection + gdm.RequestType_changeStage, // -> challenge + gdm.RequestType_changeStage, // -> authMode Selection + gdm.RequestType_changeStage, // -> challenge + }, + wantMessages: []tea.Msg{ + startAuthentication{}, + startAuthentication{}, + }, + wantGdmEvents: []gdm.EventType{ + gdm.EventType_userSelected, + gdm.EventType_brokersReceived, + gdm.EventType_brokerSelected, + gdm.EventType_authModeSelected, + gdm.EventType_uiLayoutReceived, + gdm.EventType_startAuthentication, + gdm.EventType_authEvent, + gdm.EventType_authModeSelected, + gdm.EventType_uiLayoutReceived, + gdm.EventType_startAuthentication, + gdm.EventType_authEvent, + }, + wantStage: pam_proto.Stage_challenge, + wantGdmAuthRes: []*authd.IAResponse{ + {Access: brokers.AuthCancelled}, + {Access: brokers.AuthGranted}, + }, + wantExitStatus: PamSuccess{BrokerID: firstBrokerInfo.Id}, + }, "Broker selection stage from client after client-side broker and auth mode selection if there is only one auth mode": { gdmEvents: []*gdm.EventData{ gdm_test.SelectUserEvent("gdm-selected-user-broker-and-auth-mode"), From 49740818bead3377f7ae234484b37f071940bc1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 8 Apr 2024 00:25:57 +0200 Subject: [PATCH 03/14] pam/integration-tests/gdm: Increase timeout and do not end Pam transaction in such case If we're already about to fail because of a timeout don't cleanup the transaction because we may end up stopping ongoing conversations which will lead to a panic, without clear explaination --- pam/integration-tests/gdm_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pam/integration-tests/gdm_test.go b/pam/integration-tests/gdm_test.go index 70d6e7db1..e57268224 100644 --- a/pam/integration-tests/gdm_test.go +++ b/pam/integration-tests/gdm_test.go @@ -290,8 +290,13 @@ func TestGdmModule(t *testing.T) { serviceFile := createServiceFile(t, "gdm-authd", libPath, moduleArgs) + timedOut := false gh := newGdmTestModuleHandler(t, serviceFile, tc.pamUser) - t.Cleanup(func() { require.NoError(t, gh.tx.End(), "PAM: can't end transaction") }) + t.Cleanup(func() { + if !timedOut { + require.NoError(t, gh.tx.End(), "PAM: can't end transaction") + } + }) gh.eventPollResponses = tc.eventPollResponses if tc.supportedLayouts == nil { @@ -325,7 +330,8 @@ func TestGdmModule(t *testing.T) { var err error select { - case <-time.After(10 * time.Second): + case <-time.After(30 * time.Second): + timedOut = true t.Fatal("Authentication timed out!") case err = <-authResult: } From a1b812716cea9eafdf7cfd7af85ad065149f3bcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 16:57:50 +0200 Subject: [PATCH 04/14] pam/integration-tests/gdm: Rename authModeIDs to wantAuthModeIDs This is an expectation, not really something we're doing in the test so change the value as expected --- pam/integration-tests/gdm_test.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/pam/integration-tests/gdm_test.go b/pam/integration-tests/gdm_test.go index e57268224..1b7c66df1 100644 --- a/pam/integration-tests/gdm_test.go +++ b/pam/integration-tests/gdm_test.go @@ -57,10 +57,10 @@ func TestGdmModule(t *testing.T) { pamUser string protoVersion uint32 brokerName string - authModeIDs []string eventPollResponses map[gdm.EventType][]*gdm.EventData wantError error + wantAuthModeIDs []string wantPamInfoMessages []string wantPamErrorMessages []string wantAcctMgmtErr error @@ -76,8 +76,8 @@ func TestGdmModule(t *testing.T) { }, }, "Authenticates user2 with multiple retries": { - pamUser: "user2", - authModeIDs: []string{passwordAuthID, passwordAuthID, passwordAuthID}, + pamUser: "user2", + wantAuthModeIDs: []string{passwordAuthID, passwordAuthID, passwordAuthID}, eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { gdm_test.IsAuthenticatedEvent(&authd.IARequest_AuthenticationData_Challenge{ @@ -93,8 +93,8 @@ func TestGdmModule(t *testing.T) { }, }, "Authenticates user-mfa": { - pamUser: "user-mfa", - authModeIDs: []string{passwordAuthID, fido1AuthID, phoneAck1ID}, + pamUser: "user-mfa", + wantAuthModeIDs: []string{passwordAuthID, fido1AuthID, phoneAck1ID}, eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { gdm_test.IsAuthenticatedEvent(&authd.IARequest_AuthenticationData_Challenge{ @@ -110,8 +110,8 @@ func TestGdmModule(t *testing.T) { }, }, "Authenticates user-mfa after retry": { - pamUser: "user-mfa", - authModeIDs: []string{passwordAuthID, passwordAuthID, fido1AuthID, phoneAck1ID}, + pamUser: "user-mfa", + wantAuthModeIDs: []string{passwordAuthID, passwordAuthID, fido1AuthID, phoneAck1ID}, eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { gdm_test.IsAuthenticatedEvent(&authd.IARequest_AuthenticationData_Challenge{ @@ -130,8 +130,8 @@ func TestGdmModule(t *testing.T) { }, }, "Authenticates user2 after switching to phone ack": { - pamUser: "user2", - authModeIDs: []string{passwordAuthID, phoneAck1ID}, + pamUser: "user2", + wantAuthModeIDs: []string{passwordAuthID, phoneAck1ID}, eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { gdm_test.ChangeStageEvent(proto.Stage_authModeSelection), @@ -197,7 +197,7 @@ func TestGdmModule(t *testing.T) { }, "Error on authenticating user2 with too many retries": { pamUser: "user2", - authModeIDs: []string{ + wantAuthModeIDs: []string{ passwordAuthID, passwordAuthID, passwordAuthID, @@ -249,8 +249,8 @@ func TestGdmModule(t *testing.T) { wantAcctMgmtErr: pam_test.ErrIgnore, }, "Error on invalid fido ack": { - pamUser: "user-mfa", - authModeIDs: []string{passwordAuthID, fido1AuthID}, + pamUser: "user-mfa", + wantAuthModeIDs: []string{passwordAuthID, fido1AuthID}, eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { gdm_test.IsAuthenticatedEvent(&authd.IARequest_AuthenticationData_Challenge{ @@ -313,7 +313,7 @@ func TestGdmModule(t *testing.T) { gh.selectedBrokerName = exampleBrokerName } - gh.selectedAuthModeIDs = tc.authModeIDs + gh.selectedAuthModeIDs = tc.wantAuthModeIDs if gh.selectedAuthModeIDs == nil { gh.selectedAuthModeIDs = []string{passwordAuthID} } From 05c533b9bc4f6fa4a6c2ae13e4db3196bd30afc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 16:38:02 +0200 Subject: [PATCH 05/14] gdm-utils: Allow providing a formatted message to RequireEqualData check --- pam/internal/gdm_test/gdm_utils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pam/internal/gdm_test/gdm_utils.go b/pam/internal/gdm_test/gdm_utils.go index 65794b506..3217fe0eb 100644 --- a/pam/internal/gdm_test/gdm_utils.go +++ b/pam/internal/gdm_test/gdm_utils.go @@ -11,7 +11,7 @@ import ( ) // RequireEqualData ensures that data is equal by checking the marshalled values. -func RequireEqualData(t *testing.T, want any, actual any) { +func RequireEqualData(t *testing.T, want any, actual any, args ...any) { t.Helper() wantJSON, err := json.MarshalIndent(want, "", " ") @@ -19,7 +19,7 @@ func RequireEqualData(t *testing.T, want any, actual any) { actualJSON, err := json.MarshalIndent(actual, "", " ") require.NoError(t, err) - require.Equal(t, string(wantJSON), string(actualJSON)) + require.Equal(t, string(wantJSON), string(actualJSON), args...) } // DataToJSON is a test helper function to convert GDM data to JSON. From 7a77d331b67238ec62869ef8a06561f4f1b63b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 19:49:57 +0200 Subject: [PATCH 06/14] pam/integration-tests/gdm: Always use unique user names in tests Since we are sharing the daemon we should not reuse the same user multiple time, so generate them based on the test name to avoid even manual clashes --- examplebroker/broker.go | 5 ++++ pam/integration-tests/gdm_test.go | 46 +++++++++++++++---------------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/examplebroker/broker.go b/examplebroker/broker.go index 4f4787113..353315530 100644 --- a/examplebroker/broker.go +++ b/examplebroker/broker.go @@ -162,6 +162,11 @@ func (b *Broker) NewSession(ctx context.Context, username, lang, mode string) (s exampleUsers[username] = userInfoBroker{Password: "goodpass"} } + if _, ok := exampleUsers[username]; !ok && strings.HasPrefix(username, "user-mfa-integration") { + exampleUsers[username] = userInfoBroker{Password: "goodpass"} + info.neededAuthSteps = 3 + } + pubASN1, err := x509.MarshalPKIXPublicKey(&b.privateKey.PublicKey) if err != nil { return "", "", err diff --git a/pam/integration-tests/gdm_test.go b/pam/integration-tests/gdm_test.go index 1b7c66df1..76d8ea21c 100644 --- a/pam/integration-tests/gdm_test.go +++ b/pam/integration-tests/gdm_test.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" "time" @@ -54,7 +55,7 @@ func TestGdmModule(t *testing.T) { testCases := map[string]struct { supportedLayouts []*authd.UILayout - pamUser string + pamUser *string protoVersion uint32 brokerName string eventPollResponses map[gdm.EventType][]*gdm.EventData @@ -65,8 +66,7 @@ func TestGdmModule(t *testing.T) { wantPamErrorMessages []string wantAcctMgmtErr error }{ - "Authenticates user1": { - pamUser: "user1", + "Authenticates user": { eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { gdm_test.IsAuthenticatedEvent(&authd.IARequest_AuthenticationData_Challenge{ @@ -75,8 +75,7 @@ func TestGdmModule(t *testing.T) { }, }, }, - "Authenticates user2 with multiple retries": { - pamUser: "user2", + "Authenticates user with multiple retries": { wantAuthModeIDs: []string{passwordAuthID, passwordAuthID, passwordAuthID}, eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { @@ -93,7 +92,7 @@ func TestGdmModule(t *testing.T) { }, }, "Authenticates user-mfa": { - pamUser: "user-mfa", + pamUser: ptrValue("user-mfa"), wantAuthModeIDs: []string{passwordAuthID, fido1AuthID, phoneAck1ID}, eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { @@ -110,7 +109,7 @@ func TestGdmModule(t *testing.T) { }, }, "Authenticates user-mfa after retry": { - pamUser: "user-mfa", + pamUser: ptrValue("user-mfa-integration-retry"), wantAuthModeIDs: []string{passwordAuthID, passwordAuthID, fido1AuthID, phoneAck1ID}, eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { @@ -129,8 +128,7 @@ func TestGdmModule(t *testing.T) { }, }, }, - "Authenticates user2 after switching to phone ack": { - pamUser: "user2", + "Authenticates user switching to phone ack": { wantAuthModeIDs: []string{passwordAuthID, phoneAck1ID}, eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { @@ -147,7 +145,6 @@ func TestGdmModule(t *testing.T) { // Error cases "Error on unknown protocol": { - pamUser: "user-foo", protoVersion: 9999, wantPamErrorMessages: []string{ "GDM protocol initialization failed, type hello, version 9999", @@ -156,7 +153,7 @@ func TestGdmModule(t *testing.T) { wantAcctMgmtErr: pam_test.ErrIgnore, }, "Error on missing user": { - pamUser: "", + pamUser: ptrValue(""), wantPamErrorMessages: []string{ "can't select broker: rpc error: code = InvalidArgument desc = can't start authentication transaction: rpc error: code = InvalidArgument desc = no user name provided", }, @@ -164,7 +161,6 @@ func TestGdmModule(t *testing.T) { wantAcctMgmtErr: pam_test.ErrIgnore, }, "Error on no supported layouts": { - pamUser: "user-bar", supportedLayouts: []*authd.UILayout{}, wantPamErrorMessages: []string{ "UI does not support any layouts", @@ -173,7 +169,6 @@ func TestGdmModule(t *testing.T) { wantAcctMgmtErr: pam_test.ErrIgnore, }, "Error on unknown broker": { - pamUser: "user-foo", brokerName: "Not a valid broker!", eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_brokersReceived: { @@ -187,7 +182,6 @@ func TestGdmModule(t *testing.T) { wantAcctMgmtErr: pam_test.ErrIgnore, }, "Error (ignored) on local broker causes fallback error": { - pamUser: "user-foo", brokerName: brokers.LocalBrokerName, wantPamInfoMessages: []string{ "auth=incomplete", @@ -195,8 +189,7 @@ func TestGdmModule(t *testing.T) { wantError: pam_test.ErrIgnore, wantAcctMgmtErr: pam.ErrAbort, }, - "Error on authenticating user2 with too many retries": { - pamUser: "user2", + "Error on authenticating user with too many retries": { wantAuthModeIDs: []string{ passwordAuthID, passwordAuthID, @@ -234,7 +227,7 @@ func TestGdmModule(t *testing.T) { wantAcctMgmtErr: pam_test.ErrIgnore, }, "Error on authenticating unknown user": { - pamUser: "user-unknown", + pamUser: ptrValue("user-unknown"), eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { gdm_test.IsAuthenticatedEvent(&authd.IARequest_AuthenticationData_Challenge{ @@ -249,7 +242,7 @@ func TestGdmModule(t *testing.T) { wantAcctMgmtErr: pam_test.ErrIgnore, }, "Error on invalid fido ack": { - pamUser: "user-mfa", + pamUser: ptrValue("user-mfa-integration-error-fido-ack"), wantAuthModeIDs: []string{passwordAuthID, fido1AuthID}, eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { @@ -290,8 +283,13 @@ func TestGdmModule(t *testing.T) { serviceFile := createServiceFile(t, "gdm-authd", libPath, moduleArgs) + pamUser := "user-integration-" + strings.ReplaceAll(filepath.Base(t.Name()), "_", "-") + if tc.pamUser != nil { + pamUser = *tc.pamUser + } + timedOut := false - gh := newGdmTestModuleHandler(t, serviceFile, tc.pamUser) + gh := newGdmTestModuleHandler(t, serviceFile, pamUser) t.Cleanup(func() { if !timedOut { require.NoError(t, gh.tx.End(), "PAM: can't end transaction") @@ -342,19 +340,19 @@ func TestGdmModule(t *testing.T) { require.Equal(t, tc.wantPamInfoMessages, gh.pamInfoMessages, "PAM Info messages do not match") - requirePreviousBrokerForUser(t, socketPath, "", tc.pamUser) + requirePreviousBrokerForUser(t, socketPath, "", pamUser) require.ErrorIs(t, gh.tx.AcctMgmt(pamFlags), tc.wantAcctMgmtErr, "Account Management PAM Error messages do not match") if tc.wantError != nil { - requirePreviousBrokerForUser(t, socketPath, "", tc.pamUser) + requirePreviousBrokerForUser(t, socketPath, "", pamUser) return } user, err := gh.tx.GetItem(pam.User) require.NoError(t, err, "Can't get the pam user") - require.Equal(t, tc.pamUser, user, "PAM user name does not match expected") + require.Equal(t, pamUser, user, "PAM user name does not match expected") requirePreviousBrokerForUser(t, socketPath, gh.selectedBrokerName, user) }) @@ -384,7 +382,7 @@ func TestGdmModuleAuthenticateWithoutGdmExtension(t *testing.T) { moduleArgs = append(moduleArgs, "debug=true", "logfile="+gdmLog) serviceFile := createServiceFile(t, "gdm-authd", libPath, moduleArgs) - pamUser := "user1" + pamUser := "user-integration-auth-no-gdm-extension" gh := newGdmTestModuleHandler(t, serviceFile, pamUser) t.Cleanup(func() { require.NoError(t, gh.tx.End(), "PAM: can't end transaction") }) @@ -426,7 +424,7 @@ func TestGdmModuleAcctMgmtWithoutGdmExtension(t *testing.T) { moduleArgs = append(moduleArgs, "debug=true", "logfile="+gdmLog) serviceFile := createServiceFile(t, "gdm-authd", libPath, moduleArgs) - pamUser := "user1" + pamUser := "user-integration-acctmgmt-no-gdm-extension" gh := newGdmTestModuleHandler(t, serviceFile, pamUser) t.Cleanup(func() { require.NoError(t, gh.tx.End(), "PAM: can't end transaction") }) From 01538528f6d30f651fe882f4e5e0327c386937cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 16:40:13 +0200 Subject: [PATCH 07/14] pam/integration-tests/gdm: Also check for the current UI layout at auth phase --- .../gdm-module-handler_test.go | 18 +++++++++++++++++- pam/integration-tests/gdm_test.go | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/pam/integration-tests/gdm-module-handler_test.go b/pam/integration-tests/gdm-module-handler_test.go index 2a37d1e2c..97234818c 100644 --- a/pam/integration-tests/gdm-module-handler_test.go +++ b/pam/integration-tests/gdm-module-handler_test.go @@ -23,7 +23,9 @@ type gdmTestModuleHandler struct { protoVersion uint32 - supportedLayouts []*authd.UILayout + supportedLayouts []*authd.UILayout + currentUILayout *authd.UILayout + selectedUILayouts []*authd.UILayout currentStage proto.Stage pollResponses []*gdm.EventData @@ -126,6 +128,11 @@ func (gh *gdmTestModuleHandler) exampleHandleEvent(event *gdm.EventData) error { if layout.Label != nil { gh.t.Logf("%s:", *layout.Label) } + if layout.Content != nil { + gh.t.Logf("%s:", *layout.Content) + } + + gh.currentUILayout = layout case *gdm.EventData_StartAuthentication: idx := slices.IndexFunc(gh.authModes, func(mode *authd.GAMResponse_AuthenticationMode) bool { @@ -142,6 +149,15 @@ func (gh *gdmTestModuleHandler) exampleHandleEvent(event *gdm.EventData) error { "Selected authentication mode ID does not match expected one") gh.selectedAuthModeIDs = slices.Delete(gh.selectedAuthModeIDs, 0, 1) + if len(gh.selectedUILayouts) < 1 { + // TODO: Make this an error but we don't support checking the layout in all tests yet. + return nil + } + + gdm_test.RequireEqualData(gh.t, gh.selectedUILayouts[0], gh.currentUILayout, + "Selected UI layout does not match expected one") + gh.selectedUILayouts = slices.Delete(gh.selectedUILayouts, 0, 1) + case *gdm.EventData_AuthEvent: gh.t.Logf("Authentication event: %s", ev.AuthEvent.Response) if msg := ev.AuthEvent.Response.Msg; msg != "" { diff --git a/pam/integration-tests/gdm_test.go b/pam/integration-tests/gdm_test.go index 76d8ea21c..6ed9b8209 100644 --- a/pam/integration-tests/gdm_test.go +++ b/pam/integration-tests/gdm_test.go @@ -38,6 +38,16 @@ const ( phoneAck1ID = "phoneack1" ) +var testPasswordUILayout = authd.UILayout{ + Type: "form", + Label: ptrValue("Gimme your password"), + Entry: ptrValue("chars_password"), + Button: ptrValue(""), + Code: ptrValue(""), + Content: ptrValue(""), + Wait: ptrValue(""), +} + func TestGdmModule(t *testing.T) { t.Parallel() t.Cleanup(pam_test.MaybeDoLeakCheck) @@ -62,6 +72,7 @@ func TestGdmModule(t *testing.T) { wantError error wantAuthModeIDs []string + wantUILayouts []*authd.UILayout wantPamInfoMessages []string wantPamErrorMessages []string wantAcctMgmtErr error @@ -316,6 +327,13 @@ func TestGdmModule(t *testing.T) { gh.selectedAuthModeIDs = []string{passwordAuthID} } + gh.selectedUILayouts = tc.wantUILayouts + if gh.selectedAuthModeIDs == nil && + len(gh.selectedAuthModeIDs) == 1 && + gh.selectedAuthModeIDs[0] == passwordAuthID { + gh.selectedUILayouts = []*authd.UILayout{&testPasswordUILayout} + } + var pamFlags pam.Flags if !testutils.IsVerbose() { pamFlags = pam.Silent From 58f0335ed5c833e71cdce97aa579e61ee69c006a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 04:16:25 +0200 Subject: [PATCH 08/14] pam/integration-tests/gdm: Mimic more the gdm behavior on auth events In gdm when the authentication stage is changed we reset the currently svaed states, so do it also in the integration test mock to respect what the UI would do --- pam/integration-tests/gdm-module-handler_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pam/integration-tests/gdm-module-handler_test.go b/pam/integration-tests/gdm-module-handler_test.go index 97234818c..c592cc9e4 100644 --- a/pam/integration-tests/gdm-module-handler_test.go +++ b/pam/integration-tests/gdm-module-handler_test.go @@ -189,6 +189,14 @@ func (gh *gdmTestModuleHandler) exampleHandleAuthDRequest(gdmData *gdm.Data) (*g gh.currentStage = req.ChangeStage.Stage log.Debugf(context.TODO(), "Switching to stage %d", gh.currentStage) + switch req.ChangeStage.Stage { + case proto.Stage_brokerSelection: + gh.authModes = nil + gh.brokerID = "" + case proto.Stage_authModeSelection: + gh.currentUILayout = nil + } + return &gdm.Data{ Type: gdm.DataType_response, Response: &gdm.ResponseData{ From e82ae9e3c08f35bd0c9a9aee5841638fbba25c0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 04:26:11 +0200 Subject: [PATCH 09/14] pam/gdm_test: Support replying to a GDM event with multiple events Sometimes we may want to respond to a specific gdm event with multiple poll responses, to allow this, without refactor the world, we can just create a simple fake event and use its type as a counter for the events we want to group. When preceding events with this fake event, then the following N events will be added to the poll responses --- .../gdm-module-handler_test.go | 16 +++++++++++++--- pam/integration-tests/gdm_test.go | 7 ++++--- pam/internal/gdm_test/gdm_utils.go | 18 ++++++++++++++++++ 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/pam/integration-tests/gdm-module-handler_test.go b/pam/integration-tests/gdm-module-handler_test.go index c592cc9e4..26899a75d 100644 --- a/pam/integration-tests/gdm-module-handler_test.go +++ b/pam/integration-tests/gdm-module-handler_test.go @@ -80,9 +80,19 @@ func (gh *gdmTestModuleHandler) exampleHandleGdmData(gdmData *gdm.Data) (*gdm.Da func (gh *gdmTestModuleHandler) exampleHandleEvent(event *gdm.EventData) error { events, ok := gh.eventPollResponses[event.Type] if ok && len(events) > 0 { - pollResp := events[0] - gh.eventPollResponses[event.Type] = slices.Delete(events, 0, 1) - gh.pollResponses = append(gh.pollResponses, pollResp) + numEvents := 1 + if events[0].Type == gdm_test.EventsGroupBegin().Type { + numEvents = slices.IndexFunc(events, func(ev *gdm.EventData) bool { + return ev.Type == gdm_test.EventsGroupEnd().Type + }) + require.Greater(gh.t, numEvents, 1, "No valid events group found") + events = slices.Delete(events, numEvents, numEvents+1) + events = slices.Delete(events, 0, 1) + numEvents-- + } + pollEvents := slices.Clone(events[0:numEvents]) + gh.eventPollResponses[event.Type] = slices.Delete(events, 0, numEvents) + gh.pollResponses = append(gh.pollResponses, pollEvents...) } switch ev := event.Data.(type) { diff --git a/pam/integration-tests/gdm_test.go b/pam/integration-tests/gdm_test.go index 6ed9b8209..34f489f58 100644 --- a/pam/integration-tests/gdm_test.go +++ b/pam/integration-tests/gdm_test.go @@ -143,14 +143,15 @@ func TestGdmModule(t *testing.T) { wantAuthModeIDs: []string{passwordAuthID, phoneAck1ID}, eventPollResponses: map[gdm.EventType][]*gdm.EventData{ gdm.EventType_startAuthentication: { + gdm_test.EventsGroupBegin(), gdm_test.ChangeStageEvent(proto.Stage_authModeSelection), + gdm_test.AuthModeSelectedEvent(phoneAck1ID), + gdm_test.EventsGroupEnd(), + gdm_test.IsAuthenticatedEvent(&authd.IARequest_AuthenticationData_Wait{ Wait: "true", }), }, - gdm.EventType_authEvent: { - gdm_test.AuthModeSelectedEvent(phoneAck1ID), - }, }, }, diff --git a/pam/internal/gdm_test/gdm_utils.go b/pam/internal/gdm_test/gdm_utils.go index 3217fe0eb..f95f8a5d1 100644 --- a/pam/internal/gdm_test/gdm_utils.go +++ b/pam/internal/gdm_test/gdm_utils.go @@ -31,6 +31,24 @@ func DataToJSON(t *testing.T, data *gdm.Data) string { return string(json) } +// EventsGroupBegin returns a fake [gdm.EventData] that allows to begin a group multiple events +// so that it's possible to use this as an header to tell the test module handler that we should +// respond to an event with multiple events starting from the next one. +func EventsGroupBegin() *gdm.EventData { + return &gdm.EventData{ + Type: gdm.EventType(-1000), + } +} + +// EventsGroupEnd returns a fake [gdm.EventData] that allows to end a group multiple events +// so that it's possible to use this as a footer to tell the test module handler that we should +// respond to an event with multiple events finishing with the previous one. +func EventsGroupEnd() *gdm.EventData { + return &gdm.EventData{ + Type: gdm.EventType(-1001), + } +} + // SelectUserEvent generates a SelectUser event. func SelectUserEvent(username string) *gdm.EventData { return &gdm.EventData{ From 5ac89c52b178bd2a45665b376f1147609d13725b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 04:15:25 +0200 Subject: [PATCH 10/14] pam/integration-tests/gdm: Add simple test for qrcode authentication --- pam/integration-tests/gdm_test.go | 47 +++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/pam/integration-tests/gdm_test.go b/pam/integration-tests/gdm_test.go index 34f489f58..484d159a8 100644 --- a/pam/integration-tests/gdm_test.go +++ b/pam/integration-tests/gdm_test.go @@ -36,6 +36,7 @@ const ( passwordAuthID = "password" fido1AuthID = "fidodevice1" phoneAck1ID = "phoneack1" + qrcodeID = "qrcodewithtypo" ) var testPasswordUILayout = authd.UILayout{ @@ -48,6 +49,16 @@ var testPasswordUILayout = authd.UILayout{ Wait: ptrValue(""), } +var testQrcodeUILayout = authd.UILayout{ + Type: "qrcode", + Label: ptrValue("Enter the following code after flashing the address: 1337"), + Content: ptrValue("https://ubuntu.com"), + Wait: ptrValue("true"), + Button: ptrValue("Regenerate code"), + Code: ptrValue(""), + Entry: ptrValue(""), +} + func TestGdmModule(t *testing.T) { t.Parallel() t.Cleanup(pam_test.MaybeDoLeakCheck) @@ -154,6 +165,41 @@ func TestGdmModule(t *testing.T) { }, }, }, + "Authenticates user with qrcode": { + wantAuthModeIDs: []string{qrcodeID}, + supportedLayouts: []*authd.UILayout{pam_test.QrCodeUILayout()}, + eventPollResponses: map[gdm.EventType][]*gdm.EventData{ + gdm.EventType_startAuthentication: { + gdm_test.IsAuthenticatedEvent(&authd.IARequest_AuthenticationData_Wait{ + Wait: "true", + }), + }, + }, + wantUILayouts: []*authd.UILayout{&testQrcodeUILayout}, + }, + "Authenticates user after switching to qrcode": { + wantAuthModeIDs: []string{passwordAuthID, qrcodeID}, + supportedLayouts: []*authd.UILayout{ + pam_test.FormUILayout(), + pam_test.QrCodeUILayout(), + }, + eventPollResponses: map[gdm.EventType][]*gdm.EventData{ + gdm.EventType_startAuthentication: { + gdm_test.EventsGroupBegin(), + gdm_test.ChangeStageEvent(proto.Stage_authModeSelection), + gdm_test.AuthModeSelectedEvent(qrcodeID), + gdm_test.EventsGroupEnd(), + + gdm_test.IsAuthenticatedEvent(&authd.IARequest_AuthenticationData_Wait{ + Wait: "true", + }), + }, + }, + wantUILayouts: []*authd.UILayout{ + &testPasswordUILayout, + &testQrcodeUILayout, + }, + }, // Error cases "Error on unknown protocol": { @@ -309,6 +355,7 @@ func TestGdmModule(t *testing.T) { }) gh.eventPollResponses = tc.eventPollResponses + gh.supportedLayouts = tc.supportedLayouts if tc.supportedLayouts == nil { gh.supportedLayouts = []*authd.UILayout{pam_test.FormUILayout()} } From f4ed03c076e390dbb0f3459ea8f0a517f7901b4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 18:26:37 +0200 Subject: [PATCH 11/14] pam/gdmmodel: Support cancelling authentication all the times Cancellation needs to be possible everytime we're in challenge mode, but this is handled already by the authentication backend so don't bother adding the logic here. Since in this way we'll prevent being able to cancel authentications that are in "wait" state and so when we're not anymore waiting for user authentication request, but only for the broker reply. --- pam/internal/adapter/gdmmodel.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pam/internal/adapter/gdmmodel.go b/pam/internal/adapter/gdmmodel.go index ada7fee90..0a0c940ce 100644 --- a/pam/internal/adapter/gdmmodel.go +++ b/pam/internal/adapter/gdmmodel.go @@ -141,9 +141,7 @@ func (m *gdmModel) pollGdm() tea.Cmd { commands = append(commands, sendEvent(reselectAuthMode{})) case *gdm.EventData_IsAuthenticatedCancelled: - if m.waitingAuth { - commands = append(commands, sendEvent(isAuthenticatedCancelled{})) - } + commands = append(commands, sendEvent(isAuthenticatedCancelled{})) case *gdm.EventData_StageChanged: if res.StageChanged == nil { From c912c143008cef98524585f0614f829d1671d664 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 04:20:54 +0200 Subject: [PATCH 12/14] pam/gdmmodel: Reset the waiting authentication on reselect auth mode After this happens we're gonna receive a further start authentication event, so we need to stop considering the model being in auth phase --- pam/internal/adapter/gdmmodel.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pam/internal/adapter/gdmmodel.go b/pam/internal/adapter/gdmmodel.go index 0a0c940ce..29612903e 100644 --- a/pam/internal/adapter/gdmmodel.go +++ b/pam/internal/adapter/gdmmodel.go @@ -230,6 +230,9 @@ func (m gdmModel) Update(msg tea.Msg) (gdmModel, tea.Cmd) { StartAuthentication: &gdm.Events_StartAuthentication{}, })) + case reselectAuthMode: + m.waitingAuth = false + case isAuthenticatedResultReceived: access := msg.access authMsg, err := dataToMsg(msg.msg) From 7c4e2e48e693109fdf15d348e814020b348a3da2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 23:09:50 +0200 Subject: [PATCH 13/14] pam/client-dummy: Add support for cancelling isAuthenticated request When waiting we may want to be able to cancel it, so support it --- pam/internal/pam_test/pam-client-dummy.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pam/internal/pam_test/pam-client-dummy.go b/pam/internal/pam_test/pam-client-dummy.go index e27d83c8d..07b9a320e 100644 --- a/pam/internal/pam_test/pam-client-dummy.go +++ b/pam/internal/pam_test/pam-client-dummy.go @@ -428,7 +428,14 @@ func (dc *DummyClient) IsAuthenticated(ctx context.Context, in *authd.IARequest, if dc.isAuthenticatedWantWait == 0 { return nil, errors.New("no wanted wait provided") } - time.Sleep(dc.isAuthenticatedWantWait) + select { + case <-time.After(dc.isAuthenticatedWantWait): + case <-ctx.Done(): + return &authd.IAResponse{ + Access: brokers.AuthCancelled, + Msg: fmt.Sprintf(`{"message": "Cancelled: %s"}`, dc.isAuthenticatedMessage), + }, nil + } return &authd.IAResponse{ Access: brokers.AuthGranted, Msg: msg, From fe1e2d53ac0134f3278809b4428e53808f165f35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 27 Jun 2024 21:46:16 +0200 Subject: [PATCH 14/14] pam/gdmmodel: Add test for regenerating Qr code --- pam/internal/adapter/gdmmodel_test.go | 90 ++++++++++++++++++- pam/internal/adapter/gdmmodel_uimodel_test.go | 4 +- pam/internal/gdm_test/gdm_utils.go | 10 +++ 3 files changed, 101 insertions(+), 3 deletions(-) diff --git a/pam/internal/adapter/gdmmodel_test.go b/pam/internal/adapter/gdmmodel_test.go index cd9b4bc65..54e263066 100644 --- a/pam/internal/adapter/gdmmodel_test.go +++ b/pam/internal/adapter/gdmmodel_test.go @@ -83,6 +83,7 @@ func TestGdmModel(t *testing.T) { pamUser string protoVersion uint32 convError map[string]error + timeout time.Duration wantExitStatus PamReturnStatus wantGdmRequests []gdm.RequestType @@ -914,6 +915,7 @@ func TestGdmModel(t *testing.T) { gdm.EventType_uiLayoutReceived, gdm.EventType_startAuthentication, gdm.EventType_authEvent, + gdm.EventType_authEvent, }, wantStage: pam_proto.Stage_challenge, wantGdmAuthRes: []*authd.IAResponse{ @@ -995,6 +997,88 @@ func TestGdmModel(t *testing.T) { }, wantExitStatus: PamSuccess{BrokerID: firstBrokerInfo.Id}, }, + "Authenticated with qrcode regenerated after auth selection stage from client after client-side broker and auth mode selection": { + timeout: 10 * time.Second, + supportedLayouts: []*authd.UILayout{ + pam_test.FormUILayout(), + pam_test.QrCodeUILayout(), + }, + clientOptions: append(slices.Clone(singleBrokerClientOptions), + pam_test.WithUILayout("qrcode", "Hello QR!", pam_test.QrCodeUILayout()), + pam_test.WithIsAuthenticatedWantWait(time.Millisecond*500), + ), + gdmEvents: []*gdm.EventData{ + gdm_test.SelectUserEvent("gdm-selected-user-broker-and-auth-mode"), + }, + messages: []tea.Msg{ + gdmTestWaitForStage{ + stage: pam_proto.Stage_brokerSelection, + events: []*gdm.EventData{ + gdm_test.SelectBrokerEvent(firstBrokerInfo.Id), + }, + }, + gdmTestWaitForStage{ + stage: pam_proto.Stage_challenge, + events: []*gdm.EventData{ + gdm_test.ChangeStageEvent(pam_proto.Stage_authModeSelection), + }, + commands: []tea.Cmd{ + sendEvent(gdmTestWaitForStage{ + stage: pam_proto.Stage_authModeSelection, + events: []*gdm.EventData{ + gdm_test.AuthModeSelectedEvent("qrcode"), + }, + commands: []tea.Cmd{ + sendEvent(gdmTestSendAuthDataWhenReady{}), + sendEvent(gdmTestWaitForStage{ + stage: pam_proto.Stage_challenge, + events: []*gdm.EventData{ + gdm_test.ReselectAuthMode(), + }, + }), + sendEvent(gdmTestSendAuthDataWhenReady{&authd.IARequest_AuthenticationData_Wait{ + Wait: "true", + }}), + }, + }), + }, + }, + }, + wantUsername: "gdm-selected-user-broker-and-auth-mode", + wantSelectedBroker: firstBrokerInfo.Id, + wantGdmRequests: []gdm.RequestType{ + gdm.RequestType_uiLayoutCapabilities, + gdm.RequestType_changeStage, // -> broker Selection + gdm.RequestType_changeStage, // -> authMode Selection + gdm.RequestType_changeStage, // -> challenge + gdm.RequestType_changeStage, // -> authMode Selection + gdm.RequestType_changeStage, // -> challenge + }, + wantMessages: []tea.Msg{ + startAuthentication{}, + startAuthentication{}, + startAuthentication{}, + }, + wantGdmEvents: []gdm.EventType{ + gdm.EventType_userSelected, + gdm.EventType_brokersReceived, + gdm.EventType_brokerSelected, + gdm.EventType_authModeSelected, + gdm.EventType_uiLayoutReceived, + gdm.EventType_startAuthentication, + gdm.EventType_authModeSelected, + gdm.EventType_authEvent, + gdm.EventType_uiLayoutReceived, + gdm.EventType_startAuthentication, + gdm.EventType_authEvent, + }, + wantStage: pam_proto.Stage_challenge, + wantGdmAuthRes: []*authd.IAResponse{ + {Access: brokers.AuthCancelled}, + {Access: brokers.AuthGranted}, + }, + wantExitStatus: PamSuccess{BrokerID: firstBrokerInfo.Id}, + }, "Broker selection stage from client after client-side broker and auth mode selection if there is only one auth mode": { gdmEvents: []*gdm.EventData{ gdm_test.SelectUserEvent("gdm-selected-user-broker-and-auth-mode"), @@ -1924,7 +2008,6 @@ func TestGdmModel(t *testing.T) { p := tea.NewProgram(&appState, teaOpts...) appState.program = p - // testHadTimeout := false controlDone := make(chan struct{}) go func() { wg := sync.WaitGroup{} @@ -1970,13 +2053,16 @@ func TestGdmModel(t *testing.T) { } t.Log("Waiting for expected events") + if tc.timeout == 0 { + tc.timeout = 5 * time.Second + } waitChan := make(chan struct{}) go func() { wg.Wait() close(waitChan) }() select { - case <-time.After(5 * time.Second): + case <-time.After(tc.timeout): case <-waitChan: } t.Log("Waiting for events done...") diff --git a/pam/internal/adapter/gdmmodel_uimodel_test.go b/pam/internal/adapter/gdmmodel_uimodel_test.go index 38360c322..8860fbda1 100644 --- a/pam/internal/adapter/gdmmodel_uimodel_test.go +++ b/pam/internal/adapter/gdmmodel_uimodel_test.go @@ -141,7 +141,9 @@ func (m *gdmTestUIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { go func() { m.gdmHandler.waitForAuthenticationStarted() - m.gdmHandler.appendPollResultEvents(gdm_test.IsAuthenticatedEvent(msg.item)) + if msg.item != nil { + m.gdmHandler.appendPollResultEvents(gdm_test.IsAuthenticatedEvent(msg.item)) + } m.program.Send(tea.Sequence(tea.Tick(gdmPollFrequency, func(t time.Time) tea.Msg { return sendEvent(doneMsg) }), sendEvent(doneMsg))()) diff --git a/pam/internal/gdm_test/gdm_utils.go b/pam/internal/gdm_test/gdm_utils.go index f95f8a5d1..7bfa17273 100644 --- a/pam/internal/gdm_test/gdm_utils.go +++ b/pam/internal/gdm_test/gdm_utils.go @@ -91,6 +91,16 @@ func AuthModeSelectedEvent(authModeID string) *gdm.EventData { } } +// ReselectAuthMode generates a ReselectAuthMode event. +func ReselectAuthMode() *gdm.EventData { + return &gdm.EventData{ + Type: gdm.EventType_reselectAuthMode, + Data: &gdm.EventData_ReselectAuthMode{ + ReselectAuthMode: &gdm.Events_ReselectAuthMode{}, + }, + } +} + // IsAuthenticatedEvent generates a IsAuthenticated event. func IsAuthenticatedEvent(item authd.IARequestAuthenticationDataItem) *gdm.EventData { return &gdm.EventData{