From 9018e6fa348ff77a96378ec6394797a4e0cc3e1a Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Tue, 17 Jan 2023 14:43:37 +0100 Subject: [PATCH] server, refactor: Add a conversion from bool for the filters --- server/src/domain/handler.rs | 20 ++++++++++ server/src/domain/ldap/group.rs | 54 +++++++++++--------------- server/src/domain/ldap/user.rs | 65 ++++++++++++-------------------- server/src/infra/ldap_handler.rs | 44 +++++++++++---------- 4 files changed, 87 insertions(+), 96 deletions(-) diff --git a/server/src/domain/handler.rs b/server/src/domain/handler.rs index a39c256..d93657d 100644 --- a/server/src/domain/handler.rs +++ b/server/src/domain/handler.rs @@ -27,6 +27,16 @@ pub enum UserRequestFilter { MemberOfId(GroupId), } +impl From for UserRequestFilter { + fn from(val: bool) -> Self { + if val { + Self::And(vec![]) + } else { + Self::Not(Box::new(Self::And(vec![]))) + } + } +} + #[derive(PartialEq, Eq, Debug, Serialize, Deserialize, Clone)] pub enum GroupRequestFilter { And(Vec), @@ -39,6 +49,16 @@ pub enum GroupRequestFilter { Member(UserId), } +impl From for GroupRequestFilter { + fn from(val: bool) -> Self { + if val { + Self::And(vec![]) + } else { + Self::Not(Box::new(Self::And(vec![]))) + } + } +} + #[derive(PartialEq, Eq, Debug, Serialize, Deserialize, Clone, Default)] pub struct CreateUserRequest { // Same fields as User, but no creation_date, and with password. diff --git a/server/src/domain/ldap/group.rs b/server/src/domain/ldap/group.rs index 2a4e37f..4555e67 100644 --- a/server/src/domain/ldap/group.rs +++ b/server/src/domain/ldap/group.rs @@ -121,25 +121,20 @@ fn convert_group_filter( )?; Ok(GroupRequestFilter::Member(user_name)) } - "objectclass" => match value.as_str() { - "groupofuniquenames" | "groupofnames" => Ok(GroupRequestFilter::And(vec![])), - _ => Ok(GroupRequestFilter::Not(Box::new(GroupRequestFilter::And( - vec![], - )))), - }, - "dn" => Ok( - match get_group_id_from_distinguished_name( - value.to_ascii_lowercase().as_str(), - &ldap_info.base_dn, - &ldap_info.base_dn_str, - ) { - Ok(value) => GroupRequestFilter::DisplayName(value), - Err(_) => { - warn!("Invalid dn filter on group: {}", value); - GroupRequestFilter::Not(Box::new(GroupRequestFilter::And(vec![]))) - } - }, - ), + "objectclass" => Ok(GroupRequestFilter::from(matches!( + value.as_str(), + "groupofuniquenames" | "groupofnames" + ))), + "dn" => Ok(get_group_id_from_distinguished_name( + value.to_ascii_lowercase().as_str(), + &ldap_info.base_dn, + &ldap_info.base_dn_str, + ) + .map(GroupRequestFilter::DisplayName) + .unwrap_or_else(|_| { + warn!("Invalid dn filter on group: {}", value); + GroupRequestFilter::from(false) + })), _ => match map_group_field(field) { Some(GroupColumn::DisplayName) => { Ok(GroupRequestFilter::DisplayName(value.to_string())) @@ -158,9 +153,7 @@ fn convert_group_filter( field ); } - Ok(GroupRequestFilter::Not(Box::new(GroupRequestFilter::And( - vec![], - )))) + Ok(GroupRequestFilter::from(false)) } }, } @@ -174,17 +167,12 @@ fn convert_group_filter( LdapFilter::Not(filter) => Ok(GroupRequestFilter::Not(Box::new(rec(filter)?))), LdapFilter::Present(field) => { let field = &field.to_ascii_lowercase(); - if field == "objectclass" - || field == "dn" - || field == "distinguishedname" - || map_group_field(field).is_some() - { - Ok(GroupRequestFilter::And(vec![])) - } else { - Ok(GroupRequestFilter::Not(Box::new(GroupRequestFilter::And( - vec![], - )))) - } + Ok(GroupRequestFilter::from( + field == "objectclass" + || field == "dn" + || field == "distinguishedname" + || map_group_field(field).is_some(), + )) } _ => Err(LdapError { code: LdapResultCode::UnwillingToPerform, diff --git a/server/src/domain/ldap/user.rs b/server/src/domain/ldap/user.rs index 20b06c9..caddb6d 100644 --- a/server/src/domain/ldap/user.rs +++ b/server/src/domain/ldap/user.rs @@ -134,35 +134,27 @@ fn convert_user_filter(ldap_info: &LdapInfo, filter: &LdapFilter) -> LdapResult< LdapFilter::Equality(field, value) => { let field = &field.to_ascii_lowercase(); match field.as_str() { - "memberof" => { - let group_name = get_group_id_from_distinguished_name( + "memberof" => Ok(UserRequestFilter::MemberOf( + get_group_id_from_distinguished_name( &value.to_ascii_lowercase(), &ldap_info.base_dn, &ldap_info.base_dn_str, - )?; - Ok(UserRequestFilter::MemberOf(group_name)) - } - "objectclass" => match value.to_ascii_lowercase().as_str() { - "person" | "inetorgperson" | "posixaccount" | "mailaccount" => { - Ok(UserRequestFilter::And(vec![])) - } - _ => Ok(UserRequestFilter::Not(Box::new(UserRequestFilter::And( - vec![], - )))), - }, - "dn" => Ok( - match get_user_id_from_distinguished_name( - value.to_ascii_lowercase().as_str(), - &ldap_info.base_dn, - &ldap_info.base_dn_str, - ) { - Ok(value) => UserRequestFilter::UserId(value), - Err(_) => { - warn!("Invalid dn filter on user: {}", value); - UserRequestFilter::Not(Box::new(UserRequestFilter::And(vec![]))) - } - }, - ), + )?, + )), + "objectclass" => Ok(UserRequestFilter::from(matches!( + value.to_ascii_lowercase().as_str(), + "person" | "inetorgperson" | "posixaccount" | "mailaccount" + ))), + "dn" => Ok(get_user_id_from_distinguished_name( + value.to_ascii_lowercase().as_str(), + &ldap_info.base_dn, + &ldap_info.base_dn_str, + ) + .map(UserRequestFilter::UserId) + .unwrap_or_else(|_| { + warn!("Invalid dn filter on user: {}", value); + UserRequestFilter::from(false) + })), _ => match map_user_field(field) { Some(UserColumn::UserId) => Ok(UserRequestFilter::UserId(UserId::new(value))), Some(field) => Ok(UserRequestFilter::Equality(field, value.clone())), @@ -174,9 +166,7 @@ fn convert_user_filter(ldap_info: &LdapInfo, filter: &LdapFilter) -> LdapResult< field ); } - Ok(UserRequestFilter::Not(Box::new(UserRequestFilter::And( - vec![], - )))) + Ok(UserRequestFilter::from(false)) } }, } @@ -184,17 +174,12 @@ fn convert_user_filter(ldap_info: &LdapInfo, filter: &LdapFilter) -> LdapResult< LdapFilter::Present(field) => { let field = &field.to_ascii_lowercase(); // Check that it's a field we support. - if field == "objectclass" - || field == "dn" - || field == "distinguishedname" - || map_user_field(field).is_some() - { - Ok(UserRequestFilter::And(vec![])) - } else { - Ok(UserRequestFilter::Not(Box::new(UserRequestFilter::And( - vec![], - )))) - } + Ok(UserRequestFilter::from( + field == "objectclass" + || field == "dn" + || field == "distinguishedname" + || map_user_field(field).is_some(), + )) } _ => Err(LdapError { code: LdapResultCode::UnwillingToPerform, diff --git a/server/src/infra/ldap_handler.rs b/server/src/infra/ldap_handler.rs index 6287573..2c5e6ce 100644 --- a/server/src/infra/ldap_handler.rs +++ b/server/src/infra/ldap_handler.rs @@ -778,7 +778,7 @@ mod tests { mock.expect_list_users() .with( eq(Some(UserRequestFilter::And(vec![ - UserRequestFilter::And(vec![]), + UserRequestFilter::from(true), UserRequestFilter::UserId(UserId::new("test")), ]))), eq(false), @@ -813,7 +813,7 @@ mod tests { async fn test_search_readonly_user() { let mut mock = MockTestBackendHandler::new(); mock.expect_list_users() - .with(eq(Some(UserRequestFilter::And(vec![]))), eq(false)) + .with(eq(Some(UserRequestFilter::from(true))), eq(false)) .times(1) .return_once(|_, _| Ok(vec![])); let mut ldap_handler = setup_bound_readonly_handler(mock).await; @@ -830,7 +830,7 @@ mod tests { async fn test_search_member_of() { let mut mock = MockTestBackendHandler::new(); mock.expect_list_users() - .with(eq(Some(UserRequestFilter::And(vec![]))), eq(true)) + .with(eq(Some(UserRequestFilter::from(true))), eq(true)) .times(1) .return_once(|_, _| { Ok(vec![UserAndGroups { @@ -873,7 +873,7 @@ mod tests { mock.expect_list_users() .with( eq(Some(UserRequestFilter::And(vec![ - UserRequestFilter::And(vec![]), + UserRequestFilter::from(true), UserRequestFilter::UserId(UserId::new("bob")), ]))), eq(false), @@ -1131,7 +1131,7 @@ mod tests { async fn test_search_groups() { let mut mock = MockTestBackendHandler::new(); mock.expect_list_groups() - .with(eq(Some(GroupRequestFilter::And(vec![])))) + .with(eq(Some(GroupRequestFilter::from(true)))) .times(1) .return_once(|_| { Ok(vec![ @@ -1218,14 +1218,12 @@ mod tests { GroupRequestFilter::DisplayName("group_1".to_string()), GroupRequestFilter::Member(UserId::new("bob")), GroupRequestFilter::DisplayName("rockstars".to_string()), - GroupRequestFilter::And(vec![]), - GroupRequestFilter::And(vec![]), - GroupRequestFilter::And(vec![]), - GroupRequestFilter::And(vec![]), - GroupRequestFilter::Not(Box::new(GroupRequestFilter::Not(Box::new( - GroupRequestFilter::And(vec![]), - )))), - GroupRequestFilter::Not(Box::new(GroupRequestFilter::And(vec![]))), + GroupRequestFilter::from(true), + GroupRequestFilter::from(true), + GroupRequestFilter::from(true), + GroupRequestFilter::from(true), + GroupRequestFilter::Not(Box::new(GroupRequestFilter::from(false))), + GroupRequestFilter::from(false), ])))) .times(1) .return_once(|_| { @@ -1321,7 +1319,7 @@ mod tests { let mut mock = MockTestBackendHandler::new(); mock.expect_list_groups() .with(eq(Some(GroupRequestFilter::And(vec![ - GroupRequestFilter::And(vec![]), + GroupRequestFilter::from(true), GroupRequestFilter::DisplayName("rockstars".to_string()), ])))) .times(1) @@ -1409,12 +1407,12 @@ mod tests { "bob", )))), UserRequestFilter::UserId("bob_1".to_string().into()), - UserRequestFilter::And(vec![]), - UserRequestFilter::Not(Box::new(UserRequestFilter::And(vec![]))), - UserRequestFilter::And(vec![]), - UserRequestFilter::And(vec![]), - UserRequestFilter::Not(Box::new(UserRequestFilter::And(vec![]))), - UserRequestFilter::Not(Box::new(UserRequestFilter::And(vec![]))), + UserRequestFilter::from(true), + UserRequestFilter::from(false), + UserRequestFilter::from(true), + UserRequestFilter::from(true), + UserRequestFilter::from(false), + UserRequestFilter::from(false), ], )]))), eq(false), @@ -1562,7 +1560,7 @@ mod tests { }]) }); mock.expect_list_groups() - .with(eq(Some(GroupRequestFilter::And(vec![])))) + .with(eq(Some(GroupRequestFilter::from(true)))) .times(1) .return_once(|_| { Ok(vec![Group { @@ -1637,7 +1635,7 @@ mod tests { }]) }); mock.expect_list_groups() - .with(eq(Some(GroupRequestFilter::And(vec![])))) + .with(eq(Some(GroupRequestFilter::from(true)))) .returning(|_| { Ok(vec![Group { id: GroupId(1), @@ -2093,7 +2091,7 @@ mod tests { async fn test_search_filter_non_attribute() { let mut mock = MockTestBackendHandler::new(); mock.expect_list_users() - .with(eq(Some(UserRequestFilter::And(vec![]))), eq(false)) + .with(eq(Some(UserRequestFilter::from(true))), eq(false)) .times(1) .return_once(|_, _| Ok(vec![])); let mut ldap_handler = setup_bound_admin_handler(mock).await;