Address old TODOs addressing site_url and default email sender.

Author: ignatz

trailbase-core/src/admin/user/list_users.rs

@@ -61,8 +61,6 @@ pub async fn list_users_handler(
 ) -> Result<Json<ListUsersResponse>, Error> {
   let conn = state.user_conn();
 
-  // TODO: we should probably return an error if the query parsing fails rather than quietly
-  // falling back to defaults.
   let QueryParseResult {
     params: filter_params,
     cursor,

trailbase-core/src/app_state.rs

@@ -22,7 +22,7 @@ use crate::value_notifier::{Computed, ValueNotifier};
 struct InternalState {
   data_dir: DataDir,
   public_dir: Option<PathBuf>,
-  address: String,
+  site_url: Computed<url::Url, Config>,
   dev: bool,
   demo: bool,
 
@@ -74,6 +74,8 @@ impl AppState {
   pub(crate) fn new(args: AppStateArgs) -> Self {
     let config = ValueNotifier::new(args.config);
 
+    let site_url = Computed::new(&config, move |c| build_site_url(c, &args.address));
+
     let record_apis = {
       let schema_metadata_clone = args.schema_metadata.clone();
       let conn_clone = args.conn.clone();
@@ -109,7 +111,7 @@ impl AppState {
       state: Arc::new(InternalState {
         data_dir: args.data_dir,
         public_dir: args.public_dir,
-        address: args.address,
+        site_url,
         dev: args.dev,
         demo: args.demo,
         oauth: Computed::new(&config, |c| {
@@ -233,11 +235,8 @@ impl AppState {
       .collect();
   }
 
-  // TODO: Turn this into a parsed url::Url.
-  pub fn site_url(&self) -> String {
-    self
-      .access_config(|c| c.server.site_url.clone())
-      .unwrap_or_else(|| format!("http://{}", self.state.address))
+  pub fn site_url(&self) -> Arc<url::Url> {
+    return self.state.site_url.load().clone();
   }
 
   pub(crate) fn mailer(&self) -> Arc<Mailer> {
@@ -344,6 +343,7 @@ pub async fn test_state(options: Option<TestStateOptions>) -> anyhow::Result<App
     // Construct a fabricated config for tests and make sure it's valid.
     let mut config = Config::new_with_custom_defaults();
 
+    config.server.site_url = Some("https://test.org".to_string());
     config.email.smtp_host = Some("smtp.test.org".to_string());
     config.email.smtp_port = Some(587);
     config.email.smtp_username = Some("user".to_string());
@@ -429,11 +429,12 @@ pub async fn test_state(options: Option<TestStateOptions>) -> anyhow::Result<App
     });
   }
 
+  let address = "localhost:1234";
   return Ok(AppState {
     state: Arc::new(InternalState {
       data_dir,
       public_dir: None,
-      address: "localhost:1234".to_string(),
+      site_url: Computed::new(&config, move |c| build_site_url(c, address)),
       dev: true,
       demo: false,
       oauth: Computed::new(&config, |c| {
@@ -537,3 +538,20 @@ pub(crate) fn build_objectstore(
     object_store::local::LocalFileSystem::new_with_prefix(data_dir.uploads_path())?,
   ));
 }
+
+fn build_site_url(c: &Config, address: &str) -> url::Url {
+  let fallback = url::Url::parse(&format!("http://{address}")).expect("startup");
+
+  if let Some(ref site_url) = c.server.site_url {
+    match url::Url::parse(site_url) {
+      Ok(url) => {
+        return url;
+      }
+      Err(err) => {
+        error!("Failed to parse site_url '{site_url}' from config: {err}");
+      }
+    };
+  }
+
+  return fallback;
+}

trailbase-core/src/auth/api/avatar.rs

@@ -14,7 +14,7 @@ use crate::auth::util::user_by_id;
 use crate::constants::{AVATAR_TABLE, RECORD_API_PATH};
 use crate::util::{assert_uuidv7_version, id_to_b64};
 
-async fn get_avatar_url(state: &AppState, user: &DbUser) -> Option<String> {
+async fn get_avatar_url(state: &AppState, user: &DbUser) -> Option<url::Url> {
   lazy_static! {
     static ref QUERY: String =
       format!(r#"SELECT EXISTS(SELECT user FROM "{AVATAR_TABLE}" WHERE user = $1)"#);
@@ -32,12 +32,14 @@ async fn get_avatar_url(state: &AppState, user: &DbUser) -> Option<String> {
     .unwrap_or(false);
 
   if has_avatar {
-    let site = state.site_url();
     let record_user_id = id_to_b64(&user.id);
     let col_name = "file";
-    return Some(format!(
-      "{site}/{RECORD_API_PATH}/{AVATAR_TABLE}/{record_user_id}/file/{col_name}"
-    ));
+    return state
+      .site_url()
+      .join(&format!(
+        "/{RECORD_API_PATH}/{AVATAR_TABLE}/{record_user_id}/file/{col_name}"
+      ))
+      .ok();
   }
 
   return None;
@@ -68,7 +70,7 @@ pub async fn get_avatar_url_handler(
   // TODO: Allow a configurable fallback url.
   let avatar_url = get_avatar_url(&state, &db_user)
     .await
-    .or(db_user.provider_avatar_url);
+    .map_or_else(|| db_user.provider_avatar_url, |url| Some(url.to_string()));
 
   // TODO: Maybe return a JSON response with url if content-type is JSON.
   return match avatar_url {
@@ -288,11 +290,11 @@ mod tests {
       .to_str()
       .unwrap();
 
+    let mut _url = url::Url::parse(location).unwrap();
     assert_eq!(
       location,
       format!(
-        "{site}/{RECORD_API_PATH}/{AVATAR_COLLECTION_NAME}/{record_id_b64}/file/{COL_NAME}",
-        site = state.site_url(),
+        "https://test.org/{RECORD_API_PATH}/{AVATAR_COLLECTION_NAME}/{record_id_b64}/file/{COL_NAME}",
         record_id_b64 = uuid_to_b64(&record_id),
       )
     );

trailbase-core/src/auth/mod.rs

@@ -74,7 +74,7 @@ pub(super) fn router() -> Router<crate::AppState> {
   //    * refresh-token (no CSRF, safe side-effect)
   //    * logout (no CSRF, safe side-effect)
   //    * change-password (no CSRF: requires old pass),
-  //    * change-email (TODO: CSRF: requires old email so only targeted),
+  //    * change-email (CSRF: requires old email so only targeted),
   //    * delete-user (technically CSRF: however, currently DELETE method)
   //
   //  Avatar life-cycle: read+update are handled as record APIs.

trailbase-core/src/auth/oauth/oauth_test.rs

@@ -159,7 +159,7 @@ async fn test_oauth() {
   assert_eq!(auth_query.state, oauth_state.csrf_secret);
   assert_eq!(
     auth_query.redirect_uri,
-    format!("{}/{AUTH_API_PATH}/oauth/{name}/callback", state.site_url())
+    format!("http://localhost:1234/{AUTH_API_PATH}/oauth/{name}/callback")
   );
   assert_eq!(
     auth_query.code_challenge,

trailbase-core/src/auth/oauth/provider.rs

@@ -65,12 +65,13 @@ pub trait OAuthProvider {
   fn settings(&self) -> Result<OAuthClientSettings, AuthError>;
 
   fn oauth_client(&self, state: &AppState) -> Result<OAuthClient, AuthError> {
-    let redirect_url: Url = Url::parse(&format!(
-      "{site}/{AUTH_API_PATH}/oauth/{name}/callback",
-      site = state.site_url(),
-      name = self.name()
-    ))
-    .map_err(|err| AuthError::FailedDependency(err.into()))?;
+    let redirect_url: Url = state
+      .site_url()
+      .join(&format!(
+        "/{AUTH_API_PATH}/oauth/{name}/callback",
+        name = self.name()
+      ))
+      .map_err(|err| AuthError::FailedDependency(err.into()))?;
 
     let settings = self.settings()?;
     if settings.client_id.is_empty() {

trailbase-core/src/email.rs

@@ -223,15 +223,21 @@ impl Email {
 fn get_sender(state: &AppState) -> Result<Mailbox, EmailError> {
   let (sender_address, sender_name) =
     state.access_config(|c| (c.email.sender_address.clone(), c.email.sender_name.clone()));
-  // TODO: Have a better default sender, e.g. derive from SITE_URL.
-  let address = sender_address.unwrap_or_else(|| "admin@localhost".to_string());
+  let address = sender_address.unwrap_or_else(|| fallback_sender(&state.site_url()));
 
   if let Some(ref name) = sender_name {
     return Ok(format!("{} <{}>", name, address).parse::<Mailbox>()?);
   }
   return Ok(address.parse::<Mailbox>()?);
 }
 
+fn fallback_sender(site_url: &url::Url) -> String {
+  if let Some(host) = site_url.host() {
+    return format!("noreply@{}", host);
+  }
+  return "noreply@localhost".to_string();
+}
+
 #[derive(Clone)]
 pub(crate) enum Mailer {
   Smtp(Arc<dyn AsyncTransport<Ok = smtp::response::Response, Error = smtp::Error> + Send + Sync>),
@@ -435,4 +441,11 @@ pub mod testing {
       assert!(email.body.contains(code));
     }
   }
+
+  #[test]
+  fn test_fallback_sender() {
+    let url = url::Url::parse("https://test.org").unwrap();
+    let sender = fallback_sender(&url);
+    assert_eq!("[email protected]", sender);
+  }
 }

trailbase-core/tests/admin_permissions_test.rs

@@ -20,7 +20,7 @@ fn test_admin_permissions() {
       tls,
     } = Server::init(ServerOptions {
       data_dir: DataDir(data_dir.path().to_path_buf()),
-      address: "".to_string(),
+      address: "localhost:4040".to_string(),
       admin_address: None,
       public_dir: None,
       dev: false,

trailbase-core/tests/integration_test.rs

@@ -42,7 +42,7 @@ async fn test_record_apis() {
     tls,
   } = Server::init(ServerOptions {
     data_dir: DataDir(data_dir.path().to_path_buf()),
-    address: "".to_string(),
+    address: "localhost:4041".to_string(),
     admin_address: None,
     public_dir: None,
     dev: false,