Skip to content

Commit 5863e33

Browse files
authored
Use lowercase usernames (#723)
All the brokers we currently support (msentraid and google) use case-insensitive usernames. Currently, logging in with a username which differs in upper or lowercase from the username in the broker results in an authentication failure. We want to fix that. The easiest way to do that is to convert all usernames to lowercase before storing them in our database or passing them to the broker, and also convert the Name argument of GetPassdByName to lowercase before querying the database. If we (or anyone else) ever wants to add a broker for a provider which does *not* use case-insensitive usernames, we will have to revisit this and decide whether we want to support that. Review and merge together with ubuntu/authd-oidc-brokers#314 Closes #530, #531 UDENG-4518 UDENG-4519 UDENG-6636 UDENG-6822
2 parents c527221 + b6cc7cb commit 5863e33

File tree

167 files changed

+4297
-501
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

167 files changed

+4297
-501
lines changed

.github/workflows/qa.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ env:
2424
libpwquality-dev
2525
2626
test_apt_deps: >-
27+
apparmor-profiles
28+
bubblewrap
2729
cracklib-runtime
2830
git-delta
2931
openssh-client
@@ -155,6 +157,10 @@ jobs:
155157
156158
# The integration tests build the NSS crate, so we need the cargo build dependencies in order to run them.
157159
sudo apt install -y ${{ env.apt_deps }} ${{ env.protobuf_compilers}} ${{ env.test_apt_deps }}
160+
161+
# Load the apparmor profile for bubblewrap.
162+
sudo ln -s /usr/share/apparmor/extra-profiles/bwrap-userns-restrict /etc/apparmor.d/
163+
sudo apparmor_parser /etc/apparmor.d/bwrap-userns-restrict
158164
- name: Install PAM and GLib debug symbols
159165
run: |
160166
set -eu

cmd/authd/daemon/migration.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func maybeMigrateBBoltToSQLite(dbDir string) (migrated bool, err error) {
6969
return false, nil
7070
}
7171

72-
if err := db.MigrateData(dbDir); err != nil {
72+
if err := db.MigrateFromBBoltToSQLite(dbDir); err != nil {
7373
return false, fmt.Errorf("failed to migrate data: %w", err)
7474
}
7575

cmd/authd/daemon/testdata/golden/TestMaybeMigrateBBoltToSQLite/Migration_if_bbolt_exists_and_sqlite_does_not_exist

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,3 +61,4 @@ users_to_groups:
6161
gid: 44444
6262
- uid: 4444
6363
gid: 99999
64+
schema_version: 1

debian/control

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ Section: admin
33
Priority: optional
44
Maintainer: Ubuntu Developers <[email protected]>
55
Build-Depends: debhelper-compat (= 13),
6+
bubblewrap <!nocheck>,
67
cracklib-runtime <!nocheck>,
78
dbus <!nocheck>,
89
dh-apport,

examplebroker/users.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ var (
1212
"user1": {Password: "goodpass"},
1313
"user2": {Password: "goodpass"},
1414
"user3": {Password: "goodpass"},
15+
"user-ssh": {Password: "goodpass"},
16+
"user-ssh2": {Password: "goodpass"},
1517
"user-mfa": {Password: "goodpass"},
1618
"user-mfa-with-reset": {Password: "goodpass"},
1719
"user-needs-reset": {Password: "goodpass"},

internal/brokers/broker_test.go

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,13 @@ func TestGetAuthenticationModes(t *testing.T) {
105105
"Get_authentication_modes_and_generate_validators": {sessionID: "success", supportedUILayouts: []string{"required-entry", "optional-entry"}},
106106
"Get_authentication_modes_and_generate_validator_ignoring_whitespaces_in_supported_values": {sessionID: "success", supportedUILayouts: []string{"layout-with-spaces"}},
107107
"Get_authentication_modes_and_ignores_invalid_UI_layout": {sessionID: "success", supportedUILayouts: []string{"required-entry", "missing-type"}},
108-
"Get_multiple_authentication_modes_and_generate_validators": {sessionID: "GAM_multiple_modes", supportedUILayouts: []string{"required-entry", "optional-entry"}},
108+
"Get_multiple_authentication_modes_and_generate_validators": {sessionID: "gam_multiple_modes", supportedUILayouts: []string{"required-entry", "optional-entry"}},
109109

110-
"Does_not_error_out_when_no_authentication_modes_are_returned": {sessionID: "GAM_empty"},
110+
"Does_not_error_out_when_no_authentication_modes_are_returned": {sessionID: "gam_empty"},
111111

112112
// broker errors
113-
"Error_when_getting_authentication_modes": {sessionID: "GAM_error", wantErr: true},
114-
"Error_when_broker_returns_invalid_modes": {sessionID: "GAM_invalid", wantErr: true},
113+
"Error_when_getting_authentication_modes": {sessionID: "gam_error", wantErr: true},
114+
"Error_when_broker_returns_invalid_modes": {sessionID: "gam_invalid", wantErr: true},
115115
}
116116
for name, tc := range tests {
117117
t.Run(name, func(t *testing.T) {
@@ -153,23 +153,23 @@ func TestSelectAuthenticationMode(t *testing.T) {
153153

154154
wantErr bool
155155
}{
156-
"Successfully_select_mode_with_required_value": {sessionID: "SAM_success_required_entry"},
157-
"Successfully_select_mode_with_optional_value": {sessionID: "SAM_success_optional_entry", supportedUILayouts: []string{"optional-entry"}},
158-
"Successfully_select_mode_with_missing_optional_value": {sessionID: "SAM_missing_optional_entry", supportedUILayouts: []string{"optional-entry"}},
156+
"Successfully_select_mode_with_required_value": {sessionID: "sam_success_required_entry"},
157+
"Successfully_select_mode_with_optional_value": {sessionID: "sam_success_optional_entry", supportedUILayouts: []string{"optional-entry"}},
158+
"Successfully_select_mode_with_missing_optional_value": {sessionID: "sam_missing_optional_entry", supportedUILayouts: []string{"optional-entry"}},
159159

160160
// broker errors
161-
"Error_when_selecting_invalid_auth_mode": {sessionID: "SAM_error", wantErr: true},
161+
"Error_when_selecting_invalid_auth_mode": {sessionID: "sam_error", wantErr: true},
162162
"Error_when_no_validators_were_generated_for_session": {sessionID: "no-validators", wantErr: true},
163163

164164
/* Layout errors */
165-
"Error_when_returns_no_layout": {sessionID: "SAM_no_layout", wantErr: true},
166-
"Error_when_returns_empty_layout": {sessionID: "SAM_empty_layout", wantErr: true},
167-
"Error_when_returns_layout_with_no_type": {sessionID: "SAM_no_layout_type", wantErr: true},
168-
"Error_when_returns_layout_with_invalid_type": {sessionID: "SAM_invalid_layout_type", wantErr: true},
169-
"Error_when_returns_layout_without_required_value": {sessionID: "SAM_missing_required_entry", wantErr: true},
170-
"Error_when_returns_layout_with_unknown_field": {sessionID: "SAM_unknown_field", wantErr: true},
171-
"Error_when_returns_layout_with_invalid_required_value": {sessionID: "SAM_invalid_required_entry", wantErr: true},
172-
"Error_when_returns_layout_with_invalid_optional_value": {sessionID: "SAM_invalid_optional_entry", wantErr: true},
165+
"Error_when_returns_no_layout": {sessionID: "sam_no_layout", wantErr: true},
166+
"Error_when_returns_empty_layout": {sessionID: "sam_empty_layout", wantErr: true},
167+
"Error_when_returns_layout_with_no_type": {sessionID: "sam_no_layout_type", wantErr: true},
168+
"Error_when_returns_layout_with_invalid_type": {sessionID: "sam_invalid_layout_type", wantErr: true},
169+
"Error_when_returns_layout_without_required_value": {sessionID: "sam_missing_required_entry", wantErr: true},
170+
"Error_when_returns_layout_with_unknown_field": {sessionID: "sam_unknown_field", wantErr: true},
171+
"Error_when_returns_layout_with_invalid_required_value": {sessionID: "sam_invalid_required_entry", wantErr: true},
172+
"Error_when_returns_layout_with_invalid_optional_value": {sessionID: "sam_invalid_optional_entry", wantErr: true},
173173
}
174174
for name, tc := range tests {
175175
t.Run(name, func(t *testing.T) {
@@ -213,30 +213,30 @@ func TestIsAuthenticated(t *testing.T) {
213213
cancelFirstCall bool
214214
}{
215215
"Successfully_authenticate": {sessionID: "success"},
216-
"Successfully_authenticate_after_cancelling_first_call": {sessionID: "IA_second_call", secondCall: true},
217-
"Denies_authentication_when_broker_times_out": {sessionID: "IA_timeout"},
218-
"Adds_default_groups_even_if_broker_did_not_set_them": {sessionID: "IA_info_empty_groups"},
219-
"No_error_when_auth.Next_and_no_data": {sessionID: "IA_next"},
220-
"No_error_when_auth.Next_and_message": {sessionID: "IA_next_with_data"},
221-
"No_error_when_broker_returns_userinfo_with_empty_gecos": {sessionID: "IA_info_empty_gecos"},
222-
"No_error_when_broker_returns_userinfo_with_group_with_empty_UGID": {sessionID: "IA_info_empty_ugid"},
223-
"No_error_when_broker_returns_userinfo_with_mismatching_username": {sessionID: "IA_info_mismatching_user_name"},
216+
"Successfully_authenticate_after_cancelling_first_call": {sessionID: "ia_second_call", secondCall: true},
217+
"Denies_authentication_when_broker_times_out": {sessionID: "ia_timeout"},
218+
"Adds_default_groups_even_if_broker_did_not_set_them": {sessionID: "ia_info_empty_groups"},
219+
"No_error_when_auth.Next_and_no_data": {sessionID: "ia_next"},
220+
"No_error_when_auth.Next_and_message": {sessionID: "ia_next_with_data"},
221+
"No_error_when_broker_returns_userinfo_with_empty_gecos": {sessionID: "ia_info_empty_gecos"},
222+
"No_error_when_broker_returns_userinfo_with_group_with_empty_UGID": {sessionID: "ia_info_empty_ugid"},
223+
"No_error_when_broker_returns_userinfo_with_mismatching_username": {sessionID: "ia_info_mismatching_user_name"},
224224

225225
// broker errors
226-
"Error_when_authenticating": {sessionID: "IA_error"},
227-
"Error_on_empty_data_even_if_granted": {sessionID: "IA_empty_data"},
228-
"Error_when_broker_returns_invalid_data": {sessionID: "IA_invalid_data"},
229-
"Error_when_broker_returns_invalid_access": {sessionID: "IA_invalid_access"},
230-
"Error_when_broker_returns_invalid_userinfo": {sessionID: "IA_invalid_userinfo"},
231-
"Error_when_broker_returns_userinfo_with_empty_username": {sessionID: "IA_info_empty_user_name"},
232-
"Error_when_broker_returns_userinfo_with_empty_group_name": {sessionID: "IA_info_empty_group_name"},
233-
"Error_when_broker_returns_userinfo_with_invalid_homedir": {sessionID: "IA_info_invalid_home"},
234-
"Error_when_broker_returns_userinfo_with_invalid_shell": {sessionID: "IA_info_invalid_shell"},
235-
"Error_when_broker_returns_invalid_data_on_auth.Next": {sessionID: "IA_next_with_invalid_data"},
236-
"Error_when_broker_returns_data_on_auth.Cancelled": {sessionID: "IA_cancelled_with_data"},
237-
"Error_when_broker_returns_no_data_on_auth.Denied": {sessionID: "IA_denied_without_data"},
238-
"Error_when_broker_returns_no_data_on_auth.Retry": {sessionID: "IA_retry_without_data"},
239-
"Error_when_calling_IsAuthenticated_a_second_time_without_cancelling": {sessionID: "IA_second_call", secondCall: true, cancelFirstCall: true},
226+
"Error_when_authenticating": {sessionID: "ia_error"},
227+
"Error_on_empty_data_even_if_granted": {sessionID: "ia_empty_data"},
228+
"Error_when_broker_returns_invalid_data": {sessionID: "ia_invalid_data"},
229+
"Error_when_broker_returns_invalid_access": {sessionID: "ia_invalid_access"},
230+
"Error_when_broker_returns_invalid_userinfo": {sessionID: "ia_invalid_userinfo"},
231+
"Error_when_broker_returns_userinfo_with_empty_username": {sessionID: "ia_info_empty_user_name"},
232+
"Error_when_broker_returns_userinfo_with_empty_group_name": {sessionID: "ia_info_empty_group_name"},
233+
"Error_when_broker_returns_userinfo_with_invalid_homedir": {sessionID: "ia_info_invalid_home"},
234+
"Error_when_broker_returns_userinfo_with_invalid_shell": {sessionID: "ia_info_invalid_shell"},
235+
"Error_when_broker_returns_invalid_data_on_auth.Next": {sessionID: "ia_next_with_invalid_data"},
236+
"Error_when_broker_returns_data_on_auth.Cancelled": {sessionID: "ia_cancelled_with_data"},
237+
"Error_when_broker_returns_no_data_on_auth.Denied": {sessionID: "ia_denied_without_data"},
238+
"Error_when_broker_returns_no_data_on_auth.Retry": {sessionID: "ia_retry_without_data"},
239+
"Error_when_calling_IsAuthenticated_a_second_time_without_cancelling": {sessionID: "ia_second_call", secondCall: true, cancelFirstCall: true},
240240
}
241241
for name, tc := range tests {
242242
t.Run(name, func(t *testing.T) {
@@ -289,8 +289,8 @@ func TestCancelIsAuthenticated(t *testing.T) {
289289

290290
wantAnswer string
291291
}{
292-
"Successfully_cancels_IsAuthenticated": {sessionID: "IA_wait", wantAnswer: auth.Cancelled},
293-
"Call_returns_denied_if_not_cancelled": {sessionID: "IA_timeout", wantAnswer: auth.Denied},
292+
"Successfully_cancels_IsAuthenticated": {sessionID: "ia_wait", wantAnswer: auth.Cancelled},
293+
"Call_returns_denied_if_not_cancelled": {sessionID: "ia_timeout", wantAnswer: auth.Denied},
294294
}
295295
for name, tc := range tests {
296296
t.Run(name, func(t *testing.T) {
@@ -305,7 +305,7 @@ func TestCancelIsAuthenticated(t *testing.T) {
305305
}()
306306
defer cancel()
307307

308-
if tc.sessionID == "IA_wait" {
308+
if tc.sessionID == "ia_wait" {
309309
// Give some time for the IsAuthenticated routine to start.
310310
time.Sleep(time.Second)
311311
cancel()

internal/brokers/manager.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ func (m *Manager) SetDefaultBrokerForUser(brokerID, username string) error {
132132
return fmt.Errorf("invalid broker: %v", err)
133133
}
134134

135+
// authd uses lowercase usernames
136+
username = strings.ToLower(username)
137+
135138
m.usersToBrokerMu.Lock()
136139
defer m.usersToBrokerMu.Unlock()
137140
m.usersToBroker[username] = broker

internal/brokers/manager_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,8 @@ func TestNewSession(t *testing.T) {
186186
"Successfully_start_a_new_session_with_the_correct_broker": {username: "success", configuredBrokers: []string{t.Name() + "_Broker1.conf", t.Name() + "_Broker2.conf"}},
187187

188188
"Error_when_broker_does_not_exist": {brokerID: "does_not_exist", wantErr: true},
189-
"Error_when_broker_does_not_provide_an_ID": {username: "NS_no_id", wantErr: true},
190-
"Error_when_starting_a_new_session": {username: "NS_error", wantErr: true},
189+
"Error_when_broker_does_not_provide_an_ID": {username: "ns_no_id", wantErr: true},
190+
"Error_when_starting_a_new_session": {username: "ns_error", wantErr: true},
191191
"Error_when_broker_is_not_available_on_dbus": {unavailableBroker: true, wantErr: true},
192192
}
193193
for name, tc := range tests {
@@ -270,7 +270,7 @@ func TestEndSession(t *testing.T) {
270270
"Successfully_end_session_on_the_correct_broker": {sessionID: "success", configuredBrokers: []string{t.Name() + "_Broker1", t.Name() + "_Broker2"}},
271271

272272
"Error_when_broker_does_not_exist": {brokerID: "does not exist", sessionID: "dont matter", wantErr: true},
273-
"Error_when_ending_session": {sessionID: "ES_error", wantErr: true},
273+
"Error_when_ending_session": {sessionID: "es_error", wantErr: true},
274274
}
275275
for name, tc := range tests {
276276
t.Run(name, func(t *testing.T) {
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
FIRST CALL:
22
access: granted
3-
data: {"Name":"TestIsAuthenticated/Adds_default_groups_even_if_broker_did_not_set_them_separator_IA_info_empty_groups","UID":0,"Gecos":"gecos for IA_info_empty_groups","Dir":"/home/IA_info_empty_groups","Shell":"/bin/sh/IA_info_empty_groups","Groups":[]}
3+
data: {"Name":"TestIsAuthenticated/Adds_default_groups_even_if_broker_did_not_set_them_separator_ia_info_empty_groups","UID":0,"Gecos":"gecos for ia_info_empty_groups","Dir":"/home/ia_info_empty_groups","Shell":"/bin/sh/ia_info_empty_groups","Groups":[]}
44
err: <nil>
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
FIRST CALL:
22
access: granted
3-
data: {"Name":"TestIsAuthenticated/Error_when_calling_IsAuthenticated_a_second_time_without_cancelling_separator_IA_second_call","UID":0,"Gecos":"gecos for IA_second_call","Dir":"/home/IA_second_call","Shell":"/bin/sh/IA_second_call","Groups":[{"Name":"group-IA_second_call","GID":null,"UGID":"ugid-IA_second_call"}]}
3+
data: {"Name":"TestIsAuthenticated/Error_when_calling_IsAuthenticated_a_second_time_without_cancelling_separator_ia_second_call","UID":0,"Gecos":"gecos for ia_second_call","Dir":"/home/ia_second_call","Shell":"/bin/sh/ia_second_call","Groups":[{"Name":"group-ia_second_call","GID":null,"UGID":"ugid-ia_second_call"}]}
44
err: <nil>
55
SECOND CALL:
66
access:
77
data:
8-
err: broker "TestIsAuthenticated": IsAuthenticated already running for session "TestIsAuthenticated/Error_when_calling_IsAuthenticated_a_second_time_without_cancelling_separator_IA_second_call"
8+
err: broker "TestIsAuthenticated": IsAuthenticated already running for session "TestIsAuthenticated/Error_when_calling_IsAuthenticated_a_second_time_without_cancelling_separator_ia_second_call"

0 commit comments

Comments
 (0)