fix: security hardening and code health improvements in domain layer

- Remove danger-allow-state-serialisation feature from webauthn-rs
- Add #[serde(skip_serializing)] to Session and RegistrationToken hash fields
- Add custom Debug impls to redact hashes in Session and NewToken
- Add MAX_SESSION_DURATION (30d) and MAX_TOKEN_DURATION (7d) with clamping
- Add domain-level username validation (length + character constraints)
- Extract shared normalize_optional_field to coffee/mod.rs (DRY)
- Implement FromStr for QuickNote, delegate from_str_value to it
- Refactor UpdateRoaster/UpdateCafe normalize() to use shared helper
This commit is contained in:
Jon Seager 2026-02-13 14:04:50 +00:00
parent 7330e5b59e
commit c58c60c783
No known key found for this signature in database
10 changed files with 104 additions and 60 deletions

View file

@ -38,7 +38,7 @@ tower-http = { version = "0.6", features = ["compression-gzip", "limit", "set-he
slug = "0.1.6"
url = "2"
uuid = { version = "1", features = ["v4"] }
webauthn-rs = { version = "0.5", features = ["danger-allow-state-serialisation"] }
webauthn-rs = "0.5"
webauthn-rs-proto = "0.5"
kamadak-exif = "0.6.1"

View file

@ -87,6 +87,10 @@ pub(crate) async fn register_start(
// Create the user
let user_uuid = Uuid::new_v4().to_string();
let new_user = NewUser::new(payload.display_name, user_uuid.clone());
if let Err(msg) = new_user.validate() {
warn!(error = msg, "invalid username during registration");
return Err(StatusCode::BAD_REQUEST);
}
let user = state.user_repo.insert(new_user).await.map_err(|err| {
error!(error = %err, "failed to create user during registration");
StatusCode::INTERNAL_SERVER_ERROR

View file

@ -1,11 +1,16 @@
use chrono::{DateTime, Utc};
use chrono::{DateTime, Duration, Utc};
use serde::{Deserialize, Serialize};
use crate::domain::ids::{RegistrationTokenId, UserId};
/// Maximum registration token lifetime (7 days). Tokens with `expires_at`
/// beyond this are clamped to `created_at + MAX_TOKEN_DURATION`.
pub const MAX_TOKEN_DURATION: Duration = Duration::days(7);
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct RegistrationToken {
pub id: RegistrationTokenId,
#[serde(skip_serializing)]
pub token_hash: String,
pub created_at: DateTime<Utc>,
pub expires_at: DateTime<Utc>,
@ -36,10 +41,11 @@ pub struct NewRegistrationToken {
impl NewRegistrationToken {
pub fn new(token_hash: String, created_at: DateTime<Utc>, expires_at: DateTime<Utc>) -> Self {
let max_expires = created_at + MAX_TOKEN_DURATION;
Self {
token_hash,
created_at,
expires_at,
expires_at: expires_at.min(max_expires),
}
}
}

View file

@ -1,17 +1,34 @@
use chrono::{DateTime, Utc};
use chrono::{DateTime, Duration, Utc};
use serde::{Deserialize, Serialize};
use crate::domain::ids::{SessionId, UserId};
#[derive(Debug, Clone, Serialize, Deserialize)]
/// Maximum session duration (30 days). Sessions with `expires_at` beyond this
/// are clamped to `created_at + MAX_SESSION_DURATION`.
pub const MAX_SESSION_DURATION: Duration = Duration::days(30);
#[derive(Clone, Serialize, Deserialize)]
pub struct Session {
pub id: SessionId,
pub user_id: UserId,
#[serde(skip_serializing)]
pub session_token_hash: String,
pub created_at: DateTime<Utc>,
pub expires_at: DateTime<Utc>,
}
impl std::fmt::Debug for Session {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("Session")
.field("id", &self.id)
.field("user_id", &self.user_id)
.field("session_token_hash", &"<redacted>")
.field("created_at", &self.created_at)
.field("expires_at", &self.expires_at)
.finish()
}
}
impl Session {
pub fn new(
id: SessionId,
@ -49,11 +66,12 @@ impl NewSession {
created_at: DateTime<Utc>,
expires_at: DateTime<Utc>,
) -> Self {
let max_expires = created_at + MAX_SESSION_DURATION;
Self {
user_id,
session_token_hash,
created_at,
expires_at,
expires_at: expires_at.min(max_expires),
}
}
}

View file

@ -15,13 +15,23 @@ pub struct Token {
pub revoked_at: Option<DateTime<Utc>>,
}
#[derive(Debug, Clone)]
#[derive(Clone)]
pub struct NewToken {
pub user_id: UserId,
pub token_hash: String,
pub name: String,
}
impl std::fmt::Debug for NewToken {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("NewToken")
.field("user_id", &self.user_id)
.field("token_hash", &"<redacted>")
.field("name", &self.name)
.finish()
}
}
impl Token {
pub fn new(
id: TokenId,

View file

@ -3,6 +3,18 @@ use serde::{Deserialize, Serialize};
use crate::domain::ids::UserId;
pub const MIN_USERNAME_LEN: usize = 3;
pub const MAX_USERNAME_LEN: usize = 32;
/// Returns `true` if the username contains only allowed characters
/// (alphanumeric, underscore, hyphen) and is within length bounds.
pub fn is_valid_username(s: &str) -> bool {
let len = s.len();
(MIN_USERNAME_LEN..=MAX_USERNAME_LEN).contains(&len)
&& s.chars()
.all(|c| c.is_alphanumeric() || c == '_' || c == '-')
}
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct User {
pub id: UserId,
@ -32,4 +44,14 @@ impl NewUser {
pub fn new(username: String, uuid: String) -> Self {
Self { username, uuid }
}
/// Validates that the username meets length and character constraints.
pub fn validate(&self) -> Result<(), &'static str> {
if !is_valid_username(&self.username) {
return Err(
"Username must be 3-32 characters and contain only alphanumeric, underscore, or hyphen characters",
);
}
Ok(())
}
}

View file

@ -1,3 +1,5 @@
use std::str::FromStr;
use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize};
@ -39,15 +41,7 @@ impl QuickNote {
}
pub fn from_str_value(s: &str) -> Option<Self> {
match s {
"good" | "Good" => Some(Self::Good),
"too-fast" | "Too Fast" => Some(Self::TooFast),
"too-slow" | "Too Slow" => Some(Self::TooSlow),
"too-hot" | "Too Hot" => Some(Self::TooHot),
"under-extracted" | "Under Extracted" => Some(Self::UnderExtracted),
"over-extracted" | "Over Extracted" => Some(Self::OverExtracted),
_ => None,
}
s.parse().ok()
}
pub fn is_positive(self) -> bool {
@ -66,6 +60,22 @@ impl QuickNote {
}
}
impl FromStr for QuickNote {
type Err = ();
fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"good" | "Good" => Ok(Self::Good),
"too-fast" | "Too Fast" => Ok(Self::TooFast),
"too-slow" | "Too Slow" => Ok(Self::TooSlow),
"too-hot" | "Too Hot" => Ok(Self::TooHot),
"under-extracted" | "Under Extracted" => Ok(Self::UnderExtracted),
"over-extracted" | "Over Extracted" => Ok(Self::OverExtracted),
_ => Err(()),
}
}
}
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Brew {
pub id: BrewId,

View file

@ -1,6 +1,7 @@
use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize};
use super::normalize_optional_field;
use crate::domain::ids::CafeId;
use crate::domain::listing::{SortDirection, SortKey};
use crate::domain::roasters::is_valid_url_scheme;
@ -73,17 +74,6 @@ impl NewCafe {
}
}
fn normalize_optional_field(value: Option<String>) -> Option<String> {
value.and_then(|raw| {
let trimmed = raw.trim();
if trimmed.is_empty() {
None
} else {
Some(trimmed.to_string())
}
})
}
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
pub struct UpdateCafe {
pub name: Option<String>,
@ -98,17 +88,8 @@ pub struct UpdateCafe {
impl UpdateCafe {
pub fn normalize(mut self) -> Self {
self.website = self
.website
.and_then(|w| {
let trimmed = w.trim().to_string();
if trimmed.is_empty() {
None
} else {
Some(trimmed)
}
})
.filter(|url| is_valid_url_scheme(url));
self.website =
normalize_optional_field(self.website).filter(|url| is_valid_url_scheme(url));
self
}
}

View file

@ -5,3 +5,15 @@ pub mod cups;
pub mod gear;
pub mod roasters;
pub mod roasts;
/// Trims an optional string field, converting empty/whitespace-only values to `None`.
pub(crate) fn normalize_optional_field(value: Option<String>) -> Option<String> {
value.and_then(|raw| {
let trimmed = raw.trim();
if trimmed.is_empty() {
None
} else {
Some(trimmed.to_string())
}
})
}

View file

@ -1,6 +1,7 @@
use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize};
use super::normalize_optional_field;
use crate::domain::ids::RoasterId;
use crate::domain::listing::{SortDirection, SortKey};
use crate::domain::timeline::{NewTimelineEvent, TimelineEventDetail};
@ -45,17 +46,6 @@ impl NewRoaster {
}
}
fn normalize_optional_field(value: Option<String>) -> Option<String> {
value.and_then(|raw| {
let trimmed = raw.trim();
if trimmed.is_empty() {
None
} else {
Some(trimmed.to_string())
}
})
}
/// Returns `true` if the URL starts with `http://` or `https://`.
/// Rejects `javascript:`, `data:`, and other potentially dangerous schemes.
pub(crate) fn is_valid_url_scheme(url: &str) -> bool {
@ -102,17 +92,8 @@ pub struct UpdateRoaster {
impl UpdateRoaster {
pub fn normalize(mut self) -> Self {
self.homepage = self
.homepage
.and_then(|h| {
let trimmed = h.trim().to_string();
if trimmed.is_empty() {
None
} else {
Some(trimmed)
}
})
.filter(|url| is_valid_url_scheme(url));
self.homepage =
normalize_optional_field(self.homepage).filter(|url| is_valid_url_scheme(url));
self
}
}