From 6c7ddedd342553b53dd98c8de9cbe9e8e2e8cd7c Mon Sep 17 00:00:00 2001 From: Michael Manfre Date: Sun, 27 Nov 2022 13:09:46 -0500 Subject: [PATCH] Post editing --- activities/models/fan_out.py | 86 +++++++++++++++++++--------- activities/models/post.py | 40 +++++++++++++ activities/views/posts.py | 85 +++++++++++++++++++-------- requirements-dev.txt | 1 + static/css/style.css | 8 ++- stator/graph.py | 3 + stator/runner.py | 52 ++++++++++------- takahe/settings.py | 5 ++ takahe/urls.py | 1 + templates/activities/_post.html | 23 +++++--- templates/activities/compose.html | 3 +- tests/activities/models/test_post.py | 47 ++++++++++++++- tests/activities/views/test_posts.py | 44 +++++++++++++- tests/conftest.py | 26 +++++++++ 14 files changed, 341 insertions(+), 83 deletions(-) diff --git a/activities/models/fan_out.py b/activities/models/fan_out.py index 64df929..a86e30a 100644 --- a/activities/models/fan_out.py +++ b/activities/models/fan_out.py @@ -17,11 +17,15 @@ class FanOutStates(StateGraph): """ Sends the fan-out to the right inbox. """ + LOCAL_IDENTITY = True + REMOTE_IDENTITY = False + fan_out = await instance.afetch_full() - # Handle Posts - if fan_out.type == FanOut.Types.post: - post = await fan_out.subject_post.afetch_full() - if fan_out.identity.local: + + match (fan_out.type, fan_out.identity.local): + # Handle creating/updating local posts + case (FanOut.Types.post | FanOut.Types.post_edited, LOCAL_IDENTITY): + post = await fan_out.subject_post.afetch_full() # Make a timeline event directly # If it's a reply, we only add it if we follow at least one # of the people mentioned. @@ -44,63 +48,91 @@ class FanOutStates(StateGraph): identity=fan_out.identity, post=post, ) - else: + + # Handle sending remote posts create + case (FanOut.Types.post, REMOTE_IDENTITY): + post = await fan_out.subject_post.afetch_full() # Sign it and send it await post.author.signed_request( method="post", uri=fan_out.identity.inbox_uri, body=canonicalise(post.to_create_ap()), ) - # Handle deleting posts - elif fan_out.type == FanOut.Types.post_deleted: - post = await fan_out.subject_post.afetch_full() - if fan_out.identity.local: - # Remove all timeline events mentioning it - await TimelineEvent.objects.filter( - identity=fan_out.identity, - subject_post=post, - ).adelete() - else: + + # Handle sending remote posts update + case (FanOut.Types.post_edited, REMOTE_IDENTITY): + post = await fan_out.subject_post.afetch_full() + # Sign it and send it + await post.author.signed_request( + method="post", + uri=fan_out.identity.inbox_uri, + body=canonicalise(post.to_update_ap()), + ) + + # Handle deleting local posts + case (FanOut.Types.post_deleted, LOCAL_IDENTITY): + post = await fan_out.subject_post.afetch_full() + if fan_out.identity.local: + # Remove all timeline events mentioning it + await TimelineEvent.objects.filter( + identity=fan_out.identity, + subject_post=post, + ).adelete() + + # Handle sending remote post deletes + case (FanOut.Types.post_deleted, REMOTE_IDENTITY): + post = await fan_out.subject_post.afetch_full() # Send it to the remote inbox await post.author.signed_request( method="post", uri=fan_out.identity.inbox_uri, body=canonicalise(post.to_delete_ap()), ) - # Handle boosts/likes - elif fan_out.type == FanOut.Types.interaction: - interaction = await fan_out.subject_post_interaction.afetch_full() - if fan_out.identity.local: + + # Handle local boosts/likes + case (FanOut.Types.interaction, LOCAL_IDENTITY): + interaction = await fan_out.subject_post_interaction.afetch_full() # Make a timeline event directly await sync_to_async(TimelineEvent.add_post_interaction)( identity=fan_out.identity, interaction=interaction, ) - else: + + # Handle sending remote boosts/likes + case (FanOut.Types.interaction, REMOTE_IDENTITY): + interaction = await fan_out.subject_post_interaction.afetch_full() # Send it to the remote inbox await interaction.identity.signed_request( method="post", uri=fan_out.identity.inbox_uri, body=canonicalise(interaction.to_ap()), ) - # Handle undoing boosts/likes - elif fan_out.type == FanOut.Types.undo_interaction: - interaction = await fan_out.subject_post_interaction.afetch_full() - if fan_out.identity.local: + + # Handle undoing local boosts/likes + case (FanOut.Types.undo_interaction, LOCAL_IDENTITY): # noqa:F841 + interaction = await fan_out.subject_post_interaction.afetch_full() + # Delete any local timeline events await sync_to_async(TimelineEvent.delete_post_interaction)( identity=fan_out.identity, interaction=interaction, ) - else: + + # Handle sending remote undoing boosts/likes + case (FanOut.Types.undo_interaction, REMOTE_IDENTITY): # noqa:F841 + interaction = await fan_out.subject_post_interaction.afetch_full() # Send an undo to the remote inbox await interaction.identity.signed_request( method="post", uri=fan_out.identity.inbox_uri, body=canonicalise(interaction.to_undo_ap()), ) - else: - raise ValueError(f"Cannot fan out with type {fan_out.type}") + + case _: + raise ValueError( + f"Cannot fan out with type {fan_out.type} local={fan_out.identity.local}" + ) + return cls.sent diff --git a/activities/models/post.py b/activities/models/post.py index f75c526..23194b3 100644 --- a/activities/models/post.py +++ b/activities/models/post.py @@ -22,9 +22,17 @@ class PostStates(StateGraph): deleted = State(try_interval=300) deleted_fanned_out = State() + edited = State(try_interval=300) + edited_fanned_out = State(externally_progressed=True) + new.transitions_to(fanned_out) fanned_out.transitions_to(deleted) + fanned_out.transitions_to(edited) + deleted.transitions_to(deleted_fanned_out) + edited.transitions_to(edited_fanned_out) + edited_fanned_out.transitions_to(edited) + edited_fanned_out.transitions_to(deleted) @classmethod async def handle_new(cls, instance: "Post"): @@ -56,6 +64,21 @@ class PostStates(StateGraph): ) return cls.deleted_fanned_out + @classmethod + async def handle_edited(cls, instance: "Post"): + """ + Creates all needed fan-out objects for an edited Post. + """ + post = await instance.afetch_full() + # Fan out to each target + for follow in await post.aget_targets(): + await FanOut.objects.acreate( + identity=follow, + type=FanOut.Types.post_edited, + subject_post=post, + ) + return cls.edited_fanned_out + class Post(StatorModel): """ @@ -140,6 +163,7 @@ class Post(StatorModel): action_boost = "{view}boost/" action_unboost = "{view}unboost/" action_delete = "{view}delete/" + action_edit = "{view}edit/" action_reply = "/compose/?reply_to={self.id}" def get_scheme(self, url): @@ -305,6 +329,8 @@ class Post(StatorModel): value["summary"] = self.summary if self.in_reply_to: value["inReplyTo"] = self.in_reply_to + if self.edited: + value["updated"] = format_ld_date(self.edited) # Mentions for mention in self.mentions.all(): value["tag"].append( @@ -336,6 +362,20 @@ class Post(StatorModel): "object": object, } + def to_update_ap(self): + """ + Returns the AP JSON to update this object + """ + object = self.to_ap() + return { + "to": object["to"], + "cc": object.get("cc", []), + "type": "Update", + "id": self.object_uri + "#update", + "actor": self.author.actor_uri, + "object": object, + } + def to_delete_ap(self): """ Returns the AP JSON to create this object diff --git a/activities/views/posts.py b/activities/views/posts.py index 59b1f56..5d7b0c9 100644 --- a/activities/views/posts.py +++ b/activities/views/posts.py @@ -1,6 +1,8 @@ from django import forms -from django.http import Http404, JsonResponse +from django.core.exceptions import PermissionDenied +from django.http import JsonResponse from django.shortcuts import get_object_or_404, redirect, render +from django.utils import timezone from django.utils.decorators import method_decorator from django.views.generic import FormView, TemplateView, View @@ -143,11 +145,11 @@ class Delete(TemplateView): template_name = "activities/post_delete.html" def dispatch(self, request, handle, post_id): + # Make sure the request identity owns the post! + if handle != request.identity.handle: + raise PermissionDenied("Post author is not requestor") self.identity = by_handle_or_404(self.request, handle, local=False) self.post_obj = get_object_or_404(self.identity.posts, pk=post_id) - # Make sure the request identity owns the post! - if self.post_obj.author != request.identity: - raise Http404("Post author is not requestor") return super().dispatch(request) def get_context_data(self): @@ -164,6 +166,10 @@ class Compose(FormView): template_name = "activities/compose.html" class form_class(forms.Form): + id = forms.IntegerField( + required=False, + widget=forms.HiddenInput(), + ) text = forms.CharField( widget=forms.Textarea( @@ -206,33 +212,64 @@ class Compose(FormView): def get_initial(self): initial = super().get_initial() - initial[ - "visibility" - ] = self.request.identity.config_identity.default_post_visibility - if self.reply_to: - initial["reply_to"] = self.reply_to.pk - initial["visibility"] = self.reply_to.visibility - initial["text"] = f"@{self.reply_to.author.handle} " + if self.post_obj: + initial.update( + { + "id": self.post_obj.id, + "reply_to": self.reply_to.pk if self.reply_to else "", + "visibility": self.post_obj.visibility, + "text": self.post_obj.content, + "content_warning": self.post_obj.summary, + } + ) + else: + initial[ + "visibility" + ] = self.request.identity.config_identity.default_post_visibility + if self.reply_to: + initial["reply_to"] = self.reply_to.pk + initial["visibility"] = self.reply_to.visibility + initial["text"] = f"@{self.reply_to.author.handle} " return initial def form_valid(self, form): - post = Post.create_local( - author=self.request.identity, - content=form.cleaned_data["text"], - summary=form.cleaned_data.get("content_warning"), - visibility=form.cleaned_data["visibility"], - reply_to=self.reply_to, - ) - # Add their own timeline event for immediate visibility - TimelineEvent.add_post(self.request.identity, post) + post_id = form.cleaned_data.get("id") + if post_id: + post = get_object_or_404(self.request.identity.posts, pk=post_id) + post.edited = timezone.now() + post.content = form.cleaned_data["text"] + post.summary = form.cleaned_data.get("content_warning") + post.visibility = form.cleaned_data["visibility"] + post.save() + + # Should there be a timeline event for edits? + # E.g. "@user edited #123" + + post.transition_perform(PostStates.edited) + else: + post = Post.create_local( + author=self.request.identity, + content=form.cleaned_data["text"], + summary=form.cleaned_data.get("content_warning"), + visibility=form.cleaned_data["visibility"], + reply_to=self.reply_to, + ) + # Add their own timeline event for immediate visibility + TimelineEvent.add_post(self.request.identity, post) return redirect("/") - def dispatch(self, request, *args, **kwargs): + def dispatch(self, request, handle=None, post_id=None, *args, **kwargs): + self.post_obj = None + if handle and post_id: + # Make sure the request identity owns the post! + if handle != request.identity.handle: + raise PermissionDenied("Post author is not requestor") + + self.post_obj = get_object_or_404(request.identity.posts, pk=post_id) + # Grab the reply-to post info now self.reply_to = None - reply_to_id = self.request.POST.get("reply_to") or self.request.GET.get( - "reply_to" - ) + reply_to_id = request.POST.get("reply_to") or request.GET.get("reply_to") if reply_to_id: try: self.reply_to = Post.objects.get(pk=reply_to_id) diff --git a/requirements-dev.txt b/requirements-dev.txt index 7f49057..4ff8166 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -4,5 +4,6 @@ black==22.10.0 flake8==5.0.4 isort==5.10.1 mock~=4.0.3 +pytest-asyncio~=0.20.2 pytest-django~=4.5.2 pytest-httpx~=0.21 diff --git a/static/css/style.css b/static/css/style.css index dae2253..93ad329 100644 --- a/static/css/style.css +++ b/static/css/style.css @@ -768,11 +768,17 @@ h1.identity small { content: "HIDE"; } +.post .edited { + margin-left: 64px; + font-weight: lighter; + color: var(--color-text-duller); +} + .post .content { margin-left: 64px; } -.post.mini .content { +.post.mini .content, .post.mini .edited { margin-left: 0px; } diff --git a/stator/graph.py b/stator/graph.py index 436638b..424ea49 100644 --- a/stator/graph.py +++ b/stator/graph.py @@ -104,6 +104,9 @@ class State: def __repr__(self): return f"" + def __str__(self): + return self.name + def __eq__(self, other): if isinstance(other, State): return self is other diff --git a/stator/runner.py b/stator/runner.py index 48549bc..c78437d 100644 --- a/stator/runner.py +++ b/stator/runner.py @@ -52,28 +52,11 @@ class StatorRunner: Config.system = await Config.aload_system() print(f"{self.handled} tasks processed so far") print("Running cleaning and scheduling") - for model in self.models: - asyncio.create_task(model.atransition_clean_locks()) - asyncio.create_task(model.atransition_schedule_due()) - self.last_clean = time.monotonic() - # Calculate space left for tasks + await self.run_cleanup() + self.remove_completed_tasks() - space_remaining = self.concurrency - len(self.tasks) - # Fetch new tasks - for model in self.models: - if space_remaining > 0: - for instance in await model.atransition_get_with_lock( - number=min(space_remaining, self.concurrency_per_model), - lock_expiry=( - timezone.now() - + datetime.timedelta(seconds=self.lock_expiry) - ), - ): - self.tasks.append( - asyncio.create_task(self.run_transition(instance)) - ) - self.handled += 1 - space_remaining -= 1 + await self.fetch_and_process_tasks() + # Are we in limited run mode? if self.run_for and (time.monotonic() - self.started) > self.run_for: break @@ -92,6 +75,33 @@ class StatorRunner: print("Complete") return self.handled + async def run_cleanup(self): + """ + Do any transition cleanup tasks + """ + for model in self.models: + asyncio.create_task(model.atransition_clean_locks()) + asyncio.create_task(model.atransition_schedule_due()) + self.last_clean = time.monotonic() + + async def fetch_and_process_tasks(self): + # Calculate space left for tasks + space_remaining = self.concurrency - len(self.tasks) + # Fetch new tasks + for model in self.models: + if space_remaining > 0: + for instance in await model.atransition_get_with_lock( + number=min(space_remaining, self.concurrency_per_model), + lock_expiry=( + timezone.now() + datetime.timedelta(seconds=self.lock_expiry) + ), + ): + self.tasks.append( + asyncio.create_task(self.run_transition(instance)) + ) + self.handled += 1 + space_remaining -= 1 + async def run_transition(self, instance: StatorModel): """ Wrapper for atransition_attempt with fallback error handling diff --git a/takahe/settings.py b/takahe/settings.py index cec4f2d..0c6c2c5 100644 --- a/takahe/settings.py +++ b/takahe/settings.py @@ -30,6 +30,11 @@ TAKAHE_ENV_FILE = os.environ.get( ) +TAKAHE_ENV_FILE = os.environ.get( + "TAKAHE_ENV_FILE", "test.env" if "pytest" in sys.modules else ".env" +) + + class Settings(BaseSettings): """ Pydantic-powered settings, to provide consistent error messages, strong diff --git a/takahe/urls.py b/takahe/urls.py index 98e1050..6f5ac79 100644 --- a/takahe/urls.py +++ b/takahe/urls.py @@ -106,6 +106,7 @@ urlpatterns = [ path("@/posts//boost/", posts.Boost.as_view()), path("@/posts//unboost/", posts.Boost.as_view(undo=True)), path("@/posts//delete/", posts.Delete.as_view()), + path("@/posts//edit/", posts.Compose.as_view()), # Authentication path("auth/login/", auth.Login.as_view(), name="login"), path("auth/logout/", auth.Logout.as_view(), name="logout"), diff --git a/templates/activities/_post.html b/templates/activities/_post.html index ebe5696..e294698 100644 --- a/templates/activities/_post.html +++ b/templates/activities/_post.html @@ -18,13 +18,11 @@ {% elif post.visibility == 4 %} {% endif %} - - {% if post.published %} - {{ post.published | timedeltashort }} - {% else %} - {{ post.created | timedeltashort }} - {% endif %} - + {% if post.published %} + {{ post.published | timedeltashort }} + {% else %} + {{ post.created | timedeltashort }} + {% endif %} {% if request.identity %} @@ -32,14 +30,19 @@ {% include "activities/_reply.html" %} {% include "activities/_like.html" %} {% include "activities/_boost.html" %} + {% if post.author == request.identity %} + + Edit + Delete + {% endif %} {% endif %} @@ -57,6 +60,12 @@ {{ post.safe_content_local }} + {% if post.edited %} +
+ Edited {{ post.edited | timedeltashort }} ago +
+ {% endif %} + {% if post.attachments.exists %}
{% for attachment in post.attachments.all %} diff --git a/templates/activities/compose.html b/templates/activities/compose.html index 1a02227..e705f97 100644 --- a/templates/activities/compose.html +++ b/templates/activities/compose.html @@ -12,12 +12,13 @@ {% include "activities/_mini_post.html" with post=reply_to %} {% endif %} {{ form.reply_to }} + {{ form.id }} {% include "forms/_field.html" with field=form.text %} {% include "forms/_field.html" with field=form.content_warning %} {% include "forms/_field.html" with field=form.visibility %}
- +
{% endblock %} diff --git a/tests/activities/models/test_post.py b/tests/activities/models/test_post.py index 9d5207c..baeb55a 100644 --- a/tests/activities/models/test_post.py +++ b/tests/activities/models/test_post.py @@ -1,7 +1,10 @@ +import asyncio + import pytest +from asgiref.sync import async_to_sync from pytest_httpx import HTTPXMock -from activities.models import Post +from activities.models import Post, PostStates @pytest.mark.django_db @@ -112,3 +115,45 @@ def test_linkify_mentions_local(identity, remote_identity): local=True, ) assert post.safe_content_local() == "

@test@example.com, welcome!

" + + +async def stator_process_tasks(stator): + """ + Guarded wrapper to simply async_to_sync and ensure all stator tasks are + run to completion without blocking indefinitely. + """ + await asyncio.wait_for(stator.fetch_and_process_tasks(), timeout=1) + for _ in range(100): + if not stator.tasks: + break + stator.remove_completed_tasks() + await asyncio.sleep(0.01) + + +@pytest.mark.django_db +def test_post_transitions(identity, stator_runner): + + # Create post + post = Post.objects.create( + content="

Hello!

", + author=identity, + local=False, + visibility=Post.Visibilities.mentioned, + ) + # Test: | --> new --> fanned_out + assert post.state == str(PostStates.new) + async_to_sync(stator_process_tasks)(stator_runner) + post = Post.objects.get(id=post.id) + assert post.state == str(PostStates.fanned_out) + + # Test: fanned_out --> (forced) edited --> edited_fanned_out + Post.transition_perform(post, PostStates.edited) + async_to_sync(stator_process_tasks)(stator_runner) + post = Post.objects.get(id=post.id) + assert post.state == str(PostStates.edited_fanned_out) + + # Test: edited_fanned_out --> (forced) deleted --> deleted_fanned_out + Post.transition_perform(post, PostStates.deleted) + async_to_sync(stator_process_tasks)(stator_runner) + post = Post.objects.get(id=post.id) + assert post.state == str(PostStates.deleted_fanned_out) diff --git a/tests/activities/views/test_posts.py b/tests/activities/views/test_posts.py index b04c30f..c73dcd6 100644 --- a/tests/activities/views/test_posts.py +++ b/tests/activities/views/test_posts.py @@ -2,8 +2,10 @@ import re import mock import pytest +from django.core.exceptions import PermissionDenied -from activities.views.posts import Compose +from activities.models import Post +from activities.views.posts import Compose, Delete @pytest.mark.django_db @@ -22,3 +24,43 @@ def test_content_warning_text(identity, user, rf, config_system): assert re.search( r"\s*Content Summary\s*", content, flags=re.MULTILINE ) + + +@pytest.mark.django_db +def test_post_delete_security(identity, user, rf, other_identity): + # Create post + other_post = Post.objects.create( + content="

OTHER POST!

", + author=other_identity, + local=True, + visibility=Post.Visibilities.public, + ) + + request = rf.post(other_post.get_absolute_url() + "delete/") + request.user = user + request.identity = identity + + view = Delete.as_view() + with pytest.raises(PermissionDenied) as ex: + view(request, handle=other_identity.handle.lstrip("@"), post_id=other_post.id) + assert str(ex.value) == "Post author is not requestor" + + +@pytest.mark.django_db +def test_post_edit_security(identity, user, rf, other_identity): + # Create post + other_post = Post.objects.create( + content="

OTHER POST!

", + author=other_identity, + local=True, + visibility=Post.Visibilities.public, + ) + + request = rf.get(other_post.get_absolute_url() + "edit/") + request.user = user + request.identity = identity + + view = Compose.as_view() + with pytest.raises(PermissionDenied) as ex: + view(request, handle=other_identity.handle.lstrip("@"), post_id=other_post.id) + assert str(ex.value) == "Post author is not requestor" diff --git a/tests/conftest.py b/tests/conftest.py index d506c5c..a3feaca 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,9 @@ +import time + import pytest from core.models import Config +from stator.runner import StatorModel, StatorRunner from users.models import Domain, Identity, User @@ -120,3 +123,26 @@ def remote_identity() -> Identity: name="Test Remote User", local=False, ) + + +@pytest.fixture +def stator_runner(config_system) -> StatorRunner: + """ + Return an initialized StatorRunner for tests that need state transitioning + to happen. + + Example: + # Do some tasks with state side effects + async_to_sync(stator_runner.fetch_and_process_tasks)() + """ + runner = StatorRunner( + StatorModel.subclasses, + concurrency=100, + schedule_interval=30, + ) + runner.handled = 0 + runner.started = time.monotonic() + runner.last_clean = time.monotonic() - runner.schedule_interval + runner.tasks = [] + + return runner