From 12aad8bf3c25badd97ccfac0b3d317da509d961b Mon Sep 17 00:00:00 2001 From: Soso <51865119+sgued@users.noreply.github.com> Date: Mon, 11 Dec 2023 22:48:32 +0100 Subject: [PATCH] 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 --- docs/06_http_endpoints_axum.md | 2 +- examples/live_federation/http.rs | 2 +- examples/local_federation/actix_web/http.rs | 2 +- examples/local_federation/axum/http.rs | 2 +- src/axum/json.rs | 2 +- src/error.rs | 7 +-- src/fetch/webfinger.rs | 67 ++++++++++++++------- src/traits.rs | 2 +- 8 files changed, 55 insertions(+), 31 deletions(-) diff --git a/docs/06_http_endpoints_axum.md b/docs/06_http_endpoints_axum.md index 8ebbcc8..3a33410 100644 --- a/docs/06_http_endpoints_axum.md +++ b/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() } diff --git a/examples/live_federation/http.rs b/examples/live_federation/http.rs index d626396..e0d2869 100644 --- a/examples/live_federation/http.rs +++ b/examples/live_federation/http.rs @@ -61,7 +61,7 @@ pub async fn webfinger( data: Data, ) -> Result, 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(), diff --git a/examples/local_federation/actix_web/http.rs b/examples/local_federation/actix_web/http.rs index 12a750f..6298014 100644 --- a/examples/local_federation/actix_web/http.rs +++ b/examples/local_federation/actix_web/http.rs @@ -89,7 +89,7 @@ pub async fn webfinger( data: Data, ) -> Result { 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(), diff --git a/examples/local_federation/axum/http.rs b/examples/local_federation/axum/http.rs index 3202117..f17ea4a 100644 --- a/examples/local_federation/axum/http.rs +++ b/examples/local_federation/axum/http.rs @@ -78,7 +78,7 @@ async fn webfinger( data: Data, ) -> Result, 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(), diff --git a/src/axum/json.rs b/src/axum/json.rs index f8a649e..f99c8bd 100644 --- a/src/axum/json.rs +++ b/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, data: Data) -> Result>, 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))) diff --git a/src/error.rs b/src/error.rs index c66e16c..2d179d1 100644 --- a/src/error.rs +++ b/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), diff --git a/src/fetch/webfinger.rs b/src/fetch/webfinger.rs index a345fd4..68b110d 100644 --- a/src/fetch/webfinger.rs +++ b/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( where Kind: Object + Actor + Send + 'static + Object, for<'de2> ::Kind: serde::Deserialize<'de2>, - ::Error: From + Send + Sync, + ::Error: From + 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::::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(query: &str, data: &Data) -> Result +pub fn extract_webfinger_name<'i, T>(query: &'i str, data: &Data) -> Result<&'i str, Error> where T: Clone, { + static WEBFINGER_REGEX: Lazy = + 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 - .captures(query) - .and_then(|c| c.get(1)) - .ok_or_else(|| Error::WebfingerRegexFailed) - })?; + // TODO: This should use a URL parser + let captures = WEBFINGER_REGEX + .captures(query) + .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(()) diff --git a/src/traits.rs b/src/traits.rs index 21a1540..9fdec27 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -356,7 +356,7 @@ pub mod tests { pub async fn read_post_from_json_id(&self, _: Url) -> Result, Error> { Ok(None) } - pub async fn read_local_user(&self, _: String) -> Result { + pub async fn read_local_user(&self, _: &str) -> Result { todo!() } pub async fn upsert(&self, _: &T) -> Result<(), Error> {