From b5297b98395c851b7b565ec49ee83af6eb90ea60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 18 Oct 2024 02:10:37 +0200 Subject: [PATCH] pam/adapter/userSelection: Do not expose user name until it has been validated If an user is quick enough to type an user name on the user selection field we may end up passing such (potentially partial) value to the broker as the selected user, breaking the rest of the transaction. This happened because we returned the text input Value as is, without checking if the user ever ack'ed it via an enter press. What may happen in short is: - adapter.supportedUILayoutsReceived{} - adapter.userRequired{} * User starts typing something in the user name field - adapter.brokersListReceived{brokers:[]{...}} - adapter.UsernameOrBrokerListReceived{} * User name is set at this point as the form Value() even if not validated - adapter.brokerSelectionRequired{} - adapter.ChangeStage{Stage:1} See for example - https://github.com/3v1n0/authd/actions/runs/11390824079/job/31693266490 To prevent this, only return a valid username value after the user selection has been completed during each focus phase that the view has --- pam/internal/adapter/model.go | 2 +- pam/internal/adapter/userselection.go | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/pam/internal/adapter/model.go b/pam/internal/adapter/model.go index 9e78be3c44..94915b7458 100644 --- a/pam/internal/adapter/model.go +++ b/pam/internal/adapter/model.go @@ -431,7 +431,7 @@ func (m *UIModel) ExitStatus() PamReturnStatus { // username returns currently selected user name. func (m UIModel) username() string { - return m.userSelectionModel.Value() + return m.userSelectionModel.Username() } // availableBrokers returns currently available brokers. diff --git a/pam/internal/adapter/userselection.go b/pam/internal/adapter/userselection.go index fe654b63e3..96ceecb172 100644 --- a/pam/internal/adapter/userselection.go +++ b/pam/internal/adapter/userselection.go @@ -19,6 +19,7 @@ type userSelectionModel struct { pamMTx pam.ModuleTransaction clientType PamClientType enabled bool + selected bool } // userSelected events to report that a new username has been selected. @@ -91,6 +92,7 @@ func (m userSelectionModel) Update(msg tea.Msg) (userSelectionModel, tea.Cmd) { } if msg.username != "" { // synchronise our internal validated field and the text one. + m.selected = true m.SetValue(msg.username) return m, sendEvent(UsernameOrBrokerListReceived{}) } @@ -141,3 +143,19 @@ func (m userSelectionModel) View() string { func (m userSelectionModel) Enabled() bool { return m.enabled } + +// Username returns the approved value of the text input. +func (m userSelectionModel) Username() string { + if m.clientType == InteractiveTerminal && !m.selected { + return "" + } + return m.Model.Value() +} + +// Focus sets the focus state on the model. We also mark as the user is not +// selected so that the returned value won't be valid until the user did an +// explicit ack. +func (m *userSelectionModel) Focus() tea.Cmd { + m.selected = false + return m.Model.Focus() +}