feat: automatically sanitise usernames on create/update

crates/core/database/Cargo.toml

@@ -47,7 +47,7 @@ nanoid = "0.4.0"
 base64 = "0.21.3"
 once_cell = "1.17"
 indexmap = "1.9.1"
-decancer = "1.6.2"
+decancer = "3.3.3"
 deadqueue = "0.2.4"
 linkify = { optional = true, version = "0.8.1" }
 url-escape = { optional = true, version = "0.1.1" }

crates/core/database/src/models/users/model.rs

@@ -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,12 @@ 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)\\.gg|https?:\\/\\/)")
+        .case_insensitive(true)
+        .build().unwrap()
+});
+
 #[allow(clippy::derivable_impls)]
 impl Default for User {
     fn default() -> Self {
@@ -198,11 +205,15 @@ impl User {
         I: Into<Option<String>>,
         D: Into<Option<PartialUser>>,
     {
-        let username = User::validate_username(username)?;
+        let original_username = username;
+        let original_username_str = original_username.as_str();
+        User::validate_username(original_username_str)?;
+        let (is_username_sanitised,username) = User::sanitise_username(original_username_str);
         let mut user = User {
             id: account_id.into().unwrap_or_else(|| Ulid::new().to_string()),
             discriminator: User::find_discriminator(db, &username, None).await?,
             username,
+            display_name: if is_username_sanitised { Some(original_username) } else { Default::default() },
             last_acknowledged_policy_change: Timestamp::now_utc(),
             ..Default::default()
         };
@@ -278,16 +289,10 @@ impl User {
         }
     }
 
-    /// Sanitise and validate a username can be used
-    pub fn validate_username(username: String) -> Result<String> {
+    /// validate a username for blocked names
+    fn validate_username(username: &str) -> Result<()> {
         // Copy the username for validation
         let username_lowercase = username.to_lowercase();
-
-        // Block homoglyphs
-        if decancer::cure(&username_lowercase).into_str() != username_lowercase {
-            return Err(create_error!(InvalidUsername));
-        }
-
         // Ensure the username itself isn't blocked
         const BLOCKED_USERNAMES: &[&str] = &["admin", "revolt"];
 
@@ -297,23 +302,28 @@ impl User {
             }
         }
 
-        // 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));
-            }
+        Ok(())
+    }
+
+    /// sanitise username
+    fn sanitise_username(username: &str) -> (bool, String) {
+        let original_username = username;
+        // sanitise homoglyphs
+        let options = decancer::Options::default()
+            .retain_capitalization();
+        let mut username = decancer::cure(&username, options).unwrap().to_string();
+
+        if BLOCKED_USERNAME_PATTERNS.is_match(&username) {
+            username = BLOCKED_USERNAME_PATTERNS.replace_all(&mut username, "").into_owned();
         }
 
-        Ok(username)
+        const USERNAME_MIN_LEN: usize = 2;
+        let username_length_diff = USERNAME_MIN_LEN.saturating_sub(username.len());
+        if username_length_diff > 0 {
+            username.push_str(&"_".repeat(username_length_diff))
+        }
+
+        (original_username != username, username)
     }
 
     /// Find a user and session ID from a given token and hint
@@ -416,12 +426,24 @@ 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 original_username = username;
+        let original_username_str = original_username.as_str();
+        User::validate_username(original_username_str)?;
+        let (is_username_sanitised, username) = User::sanitise_username(&original_username);
+        if is_username_sanitised {
             self.update(
                 db,
                 PartialUser {
+                    discriminator: Some(
+                        User::find_discriminator(
+                            db,
+                            &username,
+                            Some((self.discriminator.to_string(), self.id.clone())),
+                        )
+                            .await?,
+                    ),
                     username: Some(username),
+                    display_name: if is_username_sanitised { Some(original_username) } else { Default::default() },
                     ..Default::default()
                 },
                 vec![],
@@ -431,14 +453,6 @@ impl User {
             self.update(
                 db,
                 PartialUser {
-                    discriminator: Some(
-                        User::find_discriminator(
-                            db,
-                            &username,
-                            Some((self.discriminator.to_string(), self.id.clone())),
-                        )
-                        .await?,
-                    ),
                     username: Some(username),
                     ..Default::default()
                 },
@@ -825,3 +839,111 @@ impl User {
         badges
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::{User};
+
+    #[test]
+    fn username_validation() {
+        let username_admin = "Admin";
+        let username_revolt = "Revolt";
+        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_allowed).is_ok());
+    }
+
+    #[test]
+    fn username_sanitisation_clean() {
+        let username_clean = "Test";
+
+        let (is_sanitised, username_clean_sanitised) = User::sanitise_username(username_clean);
+
+        assert!(!is_sanitised);
+        assert_eq!(username_clean, username_clean_sanitised)
+    }
+
+    #[test]
+    fn username_sanitisation_hemoglyphs() {
+        let username_hemoglyphs = "𝔽𝕌Ňℕｙ";
+
+        let (is_hemoglyphs_sanitised, username_hemoglyphs_sanitised) = User::sanitise_username(username_hemoglyphs);
+
+        assert!(is_hemoglyphs_sanitised);
+        assert_eq!("funny", username_hemoglyphs_sanitised);
+    }
+
+    #[test]
+    fn username_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_http = "http://_test";
+        let username_https = "https://_test";
+
+        let expected_username = "_test";
+
+        let (is_grave_sanitised, username_grave_sanitised) = User::sanitise_username(username_grave);
+        let (is_discord_sanitised, username_discord_sanitised) = User::sanitise_username(username_discord);
+        let (is_rvlt_sanitised, username_rvlt_sanitised) = User::sanitise_username(username_rvlt);
+        let (is_guilded_sanitised, username_guilded_sanitised) = User::sanitise_username(username_guilded);
+        let (is_http_sanitised, username_http_sanitised) = User::sanitise_username(username_http);
+        let (is_https_sanitised, username_https_sanitised) = User::sanitise_username(username_https);
+
+        assert!(is_grave_sanitised);
+        assert_eq!(expected_username, username_grave_sanitised);
+        assert!(is_discord_sanitised);
+        assert_eq!(expected_username, username_discord_sanitised);
+        assert!(is_rvlt_sanitised);
+        assert_eq!(expected_username, username_rvlt_sanitised);
+        assert!(is_guilded_sanitised);
+        assert_eq!(expected_username, username_guilded_sanitised);
+        assert!(is_http_sanitised);
+        assert_eq!(expected_username, username_http_sanitised);
+        assert!(is_https_sanitised);
+        assert_eq!(expected_username, username_https_sanitised);
+    }
+
+    #[test]
+    fn username_sanitisation_padding() {
+        let username_padding = "```a";
+
+        let (_, username) = User::sanitise_username(username_padding);
+
+        assert_eq!("a_", username);
+    }
+
+    #[async_std::test]
+    async fn create_user() {
+        database_test!(|db| async move {
+            let created_clean = User::create(&db, "Test".to_string(), None, None)
+                .await
+                .unwrap();
+
+            let mut updated_clean = User::create(&db, "Test".to_string(), None, None)
+                .await
+                .unwrap();
+            updated_clean.update_username(&db, "Test2".to_string()).await.unwrap();
+
+            let created_sanitised = User::create(&db, "http://test".to_string(), None, None)
+                .await
+                .unwrap();
+
+            let mut updated_sanitised = User::create(&db, "Test".to_string(), None, None)
+                .await
+                .unwrap();
+            updated_sanitised.update_username(&db, "http://test".to_string()).await.unwrap();
+
+            assert_eq!(None, created_clean.display_name);
+            assert_eq!(None, updated_clean.display_name);
+            assert_eq!("test", created_sanitised.username);
+            assert_eq!("http://test", created_sanitised.display_name.unwrap());
+            assert_eq!("test", updated_sanitised.username);
+            assert_eq!("http://test", updated_sanitised.display_name.unwrap());
+        });
+    }
+
+}