Skip to content

Commit

Permalink
pam/adapter/userSelection: Do not expose user name until it has been …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
3v1n0 committed Oct 18, 2024
1 parent 86743ef commit b5297b9
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pam/internal/adapter/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 18 additions & 0 deletions pam/internal/adapter/userselection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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{})
}
Expand Down Expand Up @@ -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()
}

0 comments on commit b5297b9

Please sign in to comment.