Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dont require leading ! or @ for webfinger resolve #4513

Merged
merged 3 commits into from Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions api_tests/src/user.spec.ts
Expand Up @@ -45,7 +45,7 @@ test("Create user", async () => {
if (!site.my_user) {
throw "Missing site user";
}
apShortname = `@${site.my_user.local_user_view.person.name}@lemmy-alpha:8541`;
apShortname = `${site.my_user.local_user_view.person.name}@lemmy-alpha:8541`;
});

test("Set some user settings, check that they are federated", async () => {
Expand All @@ -68,7 +68,7 @@ test("Delete user", async () => {
let user = await registerUser(alpha, alphaUrl);

// make a local post and comment
let alphaCommunity = (await resolveCommunity(user, "!main@lemmy-alpha:8541"))
let alphaCommunity = (await resolveCommunity(user, "main@lemmy-alpha:8541"))
.community;
if (!alphaCommunity) {
throw "Missing alpha community";
Expand Down Expand Up @@ -134,7 +134,7 @@ test("Create user with Arabic name", async () => {
if (!site.my_user) {
throw "Missing site user";
}
apShortname = `@${site.my_user.local_user_view.person.name}@lemmy-alpha:8541`;
apShortname = `${site.my_user.local_user_view.person.name}@lemmy-alpha:8541`;

let alphaPerson = (await resolvePerson(alpha, apShortname)).person;
expect(alphaPerson).toBeDefined();
Expand Down
27 changes: 14 additions & 13 deletions crates/apub/src/api/resolve_object.rs
@@ -1,7 +1,6 @@
use crate::fetcher::search::{
search_query_to_object_id,
search_query_to_object_id_local,
SearchableObjects,
use crate::fetcher::{
search::{search_query_to_object_id, search_query_to_object_id_local, SearchableObjects},
user_or_community::UserOrCommunity,
};
use activitypub_federation::config::Data;
use actix_web::web::{Json, Query};
Expand Down Expand Up @@ -31,7 +30,7 @@ pub async fn resolve_object(

let res = if is_authenticated {
// user is fully authenticated; allow remote lookups as well.
search_query_to_object_id(&data.q, &context).await
search_query_to_object_id(data.q.clone(), &context).await
} else {
// user isn't authenticated only allow a local search.
search_query_to_object_id_local(&data.q, &context).await
Expand All @@ -52,14 +51,6 @@ async fn convert_response(
let removed_or_deleted;
let mut res = ResolveObjectResponse::default();
match object {
Person(p) => {
removed_or_deleted = p.deleted;
res.person = Some(PersonView::read(pool, p.id).await?)
}
Community(c) => {
removed_or_deleted = c.deleted || c.removed;
res.community = Some(CommunityView::read(pool, c.id, user_id, false).await?)
}
Post(p) => {
removed_or_deleted = p.deleted || p.removed;
res.post = Some(PostView::read(pool, p.id, user_id, false).await?)
Expand All @@ -68,6 +59,16 @@ async fn convert_response(
removed_or_deleted = c.deleted || c.removed;
res.comment = Some(CommentView::read(pool, c.id, user_id).await?)
}
PersonOrCommunity(p) => match *p {
UserOrCommunity::User(u) => {
removed_or_deleted = u.deleted;
res.person = Some(PersonView::read(pool, u.id).await?)
}
UserOrCommunity::Community(c) => {
removed_or_deleted = c.deleted || c.removed;
res.community = Some(CommunityView::read(pool, c.id, user_id, false).await?)
}
},
};
// if the object was deleted from database, dont return it
if removed_or_deleted {
Expand Down
65 changes: 29 additions & 36 deletions crates/apub/src/fetcher/search.rs
@@ -1,6 +1,7 @@
use crate::{
fetcher::user_or_community::{PersonOrGroup, UserOrCommunity},
objects::{comment::ApubComment, community::ApubCommunity, person::ApubPerson, post::ApubPost},
protocol::objects::{group::Group, note::Note, page::Page, person::Person},
protocol::objects::{note::Note, page::Page},
};
use activitypub_federation::{
config::Data,
Expand All @@ -9,7 +10,7 @@ use activitypub_federation::{
};
use chrono::{DateTime, Utc};
use lemmy_api_common::context::LemmyContext;
use lemmy_utils::error::{LemmyError, LemmyErrorType};
use lemmy_utils::error::LemmyError;
use serde::Deserialize;
use url::Url;

Expand All @@ -18,28 +19,22 @@ use url::Url;
/// which gets resolved to an URL.
#[tracing::instrument(skip_all)]
pub(crate) async fn search_query_to_object_id(
query: &str,
mut query: String,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having this a a mutable param, I think it'd be better to just create a new var inside this function. immutable functions and all that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately String::remove only works on a mutable string, and I dont see a better alternative in this case. Sure I could declare a separate mut inside the function, but that wouldnt really change anything.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still an immutable function. mut before a parameter name does the same thing as putting let mut foo = foo; in the function body.

context: &Data<LemmyContext>,
) -> Result<SearchableObjects, LemmyError> {
Ok(match Url::parse(query) {
Ok(match Url::parse(&query) {
Ok(url) => {
// its already an url, just go with it
ObjectId::from(url).dereference(context).await?
}
Err(_) => {
// not an url, try to resolve via webfinger
let mut chars = query.chars();
let kind = chars.next();
let identifier = chars.as_str();
match kind {
Some('@') => SearchableObjects::Person(
webfinger_resolve_actor::<LemmyContext, ApubPerson>(identifier, context).await?,
),
Some('!') => SearchableObjects::Community(
webfinger_resolve_actor::<LemmyContext, ApubCommunity>(identifier, context).await?,
),
_ => return Err(LemmyErrorType::InvalidQuery)?,
if query.starts_with('!') || query.starts_with('@') {
query.remove(0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just build the new var here. let query_2 = if .... else

}
SearchableObjects::PersonOrCommunity(Box::new(
webfinger_resolve_actor::<LemmyContext, UserOrCommunity>(&query, context).await?,
))
}
})
}
Expand All @@ -59,19 +54,17 @@ pub(crate) async fn search_query_to_object_id_local(
/// The types of ActivityPub objects that can be fetched directly by searching for their ID.
#[derive(Debug)]
pub(crate) enum SearchableObjects {
Person(ApubPerson),
Community(ApubCommunity),
Post(ApubPost),
Comment(ApubComment),
PersonOrCommunity(Box<UserOrCommunity>),
}

#[derive(Deserialize)]
#[serde(untagged)]
pub(crate) enum SearchableKinds {
Group(Group),
Person(Person),
Page(Page),
Page(Box<Page>),
Note(Note),
PersonOrGroup(Box<PersonOrGroup>),
}

#[async_trait::async_trait]
Expand All @@ -82,10 +75,9 @@ impl Object for SearchableObjects {

fn last_refreshed_at(&self) -> Option<DateTime<Utc>> {
match self {
SearchableObjects::Person(p) => p.last_refreshed_at(),
SearchableObjects::Community(c) => c.last_refreshed_at(),
SearchableObjects::Post(p) => p.last_refreshed_at(),
SearchableObjects::Comment(c) => c.last_refreshed_at(),
SearchableObjects::PersonOrCommunity(p) => p.last_refreshed_at(),
}
}

Expand All @@ -99,13 +91,9 @@ impl Object for SearchableObjects {
object_id: Url,
context: &Data<Self::DataType>,
) -> Result<Option<Self>, LemmyError> {
let c = ApubCommunity::read_from_id(object_id.clone(), context).await?;
if let Some(c) = c {
return Ok(Some(SearchableObjects::Community(c)));
}
let p = ApubPerson::read_from_id(object_id.clone(), context).await?;
if let Some(p) = p {
return Ok(Some(SearchableObjects::Person(p)));
let uc = UserOrCommunity::read_from_id(object_id.clone(), context).await?;
if let Some(uc) = uc {
return Ok(Some(SearchableObjects::PersonOrCommunity(Box::new(uc))));
}
let p = ApubPost::read_from_id(object_id.clone(), context).await?;
if let Some(p) = p {
Expand All @@ -121,10 +109,12 @@ impl Object for SearchableObjects {
#[tracing::instrument(skip_all)]
async fn delete(self, data: &Data<Self::DataType>) -> Result<(), LemmyError> {
match self {
SearchableObjects::Person(p) => p.delete(data).await,
SearchableObjects::Community(c) => c.delete(data).await,
SearchableObjects::Post(p) => p.delete(data).await,
SearchableObjects::Comment(c) => c.delete(data).await,
SearchableObjects::PersonOrCommunity(pc) => match *pc {
UserOrCommunity::User(p) => p.delete(data).await,
UserOrCommunity::Community(c) => c.delete(data).await,
},
}
}

Expand All @@ -139,10 +129,12 @@ impl Object for SearchableObjects {
data: &Data<Self::DataType>,
) -> Result<(), LemmyError> {
match apub {
SearchableKinds::Group(a) => ApubCommunity::verify(a, expected_domain, data).await,
SearchableKinds::Person(a) => ApubPerson::verify(a, expected_domain, data).await,
SearchableKinds::Page(a) => ApubPost::verify(a, expected_domain, data).await,
SearchableKinds::Note(a) => ApubComment::verify(a, expected_domain, data).await,
SearchableKinds::PersonOrGroup(pg) => match pg.as_ref() {
PersonOrGroup::Person(a) => ApubPerson::verify(a, expected_domain, data).await,
PersonOrGroup::Group(a) => ApubCommunity::verify(a, expected_domain, data).await,
},
}
}

Expand All @@ -151,10 +143,11 @@ impl Object for SearchableObjects {
use SearchableKinds as SAT;
use SearchableObjects as SO;
Ok(match apub {
SAT::Group(g) => SO::Community(ApubCommunity::from_json(g, context).await?),
SAT::Person(p) => SO::Person(ApubPerson::from_json(p, context).await?),
SAT::Page(p) => SO::Post(ApubPost::from_json(p, context).await?),
SAT::Page(p) => SO::Post(ApubPost::from_json(*p, context).await?),
SAT::Note(n) => SO::Comment(ApubComment::from_json(n, context).await?),
SAT::PersonOrGroup(pg) => {
SO::PersonOrCommunity(Box::new(UserOrCommunity::from_json(*pg, context).await?))
}
})
}
}