forked from jmug/stoatchat
feat: automatically sanitise usernames on create/update (#689)
* feat: automatically sanitise usernames on create/update Signed-off-by: higgs01 <6546697+higgs01@users.noreply.github.com> * test: add tests for validation and sanitasion Signed-off-by: higgs01 <6546697+higgs01@users.noreply.github.com> * fix: Return only the sanitised string in sanitise_username (#424) * fix: Use as_str for role.id in insert_role * Run rustfmt for code changes Signed-off-by: jarvarvarvis <jarvistrigo@gmail.com> * feat: Make minimum username length configurable in Revolt.toml (#424) * fix: Remove redundant call to is_match with blocked username regex Signed-off-by: jarvarvarvis <jarvistrigo@gmail.com> * Update crates/core/database/src/models/users/model.rs Co-authored-by: Tom <iamtomahawkx@gmail.com> Signed-off-by: jarvarvarvis <53998846+jarvarvarvis@users.noreply.github.com> * Update crates/core/database/src/models/users/model.rs Co-authored-by: Tom <iamtomahawkx@gmail.com> Signed-off-by: jarvarvarvis <53998846+jarvarvarvis@users.noreply.github.com> * Update crates/core/database/src/models/users/model.rs Co-authored-by: Tom <iamtomahawkx@gmail.com> Signed-off-by: jarvarvarvis <53998846+jarvarvarvis@users.noreply.github.com> * Update crates/core/database/src/models/users/model.rs Co-authored-by: Tom <iamtomahawkx@gmail.com> Signed-off-by: jarvarvarvis <53998846+jarvarvarvis@users.noreply.github.com> * fix: Implement suggested changes and clean up last 4 commits Signed-off-by: jarvarvarvis <jarvistrigo@gmail.com> * fix: Disallow stoat as username, update create_user test Signed-off-by: jarvarvarvis <jarvistrigo@gmail.com> * fix: Use sanitised username to find updated discriminator Signed-off-by: jarvarvarvis <jarvistrigo@gmail.com> * feat: Sanitise revolt.chat in username Signed-off-by: jarvarvarvis <jarvistrigo@gmail.com> * fix: Implement discussed changes Signed-off-by: jarvarvarvis <jarvistrigo@gmail.com> * test: Fix create_user test Signed-off-by: jarvarvarvis <jarvistrigo@gmail.com> * fix: don't overflow the stack not entirely sure why this fixes it and I don't like it. But work it does. Signed-off-by: IAmTomahawkx <iamtomahawkx@gmail.com> * fix: revert odd file mode change Signed-off-by: jarvarvarvis <jarvistrigo@gmail.com> --------- Signed-off-by: higgs01 <6546697+higgs01@users.noreply.github.com> Signed-off-by: jarvarvarvis <jarvistrigo@gmail.com> Signed-off-by: jarvarvarvis <53998846+jarvarvarvis@users.noreply.github.com> Signed-off-by: IAmTomahawkx <iamtomahawkx@gmail.com> Co-authored-by: higgs01 <6546697+higgs01@users.noreply.github.com> Co-authored-by: Tom <iamtomahawkx@gmail.com>
This commit is contained in:
8
Cargo.lock
generated
8
Cargo.lock
generated
@@ -2113,9 +2113,13 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "decancer"
|
||||
version = "1.6.5"
|
||||
version = "3.3.3"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "080b09f6adad25c23d8c47c54e52e59b0dc09d079c4b23e0f147dac440359d0d"
|
||||
checksum = "a9244323129647178bf41ac861a2cdb9d9c81b9b09d3d0d1de9cd302b33b8a1d"
|
||||
dependencies = [
|
||||
"lazy_static",
|
||||
"regex",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "der"
|
||||
|
||||
@@ -171,7 +171,7 @@ config = "0.13.3"
|
||||
cached = "0.44.0"
|
||||
rand = "0.8.5"
|
||||
base64 = "0.21.3"
|
||||
decancer = "1.6.2"
|
||||
decancer = "3.3.3"
|
||||
linkify = "0.8.1"
|
||||
url-escape = "0.1.1"
|
||||
revolt_optional_struct = "0.2.0"
|
||||
@@ -198,4 +198,4 @@ revolt-parser = { version = "0.12.0", path = "crates/core/parser" }
|
||||
revolt-permissions = { version = "0.12.0", path = "crates/core/permissions" }
|
||||
revolt-presence = { version = "0.12.0", path = "crates/core/presence" }
|
||||
revolt-ratelimits = { version = "0.12.0", path = "crates/core/ratelimits" }
|
||||
revolt-result = { version = "0.12.0", path = "crates/core/result" }
|
||||
revolt-result = { version = "0.12.0", path = "crates/core/result" }
|
||||
|
||||
@@ -78,6 +78,8 @@ call_ring_duration = 30
|
||||
[api.livekit.nodes]
|
||||
|
||||
[api.users]
|
||||
# Minimum allowed length of usernames
|
||||
min_username_length = 2
|
||||
|
||||
[pushd]
|
||||
# this changes the names of the queues to not overlap
|
||||
|
||||
@@ -231,6 +231,7 @@ pub struct LiveKitNode {
|
||||
#[derive(Deserialize, Debug, Clone)]
|
||||
pub struct ApiUsers {
|
||||
pub early_adopter_cutoff: Option<u64>,
|
||||
pub min_username_length: usize,
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug, Clone)]
|
||||
|
||||
@@ -77,7 +77,7 @@ impl AbstractServers for MongoDb {
|
||||
},
|
||||
doc! {
|
||||
"$set": {
|
||||
"roles.".to_owned() + &role.id: to_document(role)
|
||||
"roles.".to_owned() + role.id.as_str(): to_document(role)
|
||||
.map_err(|_| create_database_error!("to_document", "role"))?
|
||||
}
|
||||
},
|
||||
|
||||
@@ -7,6 +7,7 @@ use futures::future::join_all;
|
||||
use iso8601_timestamp::Timestamp;
|
||||
use once_cell::sync::Lazy;
|
||||
use rand::seq::SliceRandom;
|
||||
use regex::{Regex, RegexBuilder};
|
||||
use revolt_config::{config, FeaturesLimits};
|
||||
use revolt_models::v0::{self, UserBadges, UserFlags};
|
||||
use revolt_presence::filter_online;
|
||||
@@ -163,6 +164,13 @@ pub static DISCRIMINATOR_SEARCH_SPACE: Lazy<HashSet<String>> = Lazy::new(|| {
|
||||
set.into_iter().collect()
|
||||
});
|
||||
|
||||
static BLOCKED_USERNAME_PATTERNS: Lazy<Regex> = Lazy::new(|| {
|
||||
RegexBuilder::new("`{3}|(discord|rvlt|guilded|stt)\\.gg|(revolt|stoat)\\.chat|https?:\\/\\/")
|
||||
.case_insensitive(true)
|
||||
.build()
|
||||
.unwrap()
|
||||
});
|
||||
|
||||
#[allow(clippy::derivable_impls)]
|
||||
impl Default for User {
|
||||
fn default() -> Self {
|
||||
@@ -198,11 +206,13 @@ impl User {
|
||||
I: Into<Option<String>>,
|
||||
D: Into<Option<PartialUser>>,
|
||||
{
|
||||
let username = User::validate_username(username)?;
|
||||
let new_username = User::sanitise_username(&username).await?;
|
||||
User::validate_username(&new_username)?;
|
||||
|
||||
let mut user = User {
|
||||
id: account_id.into().unwrap_or_else(|| Ulid::new().to_string()),
|
||||
discriminator: User::find_discriminator(db, &username, None).await?,
|
||||
username,
|
||||
discriminator: User::find_discriminator(db, &new_username, None).await?,
|
||||
username: new_username.clone(),
|
||||
last_acknowledged_policy_change: Timestamp::now_utc(),
|
||||
..Default::default()
|
||||
};
|
||||
@@ -278,39 +288,40 @@ impl User {
|
||||
}
|
||||
}
|
||||
|
||||
/// Sanitise and validate a username can be used
|
||||
pub fn validate_username(username: String) -> Result<String> {
|
||||
// Copy the username for validation
|
||||
/// Validate a username
|
||||
///
|
||||
/// This will check if the username is a blocked name or contains a blocked pattern.
|
||||
fn validate_username(username: &str) -> Result<()> {
|
||||
let username_lowercase = username.to_lowercase();
|
||||
|
||||
// Block homoglyphs
|
||||
if decancer::cure(&username_lowercase).into_str() != username_lowercase {
|
||||
const BLOCKED_USERNAMES: &[&str] = &["admin", "revolt", "stoat"];
|
||||
|
||||
if BLOCKED_USERNAMES.contains(&username_lowercase.as_str())
|
||||
|| BLOCKED_USERNAME_PATTERNS.is_match(username)
|
||||
{
|
||||
return Err(create_error!(InvalidUsername));
|
||||
}
|
||||
|
||||
// Ensure the username itself isn't blocked
|
||||
const BLOCKED_USERNAMES: &[&str] = &["admin", "revolt"];
|
||||
Ok(())
|
||||
}
|
||||
|
||||
for username in BLOCKED_USERNAMES {
|
||||
if username_lowercase == *username {
|
||||
return Err(create_error!(InvalidUsername));
|
||||
}
|
||||
}
|
||||
/// Sanitise a username
|
||||
///
|
||||
/// This will clean up Unicode homoglyphs and pad to the min username length with underscores.
|
||||
async fn sanitise_username(username: &str) -> Result<String> {
|
||||
let options = decancer::Options::default().retain_capitalization();
|
||||
let mut username = decancer::cure(username, options)
|
||||
.map_err(|_| create_error!(InvalidUsername))?
|
||||
.to_string();
|
||||
|
||||
// Ensure none of the following substrings show up in the username
|
||||
const BLOCKED_SUBSTRINGS: &[&str] = &[
|
||||
"```",
|
||||
"discord.gg",
|
||||
"rvlt.gg",
|
||||
"guilded.gg",
|
||||
"https://",
|
||||
"http://",
|
||||
];
|
||||
|
||||
for substr in BLOCKED_SUBSTRINGS {
|
||||
if username_lowercase.contains(substr) {
|
||||
return Err(create_error!(InvalidUsername));
|
||||
}
|
||||
let config = revolt_config::config().await;
|
||||
let username_length_diff = config
|
||||
.api
|
||||
.users
|
||||
.min_username_length
|
||||
.saturating_sub(username.len());
|
||||
if username_length_diff > 0 {
|
||||
username.push_str(&"_".repeat(username_length_diff))
|
||||
}
|
||||
|
||||
Ok(username)
|
||||
@@ -416,12 +427,14 @@ impl User {
|
||||
|
||||
/// Update a user's username
|
||||
pub async fn update_username(&mut self, db: &Database, username: String) -> Result<()> {
|
||||
let username = User::validate_username(username)?;
|
||||
if self.username.to_lowercase() == username.to_lowercase() {
|
||||
let new_username = User::sanitise_username(&username).await?;
|
||||
User::validate_username(&new_username)?;
|
||||
|
||||
if self.username.to_lowercase() == new_username.to_lowercase() {
|
||||
self.update(
|
||||
db,
|
||||
PartialUser {
|
||||
username: Some(username),
|
||||
username: Some(new_username),
|
||||
..Default::default()
|
||||
},
|
||||
vec![],
|
||||
@@ -434,12 +447,12 @@ impl User {
|
||||
discriminator: Some(
|
||||
User::find_discriminator(
|
||||
db,
|
||||
&username,
|
||||
&new_username,
|
||||
Some((self.discriminator.to_string(), self.id.clone())),
|
||||
)
|
||||
.await?,
|
||||
),
|
||||
username: Some(username),
|
||||
username: Some(new_username),
|
||||
..Default::default()
|
||||
},
|
||||
vec![],
|
||||
@@ -825,3 +838,109 @@ impl User {
|
||||
badges
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::User;
|
||||
|
||||
#[test]
|
||||
fn username_validation_blocked_names() {
|
||||
let username_admin = "Admin";
|
||||
let username_revolt = "Revolt";
|
||||
let username_stoat = "Stoat";
|
||||
let username_allowed = "Allowed";
|
||||
|
||||
assert!(User::validate_username(username_admin).is_err());
|
||||
assert!(User::validate_username(username_revolt).is_err());
|
||||
assert!(User::validate_username(username_stoat).is_err());
|
||||
assert!(User::validate_username(username_allowed).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn username_validation_blocked_patterns() {
|
||||
let username_grave = "```_test";
|
||||
let username_discord = "discord.gg_test";
|
||||
let username_rvlt = "rvlt.gg_test";
|
||||
let username_guilded = "guilded.gg_test";
|
||||
let username_stt = "stt.gg_test";
|
||||
let username_revolt = "revolt.chat_test";
|
||||
let username_stoat = "stoat.chat_test";
|
||||
let username_http = "http://_test";
|
||||
let username_https = "https://_test";
|
||||
|
||||
assert!(User::validate_username(username_grave).is_err());
|
||||
assert!(User::validate_username(username_discord).is_err());
|
||||
assert!(User::validate_username(username_rvlt).is_err());
|
||||
assert!(User::validate_username(username_guilded).is_err());
|
||||
assert!(User::validate_username(username_stt).is_err());
|
||||
assert!(User::validate_username(username_revolt).is_err());
|
||||
assert!(User::validate_username(username_stoat).is_err());
|
||||
assert!(User::validate_username(username_http).is_err());
|
||||
assert!(User::validate_username(username_https).is_err());
|
||||
}
|
||||
|
||||
#[async_std::test]
|
||||
async fn username_sanitisation_clean() {
|
||||
let username_clean = "Test";
|
||||
|
||||
let username_clean_sanitised = User::sanitise_username(username_clean).await;
|
||||
|
||||
assert!(username_clean_sanitised.is_ok());
|
||||
assert_eq!(username_clean, username_clean_sanitised.unwrap());
|
||||
}
|
||||
|
||||
#[async_std::test]
|
||||
async fn username_sanitisation_homoglyphs() {
|
||||
let username_homoglyphs = "𝔽𝕌Ňℕy";
|
||||
|
||||
let username_homoglyphs_sanitised =
|
||||
User::sanitise_username(username_homoglyphs).await.unwrap();
|
||||
|
||||
assert_ne!(username_homoglyphs, username_homoglyphs_sanitised);
|
||||
assert_eq!("funny", username_homoglyphs_sanitised);
|
||||
}
|
||||
|
||||
#[async_std::test]
|
||||
async fn username_sanitisation_padding() {
|
||||
let username_padding = "a";
|
||||
|
||||
let username = User::sanitise_username(username_padding).await.unwrap();
|
||||
|
||||
assert_eq!("a_", username);
|
||||
}
|
||||
|
||||
#[async_std::test]
|
||||
async fn create_user() {
|
||||
use revolt_result::Result;
|
||||
|
||||
database_test!(|db| async move {
|
||||
let mut created_clean = User::create(&db, "Test".to_string(), None, None)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!("Test", created_clean.username);
|
||||
|
||||
created_clean
|
||||
.update_username(&db, "Test2".to_string())
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!("Test2", created_clean.username);
|
||||
|
||||
let created_invalid_result: Result<_> =
|
||||
User::create(&db, "stoat.chat".to_string(), None, None).await;
|
||||
|
||||
assert!(created_invalid_result.is_err());
|
||||
|
||||
let mut updated_invalid = User::create(&db, "Test".to_string(), None, None)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let updated_invalid_update_result = updated_invalid
|
||||
.update_username(&db, "http://test".to_string())
|
||||
.await;
|
||||
|
||||
assert!(updated_invalid_update_result.is_err());
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user