From 3a3c452e9276fb57017f64c6ff1a385cd5040bf8 Mon Sep 17 00:00:00 2001 From: Massedil Date: Fri, 12 Sep 2025 20:54:35 +0200 Subject: [PATCH] Validate external_links on both server and client sides. Display errors from SaveAdminSettings to the user. --- lib/mobilizon/admin/admin.ex | 67 +++++++++++++++++++++++++++----- src/i18n/fr_FR.json | 2 +- src/views/Admin/SettingsView.vue | 16 ++++++-- 3 files changed, 71 insertions(+), 14 deletions(-) diff --git a/lib/mobilizon/admin/admin.ex b/lib/mobilizon/admin/admin.ex index 16ae49aa0..1929d70de 100644 --- a/lib/mobilizon/admin/admin.ex +++ b/lib/mobilizon/admin/admin.ex @@ -5,6 +5,7 @@ defmodule Mobilizon.Admin do import Ecto.Query import EctoEnum + import Mobilizon.Web.Gettext, only: [dgettext: 2] alias Mobilizon.Actors.Actor alias Mobilizon.{Admin, Users} @@ -182,10 +183,19 @@ defmodule Mobilizon.Admin do def save_settings(group, args) do {medias, values} = Map.split(args, [:instance_logo, :instance_favicon, :default_picture]) - Multi.new() - |> do_save_media_setting(group, medias) - |> do_save_value_setting(group, values) - |> Repo.transaction() + multi = + Multi.new() + |> do_save_media_setting(group, medias) + |> do_save_value_setting(group, values) + |> Repo.transaction() + + case multi do + {:ok, result} -> + {:ok, result} + + {:error, _err, %Ecto.Changeset{} = err, _} -> + {:error, err} + end end @spec do_save_value_setting(Ecto.Multi.t(), String.t(), map()) :: Ecto.Multi.t() @@ -195,15 +205,20 @@ defmodule Mobilizon.Admin do key = hd(Map.keys(args)) {val, rest} = Map.pop(args, key) + changeset = + %Setting{} + |> Setting.changeset(%{ + group: group, + name: Atom.to_string(key), + value: convert_to_string(val) + }) + |> maybe_validate_external_links(key, val) + transaction = Multi.insert( transaction, key, - Setting.changeset(%Setting{}, %{ - group: group, - name: Atom.to_string(key), - value: convert_to_string(val) - }), + changeset, on_conflict: :replace_all, conflict_target: [:group, :name] ) @@ -251,4 +266,38 @@ defmodule Mobilizon.Admin do val -> to_string(val) end end + + defp maybe_validate_external_links(changeset, :external_links, external_links) do + external_links + |> List.wrap() + |> Enum.reduce(changeset, fn link, cs -> + case validate_external_link(link) do + {:ok, _} -> cs + {:error, msg} -> Ecto.Changeset.add_error(cs, :external_links, msg) + end + end) + end + + defp maybe_validate_external_links(changeset, _key, _val), do: changeset + + defp validate_external_link(%{:url => url, :label => label} = link) do + uri = URI.parse(url) + + cond do + is_nil(label) or String.trim(label) == "" -> + {:error, dgettext("errors", "External link label cannot be blank")} + + String.length(label) < 2 -> + {:error, dgettext("errors", "External link label must be at least 2 characters")} + + String.length(label) > 256 -> + {:error, dgettext("errors", "External link label must be at most 256 characters")} + + not (uri.scheme in ["http", "https"] and is_binary(uri.host) and uri.host != "") -> + {:error, dgettext("errors", "External link URL must be a valid http/https URL")} + + true -> + {:ok, link} + end + end end diff --git a/src/i18n/fr_FR.json b/src/i18n/fr_FR.json index 0f9bf4f1b..3e615e653 100644 --- a/src/i18n/fr_FR.json +++ b/src/i18n/fr_FR.json @@ -1292,7 +1292,7 @@ "This profile is from another instance, the informations shown here may be incomplete.": "Ce profil provient d'une autre instance, les informations montrées ici peuvent être incomplètes.", "This profile is located on this instance, so you need to {access_the_corresponding_account} to suspend it.": "Ce profil se situe sur cette instance, vous devez donc {access_the_corresponding_account} afin de le suspendre.", "This profile was not found": "Ce profil n'a pas été trouvé", - "This section lets you add links to external websites to the menu.": "Cette section vous permet d'ajouter des liens vers des sites internets externes au menu.", + "This section lets you add links to external websites to the menu.": "Cette section vous permet d'ajouter des liens vers des sites internet externes au menu.", "This setting will be used to display the website and send you emails in the correct language.": "Ce paramètre sera utilisé pour l'affichage du site et pour vous envoyer des courriels dans la bonne langue.", "This URL doesn't seem to be valid": "Cette URL ne semble pas être valide", "This URL is not supported": "Cette URL n'est pas supportée", diff --git a/src/views/Admin/SettingsView.vue b/src/views/Admin/SettingsView.vue index 49cd3a991..0eeb1883f 100644 --- a/src/views/Admin/SettingsView.vue +++ b/src/views/Admin/SettingsView.vue @@ -480,11 +480,17 @@ :key="index" > { settingsToWrite.value.externalLinks.push({ url: "", label: "", - enabled: false, + enabled: true, }); }; @@ -700,7 +706,9 @@ saveAdminSettingsDone(() => { saveAdminSettingsError((e) => { console.error(e); - notifier?.error(t("Failed to save admin settings") as string); + notifier?.error( + (t("Failed to save admin settings") as string) + ": " + e.message + ); }); const updateSettings = async (): Promise => {