Webfinger: don't discard consumer errors (#85)

* Improve WebFinger errors

* Improve webfinger extraction

* Fix typo

* Document webfinger parsing

* Reimplement Regex based webfinger parsing

* clippy

* no unwrap

---------

Co-authored-by: Felix Ableitner <me@nutomic.com>

docs/06_http_endpoints_axum.md

@@ -48,7 +48,7 @@ async fn http_get_user(
 ) -> impl IntoResponse {
     let accept = header_map.get("accept").map(|v| v.to_str().unwrap());
     if accept == Some(FEDERATION_CONTENT_TYPE) {
-        let db_user = data.read_local_user(name).await.unwrap();
+        let db_user = data.read_local_user(&name).await.unwrap();
         let json_user = db_user.into_json(&data).await.unwrap();
         FederationJson(WithContext::new_default(json_user)).into_response()
     }

examples/live_federation/http.rs

@@ -61,7 +61,7 @@ pub async fn webfinger(
     data: Data<DatabaseHandle>,
 ) -> Result<Json<Webfinger>, Error> {
     let name = extract_webfinger_name(&query.resource, &data)?;
-    let db_user = data.read_user(&name)?;
+    let db_user = data.read_user(name)?;
     Ok(Json(build_webfinger_response(
         query.resource,
         db_user.ap_id.into_inner(),

examples/local_federation/actix_web/http.rs

@@ -89,7 +89,7 @@ pub async fn webfinger(
     data: Data<DatabaseHandle>,
 ) -> Result<HttpResponse, Error> {
     let name = extract_webfinger_name(&query.resource, &data)?;
-    let db_user = data.read_user(&name)?;
+    let db_user = data.read_user(name)?;
     Ok(HttpResponse::Ok().json(build_webfinger_response(
         query.resource.clone(),
         db_user.ap_id.into_inner(),

examples/local_federation/axum/http.rs

@@ -78,7 +78,7 @@ async fn webfinger(
     data: Data<DatabaseHandle>,
 ) -> Result<Json<Webfinger>, Error> {
     let name = extract_webfinger_name(&query.resource, &data)?;
-    let db_user = data.read_user(&name)?;
+    let db_user = data.read_user(name)?;
     Ok(Json(build_webfinger_response(
         query.resource,
         db_user.ap_id.into_inner(),

src/axum/json.rs

@@ -9,7 +9,7 @@
 //! # use activitypub_federation::traits::Object;
 //! # use activitypub_federation::traits::tests::{DbConnection, DbUser, Person};
 //! async fn http_get_user(Path(name): Path<String>, data: Data<DbConnection>) -> Result<FederationJson<WithContext<Person>>, Error> {
-//!     let user: DbUser = data.read_local_user(name).await?;
+//!     let user: DbUser = data.read_local_user(&name).await?;
 //!     let person = user.into_json(&data).await?;
 //!
 //!     Ok(FederationJson(WithContext::new_default(person)))

src/error.rs

@@ -6,6 +6,8 @@ use http_signature_normalization_reqwest::SignError;
 use openssl::error::ErrorStack;
 use url::Url;
 
+use crate::fetch::webfinger::WebFingerError;
+
 /// Error messages returned by this library
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
@@ -32,10 +34,7 @@ pub enum Error {
     ActivitySignatureInvalid,
     /// Failed to resolve actor via webfinger
     #[error("Failed to resolve actor via webfinger")]
-    WebfingerResolveFailed,
-    /// Failed to resolve actor via webfinger
-    #[error("Webfinger regex failed to match")]
-    WebfingerRegexFailed,
+    WebfingerResolveFailed(#[from] WebFingerError),
     /// JSON Error
     #[error(transparent)]
     Json(#[from] serde_json::Error),

src/fetch/webfinger.rs

@@ -1,17 +1,38 @@
 use crate::{
     config::Data,
-    error::{Error, Error::WebfingerResolveFailed},
+    error::Error,
     fetch::{fetch_object_http_with_accept, object_id::ObjectId},
     traits::{Actor, Object},
     FEDERATION_CONTENT_TYPE,
 };
 use itertools::Itertools;
+use once_cell::sync::Lazy;
 use regex::Regex;
 use serde::{Deserialize, Serialize};
-use std::collections::HashMap;
+use std::{collections::HashMap, fmt::Display};
 use tracing::debug;
 use url::Url;
 
+/// Errors relative to webfinger handling
+#[derive(thiserror::Error, Debug)]
+pub enum WebFingerError {
+    /// The webfinger identifier is invalid
+    #[error("The webfinger identifier is invalid")]
+    WrongFormat,
+    /// The webfinger identifier doesn't match the expected instance domain name
+    #[error("The webfinger identifier doesn't match the expected instance domain name")]
+    WrongDomain,
+    /// The wefinger object did not contain any link to an activitypub item
+    #[error("The webfinger object did not contain any link to an activitypub item")]
+    NoValidLink,
+}
+
+impl WebFingerError {
+    fn into_crate_error(self) -> Error {
+        self.into()
+    }
+}
+
 /// Takes an identifier of the form `name@example.com`, and returns an object of `Kind`.
 ///
 /// For this the identifier is first resolved via webfinger protocol to an Activitypub ID. This ID
@@ -23,12 +44,12 @@ pub async fn webfinger_resolve_actor<T: Clone, Kind>(
 where
     Kind: Object + Actor + Send + 'static + Object<DataType = T>,
     for<'de2> <Kind as Object>::Kind: serde::Deserialize<'de2>,
-    <Kind as Object>::Error: From<crate::error::Error> + Send + Sync,
+    <Kind as Object>::Error: From<crate::error::Error> + Send + Sync + Display,
 {
     let (_, domain) = identifier
         .splitn(2, '@')
         .collect_tuple()
-        .ok_or(WebfingerResolveFailed)?;
+        .ok_or(WebFingerError::WrongFormat.into_crate_error())?;
     let protocol = if data.config.debug { "http" } else { "https" };
     let fetch_url =
         format!("{protocol}://{domain}/.well-known/webfinger?resource=acct:{identifier}");
@@ -55,13 +76,15 @@ where
         })
         .filter_map(|l| l.href.clone())
         .collect();
+
     for l in links {
         let object = ObjectId::<Kind>::from(l).dereference(data).await;
-        if object.is_ok() {
-            return object;
+        match object {
+            Ok(obj) => return Ok(obj),
+            Err(error) => debug!(%error, "Failed to dereference link"),
         }
     }
-    Err(WebfingerResolveFailed.into())
+    Err(WebFingerError::NoValidLink.into_crate_error().into())
 }
 
 /// Extracts username from a webfinger resource parameter.
@@ -89,22 +112,24 @@ where
 /// # Ok::<(), anyhow::Error>(())
 /// }).unwrap();
 ///```
-pub fn extract_webfinger_name<T>(query: &str, data: &Data<T>) -> Result<String, Error>
+pub fn extract_webfinger_name<'i, T>(query: &'i str, data: &Data<T>) -> Result<&'i str, Error>
 where
     T: Clone,
 {
+    static WEBFINGER_REGEX: Lazy<Regex> =
+        Lazy::new(|| Regex::new(r"^acct:([\p{L}0-9_]+)@(.*)$").expect("compile regex"));
     // Regex to extract usernames from webfinger query. Supports different alphabets using `\p{L}`.
-    // TODO: would be nice if we could implement this without regex and remove the dependency
-    let result = Regex::new(&format!(r"^acct:([\p{{L}}0-9_]+)@{}$", data.domain()))
-        .map_err(|_| Error::WebfingerRegexFailed)
-        .and_then(|regex| {
-            regex
+    // TODO: This should use a URL parser
+    let captures = WEBFINGER_REGEX
         .captures(query)
-                .and_then(|c| c.get(1))
-                .ok_or_else(|| Error::WebfingerRegexFailed)
-        })?;
+        .ok_or(WebFingerError::WrongFormat)?;
 
-    return Ok(result.as_str().to_string());
+    let account_name = captures.get(1).ok_or(WebFingerError::WrongFormat)?;
+
+    if captures.get(2).map(|m| m.as_str()) != Some(data.domain()) {
+        return Err(WebFingerError::WrongDomain.into());
+    }
+    Ok(account_name.as_str())
 }
 
 /// Builds a basic webfinger response for the actor.
@@ -252,15 +277,15 @@ mod tests {
             request_counter: Default::default(),
         };
         assert_eq!(
-            Ok("test123".to_string()),
+            Ok("test123"),
             extract_webfinger_name("acct:test123@example.com", &data)
         );
         assert_eq!(
-            Ok("Владимир".to_string()),
+            Ok("Владимир"),
             extract_webfinger_name("acct:Владимир@example.com", &data)
         );
         assert_eq!(
-            Ok("تجريب".to_string()),
+            Ok("تجريب"),
             extract_webfinger_name("acct:تجريب@example.com", &data)
         );
         Ok(())

src/traits.rs

@@ -356,7 +356,7 @@ pub mod tests {
         pub async fn read_post_from_json_id<T>(&self, _: Url) -> Result<Option<T>, Error> {
             Ok(None)
         }
-        pub async fn read_local_user(&self, _: String) -> Result<DbUser, Error> {
+        pub async fn read_local_user(&self, _: &str) -> Result<DbUser, Error> {
             todo!()
         }
         pub async fn upsert<T>(&self, _: &T) -> Result<(), Error> {