Restrict login names.

There's no good reason to use an empty string as your login name, or to use one so long as to annoy others. Names beginning or ending with whitespace, or containing runs of whitespace, are also a technical problem, so they're also prohibited.

This change does not implement [UTS #39], as I haven't yet fully understood how to do so.

[UTS #39]: https://www.unicode.org/reports/tr39/

10 files changed, 116 insertions, 5 deletions

diff --git a/src/invite/app.rs b/src/invite/app.rs
index 176075f..182eb67 100644
--- a/src/invite/app.rs
+++ b/src/invite/app.rs
@@ -6,7 +6,7 @@ use crate::{
     clock::DateTime,
     db::{Duplicate as _, NotFound as _},
     event::{repo::Provider as _, Broadcaster, Event},
-    login::{repo::Provider as _, Login, Password},
+    login::{repo::Provider as _, validate, Login, Password},
     name::Name,
     token::{repo::Provider as _, Secret},
 };
@@ -44,6 +44,10 @@ impl<'a> Invites<'a> {
         password: &Password,
         accepted_at: &DateTime,
     ) -> Result<(Login, Secret), AcceptError> {
+        if !validate::name(name) {
+            return Err(AcceptError::InvalidName(name.clone()));
+        }
+
         let mut tx = self.db.begin().await?;
         let invite = tx
             .invites()
@@ -92,6 +96,8 @@ impl<'a> Invites<'a> {
 pub enum AcceptError {
     #[error("invite not found: {0}")]
     NotFound(Id),
+    #[error("invalid login name: {0}")]
+    InvalidName(Name),
     #[error("name in use: {0}")]
     DuplicateLogin(Name),
     #[error(transparent)]
diff --git a/src/invite/routes/invite/post.rs b/src/invite/routes/invite/post.rs
index 627eca3..bb68e07 100644
--- a/src/invite/routes/invite/post.rs
+++ b/src/invite/routes/invite/post.rs
@@ -45,6 +45,9 @@ impl IntoResponse for Error {
         let Self(error) = self;
         match error {
             app::AcceptError::NotFound(_) => NotFound(error).into_response(),
+            app::AcceptError::InvalidName(_) => {
+                (StatusCode::BAD_REQUEST, error.to_string()).into_response()
+            }
             app::AcceptError::DuplicateLogin(_) => {
                 (StatusCode::CONFLICT, error.to_string()).into_response()
             }
diff --git a/src/invite/routes/invite/test/post.rs b/src/invite/routes/invite/test/post.rs
index 65ab61e..40e0580 100644
--- a/src/invite/routes/invite/test/post.rs
+++ b/src/invite/routes/invite/test/post.rs
@@ -206,3 +206,35 @@ async fn conflicting_name() {
         matches!(error, AcceptError::DuplicateLogin(error_name) if error_name == conflicting_name)
     );
 }
+
+#[tokio::test]
+async fn invalid_name() {
+    // Set up the environment
+
+    let app = fixtures::scratch_app().await;
+    let issuer = fixtures::login::create(&app, &fixtures::now()).await;
+    let invite = fixtures::invite::issue(&app, &issuer, &fixtures::now()).await;
+
+    // Call the endpoint
+
+    let name = fixtures::login::propose_invalid_name();
+    let password = fixtures::login::propose_password();
+    let identity = fixtures::cookie::not_logged_in();
+    let request = post::Request {
+        name: name.clone(),
+        password: password.clone(),
+    };
+    let post::Error(error) = post::handler(
+        State(app.clone()),
+        fixtures::now(),
+        identity,
+        Path(invite.id),
+        Json(request),
+    )
+    .await
+    .expect_err("using an invalid name should fail");
+
+    // Verify the response
+
+    assert!(matches!(error, AcceptError::InvalidName(error_name) if name == error_name));
+}
diff --git a/src/login/app.rs b/src/login/app.rs
index 2f5896f..c1bfe6e 100644
--- a/src/login/app.rs
+++ b/src/login/app.rs
@@ -3,7 +3,7 @@ use sqlx::sqlite::SqlitePool;
 use super::repo::Provider as _;
 
 #[cfg(test)]
-use super::{Login, Password};
+use super::{validate, Login, Password};
 #[cfg(test)]
 use crate::{
     clock::DateTime,
@@ -35,6 +35,10 @@ impl<'a> Logins<'a> {
         password: &Password,
         created_at: &DateTime,
     ) -> Result<Login, CreateError> {
+        if !validate::name(name) {
+            return Err(CreateError::InvalidName(name.clone()));
+        }
+
         let password_hash = password.hash()?;
 
         let mut tx = self.db.begin().await?;
@@ -57,9 +61,13 @@ impl<'a> Logins<'a> {
     }
 }
 
+#[cfg(test)]
 #[derive(Debug, thiserror::Error)]
-#[error(transparent)]
 pub enum CreateError {
+    #[error("invalid login name: {0}")]
+    InvalidName(Name),
+    #[error(transparent)]
     Database(#[from] sqlx::Error),
+    #[error(transparent)]
     PasswordHash(#[from] password_hash::Error),
 }
diff --git a/src/login/mod.rs b/src/login/mod.rs
index 279e9a6..6d10e17 100644
--- a/src/login/mod.rs
+++ b/src/login/mod.rs
@@ -6,6 +6,7 @@ pub mod password;
 pub mod repo;
 mod routes;
 mod snapshot;
+pub mod validate;
 
 pub use self::{
     event::Event, history::History, id::Id, password::Password, routes::router, snapshot::Login,
diff --git a/src/login/validate.rs b/src/login/validate.rs
new file mode 100644
index 0000000..ed3eff8
--- /dev/null
+++ b/src/login/validate.rs
@@ -0,0 +1,23 @@
+use unicode_segmentation::UnicodeSegmentation as _;
+
+use crate::name::Name;
+
+// Picked out of a hat. The power of two is not meaningful.
+const NAME_TOO_LONG: usize = 64;
+
+pub fn name(name: &Name) -> bool {
+    let display = name.display();
+
+    [
+        display.graphemes(true).count() < NAME_TOO_LONG,
+        display.chars().all(|ch| !ch.is_control()),
+        display.chars().next().is_some_and(char::is_alphanumeric),
+        display.chars().last().is_some_and(char::is_alphanumeric),
+        display
+            .chars()
+            .zip(display.chars().skip(1))
+            .all(|(a, b)| !(a.is_whitespace() && b.is_whitespace())),
+    ]
+    .into_iter()
+    .all(|value| value)
+}
diff --git a/src/setup/app.rs b/src/setup/app.rs
index 030b5f6..cab7c4b 100644
--- a/src/setup/app.rs
+++ b/src/setup/app.rs
@@ -4,7 +4,7 @@ use super::repo::Provider as _;
 use crate::{
     clock::DateTime,
     event::{repo::Provider as _, Broadcaster, Event},
-    login::{repo::Provider as _, Login, Password},
+    login::{repo::Provider as _, validate, Login, Password},
     name::Name,
     token::{repo::Provider as _, Secret},
 };
@@ -25,6 +25,10 @@ impl<'a> Setup<'a> {
         password: &Password,
         created_at: &DateTime,
     ) -> Result<(Login, Secret), Error> {
+        if !validate::name(name) {
+            return Err(Error::InvalidName(name.clone()));
+        }
+
         let password_hash = password.hash()?;
 
         let mut tx = self.db.begin().await?;
@@ -56,6 +60,8 @@ impl<'a> Setup<'a> {
 pub enum Error {
     #[error("initial setup previously completed")]
     SetupCompleted,
+    #[error("invalid login name: {0}")]
+    InvalidName(Name),
     #[error(transparent)]
     Database(#[from] sqlx::Error),
     #[error(transparent)]
diff --git a/src/setup/routes/post.rs b/src/setup/routes/post.rs
index f7b256e..2a46b04 100644
--- a/src/setup/routes/post.rs
+++ b/src/setup/routes/post.rs
@@ -42,6 +42,9 @@ impl IntoResponse for Error {
     fn into_response(self) -> Response {
         let Self(error) = self;
         match error {
+            app::Error::InvalidName(_) => {
+                (StatusCode::BAD_REQUEST, error.to_string()).into_response()
+            }
             app::Error::SetupCompleted => (StatusCode::CONFLICT, error.to_string()).into_response(),
             other => Internal::from(other).into_response(),
         }
diff --git a/src/setup/routes/test.rs b/src/setup/routes/test.rs
index f7562ae..5794b78 100644
--- a/src/setup/routes/test.rs
+++ b/src/setup/routes/test.rs
@@ -67,3 +67,28 @@ async fn login_exists() {
 
     assert!(matches!(error, app::Error::SetupCompleted));
 }
+
+#[tokio::test]
+async fn invalid_name() {
+    // Set up the environment
+
+    let app = fixtures::scratch_app().await;
+
+    // Call the endpoint
+
+    let name = fixtures::login::propose_invalid_name();
+    let password = fixtures::login::propose_password();
+    let identity = fixtures::cookie::not_logged_in();
+    let request = post::Request {
+        name: name.clone(),
+        password: password.clone(),
+    };
+    let post::Error(error) =
+        post::handler(State(app.clone()), fixtures::now(), identity, Json(request))
+            .await
+            .expect_err("setup with an invalid name fails");
+
+    // Verify the response
+
+    assert!(matches!(error, app::Error::InvalidName(error_name) if name == error_name));
+}
diff --git a/src/test/fixtures/login.rs b/src/test/fixtures/login.rs
index e308289..86e3e39 100644
--- a/src/test/fixtures/login.rs
+++ b/src/test/fixtures/login.rs
@@ -1,4 +1,4 @@
-use faker_rand::en_us::internet;
+use faker_rand::{en_us::internet, lorem::Paragraphs};
 use uuid::Uuid;
 
 use crate::{
@@ -38,6 +38,10 @@ pub fn propose() -> (Name, Password) {
     (propose_name(), propose_password())
 }
 
+pub fn propose_invalid_name() -> Name {
+    rand::random::<Paragraphs>().to_string().into()
+}
+
 fn propose_name() -> Name {
     rand::random::<internet::Username>().to_string().into()
 }