From a1eb708cf390682b240e96f94ef79fc4635c1848 Mon Sep 17 00:00:00 2001 From: Valentin Tolmer Date: Mon, 26 Feb 2024 09:39:12 +0100 Subject: [PATCH] server: Add missing unique indices on lowercase email/group names, fix memberof lookup --- server/src/domain/ldap/user.rs | 8 ++++ .../src/domain/sql_group_backend_handler.rs | 23 +++++++++- server/src/domain/sql_migrations.rs | 42 +++++++++++++++++++ server/src/domain/sql_tables.rs | 2 +- server/src/domain/sql_user_backend_handler.rs | 35 +++++++++++++++- 5 files changed, 106 insertions(+), 4 deletions(-) diff --git a/server/src/domain/ldap/user.rs b/server/src/domain/ldap/user.rs index dc26a38..a16836f 100644 --- a/server/src/domain/ldap/user.rs +++ b/server/src/domain/ldap/user.rs @@ -195,6 +195,10 @@ fn convert_user_filter( UserFieldType::PrimaryField(UserColumn::UserId) => { Ok(UserRequestFilter::UserId(UserId::new(&value))) } + UserFieldType::PrimaryField(UserColumn::Email) => Ok(UserRequestFilter::Equality( + UserColumn::LowercaseEmail, + value, + )), UserFieldType::PrimaryField(field) => Ok(UserRequestFilter::Equality(field, value)), UserFieldType::Attribute(field, typ, is_list) => { get_user_attribute_equality_filter(&field, typ, is_list, &value) @@ -269,6 +273,10 @@ fn convert_user_filter( ), }), UserFieldType::NoMatch => Ok(UserRequestFilter::from(false)), + UserFieldType::PrimaryField(UserColumn::Email) => Ok(UserRequestFilter::SubString( + UserColumn::LowercaseEmail, + substring_filter.clone().into(), + )), UserFieldType::PrimaryField(field) => Ok(UserRequestFilter::SubString( field, substring_filter.clone().into(), diff --git a/server/src/domain/sql_group_backend_handler.rs b/server/src/domain/sql_group_backend_handler.rs index a0da301..6f88fb2 100644 --- a/server/src/domain/sql_group_backend_handler.rs +++ b/server/src/domain/sql_group_backend_handler.rs @@ -67,7 +67,7 @@ fn get_group_filter_expr(filter: GroupRequestFilter) -> Cond { .into_condition(), DisplayNameSubString(filter) => SimpleExpr::FunctionCall(Func::lower(Expr::col(( group_table, - GroupColumn::DisplayName, + GroupColumn::LowercaseDisplayName, )))) .like(filter.to_sql_filter()) .into_condition(), @@ -606,4 +606,25 @@ mod tests { let details = fixture.handler.get_group_details(group_id).await.unwrap(); assert_eq!(details.attributes, Vec::new()); } + + #[tokio::test] + async fn test_create_group_duplicate_name() { + let fixture = TestFixture::new().await; + fixture + .handler + .create_group(CreateGroupRequest { + display_name: "New Group".into(), + ..Default::default() + }) + .await + .unwrap(); + fixture + .handler + .create_group(CreateGroupRequest { + display_name: "neW group".into(), + ..Default::default() + }) + .await + .unwrap_err(); + } } diff --git a/server/src/domain/sql_migrations.rs b/server/src/domain/sql_migrations.rs index 587971e..4bd54cf 100644 --- a/server/src/domain/sql_migrations.rs +++ b/server/src/domain/sql_migrations.rs @@ -1090,6 +1090,47 @@ async fn migrate_to_v9(transaction: DatabaseTransaction) -> Result Result { + let builder = transaction.get_database_backend(); + if let Err(e) = transaction + .execute( + builder.build( + Index::create() + .if_not_exists() + .name("unique-group-id") + .table(Groups::Table) + .col(Groups::LowercaseDisplayName) + .unique(), + ), + ) + .await + { + error!( + r#"Found several groups with the same (case-insensitive) display name. Please delete the duplicates"# + ); + return Err(e); + } + if let Err(e) = transaction + .execute( + builder.build( + Index::create() + .if_not_exists() + .name("unique-user-lower-email") + .table(Users::Table) + .col(Users::LowercaseEmail) + .unique(), + ), + ) + .await + { + error!( + r#"Found several users with the same (case-insensitive) email. Please delete the duplicates"# + ); + return Err(e); + } + Ok(transaction) +} + // This is needed to make an array of async functions. macro_rules! to_sync { ($l:ident) => { @@ -1119,6 +1160,7 @@ pub async fn migrate_from_version( to_sync!(migrate_to_v7), to_sync!(migrate_to_v8), to_sync!(migrate_to_v9), + to_sync!(migrate_to_v10), ]; assert_eq!(migrations.len(), (LAST_SCHEMA_VERSION.0 - 1) as usize); for migration in 2..=last_version.0 { diff --git a/server/src/domain/sql_tables.rs b/server/src/domain/sql_tables.rs index 9b50d51..6745e61 100644 --- a/server/src/domain/sql_tables.rs +++ b/server/src/domain/sql_tables.rs @@ -11,7 +11,7 @@ pub type DbConnection = sea_orm::DatabaseConnection; #[derive(Copy, PartialEq, Eq, Debug, Clone, PartialOrd, Ord, DeriveValueType)] pub struct SchemaVersion(pub i16); -pub const LAST_SCHEMA_VERSION: SchemaVersion = SchemaVersion(9); +pub const LAST_SCHEMA_VERSION: SchemaVersion = SchemaVersion(10); #[derive(Copy, PartialEq, Eq, Debug, Clone, PartialOrd, Ord)] pub struct PrivateKeyHash(pub [u8; 32]); diff --git a/server/src/domain/sql_user_backend_handler.rs b/server/src/domain/sql_user_backend_handler.rs index ef53271..5372a80 100644 --- a/server/src/domain/sql_user_backend_handler.rs +++ b/server/src/domain/sql_user_backend_handler.rs @@ -67,8 +67,8 @@ fn get_user_filter_expr(filter: UserRequestFilter) -> Cond { } } AttributeEquality(column, value) => attribute_condition(column, value), - MemberOf(group) => Expr::col((group_table, GroupColumn::DisplayName)) - .eq(group) + MemberOf(group) => Expr::col((group_table, GroupColumn::LowercaseDisplayName)) + .eq(group.as_str().to_lowercase()) .into_condition(), MemberOfId(group_id) => Expr::col((group_table, GroupColumn::GroupId)) .eq(group_id) @@ -542,6 +542,12 @@ mod tests { ) .await; assert_eq!(users, vec!["bob", "patrick"]); + let users = get_user_names( + &fixture.handler, + Some(UserRequestFilter::MemberOf("best grOUp".into())), + ) + .await; + assert_eq!(users, vec!["bob", "patrick"]); } #[tokio::test] @@ -1121,4 +1127,29 @@ mod tests { .await .expect_err("Should have failed"); } + + #[tokio::test] + async fn test_create_user_duplicate_email() { + let fixture = TestFixture::new().await; + + fixture + .handler + .create_user(CreateUserRequest { + user_id: UserId::new("james"), + email: "email".into(), + ..Default::default() + }) + .await + .unwrap(); + + fixture + .handler + .create_user(CreateUserRequest { + user_id: UserId::new("john"), + email: "eMail".into(), + ..Default::default() + }) + .await + .unwrap_err(); + } }