Merge branch 'make-sure-person-profiles-are-giving-404' into 'master'

Make sure a person profile page returns 404

See merge request framasoft/mobilizon!604

js/src/mixins/group.ts

@@ -1,5 +1,6 @@
 import { PERSON_MEMBERSHIPS, CURRENT_ACTOR_CLIENT } from "@/graphql/actor";
 import { FETCH_GROUP } from "@/graphql/group";
+import RouteName from "@/router/name";
 import { Group, IActor, IGroup, IPerson, MemberRole } from "@/types/actor";
 import { Component, Vue } from "vue-property-decorator";
 
@@ -16,6 +17,9 @@ import { Component, Vue } from "vue-property-decorator";
       skip() {
         return !this.$route.params.preferredUsername;
       },
+      error({ graphQLErrors }) {
+        this.handleErrors(graphQLErrors);
+      },
     },
     person: {
       query: PERSON_MEMBERSHIPS,
@@ -46,4 +50,13 @@ export default class GroupMixin extends Vue {
       )
     );
   }
+
+  handleErrors(errors: any[]) {
+    if (
+      errors.some((error) => error.status_code === 404) ||
+      errors.some(({ message }) => message.includes("has invalid value $uuid"))
+    ) {
+      this.$router.replace({ name: RouteName.PAGE_NOT_FOUND });
+    }
+  }
 }

lib/graphql/error.ex

@@ -87,6 +87,7 @@ defmodule Mobilizon.GraphQL.Error do
   defp metadata(:user_not_found), do: {404, dgettext("errors", "User not found")}
   defp metadata(:post_not_found), do: {404, dgettext("errors", "Post not found")}
   defp metadata(:event_not_found), do: {404, dgettext("errors", "Event not found")}
+  defp metadata(:group_not_found), do: {404, dgettext("errors", "Group not found")}
   defp metadata(:unknown), do: {500, dgettext("errors", "Something went wrong")}
 
   defp metadata(code) do

lib/graphql/resolvers/group.ex

@@ -38,7 +38,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Group do
         find_group(parent, args, nil)
 
       _ ->
-        {:error, dgettext("errors", "Group with name %{name} not found", name: name)}
+        {:error, :group_not_found}
     end
   end
 
@@ -52,7 +52,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Group do
       {:ok, actor}
     else
       _ ->
-        {:error, dgettext("errors", "Group with name %{name} not found", name: name)}
+        {:error, :group_not_found}
     end
   end
 

lib/web/controllers/page_controller.ex

@@ -4,6 +4,7 @@ defmodule Mobilizon.Web.PageController do
   """
   use Mobilizon.Web, :controller
 
+  alias Mobilizon.Actors.Actor
   alias Mobilizon.Discussions.Comment
   alias Mobilizon.Events.Event
   alias Mobilizon.Federation.ActivityPub
@@ -28,7 +29,7 @@ defmodule Mobilizon.Web.PageController do
   @spec actor(Plug.Conn.t(), map) :: {:error, :not_found} | Plug.Conn.t()
   def actor(conn, %{"name" => name}) do
     {status, actor} = Cache.get_actor_by_name(name)
-    render_or_error(conn, &ok_status?/3, status, :actor, actor)
+    render_or_error(conn, &checks?/3, status, :actor, actor)
   end
 
   @spec event(Plug.Conn.t(), map) :: {:error, :not_found} | Plug.Conn.t()
@@ -140,16 +141,20 @@ defmodule Mobilizon.Web.PageController do
   defp is_visible?(_), do: true
 
   defp ok_status?(status), do: status in [:ok, :commit]
-  defp ok_status?(_conn, status, _), do: ok_status?(status)
 
   defp ok_status_and_is_visible?(_conn, status, o),
     do: ok_status?(status) and is_visible?(o)
 
   defp checks?(conn, status, o) do
-    if ok_status_and_is_visible?(conn, status, o) do
-      if is_local?(o) == :remote && get_format(conn) == "activity-json", do: :remote, else: true
-    else
-      false
+    cond do
+      ok_status_and_is_visible?(conn, status, o) ->
+        if is_local?(o) == :remote && get_format(conn) == "activity-json", do: :remote, else: true
+
+      is_person?(o) && get_format(conn) == "activity-json" ->
+        true
+
+      true ->
+        false
     end
   end
 
@@ -162,4 +167,7 @@ defmodule Mobilizon.Web.PageController do
   end
 
   defp maybe_add_noindex_header(conn, _), do: conn
+
+  defp is_person?(%Actor{type: :Person}), do: true
+  defp is_person?(_), do: false
 end

lib/web/views/utils.ex

@@ -19,7 +19,9 @@ defmodule Mobilizon.Web.Views.Utils do
 
   @spec replace_meta(String.t(), String.t()) :: String.t()
   defp replace_meta(index_content, tags) do
-    String.replace(index_content, "<meta name=\"server-injected-data\">", tags)
+    index_content
+    |> String.replace("<meta name=\"server-injected-data\">", tags)
+    |> String.replace("<meta name=\"server-injected-data\" />", tags)
   end
 
   @spec do_replacements(String.t(), String.t(), String.t()) :: {:safe, String.t()}

priv/gettext/fr/LC_MESSAGES/errors.po

@@ -454,12 +454,12 @@ msgstr "Participant·e non trouvé·e"
 #, elixir-format
 #: lib/graphql/resolvers/person.ex:31
 msgid "Person with ID %{id} not found"
-msgstr "Groupe avec l'ID %{id} non trouvé"
+msgstr "Personne avec l'ID %{id} non trouvé"
 
 #, elixir-format
 #: lib/graphql/resolvers/person.ex:52
 msgid "Person with username %{username} not found"
-msgstr "Groupe avec le nom %{name} non trouvé"
+msgstr "Personne avec le nom %{name} non trouvé"
 
 #, elixir-format
 #: lib/graphql/resolvers/picture.ex:45

test/graphql/resolvers/group_test.exs

@@ -210,8 +210,7 @@ defmodule Mobilizon.Web.Resolvers.GroupTest do
 
       assert res["data"]["group"] == nil
 
-      assert hd(res["errors"])["message"] ==
-               "Group with name #{@non_existent_username} not found"
+      assert hd(res["errors"])["message"] == "Group not found"
     end
 
     test "find_group doesn't list group members access if group is private", %{

test/web/controllers/page_controller_test.exs

@@ -13,20 +13,30 @@ defmodule Mobilizon.Web.PageControllerTest do
     {:ok, conn: conn}
   end
 
-  test "GET /", %{conn: conn} do
-    conn = get(conn, "/")
-    assert html_response(conn, 200)
+  describe "GET /" do
+    test "GET /", %{conn: conn} do
+      conn = get(conn, "/")
+      assert html_response(conn, 200)
+    end
   end
 
-  test "GET /@actor with existing actor", %{conn: conn} do
-    actor = insert(:actor)
-    conn = get(conn, Actor.build_url(actor.preferred_username, :page))
-    assert html_response(conn, 200) =~ actor.preferred_username
-  end
+  describe "GET /@actor" do
+    test "GET /@actor with existing group", %{conn: conn} do
+      actor = insert(:group)
+      conn = get(conn, Actor.build_url(actor.preferred_username, :page))
+      assert html_response(conn, 200) =~ actor.preferred_username
+    end
 
-  test "GET /@actor with not existing actor", %{conn: conn} do
-    conn = get(conn, Actor.build_url("not_existing", :page))
-    assert html_response(conn, 404)
+    test "GET /@actor with existing person", %{conn: conn} do
+      actor = insert(:actor, visibility: :private)
+      conn = get(conn, Actor.build_url(actor.preferred_username, :page))
+      assert html_response(conn, 404)
+    end
+
+    test "GET /@actor with not existing group", %{conn: conn} do
+      conn = get(conn, Actor.build_url("not_existing", :page))
+      assert html_response(conn, 404)
+    end
   end
 
   test "GET /events/:uuid", %{conn: conn} do