From 09c5d9f92569216b81ccf208f1eda9c9051e7bb7 Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Fri, 16 Aug 2024 23:48:15 +0200 Subject: [PATCH] server: Fix implementation of attribute present filter Instead of just doing a schema check, this actually looks for users that have a value for this attribute. --- server/src/domain/handler.rs | 2 + server/src/domain/ldap/group.rs | 11 +++-- server/src/domain/ldap/user.rs | 14 +++---- .../src/domain/sql_group_backend_handler.rs | 11 +++-- .../src/domain/sql_schema_backend_handler.rs | 41 ++++++++++++++++++- server/src/domain/sql_user_backend_handler.rs | 11 +++-- 6 files changed, 72 insertions(+), 18 deletions(-) diff --git a/server/src/domain/handler.rs b/server/src/domain/handler.rs index 06ee879..c5d5ad3 100644 --- a/server/src/domain/handler.rs +++ b/server/src/domain/handler.rs @@ -61,6 +61,7 @@ pub enum UserRequestFilter { MemberOf(GroupName), // Same, by id. MemberOfId(GroupId), + CustomAttributePresent(AttributeName), } impl From for UserRequestFilter { @@ -85,6 +86,7 @@ pub enum GroupRequestFilter { // Check if the group contains a user identified by uid. Member(UserId), AttributeEquality(AttributeName, Serialized), + CustomAttributePresent(AttributeName), } impl From for GroupRequestFilter { diff --git a/server/src/domain/ldap/group.rs b/server/src/domain/ldap/group.rs index 313d40d..a3584ea 100644 --- a/server/src/domain/ldap/group.rs +++ b/server/src/domain/ldap/group.rs @@ -228,10 +228,13 @@ fn convert_group_filter( LdapFilter::Not(filter) => Ok(GroupRequestFilter::Not(Box::new(rec(filter)?))), LdapFilter::Present(field) => { let field = AttributeName::from(field.as_str()); - Ok(GroupRequestFilter::from(!matches!( - map_group_field(&field, schema), - GroupFieldType::NoMatch - ))) + Ok(match map_group_field(&field, schema) { + GroupFieldType::Attribute(name, _, _) => { + GroupRequestFilter::CustomAttributePresent(name) + } + GroupFieldType::NoMatch => GroupRequestFilter::from(false), + _ => GroupRequestFilter::from(true), + }) } LdapFilter::Substring(field, substring_filter) => { let field = AttributeName::from(field.as_str()); diff --git a/server/src/domain/ldap/user.rs b/server/src/domain/ldap/user.rs index bbd6c69..325200b 100644 --- a/server/src/domain/ldap/user.rs +++ b/server/src/domain/ldap/user.rs @@ -250,13 +250,13 @@ fn convert_user_filter( } LdapFilter::Present(field) => { let field = AttributeName::from(field.as_str()); - // Check that it's a field we support. - Ok(UserRequestFilter::from( - field.as_str() == "objectclass" - || field.as_str() == "dn" - || field.as_str() == "distinguishedname" - || !matches!(map_user_field(&field, schema), UserFieldType::NoMatch), - )) + Ok(match map_user_field(&field, schema) { + UserFieldType::Attribute(name, _, _) => { + UserRequestFilter::CustomAttributePresent(name) + } + UserFieldType::NoMatch => UserRequestFilter::from(false), + _ => UserRequestFilter::from(true), + }) } LdapFilter::Substring(field, substring_filter) => { let field = AttributeName::from(field.as_str()); diff --git a/server/src/domain/sql_group_backend_handler.rs b/server/src/domain/sql_group_backend_handler.rs index 6f88fb2..ffa707a 100644 --- a/server/src/domain/sql_group_backend_handler.rs +++ b/server/src/domain/sql_group_backend_handler.rs @@ -16,14 +16,18 @@ use sea_orm::{ }; use tracing::instrument; -fn attribute_condition(name: AttributeName, value: Serialized) -> Cond { +fn attribute_condition(name: AttributeName, value: Option) -> Cond { Expr::in_subquery( Expr::col(GroupColumn::GroupId.as_column_ref()), model::GroupAttributes::find() .select_only() .column(model::GroupAttributesColumn::GroupId) .filter(model::GroupAttributesColumn::AttributeName.eq(name)) - .filter(model::GroupAttributesColumn::Value.eq(value)) + .filter( + value + .map(|value| model::GroupAttributesColumn::Value.eq(value)) + .unwrap_or_else(|| SimpleExpr::Value(true.into())), + ) .into_query(), ) .into_condition() @@ -71,7 +75,8 @@ fn get_group_filter_expr(filter: GroupRequestFilter) -> Cond { )))) .like(filter.to_sql_filter()) .into_condition(), - AttributeEquality(name, value) => attribute_condition(name, value), + AttributeEquality(name, value) => attribute_condition(name, Some(value)), + CustomAttributePresent(name) => attribute_condition(name, None), } } diff --git a/server/src/domain/sql_schema_backend_handler.rs b/server/src/domain/sql_schema_backend_handler.rs index c52f998..288055a 100644 --- a/server/src/domain/sql_schema_backend_handler.rs +++ b/server/src/domain/sql_schema_backend_handler.rs @@ -175,7 +175,9 @@ impl SqlBackendHandler { mod tests { use super::*; use crate::domain::{ - handler::AttributeList, sql_backend_handler::tests::*, types::AttributeType, + handler::{AttributeList, UpdateUserRequest, UserBackendHandler, UserRequestFilter}, + sql_backend_handler::tests::*, + types::{AttributeType, AttributeValue, Serialized}, }; use pretty_assertions::assert_eq; @@ -268,6 +270,43 @@ mod tests { .contains(&expected_value)); } + #[tokio::test] + async fn test_user_attribute_present_filter() { + let fixture = TestFixture::new().await; + let new_attribute = CreateAttributeRequest { + name: "new_attribute".into(), + attribute_type: AttributeType::Integer, + is_list: true, + is_visible: false, + is_editable: false, + }; + fixture + .handler + .add_user_attribute(new_attribute) + .await + .unwrap(); + fixture + .handler + .update_user(UpdateUserRequest { + user_id: "bob".into(), + insert_attributes: vec![AttributeValue { + name: "new_attribute".into(), + value: Serialized::from(&3), + }], + ..Default::default() + }) + .await + .unwrap(); + let users = get_user_names( + &fixture.handler, + Some(UserRequestFilter::CustomAttributePresent( + "new_attribute".into(), + )), + ) + .await; + assert_eq!(users, vec!["bob"]); + } + #[tokio::test] async fn test_group_attribute_add_and_delete() { let fixture = TestFixture::new().await; diff --git a/server/src/domain/sql_user_backend_handler.rs b/server/src/domain/sql_user_backend_handler.rs index 1ba60b6..a3768b5 100644 --- a/server/src/domain/sql_user_backend_handler.rs +++ b/server/src/domain/sql_user_backend_handler.rs @@ -22,14 +22,18 @@ use sea_orm::{ use std::collections::HashSet; use tracing::instrument; -fn attribute_condition(name: AttributeName, value: Serialized) -> Cond { +fn attribute_condition(name: AttributeName, value: Option) -> Cond { Expr::in_subquery( Expr::col(UserColumn::UserId.as_column_ref()), model::UserAttributes::find() .select_only() .column(model::UserAttributesColumn::UserId) .filter(model::UserAttributesColumn::AttributeName.eq(name)) - .filter(model::UserAttributesColumn::Value.eq(value)) + .filter( + value + .map(|value| model::UserAttributesColumn::Value.eq(value)) + .unwrap_or_else(|| SimpleExpr::Constant(true.into())), + ) .into_query(), ) .into_condition() @@ -79,7 +83,7 @@ fn get_user_filter_expr(filter: UserRequestFilter) -> Cond { ColumnTrait::eq(&column, value).into_condition() } } - AttributeEquality(column, value) => attribute_condition(column, value), + AttributeEquality(column, value) => attribute_condition(column, Some(value)), MemberOf(group) => user_id_subcondition( Expr::col((group_table, GroupColumn::LowercaseDisplayName)) .eq(group.as_str().to_lowercase()) @@ -98,6 +102,7 @@ fn get_user_filter_expr(filter: UserRequestFilter) -> Cond { .like(filter.to_sql_filter()) .into_condition() } + CustomAttributePresent(name) => attribute_condition(name, None), } }