From cd5418825b4a4bf7c420d913328ca7b807a71075 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Mon, 12 Oct 2020 12:16:36 +0200 Subject: [PATCH] Make sure a person profile page returns 404 Signed-off-by: Thomas Citharel --- 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, "", tags) + index_content + |> String.replace("", tags) + |> String.replace("", 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