Skip to content

Commit

Permalink
pam/authentication: Start new auth only after cancellation is notified
Browse files Browse the repository at this point in the history
In case we're still in the process of cancelling the authentication we
should not continue with the current action until we're notified that
the cancellation is completed.

This is needed because otherwise we may end up into this situation:
 - reselectAuthMode
 - AuthModeSelected
 - ...
 - startAuthentication
 - isAuthenticatedResultReceived: cancelled

Or a bit less extreme, but still getting the cancelled event while we're
selecting the authentication mode. And this mis-order causes troubles
when passing the data to gdm
  • Loading branch information
3v1n0 committed Jun 27, 2024
1 parent 3d3a967 commit cac8ce0
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 14 deletions.
43 changes: 29 additions & 14 deletions pam/internal/adapter/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,12 @@ type authenticationModel struct {
client authd.PAMClient
clientType PamClientType

currentModel authenticationComponent
currentSessionID string
currentBrokerID string
currentChallenge string
cancelIsAuthenticated func()
currentModel authenticationComponent
currentSessionID string
currentBrokerID string
currentChallenge string
cancelAuthFunc func()
postCancellation []tea.Cmd

encryptionKey *rsa.PublicKey

Expand Down Expand Up @@ -124,9 +125,8 @@ type newPasswordCheckResult struct {
// newAuthenticationModel initializes a authenticationModel which needs to be Compose then.
func newAuthenticationModel(client authd.PAMClient, clientType PamClientType) authenticationModel {
return authenticationModel{
client: client,
clientType: clientType,
cancelIsAuthenticated: func() {},
client: client,
clientType: clientType,
}
}

Expand All @@ -135,12 +135,27 @@ func (m *authenticationModel) Init() tea.Cmd {
return nil
}

func (m *authenticationModel) cancelIsAuthenticated() {
if m.cancelAuthFunc == nil {
return
}
m.cancelAuthFunc()
m.cancelAuthFunc = nil
}

// Update handles events and actions.
func (m *authenticationModel) Update(msg tea.Msg) (authenticationModel, tea.Cmd) {
switch msg := msg.(type) {
case reselectAuthMode:
cmd := sendEvent(AuthModeSelected{})
if m.cancelAuthFunc == nil {
return *m, cmd
}

// We need to wait until the cancellation actually happens before
// starting again the authentication.
m.postCancellation = append(m.postCancellation, cmd)
m.cancelIsAuthenticated()
return *m, sendEvent(AuthModeSelected{})

case newPasswordCheck:
res := newPasswordCheckResult{challenge: msg.challenge}
Expand All @@ -153,7 +168,7 @@ func (m *authenticationModel) Update(msg tea.Msg) (authenticationModel, tea.Cmd)
log.Debugf(context.TODO(), "%#v", msg)
m.cancelIsAuthenticated()
ctx, cancel := context.WithCancel(context.Background())
m.cancelIsAuthenticated = cancel
m.cancelAuthFunc = cancel

// Store the current challenge, if present, for password verifications.
challenge, ok := msg.item.(*authd.IARequest_AuthenticationData_Challenge)
Expand Down Expand Up @@ -213,8 +228,8 @@ func (m *authenticationModel) Update(msg tea.Msg) (authenticationModel, tea.Cmd)
return *m, sendEvent(GetAuthenticationModesRequested{})

case brokers.AuthCancelled:
// nothing to do
return *m, nil
defer func() { m.postCancellation = nil }()
return *m, tea.Sequence(m.postCancellation...)
}

case errMsgToDisplay:
Expand Down Expand Up @@ -271,7 +286,7 @@ func (m *authenticationModel) Compose(brokerID, sessionID string, encryptionKey
m.currentBrokerID = brokerID
m.currentSessionID = sessionID
m.encryptionKey = encryptionKey
m.cancelIsAuthenticated = func() {}
m.cancelAuthFunc = nil

m.errorMsg = ""

Expand Down Expand Up @@ -328,7 +343,7 @@ func (m authenticationModel) View() string {
// Resets zeroes any internal state on the authenticationModel.
func (m *authenticationModel) Reset() {
m.cancelIsAuthenticated()
m.cancelIsAuthenticated = func() {}
m.cancelAuthFunc = nil
m.currentModel = nil
m.currentSessionID = ""
m.currentBrokerID = ""
Expand Down
87 changes: 87 additions & 0 deletions pam/internal/adapter/gdmmodel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,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{
Expand Down Expand Up @@ -1076,6 +1077,92 @@ func TestGdmModel(t *testing.T) {
},
wantExitStatus: PamSuccess{BrokerID: firstBrokerInfo.Id},
},
"Authenticated with qrcode regenerated after wait started at 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()),
// It's long because we want to cancel this!
pam_test.WithIsAuthenticatedWantWait(time.Millisecond*1500),
),
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",
}}),
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,
gdm.EventType_authEvent,
},
wantStage: pam_proto.Stage_challenge,
wantGdmAuthRes: []*authd.IAResponse{
{Access: brokers.AuthCancelled},
{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"),
Expand Down
19 changes: 19 additions & 0 deletions pam/internal/pam_test/pam-client-dummy.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type options struct {
isAuthenticatedWantWait time.Duration
isAuthenticatedMessage string
isAuthenticatedMaxRetries int
isAuthenticatedCancelFistN int

endSessionErr error

Expand Down Expand Up @@ -157,6 +158,13 @@ func WithIsAuthenticatedMaxRetries(maxRetries int) func(o *options) {
}
}

// WithIsAuthenticatedCancelFistN is the option to define the IsAuthenticated cancellation steps.
func WithIsAuthenticatedCancelFistN(cancellationsN int) func(o *options) {
return func(o *options) {
o.isAuthenticatedCancelFistN = cancellationsN
}
}

// WithIsAuthenticatedMessage is the option to define the IsAuthenticated message return values.
func WithIsAuthenticatedMessage(message string) func(o *options) {
return func(o *options) {
Expand Down Expand Up @@ -413,6 +421,16 @@ func (dc *DummyClient) IsAuthenticated(ctx context.Context, in *authd.IARequest,
return nil, errors.New("no authentication data provided")
}

// fmt.Println("req Cancellation", dc.isAuthenticatedCancelFistN)
// if dc.isAuthenticatedCancelFistN > 0 {
// defer func() { dc.isAuthenticatedCancelFistN-- }()
// return &authd.IAResponse{
// Access: brokers.AuthCancelled,
// Msg: fmt.Sprintf(`{"message": "Cancellation %d | %s"}`,
// dc.isAuthenticatedCancelFistN, dc.isAuthenticatedMessage),
// }, nil
// }

var msg string
if dc.isAuthenticatedMessage != "" {
msg = fmt.Sprintf(`{"message": "%s"}`, dc.isAuthenticatedMessage)
Expand All @@ -428,6 +446,7 @@ func (dc *DummyClient) IsAuthenticated(ctx context.Context, in *authd.IARequest,
if dc.isAuthenticatedWantWait == 0 {
return nil, errors.New("no wanted wait provided")
}
// ctx, _ := context.WithCancel(ctx)
select {
case <-time.After(dc.isAuthenticatedWantWait):
case <-ctx.Done():
Expand Down

0 comments on commit cac8ce0

Please sign in to comment.