Skip to content

Commit

Permalink
fix(pam/brokers): Small adjustments to PAM and the UI validators (#127)
Browse files Browse the repository at this point in the history
Some things were not working as they should in the PAM module, so we
should update them. The details for each fix are in their respective
commit message.

UDENG-1916
  • Loading branch information
denisonbarbosa authored Dec 7, 2023
2 parents 746bcfc + ca985a4 commit b11f5cb
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 14 deletions.
25 changes: 16 additions & 9 deletions internal/brokers/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,33 +259,40 @@ func (b Broker) validateUILayout(sessionID string, layout map[string]string) (r
b.layoutValidatorsMu.Lock()
defer b.layoutValidatorsMu.Unlock()

layoutValidator, exists := b.layoutValidators[sessionID]
layoutValidators, exists := b.layoutValidators[sessionID]
if !exists {
return nil, fmt.Errorf("session %q does not have any layout validator", sessionID)
}

typ := layout["type"]
layoutTypeValidator, exists := layoutValidator[typ]
// layoutValidator is UI Layout validator generated based on the supported layouts.
layoutValidator, exists := layoutValidators[layout["type"]]
if !exists {
return nil, fmt.Errorf("no validator for UI layout type %q", typ)
return nil, fmt.Errorf("no validator for UI layout type %q", layout["type"])
}

r = map[string]string{"type": typ}
for key, validator := range layoutTypeValidator {
// Ensure that all fields provided in the layout returned by the broker are valid.
for key := range layout {
if key == "type" {
continue
}
if _, exists := layoutValidator[key]; !exists {
return nil, fmt.Errorf("unrecognized field %q provided for layout %q", key, layout["type"])
}
}
// Ensure that all required fields were provided and that the values are valid.
for key, validator := range layoutValidator {
value, exists := layout[key]
if !exists || value == "" {
if validator.required {
return nil, fmt.Errorf("required field %q was not provided", key)
}
continue
}

if validator.supportedValues != nil && !slices.Contains(validator.supportedValues, value) {
return nil, fmt.Errorf("field %q has invalid value %q, expected one of %s", key, value, strings.Join(validator.supportedValues, ","))
}
r[key] = value
}
return r, nil
return layout, nil
}

// parseSessionID strips broker ID prefix from sessionID.
Expand Down
11 changes: 8 additions & 3 deletions internal/brokers/broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,16 @@ func TestSelectAuthenticationMode(t *testing.T) {
"Successfully select mode with missing optional value": {sessionID: "SAM_missing_optional_entry", supportedUILayouts: []string{"optional-entry"}},

// broker errors
"Error when selecting invalid auth mode": {sessionID: "SAM_error", wantErr: true},
"Error when selecting invalid auth mode": {sessionID: "SAM_error", wantErr: true},
"Error when no validators were generated for session": {sessionID: "no-validators", wantErr: true},

/* Layout errors */
"Error when returns no layout": {sessionID: "SAM_no_layout", wantErr: true},
"Error when returns empty layout": {sessionID: "SAM_empty_layout", wantErr: true},
"Error when returns layout with no type": {sessionID: "SAM_no_layout_type", wantErr: true},
"Error when returns layout with invalid type": {sessionID: "SAM_invalid_layout_type", wantErr: true},
"Error when returns layout without required value": {sessionID: "SAM_missing_required_entry", wantErr: true},
"Error when returns layout with unknown field": {sessionID: "SAM_unknown_field", wantErr: true},
"Error when returns layout with invalid required value": {sessionID: "SAM_invalid_required_entry", wantErr: true},
"Error when returns layout with invalid optional value": {sessionID: "SAM_invalid_optional_entry", wantErr: true},
}
Expand All @@ -182,8 +184,11 @@ func TestSelectAuthenticationMode(t *testing.T) {
for _, layout := range tc.supportedUILayouts {
supportedUILayouts = append(supportedUILayouts, supportedLayouts[layout])
}
// This is normally done in the broker's GetAuthenticationModes method, but we need to do it here to test the SelectAuthenticationMode method.
brokers.GenerateLayoutValidators(&b, prefixID(t, tc.sessionID), supportedUILayouts)

if tc.sessionID != "no-validators" {
// This is normally done in the broker's GetAuthenticationModes method, but we need to do it here to test the SelectAuthenticationMode method.
brokers.GenerateLayoutValidators(&b, prefixID(t, tc.sessionID), supportedUILayouts)
}

gotUI, err := b.SelectAuthenticationMode(context.Background(), prefixID(t, tc.sessionID), "mode1")
if tc.wantErr {
Expand Down
1 change: 1 addition & 0 deletions internal/services/pam/pam_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ func TestSelectAuthenticationMode(t *testing.T) {
"Error when returns no layout": {username: "SAM_no_layout", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true},
"Error when returns layout with no type": {username: "SAM_no_layout_type", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true},
"Error when returns layout without required value": {username: "SAM_missing_required_entry", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true},
"Error when returns layout with unknown field": {username: "SAM_unknown_field", supportedUILayouts: []*authd.UILayout{requiredEntry}, wantErr: true},
}
for name, tc := range tests {
tc := tc
Expand Down
6 changes: 6 additions & 0 deletions internal/testutils/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,12 @@ func (b *BrokerBusMock) SelectAuthenticationMode(sessionID, authenticationModeNa
"type": "optional-entry",
"entry": "invalid entry",
}, nil
case "SAM_unknown_field":
return map[string]string{
"type": "required-entry",
"entry": "entry_type",
"unknown_field": "unknown",
}, nil
case "SAM_error":
return nil, dbus.MakeFailedError(fmt.Errorf("broker %q: SelectAuthenticationMode errored out", b.name))
case "SAM_no_layout":
Expand Down
1 change: 1 addition & 0 deletions pam/authmodeselection.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ func (m *authModeSelectionModel) Init() tea.Cmd {
Content: &required,
Wait: &requiredWithBooleans,
Label: &optional,
Button: &optional,
},
{
Type: "newpassword",
Expand Down
4 changes: 2 additions & 2 deletions pam/main-cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ func main() {
return "", nil
}))

authResult := module.Authenticate(mTx, pam.Flags(0), nil)
authResult := module.Authenticate(mTx, pam.Flags(0), os.Args)
fmt.Println("Auth return:", authResult)

// Simulate setting auth broker as default.
accMgmtResult := module.AcctMgmt(mTx, pam.Flags(0), nil)
accMgmtResult := module.AcctMgmt(mTx, pam.Flags(0), os.Args)
fmt.Println("Acct mgmt return:", accMgmtResult)
}

0 comments on commit b11f5cb

Please sign in to comment.