From 9c966ea7f13ac9a838e882a2a319b134389a65a5 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Sat, 19 Jul 2025 15:03:38 +0100 Subject: [PATCH] Use strongly-validated Username type for creating users --- Cargo.lock | 1 + Cargo.toml | 26 ++++++++++++++++------ src/cli.rs | 5 ++--- src/store.rs | 51 +++++++++++++++++++++++++++++++++++++++++-- src/tests/test_cli.rs | 6 ++--- 5 files changed, 74 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c256cb4..35d6d3a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1892,6 +1892,7 @@ dependencies = [ "sha2", "sqlx", "subtle", + "thiserror 2.0.12", "time", "tokio", "tower-cookies", diff --git a/Cargo.toml b/Cargo.toml index 5381659..7df04a6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,7 @@ repository = "https://git.emunest.net/reivilibre/idcoop" argon2 = "0.5.2" async-trait = "0.1.74" axum = { version = "0.8.4", features = ["tracing", "macros"] } -axum-extra = { version = "0.10.1", features = ["typed-header"] } +axum-extra = { version = "0.10.1", features = ["typed-header"] } axum-client-ip = "0.7.0" base64 = "0.21.5" blake2 = "0.10.6" @@ -24,7 +24,9 @@ confique = { version = "0.2.4", features = ["toml"], default-features = false } eyre = "0.6.8" futures = "0.3.29" governor = "0.6.0" -hornbeam = { version = "0.0.4", path = "../hornbeam/hornbeam", features = ["formbeam"] } +hornbeam = { version = "0.0.4", path = "../hornbeam/hornbeam", features = [ + "formbeam", +] } formbeam = { version = "0.0.4", path = "../hornbeam/formbeam" } formbeam_derive = { version = "0.0.4", path = "../hornbeam/formbeam_derive" } bevy_reflect = { version = "0.14.0" } @@ -38,12 +40,25 @@ serde = { version = "1.0.188", features = ["derive"] } serde_json = "1.0.108" serde_urlencoded = "0.7.1" sha2 = "0.10.8" -sqlx = { version = "0.7.2", features = ["postgres", "runtime-tokio-native-tls", "macros", "migrate", "uuid", "chrono"] } +sqlx = { version = "0.7.2", features = [ + "postgres", + "runtime-tokio-native-tls", + "macros", + "migrate", + "uuid", + "chrono", +] } subtle = "2.5.0" time = "0.3.30" +thiserror = "2.0.12" tokio = { version = "1.33.0", features = ["rt", "macros"] } tower-cookies = "0.11.0" -tower-http = { version = "0.6.4", features = ["trace", "cors", "set-header", "fs"] } +tower-http = { version = "0.6.4", features = [ + "trace", + "cors", + "set-header", + "fs", +] } tracing = "0.1.37" tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } uuid = { version = "1.16.0", features = ["serde"] } @@ -59,9 +74,6 @@ rstest = "0.21.0" regex = "1.11.1" - - - # Enable optimisations for some perf-sensitive libraries # even in dev mode [profile.dev.package.argon2] diff --git a/src/cli.rs b/src/cli.rs index 5fadefa..3733aea 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -5,7 +5,7 @@ use std::io::stdin; use crate::config::Configuration; use crate::passwords::create_password_hash; -use crate::store::{CreateRole, CreateUser, IdCoopStore, IdCoopStoreTxn}; +use crate::store::{CreateRole, CreateUser, IdCoopStore, IdCoopStoreTxn, Username}; use clap::Parser; use comfy_table::presets::UTF8_FULL; use comfy_table::{Attribute, Cell, Color, ContentArrangement, Row, Table}; @@ -19,8 +19,7 @@ pub enum UserCommand { #[clap(alias = "new", alias = "create")] Add { /// The login name of the user. - // TODO this should be a richer newtype with validation - username: String, + username: Username, /// Set this flag if the user should be locked. #[clap(long = "locked")] diff --git a/src/store.rs b/src/store.rs index 0e02f40..e6a0327 100644 --- a/src/store.rs +++ b/src/store.rs @@ -3,6 +3,8 @@ //! This file contains PostgreSQL queries. use std::collections::BTreeSet; +use std::ops::Deref; +use std::str::FromStr; use bevy_reflect::Reflect; use chrono::DateTime; @@ -13,6 +15,7 @@ use eyre::eyre; use eyre::Context; use futures::future::BoxFuture; use sqlx::{types::Uuid, Connection, PgPool, Postgres, Transaction}; +use thiserror::Error; use crate::web::login::{ LoginSession, LOGIN_SESSION_TOKEN_HASH_BYTES, LOGIN_SESSION_XSRF_SECRET_BYTES, @@ -22,6 +25,50 @@ use crate::web::oauth_openid::application_session_access_token::ApplicationSessi /// Prefix for roles that are reserved by idCoop usage. pub const IDCOOP_RESERVED_ROLE_PREFIX: &str = "idcoop/"; +/// Username grammar, chosen to be fairly interoperable hopefully: +/// 1. at least 3 chars +/// 2. no more than 36 chars +/// 3. all ASCII alphanumeric and lowercase +/// 4. must start with an ASCII letter +pub fn is_valid_username(user_id: &str) -> bool { + user_id.len() >= 3 + && user_id.len() <= 36 + && user_id + .chars() + .all(|c| c.is_ascii_alphanumeric() && !c.is_ascii_uppercase()) + && user_id.chars().next().unwrap().is_ascii_alphabetic() +} + +#[derive(Clone, Debug, Error)] +#[error("invalid username: {0}")] +/// An invalid username error. +pub struct InvalidUsername(String); + +/// A validated username +#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)] +pub struct Username(String); + +impl FromStr for Username { + type Err = InvalidUsername; + + fn from_str(s: &str) -> Result { + let s = s.to_owned(); + if is_valid_username(&s) { + Ok(Username(s)) + } else { + Err(InvalidUsername(s)) + } + } +} + +impl Deref for Username { + type Target = str; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + /// Checks if the given role ID is valid. /// /// Does NOT check whether the role ID is reserved for use by idCoop or not! @@ -110,7 +157,7 @@ pub struct User { /// Representation of the action of creating a user. pub struct CreateUser { /// The system name for the user. - pub user_login_name: String, + pub user_login_name: Username, /// The password hash of the user. See [`crate::passwords`]. pub password_hash: Option, /// Whether the user is locked and is therefore not allowed to log in. @@ -276,7 +323,7 @@ impl IdCoopStoreTxn<'_, '_> { pub async fn create_user(&mut self, cu: CreateUser) -> eyre::Result { let r = sqlx::query!( "INSERT INTO users (user_name, user_id, created_at_utc, password_hash, locked) VALUES ($1, gen_random_uuid(), NOW(), $2, $3) RETURNING user_id", - &cu.user_login_name, cu.password_hash.as_ref(), cu.locked + &*cu.user_login_name, cu.password_hash.as_ref(), cu.locked ) .fetch_one(&mut **self.txn) .await.context("failed to create user in DB")?; diff --git a/src/tests/test_cli.rs b/src/tests/test_cli.rs index 1413558..9861044 100644 --- a/src/tests/test_cli.rs +++ b/src/tests/test_cli.rs @@ -11,7 +11,7 @@ async fn test_cli_add_user() { handle_user_command( UserCommand::Add { - username: "jonathan".to_owned(), + username: "jonathan".parse().unwrap(), locked: true, }, &sys.config, @@ -41,7 +41,7 @@ async fn test_cli_lock_and_unlock_user() { handle_user_command( UserCommand::Add { - username: "jonathan".to_owned(), + username: "jonathan".parse().unwrap(), locked: false, }, &sys.config, @@ -117,7 +117,7 @@ async fn test_cli_del_user() { handle_user_command( UserCommand::Add { - username: "jonathan".to_owned(), + username: "jonathan".parse().unwrap(), locked: true, }, &sys.config,