From 620247a015f6cd894a119bb5173a3da7e5913064 Mon Sep 17 00:00:00 2001 From: Stephanie Wilde-Hobbs Date: Tue, 12 May 2020 17:12:27 +0100 Subject: [PATCH 1/6] Add database configuration whitelist --- docs/configuration/cheatsheet.md | 11 +++++++++ .../web/admin_api/admin_api_controller.ex | 13 +++++++++- .../admin_api/admin_api_controller_test.exs | 24 +++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index 707d7fdbd..7b7a332c7 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -911,6 +911,17 @@ config :auto_linker, Boolean, enables/disables in-database configuration. Read [Transfering the config to/from the database](../administration/CLI_tasks/config.md) for more information. +## :database_config_whitelist + +List of valid configuration sections which are allowed to be configured from the database. + +Example: +```elixir +config :pleroma, :database_config_whitelist, [ + {:pleroma, :instance}, + {:pleroma, Pleroma.Web.Metadata} +] +``` ### Multi-factor authentication - :two_factor_authentication * `totp` - a list containing TOTP configuration diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 9f1fd3aeb..9c5fbfc5d 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -949,7 +949,8 @@ def config_show(conn, _params) do def config_update(conn, %{"configs" => configs}) do with :ok <- configurable_from_database(conn) do {_errors, results} = - Enum.map(configs, fn + Enum.filter(configs, &whitelisted_config?/1) + |> Enum.map(fn %{"group" => group, "key" => key, "delete" => true} = params -> ConfigDB.delete(%{group: group, key: key, subkeys: params["subkeys"]}) @@ -1011,6 +1012,16 @@ defp configurable_from_database(conn) do end end + defp whitelisted_config?(%{"group" => group, "key" => key}) do + if whitelisted_configs = Config.get(:database_config_whitelist) do + Enum.any?(whitelisted_configs, fn {whitelisted_group, whitelisted_key} -> + group == inspect(whitelisted_group) && key == inspect(whitelisted_key) + end) + else + true + end + end + def reload_emoji(conn, _params) do Pleroma.Emoji.reload() diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 4697af50e..31e73d6a5 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -2943,6 +2943,30 @@ test "proxy tuple ip", %{conn: conn} do ] } end + + test "doesn't set keys not in the whitelist", %{conn: conn} do + clear_config(:database_config_whitelist, [ + {:pleroma, :key1}, + {:pleroma, :key2}, + {:pleroma, Pleroma.Captcha.NotReal} + ]) + + post(conn, "/api/pleroma/admin/config", %{ + configs: [ + %{group: ":pleroma", key: ":key1", value: "value1"}, + %{group: ":pleroma", key: ":key2", value: "value2"}, + %{group: ":pleroma", key: ":key3", value: "value3"}, + %{group: ":pleroma", key: "Pleroma.Web.Endpoint.NotReal", value: "value4"}, + %{group: ":pleroma", key: "Pleroma.Captcha.NotReal", value: "value5"} + ] + }) + + assert Application.get_env(:pleroma, :key1) == "value1" + assert Application.get_env(:pleroma, :key2) == "value2" + assert Application.get_env(:pleroma, :key3) == nil + assert Application.get_env(:pleroma, Pleroma.Web.Endpoint.NotReal) == nil + assert Application.get_env(:pleroma, Pleroma.Captcha.NotReal) == "value5" + end end describe "GET /api/pleroma/admin/restart" do From a2fcfc78c9dcf33081db47292d96ffa7c7709abb Mon Sep 17 00:00:00 2001 From: Stephanie Wilde-Hobbs Date: Tue, 12 May 2020 21:07:33 +0100 Subject: [PATCH 2/6] Filter config descriptions by config whitelist --- lib/pleroma/docs/json.ex | 1 - .../web/admin_api/admin_api_controller.ex | 19 +++++-- .../admin_api/admin_api_controller_test.exs | 51 +++++++++++++++---- 3 files changed, 57 insertions(+), 14 deletions(-) diff --git a/lib/pleroma/docs/json.ex b/lib/pleroma/docs/json.ex index 74f8b2615..d1cf1f487 100644 --- a/lib/pleroma/docs/json.ex +++ b/lib/pleroma/docs/json.ex @@ -18,7 +18,6 @@ def compile do with config <- Pleroma.Config.Loader.read("config/description.exs") do config[:pleroma][:config_description] |> Pleroma.Docs.Generator.convert_to_strings() - |> Jason.encode!() end end end diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 9c5fbfc5d..fa064a8c7 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -37,7 +37,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do require Logger - @descriptions_json Pleroma.Docs.JSON.compile() + @descriptions Pleroma.Docs.JSON.compile() @users_page_size 50 plug( @@ -892,9 +892,14 @@ def list_log(conn, params) do end def config_descriptions(conn, _params) do + descriptions_json = + @descriptions + |> Enum.filter(&whitelisted_config?/1) + |> Jason.encode!() + conn |> Plug.Conn.put_resp_content_type("application/json") - |> Plug.Conn.send_resp(200, @descriptions_json) + |> Plug.Conn.send_resp(200, descriptions_json) end def config_show(conn, %{"only_db" => true}) do @@ -1012,7 +1017,7 @@ defp configurable_from_database(conn) do end end - defp whitelisted_config?(%{"group" => group, "key" => key}) do + defp whitelisted_config?(group, key) do if whitelisted_configs = Config.get(:database_config_whitelist) do Enum.any?(whitelisted_configs, fn {whitelisted_group, whitelisted_key} -> group == inspect(whitelisted_group) && key == inspect(whitelisted_key) @@ -1022,6 +1027,14 @@ defp whitelisted_config?(%{"group" => group, "key" => key}) do end end + defp whitelisted_config?(%{"group" => group, "key" => key}) do + whitelisted_config?(group, key) + end + + defp whitelisted_config?(%{:group => group} = config) do + whitelisted_config?(group, config[:key]) + end + def reload_emoji(conn, _params) do Pleroma.Emoji.reload() diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 31e73d6a5..7d42a400c 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -3604,19 +3604,50 @@ test "it deletes the note", %{conn: conn, report_id: report_id} do end end - test "GET /api/pleroma/admin/config/descriptions", %{conn: conn} do - admin = insert(:user, is_admin: true) + describe "GET /api/pleroma/admin/config/descriptions" do + test "structure", %{conn: conn} do + admin = insert(:user, is_admin: true) - conn = - assign(conn, :user, admin) - |> get("/api/pleroma/admin/config/descriptions") + conn = + assign(conn, :user, admin) + |> get("/api/pleroma/admin/config/descriptions") - assert [child | _others] = json_response(conn, 200) + assert [child | _others] = json_response(conn, 200) - assert child["children"] - assert child["key"] - assert String.starts_with?(child["group"], ":") - assert child["description"] + assert child["children"] + assert child["key"] + assert String.starts_with?(child["group"], ":") + assert child["description"] + end + + test "filters by database configuration whitelist", %{conn: conn} do + clear_config(:database_config_whitelist, [ + {:pleroma, :instance}, + {:pleroma, :activitypub}, + {:pleroma, Pleroma.Upload} + ]) + + admin = insert(:user, is_admin: true) + + conn = + assign(conn, :user, admin) + |> get("/api/pleroma/admin/config/descriptions") + + children = json_response(conn, 200) + + assert length(children) == 3 + + assert Enum.all?(children, fn c -> c["group"] == ":pleroma" end) + + instance = Enum.find(children, fn c -> c["key"] == ":instance" end) + assert instance["children"] + + activitypub = Enum.find(children, fn c -> c["key"] == ":activitypub" end) + assert activitypub["children"] + + web_endpoint = Enum.find(children, fn c -> c["key"] == "Pleroma.Upload" end) + assert web_endpoint["children"] + end end describe "/api/pleroma/admin/stats" do From 5c6f57531505f1e5e39836c0e0e6d2563fdaedf8 Mon Sep 17 00:00:00 2001 From: Steph Date: Thu, 14 May 2020 09:50:53 +0000 Subject: [PATCH 3/6] Style fixes --- lib/pleroma/web/admin_api/admin_api_controller.ex | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index fa064a8c7..3053f57a1 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -892,14 +892,9 @@ def list_log(conn, params) do end def config_descriptions(conn, _params) do - descriptions_json = - @descriptions - |> Enum.filter(&whitelisted_config?/1) - |> Jason.encode!() + descriptions = Enum.filter(@descriptions, &whitelisted_config?/1) - conn - |> Plug.Conn.put_resp_content_type("application/json") - |> Plug.Conn.send_resp(200, descriptions_json) + json(conn, descriptions) end def config_show(conn, %{"only_db" => true}) do @@ -954,7 +949,8 @@ def config_show(conn, _params) do def config_update(conn, %{"configs" => configs}) do with :ok <- configurable_from_database(conn) do {_errors, results} = - Enum.filter(configs, &whitelisted_config?/1) + configs + |> Enum.filter(&whitelisted_config?/1) |> Enum.map(fn %{"group" => group, "key" => key, "delete" => true} = params -> ConfigDB.delete(%{group: group, key: key, subkeys: params["subkeys"]}) From 20cbfb5cb5515044de03cc48e8464ec45ad0ca50 Mon Sep 17 00:00:00 2001 From: Stephanie Wilde-Hobbs Date: Thu, 14 May 2020 12:34:46 +0100 Subject: [PATCH 4/6] Allow whitelisting whole groups --- docs/configuration/cheatsheet.md | 3 ++- .../web/admin_api/admin_api_controller.ex | 8 ++++++-- .../web/admin_api/admin_api_controller_test.exs | 17 ++++++++++++----- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index 7b7a332c7..f0ecebc99 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -919,7 +919,8 @@ Example: ```elixir config :pleroma, :database_config_whitelist, [ {:pleroma, :instance}, - {:pleroma, Pleroma.Web.Metadata} + {:pleroma, Pleroma.Web.Metadata}, + {:auto_linker} ] ``` diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 3053f57a1..c996a2a5a 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -1015,8 +1015,12 @@ defp configurable_from_database(conn) do defp whitelisted_config?(group, key) do if whitelisted_configs = Config.get(:database_config_whitelist) do - Enum.any?(whitelisted_configs, fn {whitelisted_group, whitelisted_key} -> - group == inspect(whitelisted_group) && key == inspect(whitelisted_key) + Enum.any?(whitelisted_configs, fn + {whitelisted_group} -> + group == inspect(whitelisted_group) + + {whitelisted_group, whitelisted_key} -> + group == inspect(whitelisted_group) && key == inspect(whitelisted_key) end) else true diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 7d42a400c..e573220ba 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -2948,7 +2948,8 @@ test "doesn't set keys not in the whitelist", %{conn: conn} do clear_config(:database_config_whitelist, [ {:pleroma, :key1}, {:pleroma, :key2}, - {:pleroma, Pleroma.Captcha.NotReal} + {:pleroma, Pleroma.Captcha.NotReal}, + {:not_real} ]) post(conn, "/api/pleroma/admin/config", %{ @@ -2957,7 +2958,8 @@ test "doesn't set keys not in the whitelist", %{conn: conn} do %{group: ":pleroma", key: ":key2", value: "value2"}, %{group: ":pleroma", key: ":key3", value: "value3"}, %{group: ":pleroma", key: "Pleroma.Web.Endpoint.NotReal", value: "value4"}, - %{group: ":pleroma", key: "Pleroma.Captcha.NotReal", value: "value5"} + %{group: ":pleroma", key: "Pleroma.Captcha.NotReal", value: "value5"}, + %{group: ":not_real", key: ":anything", value: "value6"} ] }) @@ -2966,6 +2968,7 @@ test "doesn't set keys not in the whitelist", %{conn: conn} do assert Application.get_env(:pleroma, :key3) == nil assert Application.get_env(:pleroma, Pleroma.Web.Endpoint.NotReal) == nil assert Application.get_env(:pleroma, Pleroma.Captcha.NotReal) == "value5" + assert Application.get_env(:not_real, :anything) == "value6" end end @@ -3624,7 +3627,8 @@ test "filters by database configuration whitelist", %{conn: conn} do clear_config(:database_config_whitelist, [ {:pleroma, :instance}, {:pleroma, :activitypub}, - {:pleroma, Pleroma.Upload} + {:pleroma, Pleroma.Upload}, + {:esshd} ]) admin = insert(:user, is_admin: true) @@ -3635,9 +3639,9 @@ test "filters by database configuration whitelist", %{conn: conn} do children = json_response(conn, 200) - assert length(children) == 3 + assert length(children) == 4 - assert Enum.all?(children, fn c -> c["group"] == ":pleroma" end) + assert Enum.count(children, fn c -> c["group"] == ":pleroma" end) == 3 instance = Enum.find(children, fn c -> c["key"] == ":instance" end) assert instance["children"] @@ -3647,6 +3651,9 @@ test "filters by database configuration whitelist", %{conn: conn} do web_endpoint = Enum.find(children, fn c -> c["key"] == "Pleroma.Upload" end) assert web_endpoint["children"] + + esshd = Enum.find(children, fn c -> c["group"] == ":esshd" end) + assert esshd["children"] end end From 3eff54267837a2c6fd65f8600643cb9f0cd5b972 Mon Sep 17 00:00:00 2001 From: Stephanie Wilde-Hobbs Date: Thu, 14 May 2020 12:36:49 +0100 Subject: [PATCH 5/6] Add Changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b7fb603d..feda41320 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - NodeInfo: `pleroma:api/v1/notifications:include_types_filter` to the `features` list. - NodeInfo: `pleroma_emoji_reactions` to the `features` list. - Configuration: `:restrict_unauthenticated` setting, restrict access for unauthenticated users to timelines (public and federate), user profiles and statuses. +- Configuration: Add `:database_config_whitelist` setting to whitelist settings which can be configured from AdminFE. - New HTTP adapter [gun](https://github.com/ninenines/gun). Gun adapter requires minimum OTP version of 22.2 otherwise Pleroma won’t start. For hackney OTP update is not required. - Mix task to create trusted OAuth App. - Notifications: Added `follow_request` notification type. From 80308c5c262662084dc89de05e976e7166cbb304 Mon Sep 17 00:00:00 2001 From: Stephanie Wilde-Hobbs Date: Thu, 14 May 2020 15:56:14 +0100 Subject: [PATCH 6/6] Add config migration disclaimer to config whitelist documentation --- docs/configuration/cheatsheet.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index f0ecebc99..1078c4e87 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -913,7 +913,10 @@ Boolean, enables/disables in-database configuration. Read [Transfering the confi ## :database_config_whitelist -List of valid configuration sections which are allowed to be configured from the database. +List of valid configuration sections which are allowed to be configured from the +database. Settings stored in the database before the whitelist is configured are +still applied, so it is suggested to only use the whitelist on instances that +have not migrated the config to the database. Example: ```elixir