Skip to content

Commit 6126eec

Browse files
committed
fix(auth): harden token parsing and remove panics in ban logic
- Parse Bearer scheme case-insensitively per RFC 7235 §2.1 - Replace `parse_from_str` + `expect` with a `LazyLock` constant for the permanent-ban expiry date, removing two panic sites - Propagate `is_user_banned` errors with `?` instead of swallowing them via `unwrap_or(false)` - Use `saturating_sub` for token-expiry arithmetic to avoid underflow
1 parent 593276a commit 6126eec

4 files changed

Lines changed: 23 additions & 23 deletions

File tree

src/jwt.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ impl JsonWebToken {
271271
return Err(AuthError::TokenRevoked);
272272
}
273273

274-
if db.is_user_banned(claims.sub).await.unwrap_or(false) {
274+
if db.is_user_banned(claims.sub).await? {
275275
return Err(AuthError::TokenRevoked);
276276
}
277277

src/services/authentication.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ impl Service {
135135
})?;
136136

137137
// Renew token if it is valid for less than one week
138-
let token = match claims.exp - clock::now() {
138+
let token = match claims.exp.saturating_sub(clock::now()) {
139139
x if x < ONE_WEEK_IN_SECONDS => self.json_web_token.sign(user_compact.clone(), claims.token_gen).await?,
140140
_ => token.to_string(),
141141
};

src/services/user.rs

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
//! User services.
22
use std::str::FromStr;
3-
use std::sync::Arc;
3+
use std::sync::{Arc, LazyLock};
44

55
use argon2::password_hash::SaltString;
66
use argon2::{Argon2, PasswordHasher};
77
use async_trait::async_trait;
8+
use chrono::NaiveDate;
89
#[cfg(test)]
910
use mockall::automock;
1011
use pbkdf2::password_hash::rand_core::OsRng;
@@ -24,6 +25,14 @@ use crate::utils::validation::validate_email_address;
2425
use crate::web::api::server::v1::contexts::user::forms::{ChangePasswordForm, RegistrationForm};
2526
use crate::{AsCSV, mailer};
2627

28+
/// Permanent ban expiry date (year 9999).
29+
static PERMANENT_BAN_EXPIRY: LazyLock<chrono::NaiveDateTime> = LazyLock::new(|| {
30+
NaiveDate::from_ymd_opt(9999, 1, 1)
31+
.expect("valid date")
32+
.and_hms_opt(0, 0, 0)
33+
.expect("valid time")
34+
});
35+
2736
/// Since user email could be optional, we need a way to represent "no email"
2837
/// in the database. This function returns the string that should be used for
2938
/// that purpose.
@@ -551,11 +560,6 @@ impl DbBannedUserList {
551560
/// # Errors
552561
///
553562
/// It returns an error if there is a database error.
554-
///
555-
/// # Panics
556-
///
557-
/// It panics if the expiration date cannot be parsed. It should never
558-
/// happen as the date is hardcoded for now.
559563
pub async fn add(&self, user_id: &UserId) -> Result<(), Error> {
560564
// todo: add reason and `date_expiry` parameters to request.
561565

@@ -564,11 +568,7 @@ impl DbBannedUserList {
564568
// For the time being, we will not use a reason for banning a user.
565569
let reason = "no reason".to_string();
566570

567-
// User will be banned until the year 9999
568-
let date_expiry = chrono::NaiveDateTime::parse_from_str("9999-01-01 00:00:00", "%Y-%m-%d %H:%M:%S")
569-
.expect("Could not parse date from 9999-01-01 00:00:00.");
570-
571-
self.database.ban_user(*user_id, &reason, date_expiry).await
571+
self.database.ban_user(*user_id, &reason, *PERMANENT_BAN_EXPIRY).await
572572
}
573573

574574
/// Ban a user and atomically increment `token_generation`.
@@ -577,17 +577,12 @@ impl DbBannedUserList {
577577
/// # Errors
578578
///
579579
/// It returns an error if there is a database error.
580-
///
581-
/// # Panics
582-
///
583-
/// It panics if the expiration date cannot be parsed.
584580
pub async fn add_and_revoke_tokens(&self, user_id: &UserId) -> Result<(), Error> {
585581
let reason = "no reason".to_string();
586582

587-
let date_expiry = chrono::NaiveDateTime::parse_from_str("9999-01-01 00:00:00", "%Y-%m-%d %H:%M:%S")
588-
.expect("Could not parse date from 9999-01-01 00:00:00.");
589-
590-
self.database.ban_user_and_revoke_tokens(*user_id, &reason, date_expiry).await
583+
self.database
584+
.ban_user_and_revoke_tokens(*user_id, &reason, *PERMANENT_BAN_EXPIRY)
585+
.await
591586
}
592587

593588
/// Increment the user's `token_generation` counter, invalidating all

src/web/api/server/v1/auth.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,16 @@ impl Authentication {
167167
/// # Errors
168168
///
169169
/// Returns `AuthError::TokenInvalid` if the header value is not valid
170-
/// ASCII or does not contain a `Bearer <token>` pair.
170+
/// ASCII or does not contain a `Bearer <token>` pair. The scheme name
171+
/// is matched case-insensitively per RFC 7235 §2.1.
171172
pub fn parse_token(authorization: &HeaderValue) -> Result<String, AuthError> {
172173
let header_str = authorization.to_str().map_err(|_| AuthError::TokenInvalid)?;
173174

174-
let token = header_str.strip_prefix("Bearer ").ok_or(AuthError::TokenInvalid)?.trim();
175+
let token = header_str
176+
.get(7..)
177+
.filter(|_| header_str[..7].eq_ignore_ascii_case("bearer "))
178+
.ok_or(AuthError::TokenInvalid)?
179+
.trim();
175180

176181
if token.is_empty() {
177182
return Err(AuthError::TokenInvalid);

0 commit comments

Comments
 (0)