Skip to content

Commit

Permalink
Merge torrust#504: Add more contrains for usernames
Browse files Browse the repository at this point in the history
553208e feat: [torrust#503] add more contrains for usernames (Jose Celano)

Pull request description:

  Usernames will only allow the following characters:

  - Uppercase and lowercase letters (`A-Z`, `a-z`)
  - Digits (`0-9`)
  - The simple dash (`-`)
  - The underscore (`_`)

  Additionally, we'll enforce a maximum length of 20 characters for the usernames.

  We exclude emojis, blank spaces, non-valid UTF-8 characters, and non-ASCII characters

  ```regex
  ^[A-Za-z0-9-_]{1,20}$
  ```

  Explanation:

  - `^` asserts the start of the string.
  - `[A-Za-z0-9-]` is a character class that matches uppercase letters (`A-Z`), lowercase letters (`a-z`), digits (`0-9`), and the simple dash (`-`) or underscore (`_`).
  - `{1,20}` quantifier makes sure that the preceding character class matches between 1 and 20 times, inclusive, which enforces your maximum length requirement.
  - `$` asserts the end of the string.

  This regular expression ensures that usernames consist only of the specified characters and do not exceed 20 characters in length. It effectively excludes emojis, blank spaces, non-valid UTF-8 characters, and non-ASCII characters, as they are not included in the specified character set.

  The API endpoint `/register` for registration should return a Bad Request when the username does to match this regexp.

ACKs for top commit:
  josecelano:
    ACK 553208e

Tree-SHA512: b746ac9cac6a3268cd443331682fc54a89ccc185231ca24b0a2d9ab7bd9380246d68cc69154eb9920e345dbb971a5a8c8359499c2a3e48e5bba1f6635ce483b9
  • Loading branch information
josecelano committed Feb 28, 2024
2 parents 0fc5518 + 553208e commit eb87363
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub enum ServiceError {
#[display(fmt = "Username not available")]
UsernameTaken,

#[display(fmt = "Username contains illegal characters")]
#[display(fmt = "Invalid username. Usernames must consist of 1-20 alphanumeric characters, dashes, or underscore")]
UsernameInvalid,

/// email is already taken
Expand Down
83 changes: 83 additions & 0 deletions src/models/user.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
use std::fmt;
use std::str::FromStr;

use regex::Regex;
use serde::{Deserialize, Serialize};

#[allow(clippy::module_name_repetitions)]
Expand Down Expand Up @@ -57,3 +61,82 @@ pub struct UserClaims {
pub user: UserCompact,
pub exp: u64, // epoch in seconds
}

const MAX_USERNAME_LENGTH: usize = 20;
const USERNAME_VALIDATION_ERROR_MSG: &str = "Usernames must consist of 1-20 alphanumeric characters, dashes, or underscore";

#[derive(Debug, Clone)]
pub struct UsernameParseError {
message: String,
}

// Implement std::fmt::Display for UsernameParseError
impl fmt::Display for UsernameParseError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "UsernameParseError: {}", self.message)
}
}

// Implement std::error::Error for UsernameParseError
impl std::error::Error for UsernameParseError {}

pub struct Username(String);

impl fmt::Display for Username {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.0)
}
}

// Implement the parsing logic
impl FromStr for Username {
type Err = UsernameParseError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
if s.len() > MAX_USERNAME_LENGTH {
return Err(UsernameParseError {
message: format!("username '{s}' is too long. {USERNAME_VALIDATION_ERROR_MSG}."),
});
}

let pattern = format!(r"^[A-Za-z0-9-_]{{1,{MAX_USERNAME_LENGTH}}}$");
let re = Regex::new(&pattern).expect("username regexp should be valid");

if re.is_match(s) {
Ok(Username(s.to_string()))
} else {
Err(UsernameParseError {
message: format!("'{s}' is not a valid username. {USERNAME_VALIDATION_ERROR_MSG}."),
})
}
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn username_must_consist_of_1_to_20_alphanumeric_characters_or_dashes() {
let username_str = "validUsername123";
assert!(username_str.parse::<Username>().is_ok());
}

#[test]
fn username_should_be_shorter_then_21_chars() {
let username_str = "a".repeat(MAX_USERNAME_LENGTH + 1);
assert!(username_str.parse::<Username>().is_err());
}

#[test]
fn username_should_not_allow_invalid_characters() {
let username_str = "invalid*Username";
assert!(username_str.parse::<Username>().is_err());
}

#[test]
fn username_should_be_displayed() {
let username = Username("FirstLast-01".to_string());
assert_eq!(username.to_string(), "FirstLast-01");
}
}
12 changes: 6 additions & 6 deletions src/services/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::databases::database::{Database, Error};
use crate::errors::ServiceError;
use crate::mailer;
use crate::mailer::VerifyClaims;
use crate::models::user::{UserCompact, UserId, UserProfile};
use crate::models::user::{UserCompact, UserId, UserProfile, Username};
use crate::utils::validation::validate_email_address;
use crate::web::api::server::v1::contexts::user::forms::RegistrationForm;

Expand Down Expand Up @@ -68,6 +68,10 @@ impl RegistrationService {
pub async fn register_user(&self, registration_form: &RegistrationForm, api_base_url: &str) -> Result<UserId, ServiceError> {
info!("registering user: {}", registration_form.username);

let Ok(username) = registration_form.username.parse::<Username>() else {
return Err(ServiceError::UsernameInvalid);
};

let settings = self.configuration.settings.read().await;

let opt_email = match settings.auth.email_on_signup {
Expand Down Expand Up @@ -111,14 +115,10 @@ impl RegistrationService {
.hash_password(registration_form.password.as_bytes(), &salt)?
.to_string();

if registration_form.username.contains('@') {
return Err(ServiceError::UsernameInvalid);
}

let user_id = self
.user_repository
.add(
&registration_form.username,
&username.to_string(),
&opt_email.clone().unwrap_or(no_email()),
&password_hash,
)
Expand Down

0 comments on commit eb87363

Please sign in to comment.