Skip to content

Commit 93ae648

Browse files
authored
pam/cli: Factorize list views code and remove empty vertical spaces (#840)
We have two stages using lists at the moment that are basically repeating the same code in most logic, avoid such duplication and handle everything in a new list type that is used as base of both the broker and authentication selection stages. Then, Bubbletea by default shows lists in full screen mode, however this is far from optimal in our implementation because the command triggering the pam request is hidden making the user to lose the context in a security-related operation. UDENG-6445
2 parents 75499a0 + a14a505 commit 93ae648

10 files changed

+240
-338
lines changed

Diff for: pam/integration-tests/testdata/golden/TestCLIAuthenticate/Authenticate_user_switching_to_local_broker

-20
Original file line numberDiff line numberDiff line change
@@ -35,26 +35,6 @@ Gimme your password:
3535
Select your provider
3636

3737
> 1. local
38-
2. ExampleBroker
39-
40-
41-
42-
43-
44-
45-
46-
47-
48-
49-
50-
51-
52-
53-
54-
55-
56-
57-
5838
PAM Info Message: auth=incomplete
5939
PAM Authenticate()
6040
User: "user-integration-switch-broker"

Diff for: pam/integration-tests/testdata/golden/TestCLIAuthenticate/Deny_authentication_if_user_does_not_exist

-20
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,6 @@ Username: user-unexistent
1414
Select your provider
1515

1616
1. local
17-
> 2. ExampleBroker
18-
19-
20-
21-
22-
23-
24-
25-
26-
27-
28-
29-
30-
31-
32-
33-
34-
35-
36-
3717
PAM Error Message: can't select broker: user "user-unexistent" does not exist
3818
PAM Authenticate()
3919
User: "user-unexistent"

Diff for: pam/integration-tests/testdata/golden/TestCLIAuthenticate/Exit_authd_if_local_broker_is_selected

-20
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,6 @@ Username: user-local-broker
1414
Select your provider
1515

1616
> 1. local
17-
2. ExampleBroker
18-
19-
20-
21-
22-
23-
24-
25-
26-
27-
28-
29-
30-
31-
32-
33-
34-
35-
36-
3717
PAM Info Message: auth=incomplete
3818
PAM Authenticate()
3919
User: "user-local-broker"

Diff for: pam/integration-tests/testdata/golden/TestCLIChangeAuthTok/Exit_authd_if_local_broker_is_selected

-20
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,6 @@ Username: user-integration-cli-passwd-exit-authd-if-local-broker-is-selected
1414
Select your provider
1515

1616
> 1. local
17-
2. ExampleBroker
18-
19-
20-
21-
22-
23-
24-
25-
26-
27-
28-
29-
30-
31-
32-
33-
34-
35-
36-
3717
PAM Info Message: chauthtok=incomplete
3818
PAM ChangeAuthTok()
3919
User: "user-integration-cli-passwd-exit-authd-if-local-broker-is-selected"

Diff for: pam/integration-tests/testdata/golden/TestCLIChangeAuthTok/Prevent_change_password_if_user_does_not_exist

-20
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,6 @@ Username: user-unexistent
1414
Select your provider
1515

1616
1. local
17-
> 2. ExampleBroker
18-
19-
20-
21-
22-
23-
24-
25-
26-
27-
28-
29-
30-
31-
32-
33-
34-
35-
36-
3717
PAM Error Message: can't select broker: user "user-unexistent" does not exist
3818
PAM ChangeAuthTok()
3919
User: "user-unexistent"

Diff for: pam/internal/adapter/authentication.go

+2
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ func (m *authenticationModel) Update(msg tea.Msg) (authModel authenticationModel
194194
return *m, tea.Sequence(m.cancelIsAuthenticated(), sendEvent(AuthModeSelected{}))
195195

196196
case newPasswordCheck:
197+
log.Debugf(context.TODO(), "%T", msg)
197198
currentSecret := m.currentSecret
198199
return *m, func() tea.Msg {
199200
res := newPasswordCheckResult{ctx: msg.ctx, password: msg.password}
@@ -204,6 +205,7 @@ func (m *authenticationModel) Update(msg tea.Msg) (authModel authenticationModel
204205
}
205206

206207
case newPasswordCheckResult:
208+
log.Debugf(msg.ctx, "%T: %s", msg, msg.msg)
207209
if m.clientType != Gdm {
208210
// This may be handled by the current model, so don't return early.
209211
break

Diff for: pam/internal/adapter/authmodeselection.go

+14-100
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,9 @@ package adapter
33
import (
44
"context"
55
"fmt"
6-
"strconv"
76

8-
"github.com/charmbracelet/bubbles/list"
7+
tea_list "github.com/charmbracelet/bubbles/list"
98
tea "github.com/charmbracelet/bubbletea"
10-
"github.com/charmbracelet/lipgloss"
119
"github.com/msteinert/pam/v2"
1210
"github.com/ubuntu/authd/internal/brokers/layouts"
1311
"github.com/ubuntu/authd/internal/brokers/layouts/entries"
@@ -17,10 +15,8 @@ import (
1715

1816
// authModeSelectionModel is the model list to select supported authentication modes.
1917
type authModeSelectionModel struct {
20-
list.Model
21-
focused bool
18+
List
2219

23-
clientType PamClientType
2420
supportedUILayouts []*authd.UILayout
2521
availableAuthModes []*authd.GAMResponse_AuthenticationMode
2622
currentAuthModeSelectedID string
@@ -44,9 +40,6 @@ type authModeSelected struct {
4440
id string
4541
}
4642

47-
// authModeSelectionFocused is the internal event to signal that the auth mode selection view is focused.
48-
type authModeSelectionFocused struct{}
49-
5043
// selectAuthMode selects current authentication mode.
5144
func selectAuthMode(id string) tea.Cmd {
5245
return func() tea.Msg {
@@ -58,27 +51,8 @@ func selectAuthMode(id string) tea.Cmd {
5851

5952
// newAuthModeSelectionModel initializes an empty list with default options of authModeSelectionModel.
6053
func newAuthModeSelectionModel(clientType PamClientType) authModeSelectionModel {
61-
// FIXME: decouple UI from data model.
62-
if clientType != InteractiveTerminal {
63-
return authModeSelectionModel{
64-
Model: list.New(nil, itemLayout{}, 0, 0),
65-
clientType: clientType,
66-
}
67-
}
68-
69-
l := list.New(nil, itemLayout{}, 80, 24)
70-
l.Title = "Select your authentication method"
71-
l.SetShowStatusBar(false)
72-
l.SetShowHelp(false)
73-
l.DisableQuitKeybindings()
74-
75-
l.Styles.Title = lipgloss.NewStyle()
76-
/*l.Styles.PaginationStyle = paginationStyle
77-
l.Styles.HelpStyle = helpStyle*/
78-
7954
return authModeSelectionModel{
80-
Model: l,
81-
clientType: clientType,
55+
List: NewList(clientType, "Select your authentication method"),
8256
}
8357
}
8458

@@ -128,10 +102,6 @@ func (m *authModeSelectionModel) Init() tea.Cmd {
128102
// Update handles events and actions.
129103
func (m authModeSelectionModel) Update(msg tea.Msg) (authModeSelectionModel, tea.Cmd) {
130104
switch msg := msg.(type) {
131-
case authModeSelectionFocused:
132-
log.Debugf(context.TODO(), "%T: %#v", m, msg)
133-
m.focused = true
134-
135105
case supportedUILayoutsReceived:
136106
log.Debugf(context.TODO(), "%#v", msg)
137107
if len(msg.layouts) == 0 {
@@ -147,7 +117,7 @@ func (m authModeSelectionModel) Update(msg tea.Msg) (authModeSelectionModel, tea
147117
log.Debugf(context.TODO(), "%#v", msg)
148118
m.availableAuthModes = msg.authModes
149119

150-
var allAuthModes []list.Item
120+
var allAuthModes []tea_list.Item
151121
var firstAuthModeID string
152122
for _, a := range m.availableAuthModes {
153123
if firstAuthModeID == "" {
@@ -167,6 +137,15 @@ func (m authModeSelectionModel) Update(msg tea.Msg) (authModeSelectionModel, tea
167137

168138
return m, tea.Sequence(cmds...)
169139

140+
case listItemSelected:
141+
if !m.Focused() {
142+
return m, nil
143+
}
144+
145+
log.Debugf(context.TODO(), "%#v", msg)
146+
authMode := convertTo[authModeItem](msg.item)
147+
return m, selectAuthMode(authMode.id)
148+
170149
case authModeSelected:
171150
log.Debugf(context.TODO(), "%#v", msg)
172151
// Ensure auth mode id is valid
@@ -190,76 +169,11 @@ func (m authModeSelectionModel) Update(msg tea.Msg) (authModeSelectionModel, tea
190169
})
191170
}
192171

193-
if m.clientType != InteractiveTerminal {
194-
return m, nil
195-
}
196-
197-
// interaction events
198-
if !m.focused {
199-
return m, nil
200-
}
201-
switch msg := msg.(type) {
202-
// Key presses
203-
case tea.KeyMsg:
204-
switch msg.String() {
205-
case "enter":
206-
item := m.SelectedItem()
207-
if item == nil {
208-
return m, nil
209-
}
210-
authMode := convertTo[authModeItem](item)
211-
cmd := selectAuthMode(authMode.id)
212-
return m, cmd
213-
case "1", "2", "3", "4", "5", "6", "7", "8", "9":
214-
// This is necessarily an integer, so above
215-
choice, _ := strconv.Atoi(msg.String())
216-
items := m.Items()
217-
if choice > len(items) {
218-
return m, nil
219-
}
220-
item := items[choice-1]
221-
authMode := convertTo[authModeItem](item)
222-
cmd := selectAuthMode(authMode.id)
223-
return m, cmd
224-
}
225-
}
226-
227172
var cmd tea.Cmd
228-
m.Model, cmd = m.Model.Update(msg)
173+
m.List, cmd = m.List.Update(msg)
229174
return m, cmd
230175
}
231176

232-
// View renders a text view of the authentication UI.
233-
func (m authModeSelectionModel) View() string {
234-
if !m.Focused() {
235-
return ""
236-
}
237-
238-
return m.Model.View()
239-
}
240-
241-
// Focus focuses this model.
242-
func (m *authModeSelectionModel) Focus() tea.Cmd {
243-
log.Debugf(context.TODO(), "%T: Focus", m)
244-
return sendEvent(authModeSelectionFocused{})
245-
}
246-
247-
// Focused returns if this model is focused.
248-
func (m *authModeSelectionModel) Focused() bool {
249-
return m.focused
250-
}
251-
252-
// Blur releases the focus from this model.
253-
func (m *authModeSelectionModel) Blur() {
254-
log.Debugf(context.TODO(), "%T: Blur", m)
255-
m.focused = false
256-
}
257-
258-
// WillCaptureEscape returns if this broker may capture Esc typing on keyboard.
259-
func (m authModeSelectionModel) WillCaptureEscape() bool {
260-
return m.FilterState() == list.Filtering
261-
}
262-
263177
// authModeItem is the list item corresponding to an authentication mode.
264178
type authModeItem struct {
265179
id string

0 commit comments

Comments
 (0)