Skip to content

Commit 6c3cd5f

Browse files
konac-hamzakonac-hamza
committed
refactor: Improve list function
Update list function to handle user types properly.
1 parent e4774ad commit 6c3cd5f

File tree

3 files changed

+214
-13
lines changed

3 files changed

+214
-13
lines changed

‎src/api/v3/user/types.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ impl From<UserListParameters> for identity_types::UserListParameters {
353353
domain_id: value.domain_id,
354354
name: value.name,
355355
unique_id: value.unique_id,
356-
// limit: value.limit,
356+
..Default::default() // limit: value.limit,
357357
}
358358
}
359359
}

‎src/identity/backend/sql/user/list.rs‎

Lines changed: 191 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,27 +60,28 @@ pub async fn list(
6060
let db_users: Vec<db_user::Model> = user_select.all(db).await.context("fetching users data")?;
6161
let count_of_users_selected = db_users.len();
6262

63+
let user_type = params.user_type.unwrap_or(UserType::All);
6364
let (user_opts, local_users, nonlocal_users, federated_users) = tokio::join!(
6465
db_users.load_many(DbUserOption, db),
6566
// Load local users when requested, otherwise return empty results list
6667
async {
67-
if true {
68+
if user_type == UserType::Local || user_type == UserType::All {
6869
db_users.load_one(local_user_select, db).await
6970
} else {
7071
Ok(vec![None; count_of_users_selected])
7172
}
7273
},
7374
// Load nonlocal users when requested
7475
async {
75-
if true {
76+
if user_type == UserType::NonLocal || user_type == UserType::All {
7677
db_users.load_one(nonlocal_user_select, db).await
7778
} else {
7879
Ok(vec![None; count_of_users_selected])
7980
}
8081
},
8182
// Load federated users when requested
8283
async {
83-
if true {
84+
if user_type == UserType::Federated || user_type == UserType::All {
8485
db_users.load_many(federated_user_select, db).await
8586
} else {
8687
Ok(vec![Vec::new(); count_of_users_selected])
@@ -92,15 +93,19 @@ pub async fn list(
9293

9394
// For local users fetch passwords to determine password expiration
9495
let local_users_passwords: Vec<Option<Vec<db_password::Model>>> =
95-
local_user::load_local_users_passwords(
96-
db,
97-
locals
98-
.iter()
99-
.cloned()
100-
.map(|u| u.map(|x| x.id))
101-
.collect::<Vec<_>>(),
102-
)
103-
.await?;
96+
if user_type == UserType::Local || user_type == UserType::All {
97+
local_user::load_local_users_passwords(
98+
db,
99+
locals
100+
.iter()
101+
.cloned()
102+
.map(|u| u.map(|x| x.id))
103+
.collect::<Vec<_>>(),
104+
)
105+
.await?
106+
} else {
107+
vec![None; count_of_users_selected]
108+
};
104109

105110
// Determine the date for which users with the last activity earlier than are
106111
// determined as inactive.
@@ -232,4 +237,178 @@ mod tests {
232237
);
233238
}
234239
}
240+
241+
#[tokio::test]
242+
async fn test_list_local_only() {
243+
let db = MockDatabase::new(DatabaseBackend::Postgres)
244+
.append_query_results([vec![
245+
// local user
246+
get_user_mock("1"),
247+
// nonlocal user
248+
get_user_mock("2"),
249+
// federated user
250+
get_user_mock("3"),
251+
// a "bad" user with no user detail records
252+
get_user_mock("4"),
253+
]])
254+
.append_query_results([[get_user_options_mock("1", &UserOptions::default())]
255+
.into_iter()
256+
.flatten()])
257+
.append_query_results([vec![get_local_user_mock("1")]])
258+
.append_query_results([vec![db_password::Model::default()]])
259+
.into_connection();
260+
261+
let config = Config::default();
262+
let res = list(
263+
&config,
264+
&db,
265+
&UserListParameters {
266+
user_type: Some(UserType::Local),
267+
..Default::default()
268+
},
269+
)
270+
.await
271+
.unwrap();
272+
assert_eq!(res.len(), 1, "1 local user found");
273+
for (l,r) in db.into_transaction_log().iter().zip([
274+
Transaction::from_sql_and_values(
275+
DatabaseBackend::Postgres,
276+
r#"SELECT "user"."id", "user"."extra", "user"."enabled", "user"."default_project_id", "user"."created_at", "user"."last_active_at", "user"."domain_id" FROM "user""#,
277+
[]
278+
),
279+
Transaction::from_sql_and_values(
280+
DatabaseBackend::Postgres,
281+
r#"SELECT "user_option"."user_id", "user_option"."option_id", "user_option"."option_value" FROM "user_option" WHERE "user_option"."user_id" IN ($1, $2, $3, $4)"#,
282+
[]
283+
),
284+
Transaction::from_sql_and_values(
285+
DatabaseBackend::Postgres,
286+
r#"SELECT "local_user"."id", "local_user"."user_id", "local_user"."domain_id", "local_user"."name", "local_user"."failed_auth_count", "local_user"."failed_auth_at" FROM "local_user" WHERE ("local_user"."user_id", "local_user"."domain_id") IN (($1, $2), ($3, $4), ($5, $6), ($7, $8))"#,
287+
[]
288+
),
289+
Transaction::from_sql_and_values(
290+
DatabaseBackend::Postgres,
291+
r#"SELECT "password"."id", "password"."local_user_id", "password"."self_service", "password"."created_at", "password"."expires_at", "password"."password_hash", "password"."created_at_int", "password"."expires_at_int" FROM "password" WHERE "password"."local_user_id" IN ($1) ORDER BY "password"."created_at_int" DESC"#,
292+
[]
293+
),
294+
]) {
295+
assert_eq!(
296+
l.statements().iter().map(|x| x.sql.clone()).collect::<Vec<_>>(),
297+
r.statements().iter().map(|x| x.sql.clone()).collect::<Vec<_>>()
298+
);
299+
}
300+
}
301+
302+
#[tokio::test]
303+
304+
async fn test_list_nonlocal_only() {
305+
let db = MockDatabase::new(DatabaseBackend::Postgres)
306+
.append_query_results([vec![
307+
// local user
308+
get_user_mock("1"),
309+
// nonlocal user
310+
get_user_mock("2"),
311+
// federated user
312+
get_user_mock("3"),
313+
// a "bad" user with no user detail records
314+
get_user_mock("4"),
315+
]])
316+
.append_query_results([[get_user_options_mock("2", &UserOptions::default())]
317+
.into_iter()
318+
.flatten()])
319+
.append_query_results([vec![get_nonlocal_user_mock("2")]])
320+
.into_connection();
321+
322+
let config = Config::default();
323+
let res = list(
324+
&config,
325+
&db,
326+
&UserListParameters {
327+
user_type: Some(UserType::NonLocal),
328+
..Default::default()
329+
},
330+
)
331+
.await
332+
.unwrap();
333+
assert_eq!(res.len(), 1, "1 nonlocal user found");
334+
335+
for (l,r) in db.into_transaction_log().iter().zip([
336+
Transaction::from_sql_and_values(
337+
DatabaseBackend::Postgres,
338+
r#"SELECT "user"."id", "user"."extra", "user"."enabled", "user"."default_project_id", "user"."created_at", "user"."last_active_at", "user"."domain_id" FROM "user""#,
339+
[]
340+
),
341+
Transaction::from_sql_and_values(
342+
DatabaseBackend::Postgres,
343+
r#"SELECT "user_option"."user_id", "user_option"."option_id", "user_option"."option_value" FROM "user_option" WHERE "user_option"."user_id" IN ($1, $2, $3, $4)"#,
344+
[]
345+
),
346+
Transaction::from_sql_and_values(
347+
DatabaseBackend::Postgres,
348+
r#"SELECT "nonlocal_user"."domain_id", "nonlocal_user"."name", "nonlocal_user"."user_id" FROM "nonlocal_user" WHERE ("nonlocal_user"."user_id", "nonlocal_user"."domain_id") IN (($1, $2), ($3, $4), ($5, $6), ($7, $8))"#,
349+
[]
350+
),
351+
]) {
352+
assert_eq!(
353+
l.statements().iter().map(|x| x.sql.clone()).collect::<Vec<_>>(),
354+
r.statements().iter().map(|x| x.sql.clone()).collect::<Vec<_>>()
355+
);
356+
}
357+
}
358+
359+
#[tokio::test]
360+
async fn test_list_federated_only() {
361+
let db = MockDatabase::new(DatabaseBackend::Postgres)
362+
.append_query_results([vec![
363+
// local user
364+
get_user_mock("1"),
365+
// nonlocal user
366+
get_user_mock("2"),
367+
// federated user
368+
get_user_mock("3"),
369+
// a "bad" user with no user detail records
370+
get_user_mock("4"),
371+
]])
372+
.append_query_results([[get_user_options_mock("3", &UserOptions::default())]
373+
.into_iter()
374+
.flatten()])
375+
.append_query_results([vec![get_federated_user_mock("3")]])
376+
.into_connection();
377+
378+
let config = Config::default();
379+
let res = list(
380+
&config,
381+
&db,
382+
&UserListParameters {
383+
user_type: Some(UserType::Federated),
384+
..Default::default()
385+
},
386+
)
387+
.await
388+
.unwrap();
389+
assert_eq!(res.len(), 1, "1 federated user found");
390+
391+
for (l,r) in db.into_transaction_log().iter().zip([
392+
Transaction::from_sql_and_values(
393+
DatabaseBackend::Postgres,
394+
r#"SELECT "user"."id", "user"."extra", "user"."enabled", "user"."default_project_id", "user"."created_at", "user"."last_active_at", "user"."domain_id" FROM "user""#,
395+
[]
396+
),
397+
Transaction::from_sql_and_values(
398+
DatabaseBackend::Postgres,
399+
r#"SELECT "user_option"."user_id", "user_option"."option_id", "user_option"."option_value" FROM "user_option" WHERE "user_option"."user_id" IN ($1, $2, $3, $4)"#,
400+
[]
401+
),
402+
Transaction::from_sql_and_values(
403+
DatabaseBackend::Postgres,
404+
r#"SELECT "federated_user"."id", "federated_user"."user_id", "federated_user"."idp_id", "federated_user"."protocol_id", "federated_user"."unique_id", "federated_user"."display_name" FROM "federated_user" WHERE "federated_user"."user_id" IN ($1, $2, $3, $4)"#,
405+
[]
406+
),
407+
]) {
408+
assert_eq!(
409+
l.statements().iter().map(|x| x.sql.clone()).collect::<Vec<_>>(),
410+
r.statements().iter().map(|x| x.sql.clone()).collect::<Vec<_>>()
411+
);
412+
}
413+
}
235414
}

‎src/identity/types/user.rs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,28 @@ pub struct UserListParameters {
204204
#[builder(default)]
205205
#[validate(length(max = 64))]
206206
pub unique_id: Option<String>,
207+
/// Filter users by User Type (local, federated, nonlocal, all).
208+
#[builder(default)]
209+
#[serde(default, rename = "type")]
210+
pub user_type: Option<UserType>,
211+
}
212+
213+
/// User type for filtering.
214+
#[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Eq, Hash, Serialize)]
215+
#[serde(rename_all = "lowercase")]
216+
pub enum UserType {
217+
/// Local users only (with passwords).
218+
Local,
219+
220+
/// Federated users only (authenticated via external IdP).
221+
Federated,
222+
223+
/// Non-local users (users without local authentication).
224+
NonLocal,
225+
226+
/// All users (default behavior).
227+
#[default]
228+
All,
207229
}
208230

209231
/// User password information.

0 commit comments

Comments
 (0)