server: Improve the error message in case of duplicate emails

This commit is contained in:
Valentin Tolmer 2023-04-14 11:40:35 +02:00 committed by nitnelave
parent 96b7dbb1c5
commit e5ce98c874
3 changed files with 136 additions and 16 deletions

View file

@ -0,0 +1,58 @@
# Migration from 0.4 to 0.5
Welcome! If you're here, it's probably that the migration from 0.4.x to 0.5
didn't go smoothly for you. Don't worry, we can fix that.
## Multiple users with the same email
This is the most common case. You can see in the LLDAP logs that there are
several users with the same email, and they are listed.
This is not allowed anymore in v0.5, to prevent a user from setting their email
to someone else's email and gaining access to systems that identify by email.
The problem is that you currently have several users with the same email, so the
constraint cannot be enforced.
### Step 1: Take a note of the users with duplicate emails
In the LLDAP logs when you tried to start v0.5+, you'll see some warnings with
the list of users with the same emails. Take note of them.
### Step 2: Downgrade to v0.4.3
If using docker, switch to the `lldap/lldap:v0.4.3` image. Alternatively, grab
the binaries at https://github.com/lldap/lldap/releases/tag/v0.4.3.
This downgrade is safe and supported.
### Step 3: Remove duplicate emails
Restart LLDAP with the v0.4.3 version, and using your notes from step 1, change
the email of users with duplicate emails to make sure that each email is unique.
### Step 4: Upgrade again
You can now revert to the initial version.
## Multiple users/groups with the same UUID
This should be extremely rare. In this case, you'll need to find which users
have the same UUID, revert to v0.4.3 to be able to apply the changes, and delete
one of the duplicates.
## FAQ
### What if I want several users to be controlled by the same email?
You can use plus codes to set "the same" email to several users, while ensuring
that they can't identify as each other. For instance:
- Admin: `admin@example.com`
- Read-only admin: `admin+readonly@example.com`
- Jellyfin admin: `admin+jellyfin@example.com`
### I'm upgrading to a higher version than v0.5.
This guide is still relevant: you can use whatever later version in place of
v0.5. You'll still need to revert to v0.4.3 to apply the changes.

View file

@ -3,9 +3,12 @@ use crate::domain::{
types::{GroupId, UserId, Uuid},
};
use anyhow::Context;
use itertools::Itertools;
use sea_orm::{
sea_query::{self, ColumnDef, Expr, ForeignKey, ForeignKeyAction, Index, Query, Table, Value},
ConnectionTrait, FromQueryResult, Iden, Statement, TransactionTrait,
sea_query::{
self, all, ColumnDef, Expr, ForeignKey, ForeignKeyAction, Func, Index, Query, Table, Value,
},
ConnectionTrait, FromQueryResult, Iden, Order, Statement, TransactionTrait,
};
use serde::{Deserialize, Serialize};
use tracing::{info, instrument, warn};
@ -462,20 +465,73 @@ async fn migrate_to_v3(pool: &DbConnection) -> anyhow::Result<()> {
async fn migrate_to_v4(pool: &DbConnection) -> anyhow::Result<()> {
let builder = pool.get_database_backend();
// Make emails and UUIDs unique.
if let Err(e) = pool
.execute(
builder.build(
Index::create()
.if_not_exists()
.name("unique-user-email")
.table(Users::Table)
.col(Users::Email)
.unique(),
),
)
.await
.context(
r#"while enforcing unicity on emails (2 users have the same email).
See https://github.com/lldap/lldap/blob/main/docs/migration_guides/v0.5.md for details.
"#,
)
{
warn!("Found several users with the same email:");
for (email, users) in &pool
.query_all(
builder.build(
Query::select()
.from(Users::Table)
.columns([Users::Email, Users::UserId])
.order_by_columns([(Users::Email, Order::Asc), (Users::UserId, Order::Asc)])
.and_where(
Expr::col(Users::Email).in_subquery(
Query::select()
.from(Users::Table)
.column(Users::Email)
.group_by_col(Users::Email)
.cond_having(all![Expr::gt(
Expr::expr(Func::count(Expr::col(Users::Email))),
1
)])
.take(),
),
),
),
)
.await
.expect("Could not check duplicate users")
.into_iter()
.map(|row| {
(
row.try_get::<UserId>("", &Users::UserId.to_string())
.unwrap(),
row.try_get::<String>("", &Users::Email.to_string())
.unwrap(),
)
})
.group_by(|(_user, email)| email.to_owned())
{
warn!("Email: {email}");
for (user, _email) in users {
warn!(" User: {}", user.as_str());
}
}
return Err(e);
}
pool.execute(
builder.build(
Index::create()
.name("unique-user-email")
.table(Users::Table)
.col(Users::Email)
.unique(),
),
)
.await
.context("while enforcing unicity on emails (2 users have the same email)")?;
pool.execute(
builder.build(
Index::create()
.if_not_exists()
.name("unique-user-uuid")
.table(Users::Table)
.col(Users::Uuid)
@ -487,6 +543,7 @@ async fn migrate_to_v4(pool: &DbConnection) -> anyhow::Result<()> {
pool.execute(
builder.build(
Index::create()
.if_not_exists()
.name("unique-group-uuid")
.table(Groups::Table)
.col(Groups::Uuid)

View file

@ -46,6 +46,7 @@ mod tests {
use super::*;
use chrono::prelude::*;
use sea_orm::{ConnectionTrait, Database, DbBackend, FromQueryResult};
use tracing::error;
async fn get_in_memory_db() -> DbConnection {
let mut sql_opt = sea_orm::ConnectOptions::new("sqlite::memory:".to_owned());
@ -208,6 +209,7 @@ mod tests {
#[tokio::test]
async fn test_migration_to_v4() {
crate::infra::logging::init_for_tests();
let sql_pool = get_in_memory_db().await;
upgrade_to_v1(&sql_pool).await.unwrap();
migrate_from_version(&sql_pool, SchemaVersion(1), SchemaVersion(3))
@ -227,9 +229,12 @@ mod tests {
))
.await
.unwrap();
migrate_from_version(&sql_pool, SchemaVersion(3), SchemaVersion(4))
.await
.expect_err("migration should fail");
error!(
"{}",
migrate_from_version(&sql_pool, SchemaVersion(3), SchemaVersion(4))
.await
.expect_err("migration should fail")
);
assert_eq!(
sql_migrations::JustSchemaVersion::find_by_statement(raw_statement(
r#"SELECT version FROM metadata"#