diff options
| -rw-r--r-- | Cargo.lock | 7 | ||||
| -rw-r--r-- | Cargo.toml | 1 | ||||
| -rw-r--r-- | docs/api/initial-setup.md | 14 | ||||
| -rw-r--r-- | docs/api/invitations.md | 13 | ||||
| -rw-r--r-- | src/invite/app.rs | 8 | ||||
| -rw-r--r-- | src/invite/routes/invite/post.rs | 3 | ||||
| -rw-r--r-- | src/invite/routes/invite/test/post.rs | 32 | ||||
| -rw-r--r-- | src/login/app.rs | 12 | ||||
| -rw-r--r-- | src/login/mod.rs | 1 | ||||
| -rw-r--r-- | src/login/validate.rs | 23 | ||||
| -rw-r--r-- | src/setup/app.rs | 8 | ||||
| -rw-r--r-- | src/setup/routes/post.rs | 3 | ||||
| -rw-r--r-- | src/setup/routes/test.rs | 25 | ||||
| -rw-r--r-- | src/test/fixtures/login.rs | 6 |
14 files changed, 151 insertions, 5 deletions
@@ -828,6 +828,7 @@ dependencies = [ "tokio-stream", "unicode-casefold", "unicode-normalization", + "unicode-segmentation", "unix_path", "uuid", ] @@ -2143,6 +2144,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e70f2a8b45122e719eb623c01822704c4e0907e7e426a05927e1a1cfff5b75d0" [[package]] +name = "unicode-segmentation" +version = "1.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6ccf251212114b54433ec949fd6a7841275f9ada20dddd2f29e9ceea4501493" + +[[package]] name = "unicode_categories" version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -44,6 +44,7 @@ tokio = { version = "1.40.0", features = ["rt", "macros", "rt-multi-thread"] } tokio-stream = { version = "0.1.16", features = ["sync"] } unicode-casefold = "0.2.0" unicode-normalization = "0.1.24" +unicode-segmentation = "1.12.0" unix_path = "1.0.1" uuid = { version = "1.11.0", features = ["v4"] } diff --git a/docs/api/initial-setup.md b/docs/api/initial-setup.md index 306d798..b6bf270 100644 --- a/docs/api/initial-setup.md +++ b/docs/api/initial-setup.md @@ -51,6 +51,16 @@ The request must have the following fields: | `name` | string | The initial login's name. | | `password` | string | The initial login's password, in plain text. | +<!-- Reproduced in invitations.md. Edit in both places. --> + +The proposed `name` must be valid. The precise definition of valid is still up in the air, but, at minimum: + +* It must be non-empty. +* It must not be "too long." (Currently, 64 characters is too long.) +* It must begin with an alphanumeric character. +* It must end with an alphanumeric character. +* It must not contain runs of multiple whitespace characters. + ### Success <!-- This prose is duplicated from authentication.md, with small changes for context. If you edit it here, edit it there, too. --> @@ -79,6 +89,10 @@ The response will include a `Set-Cookie` header for the `identity` cookie, provi The cookie will expire if it is not used regularly. +### Name not valid + +This endpoint will respond with a status of `400 Bad Request` if the proposed `name` is not valid. + ### Setup previously completed Once completed, this operation cannot be performed a second time. Subsequent requests to this endpoint will respond with a status of `409 Conflict`. diff --git a/docs/api/invitations.md b/docs/api/invitations.md index ddbef8a..83e5145 100644 --- a/docs/api/invitations.md +++ b/docs/api/invitations.md @@ -130,6 +130,15 @@ The request must have the following fields: | `name` | string | The new login's name. | | `password` | string | The new login's password, in plain text. | +<!-- Reproduced in initial-setup.md. Edit in both places. --> +The proposed `name` must be valid. The precise definition of valid is still up in the air, but, at minimum: + +* It must be non-empty. +* It must not be "too long." (Currently, 64 characters is too long.) +* It must begin with an alphanumeric character. +* It must end with an alphanumeric character. +* It must not contain runs of multiple whitespace characters. + ### Success <!-- This prose is duplicated from authentication.md, with small changes for context. If you edit it here, edit it there, too. --> @@ -162,6 +171,10 @@ The cookie will expire if it is not used regularly. This endpoint will respond with a status of `404 Not Found` when the invitation ID either does not exist, or has already been accepted. +### Name not valid + +This endpoint will respond with a status of `400 Bad Request` if the proposed `name` is not valid. + ### Name in use This endpoint will respond with a status of `409 Conflict` if the requested login name has already been taken. 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() } |
