diff options
| author | Owen Jacobson <owen@grimoire.ca> | 2024-10-21 00:36:44 -0400 |
|---|---|---|
| committer | Owen Jacobson <owen@grimoire.ca> | 2024-10-22 10:58:11 -0400 |
| commit | 3f9648eed48cd8b6cd35d0ae2ee5bbe25fa735ac (patch) | |
| tree | 8ecdd86cd9e09d8a3bd55ec1f72888a81498cc38 /src/login | |
| parent | 379e97c2cb145bc3a495aa14746273d83b508214 (diff) | |
Canonicalize login and channel names.
Canonicalization does two things:
* It prevents duplicate names that differ only by case or only by normalization/encoding sequence; and
* It makes certain name-based comparisons "case-insensitive" (generalizing via Unicode's case-folding rules).
This change is complicated, as it means that every name now needs to be stored in two forms. Unfortunately, this is _very likely_ a breaking schema change. The migrations in this commit perform a best-effort attempt to canonicalize existing channel or login names, but it's likely any existing channels or logins with non-ASCII characters will not be canonicalize correctly. Since clients look at all channel names and all login names on boot, and since the code in this commit verifies canonicalization when reading from the database, this will effectively make the server un-usuable until any incorrectly-canonicalized values are either manually canonicalized, or removed
It might be possible to do better with [the `icu` sqlite3 extension][icu], but (a) I'm not convinced of that and (b) this commit is already huge; adding database extension support would make it far larger.
[icu]: https://sqlite.org/src/dir/ext/icu
For some references on why it's worth storing usernames this way, see <https://www.b-list.org/weblog/2018/nov/26/case/> and the refernced talk, as well as <https://www.b-list.org/weblog/2018/feb/11/usernames/>. Bennett's treatment of this issue is, to my eye, much more readable than the referenced Unicode technical reports, and I'm inclined to trust his opinion given that he maintains a widely-used, internet-facing user registration library for Django.
Diffstat (limited to 'src/login')
| -rw-r--r-- | src/login/app.rs | 3 | ||||
| -rw-r--r-- | src/login/mod.rs | 4 | ||||
| -rw-r--r-- | src/login/name.rs | 28 | ||||
| -rw-r--r-- | src/login/password.rs | 2 | ||||
| -rw-r--r-- | src/login/repo.rs | 98 | ||||
| -rw-r--r-- | src/login/routes/login/post.rs | 3 | ||||
| -rw-r--r-- | src/login/routes/logout/test.rs | 1 | ||||
| -rw-r--r-- | src/login/snapshot.rs | 3 |
8 files changed, 63 insertions, 79 deletions
diff --git a/src/login/app.rs b/src/login/app.rs index ebc1c00..37f1249 100644 --- a/src/login/app.rs +++ b/src/login/app.rs @@ -1,9 +1,10 @@ use sqlx::sqlite::SqlitePool; -use super::{repo::Provider as _, Login, Name, Password}; +use super::{repo::Provider as _, Login, Password}; use crate::{ clock::DateTime, event::{repo::Provider as _, Broadcaster, Event}, + name::Name, }; pub struct Logins<'a> { diff --git a/src/login/mod.rs b/src/login/mod.rs index 71d5bfc..98cc3d7 100644 --- a/src/login/mod.rs +++ b/src/login/mod.rs @@ -4,13 +4,11 @@ pub mod event; pub mod extract; mod history; mod id; -mod name; pub mod password; pub mod repo; mod routes; mod snapshot; pub use self::{ - event::Event, history::History, id::Id, name::Name, password::Password, routes::router, - snapshot::Login, + event::Event, history::History, id::Id, password::Password, routes::router, snapshot::Login, }; diff --git a/src/login/name.rs b/src/login/name.rs deleted file mode 100644 index d882ff9..0000000 --- a/src/login/name.rs +++ /dev/null @@ -1,28 +0,0 @@ -use std::fmt; - -use crate::nfc; - -#[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize, sqlx::Type)] -#[serde(transparent)] -#[sqlx(transparent)] -pub struct Name(nfc::String); - -impl fmt::Display for Name { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let Self(name) = self; - name.fmt(f) - } -} - -impl From<String> for Name { - fn from(name: String) -> Self { - Self(name.into()) - } -} - -impl From<Name> for String { - fn from(name: Name) -> Self { - let Name(name) = name; - name.into() - } -} diff --git a/src/login/password.rs b/src/login/password.rs index f9ecf37..c27c950 100644 --- a/src/login/password.rs +++ b/src/login/password.rs @@ -4,7 +4,7 @@ use argon2::Argon2; use password_hash::{PasswordHash, PasswordHasher, PasswordVerifier, SaltString}; use rand_core::OsRng; -use crate::nfc; +use crate::normalize::nfc; #[derive(sqlx::Type)] #[sqlx(transparent)] diff --git a/src/login/repo.rs b/src/login/repo.rs index 204329f..6021f26 100644 --- a/src/login/repo.rs +++ b/src/login/repo.rs @@ -1,9 +1,11 @@ +use futures::stream::{StreamExt as _, TryStreamExt as _}; use sqlx::{sqlite::Sqlite, SqliteConnection, Transaction}; use crate::{ clock::DateTime, event::{Instant, ResumePoint, Sequence}, - login::{password::StoredHash, History, Id, Login, Name}, + login::{password::StoredHash, History, Id, Login}, + name::{self, Name}, }; pub trait Provider { @@ -26,43 +28,43 @@ impl<'c> Logins<'c> { created: &Instant, ) -> Result<History, sqlx::Error> { let id = Id::generate(); + let display_name = name.display(); + let canonical_name = name.canonical(); - let login = sqlx::query!( + sqlx::query!( r#" insert - into login (id, name, password_hash, created_sequence, created_at) - values ($1, $2, $3, $4, $5) - returning - id as "id: Id", - name as "name: Name", - created_sequence as "created_sequence: Sequence", - created_at as "created_at: DateTime" + into login (id, display_name, canonical_name, password_hash, created_sequence, created_at) + values ($1, $2, $3, $4, $5, $6) "#, id, - name, + display_name, + canonical_name, password_hash, created.sequence, created.at, ) - .map(|row| History { + .execute(&mut *self.0) + .await?; + + let login = History { + created: *created, login: Login { - id: row.id, - name: row.name, + id, + name: name.clone(), }, - created: Instant::new(row.created_at, row.created_sequence), - }) - .fetch_one(&mut *self.0) - .await?; + }; Ok(login) } - pub async fn all(&mut self, resume_at: ResumePoint) -> Result<Vec<History>, sqlx::Error> { - let channels = sqlx::query!( + pub async fn all(&mut self, resume_at: ResumePoint) -> Result<Vec<History>, LoadError> { + let logins = sqlx::query!( r#" select id as "id: Id", - name as "name: Name", + display_name as "display_name: String", + canonical_name as "canonical_name: String", created_sequence as "created_sequence: Sequence", created_at as "created_at: DateTime" from login @@ -71,24 +73,29 @@ impl<'c> Logins<'c> { "#, resume_at, ) - .map(|row| History { - login: Login { - id: row.id, - name: row.name, - }, - created: Instant::new(row.created_at, row.created_sequence), + .map(|row| { + Ok::<_, LoadError>(History { + login: Login { + id: row.id, + name: Name::new(row.display_name, row.canonical_name)?, + }, + created: Instant::new(row.created_at, row.created_sequence), + }) }) - .fetch_all(&mut *self.0) + .fetch(&mut *self.0) + .map(|res| res?) + .try_collect() .await?; - Ok(channels) + Ok(logins) } - pub async fn replay(&mut self, resume_at: ResumePoint) -> Result<Vec<History>, sqlx::Error> { - let messages = sqlx::query!( + pub async fn replay(&mut self, resume_at: ResumePoint) -> Result<Vec<History>, LoadError> { + let logins = sqlx::query!( r#" select id as "id: Id", - name as "name: Name", + display_name as "display_name: String", + canonical_name as "canonical_name: String", created_sequence as "created_sequence: Sequence", created_at as "created_at: DateTime" from login @@ -96,22 +103,27 @@ impl<'c> Logins<'c> { "#, resume_at, ) - .map(|row| History { - login: Login { - id: row.id, - name: row.name, - }, - created: Instant::new(row.created_at, row.created_sequence), + .map(|row| { + Ok::<_, name::Error>(History { + login: Login { + id: row.id, + name: Name::new(row.display_name, row.canonical_name)?, + }, + created: Instant::new(row.created_at, row.created_sequence), + }) }) - .fetch_all(&mut *self.0) + .fetch(&mut *self.0) + .map(|res| Ok::<_, LoadError>(res??)) + .try_collect() .await?; - Ok(messages) + Ok(logins) } } -impl<'t> From<&'t mut SqliteConnection> for Logins<'t> { - fn from(tx: &'t mut SqliteConnection) -> Self { - Self(tx) - } +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub enum LoadError { + Database(#[from] sqlx::Error), + Name(#[from] name::Error), } diff --git a/src/login/routes/login/post.rs b/src/login/routes/login/post.rs index 7a685e2..20430db 100644 --- a/src/login/routes/login/post.rs +++ b/src/login/routes/login/post.rs @@ -8,7 +8,8 @@ use crate::{ app::App, clock::RequestedAt, error::Internal, - login::{Login, Name, Password}, + login::{Login, Password}, + name::Name, token::{app, extract::IdentityToken}, }; diff --git a/src/login/routes/logout/test.rs b/src/login/routes/logout/test.rs index 0e70e4c..91837fe 100644 --- a/src/login/routes/logout/test.rs +++ b/src/login/routes/logout/test.rs @@ -33,7 +33,6 @@ async fn successful() { assert_eq!(StatusCode::NO_CONTENT, response_status); // Verify the semantics - let error = app .tokens() .validate(&secret, &now) diff --git a/src/login/snapshot.rs b/src/login/snapshot.rs index 85800e4..e1eb96c 100644 --- a/src/login/snapshot.rs +++ b/src/login/snapshot.rs @@ -1,7 +1,8 @@ use super::{ event::{Created, Event}, - Id, Name, + Id, }; +use crate::name::Name; // This also implements FromRequestParts (see `./extract.rs`). As a result, it // can be used as an extractor for endpoints that want to require login, or for |
