Fix follow IDs (#455)

* Generate valid IDs for Follow

Fixes #449

* Use the new post-insert hook for all the models

* Fix plume-cli build
This commit is contained in:
Baptiste Gelez 2019-03-04 21:35:03 +01:00 committed by GitHub
parent 9b48b8a846
commit 2a188abfa1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 162 additions and 181 deletions

View File

@ -88,8 +88,7 @@ fn new<'a>(args: &ArgMatches<'a>, conn: &Connection) {
&bio,
email,
User::hash_pass(&password).expect("Couldn't hash password"),
).expect("Couldn't save new user")
.update_boxes(conn).expect("Couldn't update ActivityPub informations for new user");
).expect("Couldn't save new user");
}
fn reset_password<'a>(args: &ArgMatches<'a>, conn: &Connection) {

View File

@ -1,6 +1,6 @@
use activitypub::{actor::Group, collection::OrderedCollection, Actor, CustomObject, Object};
use chrono::NaiveDateTime;
use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl};
use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl, SaveChangesDsl};
use openssl::{
hash::MessageDigest,
pkey::{PKey, Private},
@ -30,7 +30,7 @@ use {Connection, BASE_URL, USE_HTTPS, Error, Result};
pub type CustomGroup = CustomObject<ApSignature, Group>;
#[derive(Queryable, Identifiable, Serialize, Deserialize, Clone)]
#[derive(Queryable, Identifiable, Serialize, Deserialize, Clone, AsChangeset)]
pub struct Blog {
pub id: i32,
pub actor_id: String,
@ -45,7 +45,7 @@ pub struct Blog {
pub public_key: String,
}
#[derive(Insertable)]
#[derive(Default, Insertable)]
#[table_name = "blogs"]
pub struct NewBlog {
pub actor_id: String,
@ -62,7 +62,33 @@ pub struct NewBlog {
const BLOG_PREFIX: &str = "~";
impl Blog {
insert!(blogs, NewBlog);
insert!(blogs, NewBlog, |inserted, conn| {
let instance = inserted.get_instance(conn)?;
if inserted.outbox_url.is_empty() {
inserted.outbox_url = instance.compute_box(
BLOG_PREFIX,
&inserted.actor_id,
"outbox",
);
}
if inserted.inbox_url.is_empty() {
inserted.inbox_url = instance.compute_box(
BLOG_PREFIX,
&inserted.actor_id,
"inbox",
);
}
if inserted.ap_url.is_empty() {
inserted.ap_url = instance.compute_box(
BLOG_PREFIX,
&inserted.actor_id,
"",
);
}
inserted.save_changes(conn).map_err(Error::from)
});
get!(blogs);
find_by!(blogs, find_by_ap_url, ap_url as &str);
find_by!(blogs, find_by_name, actor_id as &str, instance_id as i32);
@ -243,36 +269,6 @@ impl Blog {
Ok(CustomGroup::new(blog, ap_signature))
}
pub fn update_boxes(&self, conn: &Connection) -> Result<()> {
let instance = self.get_instance(conn)?;
if self.outbox_url.is_empty() {
diesel::update(self)
.set(blogs::outbox_url.eq(instance.compute_box(
BLOG_PREFIX,
&self.actor_id,
"outbox",
)))
.execute(conn)?;
}
if self.inbox_url.is_empty() {
diesel::update(self)
.set(blogs::inbox_url.eq(instance.compute_box(
BLOG_PREFIX,
&self.actor_id,
"inbox",
)))
.execute(conn)?;
}
if self.ap_url.is_empty() {
diesel::update(self)
.set(blogs::ap_url.eq(instance.compute_box(BLOG_PREFIX, &self.actor_id, "")))
.execute(conn)?;
}
Ok(())
}
pub fn outbox(&self, conn: &Connection) -> Result<ActivityStream<OrderedCollection>> {
let mut coll = OrderedCollection::default();
coll.collection_props.items = serde_json::to_value(self.get_activities(conn)?)?;
@ -431,12 +427,10 @@ impl NewBlog {
actor_id,
title,
summary,
outbox_url: String::from(""),
inbox_url: String::from(""),
instance_id,
ap_url: String::from(""),
public_key: String::from_utf8(pub_key).or(Err(Error::Signature))?,
private_key: Some(String::from_utf8(priv_key).or(Err(Error::Signature))?),
..NewBlog::default()
})
}
}
@ -461,21 +455,18 @@ pub(crate) mod tests {
"This is a small blog".to_owned(),
Instance::get_local(conn).unwrap().id
).unwrap()).unwrap();
blog1.update_boxes(conn).unwrap();
let blog2 = Blog::insert(conn, NewBlog::new_local(
"MyBlog".to_owned(),
"My blog".to_owned(),
"Welcome to my blog".to_owned(),
Instance::get_local(conn).unwrap().id
).unwrap()).unwrap();
blog2.update_boxes(conn).unwrap();
let blog3 = Blog::insert(conn, NewBlog::new_local(
"WhyILikePlume".to_owned(),
"Why I like Plume".to_owned(),
"In this blog I will explay you why I like Plume so much".to_owned(),
Instance::get_local(conn).unwrap().id
).unwrap()).unwrap();
blog3.update_boxes(conn).unwrap();
BlogAuthor::insert(
conn,
@ -553,7 +544,6 @@ pub(crate) mod tests {
Instance::get_local(conn).unwrap().id,
).unwrap(),
).unwrap();
b1.update_boxes(conn).unwrap();
let b2 = Blog::insert(
conn,
NewBlog::new_local(
@ -563,7 +553,6 @@ pub(crate) mod tests {
Instance::get_local(conn).unwrap().id
).unwrap(),
).unwrap();
b2.update_boxes(conn).unwrap();
let blog = vec![ b1, b2 ];
BlogAuthor::insert(
@ -719,7 +708,6 @@ pub(crate) mod tests {
Instance::get_local(conn).unwrap().id,
).unwrap(),
).unwrap();
b1.update_boxes(conn).unwrap();
let b2 = Blog::insert(
conn,
NewBlog::new_local(
@ -729,7 +717,6 @@ pub(crate) mod tests {
Instance::get_local(conn).unwrap().id,
).unwrap(),
).unwrap();
b2.update_boxes(conn).unwrap();
let blog = vec![ b1, b2 ];
BlogAuthor::insert(

View File

@ -1,6 +1,6 @@
use activitypub::{activity::{Create, Delete}, link, object::{Note, Tombstone}};
use chrono::{self, NaiveDateTime};
use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl};
use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl, SaveChangesDsl};
use serde_json;
use std::collections::HashSet;
@ -20,7 +20,7 @@ use schema::comments;
use users::User;
use {Connection, Error, Result};
#[derive(Queryable, Identifiable, Serialize, Clone)]
#[derive(Queryable, Identifiable, Serialize, Clone, AsChangeset)]
pub struct Comment {
pub id: i32,
pub content: SafeString,
@ -48,7 +48,13 @@ pub struct NewComment {
}
impl Comment {
insert!(comments, NewComment);
insert!(comments, NewComment, |inserted, conn| {
if inserted.ap_url.is_none() {
inserted.ap_url = Some(format!("{}comment/{}", inserted.get_post(conn)?.ap_url, inserted.id));
let _: Comment = inserted.save_changes(conn)?;
}
Ok(inserted)
});
get!(comments);
list_by!(comments, list_by_post, post_id as i32);
find_by!(comments, find_by_ap_url, ap_url as &str);
@ -79,21 +85,6 @@ impl Comment {
.map_err(Error::from)
}
pub fn update_ap_url(&self, conn: &Connection) -> Result<Comment> {
if self.ap_url.is_none() {
diesel::update(self)
.set(comments::ap_url.eq(self.compute_id(conn)?))
.execute(conn)?;
Comment::get(conn, self.id)
} else {
Ok(self.clone())
}
}
pub fn compute_id(&self, conn: &Connection) -> Result<String> {
Ok(format!("{}comment/{}", self.get_post(conn)?.ap_url, self.id))
}
pub fn can_see(&self, conn: &Connection, user: Option<&User>) -> bool {
self.public_visibility ||
user.as_ref().map(|u| CommentSeers::can_see(conn, self, u).unwrap_or(false))
@ -117,7 +108,7 @@ impl Comment {
note.object_props
.set_in_reply_to_link(Id::new(self.in_response_to_id.map_or_else(
|| Ok(Post::get(conn, self.post_id)?.ap_url),
|id| Ok(Comment::get(conn, id)?.compute_id(conn)?) as Result<String>,
|id| Ok(Comment::get(conn, id)?.ap_url.unwrap_or_default()) as Result<String>,
)?))?;
note.object_props
.set_published_string(chrono::Utc::now().to_rfc3339())?;

View File

@ -3,7 +3,7 @@ use activitypub::{
actor::Person,
Actor,
};
use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl};
use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl, SaveChangesDsl};
use blogs::Blog;
use notifications::*;
@ -17,7 +17,7 @@ use schema::follows;
use users::User;
use {ap_url, Connection, BASE_URL, Error, Result};
#[derive(Clone, Queryable, Identifiable, Associations)]
#[derive(Clone, Queryable, Identifiable, Associations, AsChangeset)]
#[belongs_to(User, foreign_key = "following_id")]
pub struct Follow {
pub id: i32,
@ -35,7 +35,14 @@ pub struct NewFollow {
}
impl Follow {
insert!(follows, NewFollow);
insert!(follows, NewFollow, |inserted, conn| {
if inserted.ap_url.is_empty() {
inserted.ap_url = ap_url(&format!("{}/follows/{}", *BASE_URL, inserted.id));
inserted.save_changes(conn).map_err(Error::from)
} else {
Ok(inserted)
}
});
get!(follows);
find_by!(follows, find_by_ap_url, ap_url as &str);
@ -200,3 +207,33 @@ impl IntoId for Follow {
Id::new(self.ap_url)
}
}
#[cfg(test)]
mod tests {
use diesel::Connection;
use super::*;
use tests::db;
use users::tests as user_tests;
#[test]
fn test_id() {
let conn = db();
conn.test_transaction::<_, (), _>(|| {
let users = user_tests::fill_database(&conn);
let follow = Follow::insert(&conn, NewFollow {
follower_id: users[0].id,
following_id: users[1].id,
ap_url: String::new(),
}).expect("Couldn't insert new follow");
assert_eq!(follow.ap_url, format!("https://{}/follows/{}", *BASE_URL, follow.id));
let follow = Follow::insert(&conn, NewFollow {
follower_id: users[1].id,
following_id: users[0].id,
ap_url: String::from("https://some.url/"),
}).expect("Couldn't insert new follow");
assert_eq!(follow.ap_url, String::from("https://some.url/"));
Ok(())
});
}
}

View File

@ -235,13 +235,19 @@ macro_rules! get {
/// ```
macro_rules! insert {
($table:ident, $from:ident) => {
insert!($table, $from, |x, _conn| { Ok(x) });
};
($table:ident, $from:ident, |$val:ident, $conn:ident | $after:block) => {
last!($table);
pub fn insert(conn: &crate::Connection, new: $from) -> Result<Self> {
diesel::insert_into($table::table)
.values(new)
.execute(conn)?;
Self::last(conn)
#[allow(unused_mut)]
let mut $val = Self::last(conn)?;
let $conn = conn;
$after
}
};
}

View File

@ -6,7 +6,7 @@ use activitypub::{
};
use canapi::{Error as ApiError, Provider};
use chrono::{NaiveDateTime, TimeZone, Utc};
use diesel::{self, BelongingToDsl, ExpressionMethods, QueryDsl, RunQueryDsl};
use diesel::{self, BelongingToDsl, ExpressionMethods, QueryDsl, RunQueryDsl, SaveChangesDsl};
use heck::{CamelCase, KebabCase};
use scheduled_thread_pool::ScheduledThreadPool as Worker;
use serde_json;
@ -198,7 +198,6 @@ impl<'a> Provider<(&'a Connection, &'a Worker, &'a Searcher, Option<i32>)> for P
source: query.source.expect("Post API::create: no source error"),
cover_id: query.cover_id,
}, search).map_err(|_| ApiError::NotFound("Creation error".into()))?;
post.update_ap_url(conn).map_err(|_| ApiError::NotFound("Error setting ActivityPub URLs".into()))?;;
PostAuthor::insert(conn, NewPostAuthor {
author_id: author.id,
@ -265,7 +264,17 @@ impl Post {
diesel::insert_into(posts::table)
.values(new)
.execute(conn)?;
let post = Self::last(conn)?;
let mut post = Self::last(conn)?;
if post.ap_url.is_empty() {
post.ap_url = ap_url(&format!(
"{}/~/{}/{}/",
*BASE_URL,
post.get_blog(conn)?.get_fqn(conn),
post.slug
));
let _: Post = post.save_changes(conn)?;
}
searcher.add_document(conn, &post)?;
Ok(post)
}
@ -278,7 +287,6 @@ impl Post {
Ok(post)
}
pub fn list_by_tag(conn: &Connection, tag: String, (min, max): (i32, i32)) -> Result<Vec<Post>> {
use schema::tags;
@ -505,17 +513,6 @@ impl Post {
.map_err(Error::from)
}
pub fn update_ap_url(&self, conn: &Connection) -> Result<Post> {
if self.ap_url.is_empty() {
diesel::update(self)
.set(posts::ap_url.eq(self.compute_id(conn)?))
.execute(conn)?;
Post::get(conn, self.id)
} else {
Ok(self.clone())
}
}
pub fn get_receivers_urls(&self, conn: &Connection) -> Result<Vec<String>> {
let followers = self
.get_authors(conn)?
@ -856,15 +853,6 @@ impl Post {
Ok(format!("/~/{}/{}", blog.get_fqn(conn), self.slug))
}
pub fn compute_id(&self, conn: &Connection) -> Result<String> {
Ok(ap_url(&format!(
"{}/~/{}/{}/",
BASE_URL.as_str(),
self.get_blog(conn)?.get_fqn(conn),
self.slug
)))
}
pub fn cover_url(&self, conn: &Connection) -> Option<String> {
self.cover_id.and_then(|i| Media::get(conn, i).ok()).and_then(|c| c.url(conn).ok())
}

View File

@ -4,7 +4,7 @@ use activitypub::{
};
use bcrypt;
use chrono::{NaiveDateTime, Utc};
use diesel::{self, BelongingToDsl, ExpressionMethods, QueryDsl, RunQueryDsl};
use diesel::{self, BelongingToDsl, ExpressionMethods, QueryDsl, RunQueryDsl, SaveChangesDsl};
use openssl::{
hash::MessageDigest,
pkey::{PKey, Private},
@ -44,7 +44,7 @@ use {ap_url, Connection, BASE_URL, USE_HTTPS, Error, Result};
pub type CustomPerson = CustomObject<ApSignature, Person>;
#[derive(Queryable, Identifiable, Serialize, Deserialize, Clone, Debug)]
#[derive(Queryable, Identifiable, Serialize, Deserialize, Clone, Debug, AsChangeset)]
pub struct User {
pub id: i32,
pub username: String,
@ -66,7 +66,7 @@ pub struct User {
pub last_fetched_date: NaiveDateTime,
}
#[derive(Insertable)]
#[derive(Default, Insertable)]
#[table_name = "users"]
pub struct NewUser {
pub username: String,
@ -90,7 +90,50 @@ pub const AUTH_COOKIE: &str = "user_id";
const USER_PREFIX: &str = "@";
impl User {
insert!(users, NewUser);
insert!(users, NewUser, |inserted, conn| {
let instance = inserted.get_instance(conn)?;
if inserted.outbox_url.is_empty() {
inserted.outbox_url = instance.compute_box(
USER_PREFIX,
&inserted.username,
"outbox",
);
}
if inserted.inbox_url.is_empty() {
inserted.inbox_url = instance.compute_box(
USER_PREFIX,
&inserted.username,
"inbox",
);
}
if inserted.ap_url.is_empty() {
inserted.ap_url = instance.compute_box(
USER_PREFIX,
&inserted.username,
"",
);
}
if inserted.shared_inbox_url.is_none() {
inserted.shared_inbox_url = Some(ap_url(&format!(
"{}/inbox",
Instance::get_local(conn)?
.public_domain
)));
}
if inserted.followers_endpoint.is_empty() {
inserted.followers_endpoint = instance.compute_box(
USER_PREFIX,
&inserted.username,
"followers",
);
}
inserted.save_changes(conn).map_err(Error::from)
});
get!(users);
find_by!(users, find_by_email, email as &str);
find_by!(users, find_by_name, username as &str, instance_id as i32);
@ -392,57 +435,6 @@ impl User {
Ok(())
}
pub fn update_boxes(&self, conn: &Connection) -> Result<()> {
let instance = self.get_instance(conn)?;
if self.outbox_url.is_empty() {
diesel::update(self)
.set(users::outbox_url.eq(instance.compute_box(
USER_PREFIX,
&self.username,
"outbox",
)))
.execute(conn)?;
}
if self.inbox_url.is_empty() {
diesel::update(self)
.set(users::inbox_url.eq(instance.compute_box(
USER_PREFIX,
&self.username,
"inbox",
)))
.execute(conn)?;
}
if self.ap_url.is_empty() {
diesel::update(self)
.set(users::ap_url.eq(instance.compute_box(USER_PREFIX, &self.username, "")))
.execute(conn)?;
}
if self.shared_inbox_url.is_none() {
diesel::update(self)
.set(users::shared_inbox_url.eq(ap_url(&format!(
"{}/inbox",
Instance::get_local(conn)?
.public_domain
))))
.execute(conn)?;
}
if self.followers_endpoint.is_empty() {
diesel::update(self)
.set(users::followers_endpoint.eq(instance.compute_box(
USER_PREFIX,
&self.username,
"followers",
)))
.execute(conn)?;
}
Ok(())
}
pub fn get_local_page(conn: &Connection, (min, max): (i32, i32)) -> Result<Vec<User>> {
users::table
.filter(users::instance_id.eq(Instance::get_local(conn)?.id))
@ -917,22 +909,15 @@ impl NewUser {
NewUser {
username,
display_name,
outbox_url: String::from(""),
inbox_url: String::from(""),
is_admin,
summary: SafeString::new(summary),
email: Some(email),
hashed_password: Some(password),
instance_id: Instance::get_local(conn)?.id,
ap_url: String::from(""),
public_key: String::from_utf8(pub_key)
.expect("NewUser::new_local: public key error"),
private_key: Some(
String::from_utf8(priv_key).expect("NewUser::new_local: private key error"),
),
shared_inbox_url: None,
followers_endpoint: String::from(""),
avatar_id: None,
ap_url: String::new(),
public_key: String::from_utf8(pub_key).or(Err(Error::Signature))?,
private_key: Some(String::from_utf8(priv_key).or(Err(Error::Signature))?),
..NewUser::default()
},
)
}
@ -958,7 +943,6 @@ pub(crate) mod tests {
"admin@example.com".to_owned(),
"invalid_admin_password".to_owned(),
).unwrap();
admin.update_boxes(conn).unwrap();
let user = NewUser::new_local(
conn,
"user".to_owned(),
@ -968,7 +952,6 @@ pub(crate) mod tests {
"user@example.com".to_owned(),
"invalid_user_password".to_owned(),
).unwrap();
user.update_boxes(conn).unwrap();
let other = NewUser::new_local(
conn,
"other".to_owned(),
@ -978,7 +961,6 @@ pub(crate) mod tests {
"other@example.com".to_owned(),
"invalid_other_password".to_owned(),
).unwrap();
other.update_boxes(conn).unwrap();
vec![ admin, user, other ]
}
@ -996,7 +978,6 @@ pub(crate) mod tests {
"test@example.com".to_owned(),
User::hash_pass("test_password").unwrap(),
).unwrap();
test_user.update_boxes(conn).unwrap();
assert_eq!(
test_user.id,
@ -1097,7 +1078,6 @@ pub(crate) mod tests {
"test@example.com".to_owned(),
User::hash_pass("test_password").unwrap(),
).unwrap();
test_user.update_boxes(conn).unwrap();
assert!(test_user.auth("test_password"));
assert!(!test_user.auth("other_password"));

View File

@ -40,7 +40,7 @@ msgstr ""
msgid "You need to be logged in order to create a new blog"
msgstr ""
# src/routes/blogs.rs:136
# src/routes/blogs.rs:135
msgid "You are not allowed to delete this blog."
msgstr ""
@ -92,15 +92,15 @@ msgstr ""
msgid "Sorry, but the link expired. Try again"
msgstr ""
# src/routes/user.rs:131
# src/routes/user.rs:126
msgid "You need to be logged in order to access your dashboard"
msgstr ""
# src/routes/user.rs:164
# src/routes/user.rs:159
msgid "You need to be logged in order to subscribe to someone"
msgstr ""
# src/routes/user.rs:245
# src/routes/user.rs:240
msgid "You need to be logged in order to edit your profile"
msgstr ""

View File

@ -105,7 +105,6 @@ pub fn create(conn: DbConn, form: LenientForm<NewBlogForm>, user: User, intl: I1
String::from(""),
Instance::get_local(&*conn).expect("blog::create: instance error").id
).expect("blog::create: new local error")).expect("blog::create: error");
blog.update_boxes(&*conn).expect("blog::create: insert error");
BlogAuthor::insert(&*conn, NewBlogAuthor {
blog_id: blog.id,

View File

@ -53,7 +53,7 @@ pub fn create(blog_name: String, slug: String, form: LenientForm<NewCommentForm>
sensitive: !form.warning.is_empty(),
spoiler_text: form.warning.clone(),
public_visibility: true
}).expect("comments::create: insert error").update_ap_url(&*conn).expect("comments::create: update ap url error");
}).expect("comments::create: insert error");
let new_comment = comm.create_activity(&*conn).expect("comments::create: activity error");
// save mentions

View File

@ -227,7 +227,6 @@ pub fn update(blog: String, slug: String, user: User, cl: ContentLen, form: Leni
post.license = form.license.clone();
post.cover_id = form.cover;
post.update(&*conn, &searcher).expect("post::update: update error");;
let post = post.update_ap_url(&*conn).expect("post::update: update ap url error");
if post.published {
post.update_mentions(&conn, mentions.into_iter().filter_map(|m| Mention::build_activity(&conn, &m).ok()).collect())
@ -338,7 +337,7 @@ pub fn create(blog_name: String, form: LenientForm<NewPostForm>, user: User, cl:
},
&searcher,
).expect("post::create: post save error");
let post = post.update_ap_url(&*conn).expect("post::create: update ap url error");
PostAuthor::insert(&*conn, NewPostAuthor {
post_id: post.id,
author_id: user.id

View File

@ -78,18 +78,13 @@ pub fn details(
let user_clone = user.clone();
worker.execute(move || {
for user_id in user_clone.fetch_followers_ids().expect("Remote user: fetching followers error") {
let follower =
User::find_by_ap_url(&*fetch_followers_conn, &user_id)
.unwrap_or_else(|_| {
User::fetch_from_url(&*fetch_followers_conn, &user_id)
.expect("user::details: Couldn't fetch follower")
});
let follower = User::from_url(&*fetch_followers_conn, &user_id).expect("user::details: Couldn't fetch follower");
follows::Follow::insert(
&*fetch_followers_conn,
follows::NewFollow {
follower_id: follower.id,
following_id: user_clone.id,
ap_url: format!("{}/follow/{}", follower.ap_url, user_clone.ap_url),
ap_url: String::new(),
},
).expect("Couldn't save follower for remote user");
}
@ -147,7 +142,7 @@ pub fn follow(name: String, conn: DbConn, user: User, worker: Worker) -> Result<
follows::NewFollow {
follower_id: user.id,
following_id: target.id,
ap_url: format!("{}/follow/{}", user.ap_url, target.ap_url),
ap_url: String::new(),
},
)?;
f.notify(&*conn)?;
@ -359,7 +354,7 @@ pub fn create(conn: DbConn, form: LenientForm<NewUserForm>, intl: I18n) -> Resul
"",
form.email.to_string(),
User::hash_pass(&form.password).map_err(to_validation)?,
).and_then(|u| u.update_boxes(&*conn)).map_err(to_validation)?;
).map_err(to_validation)?;
Ok(Redirect::to(uri!(super::session::new: m = _)))
})
.map_err(|err| {