feat: api error codes and redacted secrets (#6)

<!--
Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->

Provides `Secret` struct to keep sensitive data hidden, also Error Codes
support.
Makes sure core logic has correct error handling by enumerating errors
from different cases
and providing explicit Status Codes for each error.

Makes sure routes matches the ones under `account`, refer:


https://github.com/commune-os/commune-server/blob/df257384c8daede9fdf0e93c51af3c6444e15745/app/routes.go#L122-L164

Cargo.toml

@@ -12,6 +12,7 @@ resolver = "1"
 anyhow = "1.0.75"
 axum = "0.6.19"
 dotenv = "0.15.0"
+http = "0.2.11"
 reqwest = "0.11.22"
 serde = "1.0.192"
 tokio = "1.34.0"

crates/core/Cargo.toml

@@ -9,10 +9,14 @@ name = "commune"
 path = "src/lib.rs"
 
 [dependencies]
+thiserror = "1.0.50"
 validator = { version = "0.16", features = ["derive"] }
 
 # Workspace
-anyhow = { workspace = true }
+http = { workspace = true }
+serde = { workspace = true, features = ["derive"] }
+tracing = { workspace = true }
+url = { workspace = true, features = ["serde"] }
 
 # Local Dependencies
 matrix = { path = "../matrix" }

crates/core/src/error.rs

@@ -0,0 +1,41 @@
+use http::StatusCode;
+use thiserror::Error;
+
+use crate::user::error::UserErrorCode;
+
+pub type Result<T> = std::result::Result<T, Error>;
+
+pub trait HttpStatusCode {
+    fn status_code(&self) -> StatusCode;
+    fn error_code(&self) -> &'static str;
+}
+
+#[derive(Debug, Error)]
+pub enum Error {
+    #[error("User Error. {0}")]
+    User(UserErrorCode),
+    #[error("Unknown Error Occured")]
+    Unknown,
+}
+
+impl From<UserErrorCode> for Error {
+    fn from(err: UserErrorCode) -> Self {
+        Error::User(err)
+    }
+}
+
+impl HttpStatusCode for Error {
+    fn status_code(&self) -> StatusCode {
+        match self {
+            Error::User(err) => err.status_code(),
+            Error::Unknown => StatusCode::INTERNAL_SERVER_ERROR,
+        }
+    }
+
+    fn error_code(&self) -> &'static str {
+        match self {
+            Error::User(err) => err.error_code(),
+            Error::Unknown => "UNKNOWN_ERROR",
+        }
+    }
+}

crates/core/src/lib.rs

@@ -1,9 +1,10 @@
+pub mod error;
 pub mod user;
 pub mod util;
 
-use std::fmt::Debug;
+pub use error::{Error, HttpStatusCode, Result};
 
-use anyhow::Result;
+use std::fmt::Debug;
 
 use matrix::admin::Client as MatrixAdminClient;
 
@@ -24,9 +25,16 @@ pub struct Commune {
 impl Commune {
     pub fn new<C: Into<CommuneConfig>>(config: C) -> Result<Self> {
         let config: CommuneConfig = config.into();
-        let mut admin = MatrixAdminClient::new(config.synapse_host, config.synapse_server_name)?;
-
-        admin.set_token(config.synapse_admin_token)?;
+        let mut admin = MatrixAdminClient::new(config.synapse_host, config.synapse_server_name)
+            .map_err(|err| {
+                tracing::error!(?err, "Failed to create admin client");
+                Error::Unknown
+            })?;
+
+        admin.set_token(config.synapse_admin_token).map_err(|err| {
+            tracing::error!(?err, "Failed to set admin token");
+            Error::Unknown
+        })?;
 
         Ok(Self {
             user: UserService::new(admin),

crates/core/src/user/error.rs

@@ -0,0 +1,29 @@
+use http::StatusCode;
+use thiserror::Error;
+use validator::ValidationErrors;
+
+use crate::error::HttpStatusCode;
+
+#[derive(Debug, Error)]
+pub enum UserErrorCode {
+    #[error("Vaildation error. {0}")]
+    ValidationError(#[from] ValidationErrors),
+    #[error("The username {0} is already taken")]
+    UsernameTaken(String),
+}
+
+impl HttpStatusCode for UserErrorCode {
+    fn status_code(&self) -> StatusCode {
+        match self {
+            UserErrorCode::ValidationError(_) => StatusCode::BAD_REQUEST,
+            UserErrorCode::UsernameTaken(_) => StatusCode::CONFLICT,
+        }
+    }
+
+    fn error_code(&self) -> &'static str {
+        match self {
+            UserErrorCode::ValidationError(_) => "VALIDATION_ERROR",
+            UserErrorCode::UsernameTaken(_) => "USERNAME_TAKEN",
+        }
+    }
+}

crates/core/src/user/mod.rs

@@ -1,2 +1,3 @@
+pub mod error;
 pub mod model;
 pub mod service;

crates/core/src/user/service.rs

@@ -1,26 +1,76 @@
-use anyhow::{bail, Result};
-use validator::Validate;
+use tracing::instrument;
+use url::Url;
+use validator::{Validate, ValidationError};
 
-use matrix::admin::resources::user::{ThreePid, User as MatrixUser, UserCreateDto};
+use matrix::admin::resources::user::{
+    ListUsersParams, ThreePid, User as MatrixUser, UserCreateDto,
+};
 use matrix::admin::resources::user_id::UserId;
 use matrix::admin::Client as MatrixAdminClient;
 
+use crate::util::secret::Secret;
 use crate::util::time::timestamp;
+use crate::{Error, Result};
 
+use super::error::UserErrorCode;
 use super::model::User;
 
+const DEFAULT_AVATAR_URL: &str = "https://via.placeholder.com/150";
+const MIN_USERNAME_LENGTH: usize = 3;
+const MAX_USERNAME_LENGTH: usize = 12;
+const MIN_PASSWORD_LENGTH: usize = 8;
+
+pub struct LoginDto {
+    pub username: String,
+    pub password: String,
+}
+
 #[derive(Debug, Validate)]
 pub struct CreateAccountDto {
-    #[validate(length(min = 8, max = 12))]
+    #[validate(custom = "CreateAccountDto::validate_username")]
     pub username: String,
-    #[validate(length(min = 8, max = 12))]
-    pub password: String,
+    #[validate(custom = "CreateAccountDto::validate_password")]
+    pub password: Secret,
     #[validate(email)]
     pub email: String,
     pub session: String,
     pub code: String,
 }
 
+impl CreateAccountDto {
+    /// Validation logic for usernames enforced in user creation
+    fn validate_username(username: &str) -> std::result::Result<(), ValidationError> {
+        if username.len() < MIN_USERNAME_LENGTH {
+            return Err(ValidationError::new("username is too short"));
+        }
+
+        if username.len() > MAX_USERNAME_LENGTH {
+            return Err(ValidationError::new("username is too long"));
+        }
+
+        if username.contains(' ') {
+            return Err(ValidationError::new("username cannot contain spaces"));
+        }
+
+        if username.to_ascii_lowercase() != username {
+            return Err(ValidationError::new(
+                "username cannot contain uppercase letters",
+            ));
+        }
+
+        Ok(())
+    }
+
+    /// Validation logic for passwords enforced in user creation
+    fn validate_password(password: &Secret) -> std::result::Result<(), ValidationError> {
+        if password.inner().len() < MIN_PASSWORD_LENGTH {
+            return Err(ValidationError::new("password is too short"));
+        }
+
+        Ok(())
+    }
+}
+
 pub struct UserService {
     admin: MatrixAdminClient,
 }
@@ -30,18 +80,44 @@ impl UserService {
         Self { admin }
     }
 
+    #[instrument(skip(self, dto))]
     pub async fn register(&self, dto: CreateAccountDto) -> Result<User> {
-        dto.validate()?;
+        dto.validate().map_err(|err| {
+            tracing::warn!(?err, "Failed to validate user creation dto");
+            UserErrorCode::from(err)
+        })?;
 
         let user_id = UserId::new(dto.username.clone(), self.admin.server_name().to_string());
+        let exists = MatrixUser::list(
+            &self.admin,
+            ListUsersParams {
+                user_id: Some(user_id.to_string()),
+                ..Default::default()
+            },
+        )
+        .await
+        .map_err(|err| {
+            tracing::error!(?err, "Failed to list users");
+            Error::Unknown
+        })?;
+
+        if !exists.users.is_empty() {
+            return Err(UserErrorCode::UsernameTaken(dto.username).into());
+        }
+
+        let avatar_url = Url::parse(DEFAULT_AVATAR_URL).map_err(|err| {
+            tracing::error!(?err, "Failed to parse default avatar url");
+            Error::Unknown
+        })?;
+
         let matrix_user = MatrixUser::create(
             &self.admin,
             user_id,
             UserCreateDto {
                 displayname: Some(dto.username),
-                password: dto.password,
+                password: dto.password.to_string(),
                 logout_devices: false,
-                avatar_url: None,
+                avatar_url: Some(avatar_url),
                 threepids: vec![ThreePid {
                     medium: "email".to_string(),
                     address: dto.email,
@@ -55,14 +131,20 @@ impl UserService {
                 locked: false,
             },
         )
-        .await?;
+        .await
+        .map_err(|err| {
+            tracing::error!(?err, "Failed to create user");
+            Error::Unknown
+        })?;
 
         let Some(displayname) = matrix_user.displayname else {
-            bail!("Matrix displayname is empty, this value cannot be empty");
+            tracing::error!("Failed to get displayname for user");
+            return Err(Error::Unknown);
         };
 
         let Some(threepid) = matrix_user.threepids.first() else {
-            bail!("Matrix Threepid should exist, this value cannot be empty");
+            tracing::error!("Failed to get threepid for user");
+            return Err(Error::Unknown);
         };
 
         Ok(User {
@@ -73,3 +155,68 @@ impl UserService {
         })
     }
 }
+
+#[cfg(test)]
+mod test {
+    use validator::Validate;
+
+    use crate::util::secret::Secret;
+
+    use super::CreateAccountDto;
+
+    #[test]
+    fn ensure_username_is_not_too_short() {
+        let dto = CreateAccountDto {
+            username: "ab".to_string(),
+            password: Secret::new("password"),
+            email: "aby@mail.com".to_string(),
+            code: "1234".to_string(),
+            session: "synapse".to_string(),
+        };
+        let err = dto.validate().err().unwrap();
+
+        assert_eq!(err.to_string(), "username is too short");
+    }
+
+    #[test]
+    fn ensure_username_is_not_too_long() {
+        let dto = CreateAccountDto {
+            username: "abbeyroadismyfavoritealbum".to_string(),
+            password: Secret::new("password"),
+            email: "aby@mail.com".to_string(),
+            code: "1234".to_string(),
+            session: "synapse".to_string(),
+        };
+        let err = dto.validate().err().unwrap();
+
+        assert_eq!(err.to_string(), "username is too long");
+    }
+
+    #[test]
+    fn ensure_username_does_not_contain_spaces() {
+        let dto = CreateAccountDto {
+            username: "abbey road".to_string(),
+            password: Secret::new("password"),
+            email: "aby@mail.com".to_string(),
+            code: "1234".to_string(),
+            session: "synapse".to_string(),
+        };
+        let err = dto.validate().err().unwrap();
+
+        assert_eq!(err.to_string(), "username cannot contain spaces");
+    }
+
+    #[test]
+    fn ensure_username_is_lowercased() {
+        let dto = CreateAccountDto {
+            username: "AbbeyRoad".to_string(),
+            password: Secret::new("password"),
+            email: "aby@mail.com".to_string(),
+            code: "1234".to_string(),
+            session: "synapse".to_string(),
+        };
+        let err = dto.validate().err().unwrap();
+
+        assert_eq!(err.to_string(), "username cannot contain uppercase letters");
+    }
+}

crates/core/src/util/mod.rs

@@ -1 +1,2 @@
+pub mod secret;
 pub mod time;