From cd5418825b4a4bf7c420d913328ca7b807a71075 Mon Sep 17 00:00:00 2001
From: Thomas Citharel <tcit@tcit.fr>
Date: Mon, 12 Oct 2020 12:16:36 +0200
Subject: [PATCH] Make sure a person profile page returns 404
Signed-off-by: Thomas Citharel <tcit@tcit.fr>
---
js/src/mixins/group.ts | 13 ++++++++
lib/graphql/error.ex | 1 +
lib/graphql/resolvers/group.ex | 4 +--
lib/web/controllers/page_controller.ex | 20 ++++++++----
lib/web/views/utils.ex | 4 ++-
priv/gettext/fr/LC_MESSAGES/errors.po | 4 +--
test/graphql/resolvers/group_test.exs | 3 +-
test/web/controllers/page_controller_test.exs | 32 ++++++++++++-------
8 files changed, 57 insertions(+), 24 deletions(-)
diff --git a/js/src/mixins/group.ts b/js/src/mixins/group.ts
index 225e5f374..d5fd8ccdb 100644
--- a/js/src/mixins/group.ts
+++ b/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 });
+ }
+ }
}
diff --git a/lib/graphql/error.ex b/lib/graphql/error.ex
index 9483fed5d..2e20a228b 100644
--- a/lib/graphql/error.ex
+++ b/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
diff --git a/lib/graphql/resolvers/group.ex b/lib/graphql/resolvers/group.ex
index c33c967af..160d46f8f 100644
--- a/lib/graphql/resolvers/group.ex
+++ b/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
diff --git a/lib/web/controllers/page_controller.ex b/lib/web/controllers/page_controller.ex
index b85f6f540..eaab4b592 100644
--- a/lib/web/controllers/page_controller.ex
+++ b/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
diff --git a/lib/web/views/utils.ex b/lib/web/views/utils.ex
index 950efcd53..34eaa45c4 100644
--- a/lib/web/views/utils.ex
+++ b/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()}
diff --git a/priv/gettext/fr/LC_MESSAGES/errors.po b/priv/gettext/fr/LC_MESSAGES/errors.po
index e39ec6fb9..e3220f98a 100644
--- a/priv/gettext/fr/LC_MESSAGES/errors.po
+++ b/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
diff --git a/test/graphql/resolvers/group_test.exs b/test/graphql/resolvers/group_test.exs
index cdcabfe60..952e1b3fe 100644
--- a/test/graphql/resolvers/group_test.exs
+++ b/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", %{
diff --git a/test/web/controllers/page_controller_test.exs b/test/web/controllers/page_controller_test.exs
index 7d67b5489..ec41dee9c 100644
--- a/test/web/controllers/page_controller_test.exs
+++ b/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