From b3764423251c963a5ca007517189f556bfe95155 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Sat, 11 Jul 2020 10:36:36 +0300 Subject: [PATCH 01/13] MediaProxy whitelist setting now supports hosts with scheme added deprecation warning about using bare domains --- CHANGELOG.md | 1 + config/description.exs | 4 +- config/test.exs | 5 + docs/configuration/cheatsheet.md | 9 +- lib/pleroma/config/deprecation_warnings.ex | 15 +- lib/pleroma/plugs/http_security_plug.ex | 47 ++++-- lib/pleroma/web/media_proxy/media_proxy.ex | 26 ++-- test/config/deprecation_warnings_test.exs | 8 + test/plugs/http_security_plug_test.exs | 90 ++++++++--- .../media_proxy_controller_test.exs | 134 +++++++++++------ test/web/media_proxy/media_proxy_test.exs | 142 ++++++------------ 11 files changed, 282 insertions(+), 199 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e928528a..42149a2d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - MFR policy to set global expiration for all local Create activities - OGP rich media parser merged with TwitterCard - Configuration: `:instance, rewrite_policy` moved to `:mrf, policies`, `:instance, :mrf_transparency` moved to `:mrf, :transparency`, `:instance, :mrf_transparency_exclusions` moved to `:mrf, :transparency_exclusions`. Old config namespace is deprecated. +- Configuration: `:media_proxy, whitelist` format changed to host with scheme (e.g. `http://example.com` instead of `example.com`). Domain format is deprecated.
API Changes diff --git a/config/description.exs b/config/description.exs index b0cc8d527..432705307 100644 --- a/config/description.exs +++ b/config/description.exs @@ -1775,8 +1775,8 @@ %{ key: :whitelist, type: {:list, :string}, - description: "List of domains to bypass the mediaproxy", - suggestions: ["example.com"] + description: "List of hosts with scheme to bypass the mediaproxy", + suggestions: ["http://example.com"] } ] }, diff --git a/config/test.exs b/config/test.exs index d45c36b7b..abcf793e5 100644 --- a/config/test.exs +++ b/config/test.exs @@ -113,6 +113,11 @@ config :pleroma, :instances_favicons, enabled: true +config :pleroma, Pleroma.Uploaders.S3, + bucket: nil, + streaming_enabled: true, + public_endpoint: nil + if File.exists?("./config/test.secret.exs") do import_config "test.secret.exs" else diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index 1a0603892..f7885c11d 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -252,6 +252,7 @@ This section describe PWA manifest instance-specific values. Currently this opti * `background_color`: Describe the background color of the app. (Example: `"#191b22"`, `"aliceblue"`). ## :emoji + * `shortcode_globs`: Location of custom emoji files. `*` can be used as a wildcard. Example `["/emoji/custom/**/*.png"]` * `pack_extensions`: A list of file extensions for emojis, when no emoji.txt for a pack is present. Example `[".png", ".gif"]` * `groups`: Emojis are ordered in groups (tags). This is an array of key-value pairs where the key is the groupname and the value the location or array of locations. `*` can be used as a wildcard. Example `[Custom: ["/emoji/*.png", "/emoji/custom/*.png"]]` @@ -260,13 +261,14 @@ This section describe PWA manifest instance-specific values. Currently this opti memory for this amount of seconds multiplied by the number of files. ## :media_proxy + * `enabled`: Enables proxying of remote media to the instance’s proxy * `base_url`: The base URL to access a user-uploaded file. Useful when you want to proxy the media files via another host/CDN fronts. * `proxy_opts`: All options defined in `Pleroma.ReverseProxy` documentation, defaults to `[max_body_length: (25*1_048_576)]`. -* `whitelist`: List of domains to bypass the mediaproxy +* `whitelist`: List of hosts with scheme to bypass the mediaproxy (e.g. `https://example.com`) * `invalidation`: options for remove media from cache after delete object: - * `enabled`: Enables purge cache - * `provider`: Which one of the [purge cache strategy](#purge-cache-strategy) to use. + * `enabled`: Enables purge cache + * `provider`: Which one of the [purge cache strategy](#purge-cache-strategy) to use. ### Purge cache strategy @@ -278,6 +280,7 @@ Urls of attachments pass to script as arguments. * `script_path`: path to external script. Example: + ```elixir config :pleroma, Pleroma.Web.MediaProxy.Invalidation.Script, script_path: "./installation/nginx-cache-purge.example" diff --git a/lib/pleroma/config/deprecation_warnings.ex b/lib/pleroma/config/deprecation_warnings.ex index 0a6c724fb..026871c4f 100644 --- a/lib/pleroma/config/deprecation_warnings.ex +++ b/lib/pleroma/config/deprecation_warnings.ex @@ -54,6 +54,7 @@ def warn do check_hellthread_threshold() mrf_user_allowlist() check_old_mrf_config() + check_media_proxy_whitelist_config() end def check_old_mrf_config do @@ -65,7 +66,7 @@ def check_old_mrf_config do move_namespace_and_warn(@mrf_config_map, warning_preface) end - @spec move_namespace_and_warn([config_map()], String.t()) :: :ok + @spec move_namespace_and_warn([config_map()], String.t()) :: :ok | nil def move_namespace_and_warn(config_map, warning_preface) do warning = Enum.reduce(config_map, "", fn @@ -84,4 +85,16 @@ def move_namespace_and_warn(config_map, warning_preface) do Logger.warn(warning_preface <> warning) end end + + @spec check_media_proxy_whitelist_config() :: :ok | nil + def check_media_proxy_whitelist_config do + whitelist = Config.get([:media_proxy, :whitelist]) + + if Enum.any?(whitelist, &(not String.starts_with?(&1, "http"))) do + Logger.warn(""" + !!!DEPRECATION WARNING!!! + Your config is using old format (only domain) for MediaProxy whitelist option. Setting should work for now, but you are advised to change format to scheme with port to prevent possible issues later. + """) + end + end end diff --git a/lib/pleroma/plugs/http_security_plug.ex b/lib/pleroma/plugs/http_security_plug.ex index 7d65cf078..c363b193b 100644 --- a/lib/pleroma/plugs/http_security_plug.ex +++ b/lib/pleroma/plugs/http_security_plug.ex @@ -108,31 +108,48 @@ defp csp_string do |> :erlang.iolist_to_binary() end + defp build_csp_from_whitelist([], acc), do: acc + + defp build_csp_from_whitelist([last], acc) do + [build_csp_param_from_whitelist(last) | acc] + end + + defp build_csp_from_whitelist([head | tail], acc) do + build_csp_from_whitelist(tail, [[?\s, build_csp_param_from_whitelist(head)] | acc]) + end + + # TODO: use `build_csp_param/1` after removing support bare domains for media proxy whitelist + defp build_csp_param_from_whitelist("http" <> _ = url) do + build_csp_param(url) + end + + defp build_csp_param_from_whitelist(url), do: url + defp build_csp_multimedia_source_list do media_proxy_whitelist = - Enum.reduce(Config.get([:media_proxy, :whitelist]), [], fn host, acc -> - add_source(acc, host) - end) - - media_proxy_base_url = build_csp_param(Config.get([:media_proxy, :base_url])) - - upload_base_url = build_csp_param(Config.get([Pleroma.Upload, :base_url])) - - s3_endpoint = build_csp_param(Config.get([Pleroma.Uploaders.S3, :public_endpoint])) + [:media_proxy, :whitelist] + |> Config.get() + |> build_csp_from_whitelist([]) captcha_method = Config.get([Pleroma.Captcha, :method]) + captcha_endpoint = Config.get([captcha_method, :endpoint]) - captcha_endpoint = build_csp_param(Config.get([captcha_method, :endpoint])) + base_endpoints = + [ + [:media_proxy, :base_url], + [Pleroma.Upload, :base_url], + [Pleroma.Uploaders.S3, :public_endpoint] + ] + |> Enum.map(&Config.get/1) - [] - |> add_source(media_proxy_base_url) - |> add_source(upload_base_url) - |> add_source(s3_endpoint) + [captcha_endpoint | base_endpoints] + |> Enum.map(&build_csp_param/1) + |> Enum.reduce([], &add_source(&2, &1)) |> add_source(media_proxy_whitelist) - |> add_source(captcha_endpoint) end defp add_source(iodata, nil), do: iodata + defp add_source(iodata, []), do: iodata defp add_source(iodata, source), do: [[?\s, source] | iodata] defp add_csp_param(csp_iodata, nil), do: csp_iodata diff --git a/lib/pleroma/web/media_proxy/media_proxy.ex b/lib/pleroma/web/media_proxy/media_proxy.ex index 6f35826da..dfbfcea6b 100644 --- a/lib/pleroma/web/media_proxy/media_proxy.ex +++ b/lib/pleroma/web/media_proxy/media_proxy.ex @@ -60,22 +60,28 @@ defp local?(url), do: String.starts_with?(url, Pleroma.Web.base_url()) defp whitelisted?(url) do %{host: domain} = URI.parse(url) - mediaproxy_whitelist = Config.get([:media_proxy, :whitelist]) + mediaproxy_whitelist_domains = + [:media_proxy, :whitelist] + |> Config.get() + |> Enum.map(&maybe_get_domain_from_url/1) - upload_base_url_domain = - if !is_nil(Config.get([Upload, :base_url])) do - [URI.parse(Config.get([Upload, :base_url])).host] + whitelist_domains = + if base_url = Config.get([Upload, :base_url]) do + %{host: base_domain} = URI.parse(base_url) + [base_domain | mediaproxy_whitelist_domains] else - [] + mediaproxy_whitelist_domains end - whitelist = mediaproxy_whitelist ++ upload_base_url_domain - - Enum.any?(whitelist, fn pattern -> - String.equivalent?(domain, pattern) - end) + domain in whitelist_domains end + defp maybe_get_domain_from_url("http" <> _ = url) do + URI.parse(url).host + end + + defp maybe_get_domain_from_url(domain), do: domain + def encode_url(url) do base64 = Base.url_encode64(url, @base64_opts) diff --git a/test/config/deprecation_warnings_test.exs b/test/config/deprecation_warnings_test.exs index 548ee87b0..555661a71 100644 --- a/test/config/deprecation_warnings_test.exs +++ b/test/config/deprecation_warnings_test.exs @@ -54,4 +54,12 @@ test "move_namespace_and_warn/2" do assert Pleroma.Config.get(new_group2) == 2 assert Pleroma.Config.get(new_group3) == 3 end + + test "check_media_proxy_whitelist_config/0" do + clear_config([:media_proxy, :whitelist], ["https://example.com", "example2.com"]) + + assert capture_log(fn -> + Pleroma.Config.DeprecationWarnings.check_media_proxy_whitelist_config() + end) =~ "Your config is using old format (only domain) for MediaProxy whitelist option" + end end diff --git a/test/plugs/http_security_plug_test.exs b/test/plugs/http_security_plug_test.exs index 63b4d3f31..2297e3dac 100644 --- a/test/plugs/http_security_plug_test.exs +++ b/test/plugs/http_security_plug_test.exs @@ -4,17 +4,12 @@ defmodule Pleroma.Web.Plugs.HTTPSecurityPlugTest do use Pleroma.Web.ConnCase + alias Pleroma.Config alias Plug.Conn - setup do: clear_config([:http_securiy, :enabled]) - setup do: clear_config([:http_security, :sts]) - setup do: clear_config([:http_security, :referrer_policy]) - describe "http security enabled" do - setup do - Config.put([:http_security, :enabled], true) - end + setup do: clear_config([:http_security, :enabled], true) test "it sends CSP headers when enabled", %{conn: conn} do conn = get(conn, "/api/v1/instance") @@ -29,7 +24,7 @@ test "it sends CSP headers when enabled", %{conn: conn} do end test "it sends STS headers when enabled", %{conn: conn} do - Config.put([:http_security, :sts], true) + clear_config([:http_security, :sts], true) conn = get(conn, "/api/v1/instance") @@ -38,7 +33,7 @@ test "it sends STS headers when enabled", %{conn: conn} do end test "it does not send STS headers when disabled", %{conn: conn} do - Config.put([:http_security, :sts], false) + clear_config([:http_security, :sts], false) conn = get(conn, "/api/v1/instance") @@ -47,23 +42,19 @@ test "it does not send STS headers when disabled", %{conn: conn} do end test "referrer-policy header reflects configured value", %{conn: conn} do - conn = get(conn, "/api/v1/instance") + resp = get(conn, "/api/v1/instance") - assert Conn.get_resp_header(conn, "referrer-policy") == ["same-origin"] + assert Conn.get_resp_header(resp, "referrer-policy") == ["same-origin"] - Config.put([:http_security, :referrer_policy], "no-referrer") + clear_config([:http_security, :referrer_policy], "no-referrer") - conn = - build_conn() - |> get("/api/v1/instance") + resp = get(conn, "/api/v1/instance") - assert Conn.get_resp_header(conn, "referrer-policy") == ["no-referrer"] + assert Conn.get_resp_header(resp, "referrer-policy") == ["no-referrer"] end - test "it sends `report-to` & `report-uri` CSP response headers" do - conn = - build_conn() - |> get("/api/v1/instance") + test "it sends `report-to` & `report-uri` CSP response headers", %{conn: conn} do + conn = get(conn, "/api/v1/instance") [csp] = Conn.get_resp_header(conn, "content-security-policy") @@ -74,10 +65,67 @@ test "it sends `report-to` & `report-uri` CSP response headers" do assert reply_to == "{\"endpoints\":[{\"url\":\"https://endpoint.com\"}],\"group\":\"csp-endpoint\",\"max-age\":10886400}" end + + test "default values for img-src and media-src with disabled media proxy", %{conn: conn} do + conn = get(conn, "/api/v1/instance") + + [csp] = Conn.get_resp_header(conn, "content-security-policy") + assert csp =~ "media-src 'self' https:;" + assert csp =~ "img-src 'self' data: blob: https:;" + end + end + + describe "img-src and media-src" do + setup do + clear_config([:http_security, :enabled], true) + clear_config([:media_proxy, :enabled], true) + clear_config([:media_proxy, :proxy_opts, :redirect_on_failure], false) + end + + test "media_proxy with base_url", %{conn: conn} do + url = "https://example.com" + clear_config([:media_proxy, :base_url], url) + assert_media_img_src(conn, url) + end + + test "upload with base url", %{conn: conn} do + url = "https://example2.com" + clear_config([Pleroma.Upload, :base_url], url) + assert_media_img_src(conn, url) + end + + test "with S3 public endpoint", %{conn: conn} do + url = "https://example3.com" + clear_config([Pleroma.Uploaders.S3, :public_endpoint], url) + assert_media_img_src(conn, url) + end + + test "with captcha endpoint", %{conn: conn} do + clear_config([Pleroma.Captcha.Mock, :endpoint], "https://captcha.com") + assert_media_img_src(conn, "https://captcha.com") + end + + test "with media_proxy whitelist", %{conn: conn} do + clear_config([:media_proxy, :whitelist], ["https://example6.com", "https://example7.com"]) + assert_media_img_src(conn, "https://example7.com https://example6.com") + end + + # TODO: delete after removing support bare domains for media proxy whitelist + test "with media_proxy bare domains whitelist (deprecated)", %{conn: conn} do + clear_config([:media_proxy, :whitelist], ["example4.com", "example5.com"]) + assert_media_img_src(conn, "example5.com example4.com") + end + end + + defp assert_media_img_src(conn, url) do + conn = get(conn, "/api/v1/instance") + [csp] = Conn.get_resp_header(conn, "content-security-policy") + assert csp =~ "media-src 'self' #{url};" + assert csp =~ "img-src 'self' data: blob: #{url};" end test "it does not send CSP headers when disabled", %{conn: conn} do - Config.put([:http_security, :enabled], false) + clear_config([:http_security, :enabled], false) conn = get(conn, "/api/v1/instance") diff --git a/test/web/media_proxy/media_proxy_controller_test.exs b/test/web/media_proxy/media_proxy_controller_test.exs index d61cef83b..d4db44c63 100644 --- a/test/web/media_proxy/media_proxy_controller_test.exs +++ b/test/web/media_proxy/media_proxy_controller_test.exs @@ -4,82 +4,118 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyControllerTest do use Pleroma.Web.ConnCase - import Mock - alias Pleroma.Config - setup do: clear_config(:media_proxy) - setup do: clear_config([Pleroma.Web.Endpoint, :secret_key_base]) + import Mock + + alias Pleroma.Web.MediaProxy + alias Pleroma.Web.MediaProxy.MediaProxyController + alias Plug.Conn setup do on_exit(fn -> Cachex.clear(:banned_urls_cache) end) end test "it returns 404 when MediaProxy disabled", %{conn: conn} do - Config.put([:media_proxy, :enabled], false) + clear_config([:media_proxy, :enabled], false) - assert %Plug.Conn{ + assert %Conn{ status: 404, resp_body: "Not Found" } = get(conn, "/proxy/hhgfh/eeeee") - assert %Plug.Conn{ + assert %Conn{ status: 404, resp_body: "Not Found" } = get(conn, "/proxy/hhgfh/eeee/fff") end - test "it returns 403 when signature invalidated", %{conn: conn} do - Config.put([:media_proxy, :enabled], true) - Config.put([Pleroma.Web.Endpoint, :secret_key_base], "00000000000") - path = URI.parse(Pleroma.Web.MediaProxy.encode_url("https://google.fn")).path - Config.put([Pleroma.Web.Endpoint, :secret_key_base], "000") + describe "" do + setup do + clear_config([:media_proxy, :enabled], true) + clear_config([Pleroma.Web.Endpoint, :secret_key_base], "00000000000") + [url: MediaProxy.encode_url("https://google.fn/test.png")] + end - assert %Plug.Conn{ - status: 403, - resp_body: "Forbidden" - } = get(conn, path) + test "it returns 403 for invalid signature", %{conn: conn, url: url} do + Pleroma.Config.put([Pleroma.Web.Endpoint, :secret_key_base], "000") + %{path: path} = URI.parse(url) - assert %Plug.Conn{ - status: 403, - resp_body: "Forbidden" - } = get(conn, "/proxy/hhgfh/eeee") + assert %Conn{ + status: 403, + resp_body: "Forbidden" + } = get(conn, path) - assert %Plug.Conn{ - status: 403, - resp_body: "Forbidden" - } = get(conn, "/proxy/hhgfh/eeee/fff") - end + assert %Conn{ + status: 403, + resp_body: "Forbidden" + } = get(conn, "/proxy/hhgfh/eeee") - test "redirects on valid url when filename invalidated", %{conn: conn} do - Config.put([:media_proxy, :enabled], true) - Config.put([Pleroma.Web.Endpoint, :secret_key_base], "00000000000") - url = Pleroma.Web.MediaProxy.encode_url("https://google.fn/test.png") - invalid_url = String.replace(url, "test.png", "test-file.png") - response = get(conn, invalid_url) - assert response.status == 302 - assert redirected_to(response) == url - end + assert %Conn{ + status: 403, + resp_body: "Forbidden" + } = get(conn, "/proxy/hhgfh/eeee/fff") + end - test "it performs ReverseProxy.call when signature valid", %{conn: conn} do - Config.put([:media_proxy, :enabled], true) - Config.put([Pleroma.Web.Endpoint, :secret_key_base], "00000000000") - url = Pleroma.Web.MediaProxy.encode_url("https://google.fn/test.png") + test "redirects on valid url when filename is invalidated", %{conn: conn, url: url} do + invalid_url = String.replace(url, "test.png", "test-file.png") + response = get(conn, invalid_url) + assert response.status == 302 + assert redirected_to(response) == url + end - with_mock Pleroma.ReverseProxy, - call: fn _conn, _url, _opts -> %Plug.Conn{status: :success} end do - assert %Plug.Conn{status: :success} = get(conn, url) + test "it performs ReverseProxy.call with valid signature", %{conn: conn, url: url} do + with_mock Pleroma.ReverseProxy, + call: fn _conn, _url, _opts -> %Conn{status: :success} end do + assert %Conn{status: :success} = get(conn, url) + end + end + + test "it returns 404 when url is in banned_urls cache", %{conn: conn, url: url} do + MediaProxy.put_in_banned_urls("https://google.fn/test.png") + + with_mock Pleroma.ReverseProxy, + call: fn _conn, _url, _opts -> %Conn{status: :success} end do + assert %Conn{status: 404, resp_body: "Not Found"} = get(conn, url) + end end end - test "it returns 404 when url contains in banned_urls cache", %{conn: conn} do - Config.put([:media_proxy, :enabled], true) - Config.put([Pleroma.Web.Endpoint, :secret_key_base], "00000000000") - url = Pleroma.Web.MediaProxy.encode_url("https://google.fn/test.png") - Pleroma.Web.MediaProxy.put_in_banned_urls("https://google.fn/test.png") + describe "filename_matches/3" do + test "preserves the encoded or decoded path" do + assert MediaProxyController.filename_matches( + %{"filename" => "/Hello world.jpg"}, + "/Hello world.jpg", + "http://pleroma.social/Hello world.jpg" + ) == :ok - with_mock Pleroma.ReverseProxy, - call: fn _conn, _url, _opts -> %Plug.Conn{status: :success} end do - assert %Plug.Conn{status: 404, resp_body: "Not Found"} = get(conn, url) + assert MediaProxyController.filename_matches( + %{"filename" => "/Hello%20world.jpg"}, + "/Hello%20world.jpg", + "http://pleroma.social/Hello%20world.jpg" + ) == :ok + + assert MediaProxyController.filename_matches( + %{"filename" => "/my%2Flong%2Furl%2F2019%2F07%2FS.jpg"}, + "/my%2Flong%2Furl%2F2019%2F07%2FS.jpg", + "http://pleroma.social/my%2Flong%2Furl%2F2019%2F07%2FS.jpg" + ) == :ok + + assert MediaProxyController.filename_matches( + %{"filename" => "/my%2Flong%2Furl%2F2019%2F07%2FS.jp"}, + "/my%2Flong%2Furl%2F2019%2F07%2FS.jp", + "http://pleroma.social/my%2Flong%2Furl%2F2019%2F07%2FS.jpg" + ) == {:wrong_filename, "my%2Flong%2Furl%2F2019%2F07%2FS.jpg"} + end + + test "encoded url are tried to match for proxy as `conn.request_path` encodes the url" do + # conn.request_path will return encoded url + request_path = "/ANALYSE-DAI-_-LE-STABLECOIN-100-D%C3%89CENTRALIS%C3%89-BQ.jpg" + + assert MediaProxyController.filename_matches( + true, + request_path, + "https://mydomain.com/uploads/2019/07/ANALYSE-DAI-_-LE-STABLECOIN-100-DÉCENTRALISÉ-BQ.jpg" + ) == :ok end end end diff --git a/test/web/media_proxy/media_proxy_test.exs b/test/web/media_proxy/media_proxy_test.exs index 69d2a71a6..72885cfdd 100644 --- a/test/web/media_proxy/media_proxy_test.exs +++ b/test/web/media_proxy/media_proxy_test.exs @@ -5,38 +5,33 @@ defmodule Pleroma.Web.MediaProxyTest do use ExUnit.Case use Pleroma.Tests.Helpers - import Pleroma.Web.MediaProxy - alias Pleroma.Web.MediaProxy.MediaProxyController - setup do: clear_config([:media_proxy, :enabled]) - setup do: clear_config(Pleroma.Upload) + alias Pleroma.Web.Endpoint + alias Pleroma.Web.MediaProxy describe "when enabled" do - setup do - Pleroma.Config.put([:media_proxy, :enabled], true) - :ok - end + setup do: clear_config([:media_proxy, :enabled], true) test "ignores invalid url" do - assert url(nil) == nil - assert url("") == nil + assert MediaProxy.url(nil) == nil + assert MediaProxy.url("") == nil end test "ignores relative url" do - assert url("/local") == "/local" - assert url("/") == "/" + assert MediaProxy.url("/local") == "/local" + assert MediaProxy.url("/") == "/" end test "ignores local url" do - local_url = Pleroma.Web.Endpoint.url() <> "/hello" - local_root = Pleroma.Web.Endpoint.url() - assert url(local_url) == local_url - assert url(local_root) == local_root + local_url = Endpoint.url() <> "/hello" + local_root = Endpoint.url() + assert MediaProxy.url(local_url) == local_url + assert MediaProxy.url(local_root) == local_root end test "encodes and decodes URL" do url = "https://pleroma.soykaf.com/static/logo.png" - encoded = url(url) + encoded = MediaProxy.url(url) assert String.starts_with?( encoded, @@ -50,86 +45,44 @@ test "encodes and decodes URL" do test "encodes and decodes URL without a path" do url = "https://pleroma.soykaf.com" - encoded = url(url) + encoded = MediaProxy.url(url) assert decode_result(encoded) == url end test "encodes and decodes URL without an extension" do url = "https://pleroma.soykaf.com/path/" - encoded = url(url) + encoded = MediaProxy.url(url) assert String.ends_with?(encoded, "/path") assert decode_result(encoded) == url end test "encodes and decodes URL and ignores query params for the path" do url = "https://pleroma.soykaf.com/static/logo.png?93939393939&bunny=true" - encoded = url(url) + encoded = MediaProxy.url(url) assert String.ends_with?(encoded, "/logo.png") assert decode_result(encoded) == url end test "validates signature" do - secret_key_base = Pleroma.Config.get([Pleroma.Web.Endpoint, :secret_key_base]) + encoded = MediaProxy.url("https://pleroma.social") - on_exit(fn -> - Pleroma.Config.put([Pleroma.Web.Endpoint, :secret_key_base], secret_key_base) - end) - - encoded = url("https://pleroma.social") - - Pleroma.Config.put( - [Pleroma.Web.Endpoint, :secret_key_base], + clear_config( + [Endpoint, :secret_key_base], "00000000000000000000000000000000000000000000000" ) [_, "proxy", sig, base64 | _] = URI.parse(encoded).path |> String.split("/") - assert decode_url(sig, base64) == {:error, :invalid_signature} - end - - test "filename_matches preserves the encoded or decoded path" do - assert MediaProxyController.filename_matches( - %{"filename" => "/Hello world.jpg"}, - "/Hello world.jpg", - "http://pleroma.social/Hello world.jpg" - ) == :ok - - assert MediaProxyController.filename_matches( - %{"filename" => "/Hello%20world.jpg"}, - "/Hello%20world.jpg", - "http://pleroma.social/Hello%20world.jpg" - ) == :ok - - assert MediaProxyController.filename_matches( - %{"filename" => "/my%2Flong%2Furl%2F2019%2F07%2FS.jpg"}, - "/my%2Flong%2Furl%2F2019%2F07%2FS.jpg", - "http://pleroma.social/my%2Flong%2Furl%2F2019%2F07%2FS.jpg" - ) == :ok - - assert MediaProxyController.filename_matches( - %{"filename" => "/my%2Flong%2Furl%2F2019%2F07%2FS.jp"}, - "/my%2Flong%2Furl%2F2019%2F07%2FS.jp", - "http://pleroma.social/my%2Flong%2Furl%2F2019%2F07%2FS.jpg" - ) == {:wrong_filename, "my%2Flong%2Furl%2F2019%2F07%2FS.jpg"} - end - - test "encoded url are tried to match for proxy as `conn.request_path` encodes the url" do - # conn.request_path will return encoded url - request_path = "/ANALYSE-DAI-_-LE-STABLECOIN-100-D%C3%89CENTRALIS%C3%89-BQ.jpg" - - assert MediaProxyController.filename_matches( - true, - request_path, - "https://mydomain.com/uploads/2019/07/ANALYSE-DAI-_-LE-STABLECOIN-100-DÉCENTRALISÉ-BQ.jpg" - ) == :ok + assert MediaProxy.decode_url(sig, base64) == {:error, :invalid_signature} end test "uses the configured base_url" do - clear_config([:media_proxy, :base_url], "https://cache.pleroma.social") + base_url = "https://cache.pleroma.social" + clear_config([:media_proxy, :base_url], base_url) url = "https://pleroma.soykaf.com/static/logo.png" - encoded = url(url) + encoded = MediaProxy.url(url) - assert String.starts_with?(encoded, Pleroma.Config.get([:media_proxy, :base_url])) + assert String.starts_with?(encoded, base_url) end # Some sites expect ASCII encoded characters in the URL to be preserved even if @@ -140,7 +93,7 @@ test "preserve ASCII encoding" do url = "https://pleroma.com/%20/%21/%22/%23/%24/%25/%26/%27/%28/%29/%2A/%2B/%2C/%2D/%2E/%2F/%30/%31/%32/%33/%34/%35/%36/%37/%38/%39/%3A/%3B/%3C/%3D/%3E/%3F/%40/%41/%42/%43/%44/%45/%46/%47/%48/%49/%4A/%4B/%4C/%4D/%4E/%4F/%50/%51/%52/%53/%54/%55/%56/%57/%58/%59/%5A/%5B/%5C/%5D/%5E/%5F/%60/%61/%62/%63/%64/%65/%66/%67/%68/%69/%6A/%6B/%6C/%6D/%6E/%6F/%70/%71/%72/%73/%74/%75/%76/%77/%78/%79/%7A/%7B/%7C/%7D/%7E/%7F/%80/%81/%82/%83/%84/%85/%86/%87/%88/%89/%8A/%8B/%8C/%8D/%8E/%8F/%90/%91/%92/%93/%94/%95/%96/%97/%98/%99/%9A/%9B/%9C/%9D/%9E/%9F/%C2%A0/%A1/%A2/%A3/%A4/%A5/%A6/%A7/%A8/%A9/%AA/%AB/%AC/%C2%AD/%AE/%AF/%B0/%B1/%B2/%B3/%B4/%B5/%B6/%B7/%B8/%B9/%BA/%BB/%BC/%BD/%BE/%BF/%C0/%C1/%C2/%C3/%C4/%C5/%C6/%C7/%C8/%C9/%CA/%CB/%CC/%CD/%CE/%CF/%D0/%D1/%D2/%D3/%D4/%D5/%D6/%D7/%D8/%D9/%DA/%DB/%DC/%DD/%DE/%DF/%E0/%E1/%E2/%E3/%E4/%E5/%E6/%E7/%E8/%E9/%EA/%EB/%EC/%ED/%EE/%EF/%F0/%F1/%F2/%F3/%F4/%F5/%F6/%F7/%F8/%F9/%FA/%FB/%FC/%FD/%FE/%FF" - encoded = url(url) + encoded = MediaProxy.url(url) assert decode_result(encoded) == url end @@ -151,56 +104,49 @@ test "preserve non-unicode characters per RFC3986" do url = "https://pleroma.com/ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890-._~:/?#[]@!$&'()*+,;=|^`{}" - encoded = url(url) + encoded = MediaProxy.url(url) assert decode_result(encoded) == url end test "preserve unicode characters" do url = "https://ko.wikipedia.org/wiki/위키백과:대문" - encoded = url(url) + encoded = MediaProxy.url(url) assert decode_result(encoded) == url end end describe "when disabled" do - setup do - enabled = Pleroma.Config.get([:media_proxy, :enabled]) - - if enabled do - Pleroma.Config.put([:media_proxy, :enabled], false) - - on_exit(fn -> - Pleroma.Config.put([:media_proxy, :enabled], enabled) - :ok - end) - end - - :ok - end + setup do: clear_config([:media_proxy, :enabled], false) test "does not encode remote urls" do - assert url("https://google.fr") == "https://google.fr" + assert MediaProxy.url("https://google.fr") == "https://google.fr" end end defp decode_result(encoded) do [_, "proxy", sig, base64 | _] = URI.parse(encoded).path |> String.split("/") - {:ok, decoded} = decode_url(sig, base64) + {:ok, decoded} = MediaProxy.decode_url(sig, base64) decoded end describe "whitelist" do - setup do - Pleroma.Config.put([:media_proxy, :enabled], true) - :ok - end + setup do: clear_config([:media_proxy, :enabled], true) test "mediaproxy whitelist" do - Pleroma.Config.put([:media_proxy, :whitelist], ["google.com", "feld.me"]) + clear_config([:media_proxy, :whitelist], ["https://google.com", "https://feld.me"]) url = "https://feld.me/foo.png" - unencoded = url(url) + unencoded = MediaProxy.url(url) + assert unencoded == url + end + + # TODO: delete after removing support bare domains for media proxy whitelist + test "mediaproxy whitelist bare domains whitelist (deprecated)" do + clear_config([:media_proxy, :whitelist], ["google.com", "feld.me"]) + url = "https://feld.me/foo.png" + + unencoded = MediaProxy.url(url) assert unencoded == url end @@ -211,17 +157,17 @@ test "does not change whitelisted urls" do media_url = "https://mycdn.akamai.com" url = "#{media_url}/static/logo.png" - encoded = url(url) + encoded = MediaProxy.url(url) assert String.starts_with?(encoded, media_url) end test "ensure Pleroma.Upload base_url is always whitelisted" do media_url = "https://media.pleroma.social" - Pleroma.Config.put([Pleroma.Upload, :base_url], media_url) + clear_config([Pleroma.Upload, :base_url], media_url) url = "#{media_url}/static/logo.png" - encoded = url(url) + encoded = MediaProxy.url(url) assert String.starts_with?(encoded, media_url) end From b221b640a2dd443e3c2274b16ed5b62566329d09 Mon Sep 17 00:00:00 2001 From: = <=> Date: Mon, 13 Jul 2020 22:19:13 +0300 Subject: [PATCH 02/13] Transmogrifier: filtering weirdness in address fields --- .../web/activity_pub/transmogrifier.ex | 14 ++++++----- test/web/activity_pub/transmogrifier_test.exs | 23 +++++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 884646ceb..f37bcab3e 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -62,15 +62,17 @@ def fix_summary(%{"summary" => _} = object) do def fix_summary(object), do: Map.put(object, "summary", "") def fix_addressing_list(map, field) do - cond do - is_binary(map[field]) -> - Map.put(map, field, [map[field]]) + addrs = map[field] - is_nil(map[field]) -> - Map.put(map, field, []) + cond do + is_list(addrs) -> + Map.put(map, field, Enum.filter(addrs, &is_binary/1)) + + is_binary(addrs) -> + Map.put(map, field, [addrs]) true -> - map + Map.put(map, field, []) end end diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index f7b7d1a9f..248b410c6 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -774,6 +774,29 @@ test "it correctly processes messages with non-array cc field" do assert [user.follower_address] == activity.data["to"] end + test "it correctly processes messages with weirdness in address fields" do + user = insert(:user) + + message = %{ + "@context" => "https://www.w3.org/ns/activitystreams", + "to" => [nil, user.follower_address], + "cc" => ["https://www.w3.org/ns/activitystreams#Public", ["¿"]], + "type" => "Create", + "object" => %{ + "content" => "…", + "type" => "Note", + "attributedTo" => user.ap_id, + "inReplyTo" => nil + }, + "actor" => user.ap_id + } + + assert {:ok, activity} = Transmogrifier.handle_incoming(message) + + assert ["https://www.w3.org/ns/activitystreams#Public"] == activity.data["cc"] + assert [user.follower_address] == activity.data["to"] + end + test "it accepts Move activities" do old_user = insert(:user) new_user = insert(:user) From cf3f8cb72a46f0c8c798d4022cff442fae4ab401 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 19 Jul 2020 21:35:57 +0300 Subject: [PATCH 03/13] [#1940] Reinstated OAuth-less `admin_token` authentication. Refactored UserIsAdminPlug (freed from checking admin scopes presence). --- .../plugs/admin_secret_authentication_plug.ex | 12 +- lib/pleroma/plugs/user_is_admin_plug.ex | 25 +- priv/gettext/errors.pot | 230 +++++++++--------- priv/gettext/it/LC_MESSAGES/errors.po | 6 +- priv/gettext/nl/LC_MESSAGES/errors.po | 4 +- priv/gettext/pl/LC_MESSAGES/errors.po | 4 +- .../admin_secret_authentication_plug_test.exs | 4 + test/plugs/user_is_admin_plug_test.exs | 114 ++------- .../controllers/admin_api_controller_test.exs | 10 + .../controllers/report_controller_test.exs | 2 +- 10 files changed, 169 insertions(+), 242 deletions(-) diff --git a/lib/pleroma/plugs/admin_secret_authentication_plug.ex b/lib/pleroma/plugs/admin_secret_authentication_plug.ex index b4b47a31f..ff0328d4a 100644 --- a/lib/pleroma/plugs/admin_secret_authentication_plug.ex +++ b/lib/pleroma/plugs/admin_secret_authentication_plug.ex @@ -4,7 +4,9 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug do import Plug.Conn + alias Pleroma.User + alias Pleroma.Plugs.OAuthScopesPlug def init(options) do options @@ -26,7 +28,7 @@ def call(conn, _) do def authenticate(%{params: %{"admin_token" => admin_token}} = conn) do if admin_token == secret_token() do - assign(conn, :user, %User{is_admin: true}) + assign_admin_user(conn) else conn end @@ -36,8 +38,14 @@ def authenticate(conn) do token = secret_token() case get_req_header(conn, "x-admin-token") do - [^token] -> assign(conn, :user, %User{is_admin: true}) + [^token] -> assign_admin_user(conn) _ -> conn end end + + defp assign_admin_user(conn) do + conn + |> assign(:user, %User{is_admin: true}) + |> OAuthScopesPlug.skip_plug() + end end diff --git a/lib/pleroma/plugs/user_is_admin_plug.ex b/lib/pleroma/plugs/user_is_admin_plug.ex index 2748102df..488a61d1d 100644 --- a/lib/pleroma/plugs/user_is_admin_plug.ex +++ b/lib/pleroma/plugs/user_is_admin_plug.ex @@ -7,37 +7,18 @@ defmodule Pleroma.Plugs.UserIsAdminPlug do import Plug.Conn alias Pleroma.User - alias Pleroma.Web.OAuth def init(options) do options end - def call(%{assigns: %{user: %User{is_admin: true}} = assigns} = conn, _) do - token = assigns[:token] - - cond do - not Pleroma.Config.enforce_oauth_admin_scope_usage?() -> - conn - - token && OAuth.Scopes.contains_admin_scopes?(token.scopes) -> - # Note: checking for _any_ admin scope presence, not necessarily fitting requested action. - # Thus, controller must explicitly invoke OAuthScopesPlug to verify scope requirements. - # Admin might opt out of admin scope for some apps to block any admin actions from them. - conn - - true -> - fail(conn) - end + def call(%{assigns: %{user: %User{is_admin: true}}} = conn, _) do + conn end def call(conn, _) do - fail(conn) - end - - defp fail(conn) do conn - |> render_error(:forbidden, "User is not an admin or OAuth admin scope is not granted.") + |> render_error(:forbidden, "User is not an admin.") |> halt() end end diff --git a/priv/gettext/errors.pot b/priv/gettext/errors.pot index 0e1cf37eb..e337226a7 100644 --- a/priv/gettext/errors.pot +++ b/priv/gettext/errors.pot @@ -90,110 +90,100 @@ msgid "must be equal to %{number}" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:421 +#: lib/pleroma/web/common_api/common_api.ex:505 msgid "Account not found" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:249 +#: lib/pleroma/web/common_api/common_api.ex:339 msgid "Already voted" msgstr "" #, elixir-format -#: lib/pleroma/web/oauth/oauth_controller.ex:360 +#: lib/pleroma/web/oauth/oauth_controller.ex:359 msgid "Bad request" msgstr "" #, elixir-format -#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:425 +#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:426 msgid "Can't delete object" msgstr "" #, elixir-format -#: lib/pleroma/web/mastodon_api/controllers/status_controller.ex:196 -msgid "Can't delete this post" -msgstr "" - -#, elixir-format -#: lib/pleroma/web/controller_helper.ex:95 -#: lib/pleroma/web/controller_helper.ex:101 +#: lib/pleroma/web/controller_helper.ex:105 +#: lib/pleroma/web/controller_helper.ex:111 msgid "Can't display this activity" msgstr "" #, elixir-format -#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:227 -#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:254 +#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:285 msgid "Can't find user" msgstr "" #, elixir-format -#: lib/pleroma/web/pleroma_api/controllers/account_controller.ex:114 +#: lib/pleroma/web/pleroma_api/controllers/account_controller.ex:61 msgid "Can't get favorites" msgstr "" #, elixir-format -#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:437 +#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:438 msgid "Can't like object" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/utils.ex:556 +#: lib/pleroma/web/common_api/utils.ex:563 msgid "Cannot post an empty status without attachments" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/utils.ex:504 +#: lib/pleroma/web/common_api/utils.ex:511 msgid "Comment must be up to %{max_size} characters" msgstr "" #, elixir-format -#: lib/pleroma/config/config_db.ex:222 +#: lib/pleroma/config/config_db.ex:191 msgid "Config with params %{params} not found" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:95 +#: lib/pleroma/web/common_api/common_api.ex:181 +#: lib/pleroma/web/common_api/common_api.ex:185 msgid "Could not delete" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:141 +#: lib/pleroma/web/common_api/common_api.ex:231 msgid "Could not favorite" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:370 +#: lib/pleroma/web/common_api/common_api.ex:453 msgid "Could not pin" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:112 -msgid "Could not repeat" -msgstr "" - -#, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:188 +#: lib/pleroma/web/common_api/common_api.ex:278 msgid "Could not unfavorite" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:380 +#: lib/pleroma/web/common_api/common_api.ex:463 msgid "Could not unpin" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:126 +#: lib/pleroma/web/common_api/common_api.ex:216 msgid "Could not unrepeat" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:428 -#: lib/pleroma/web/common_api/common_api.ex:437 +#: lib/pleroma/web/common_api/common_api.ex:512 +#: lib/pleroma/web/common_api/common_api.ex:521 msgid "Could not update state" msgstr "" #, elixir-format -#: lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex:202 +#: lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex:207 msgid "Error." msgstr "" @@ -203,8 +193,8 @@ msgid "Invalid CAPTCHA" msgstr "" #, elixir-format -#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:117 -#: lib/pleroma/web/oauth/oauth_controller.ex:569 +#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:116 +#: lib/pleroma/web/oauth/oauth_controller.ex:568 msgid "Invalid credentials" msgstr "" @@ -214,22 +204,22 @@ msgid "Invalid credentials." msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:265 +#: lib/pleroma/web/common_api/common_api.ex:355 msgid "Invalid indices" msgstr "" #, elixir-format -#: lib/pleroma/web/admin_api/admin_api_controller.ex:1147 +#: lib/pleroma/web/admin_api/controllers/fallback_controller.ex:29 msgid "Invalid parameters" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/utils.ex:411 +#: lib/pleroma/web/common_api/utils.ex:414 msgid "Invalid password." msgstr "" #, elixir-format -#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:187 +#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:220 msgid "Invalid request" msgstr "" @@ -239,44 +229,44 @@ msgid "Kocaptcha service unavailable" msgstr "" #, elixir-format -#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:113 +#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:112 msgid "Missing parameters" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/utils.ex:540 +#: lib/pleroma/web/common_api/utils.ex:547 msgid "No such conversation" msgstr "" #, elixir-format -#: lib/pleroma/web/admin_api/admin_api_controller.ex:439 -#: lib/pleroma/web/admin_api/admin_api_controller.ex:465 lib/pleroma/web/admin_api/admin_api_controller.ex:507 +#: lib/pleroma/web/admin_api/controllers/admin_api_controller.ex:388 +#: lib/pleroma/web/admin_api/controllers/admin_api_controller.ex:414 lib/pleroma/web/admin_api/controllers/admin_api_controller.ex:456 msgid "No such permission_group" msgstr "" #, elixir-format -#: lib/pleroma/plugs/uploaded_media.ex:74 -#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:485 lib/pleroma/web/admin_api/admin_api_controller.ex:1135 -#: lib/pleroma/web/feed/user_controller.ex:73 lib/pleroma/web/ostatus/ostatus_controller.ex:143 +#: lib/pleroma/plugs/uploaded_media.ex:84 +#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:486 lib/pleroma/web/admin_api/controllers/fallback_controller.ex:11 +#: lib/pleroma/web/feed/user_controller.ex:71 lib/pleroma/web/ostatus/ostatus_controller.ex:143 msgid "Not found" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:241 +#: lib/pleroma/web/common_api/common_api.ex:331 msgid "Poll's author can't vote" msgstr "" #, elixir-format #: lib/pleroma/web/mastodon_api/controllers/fallback_controller.ex:20 #: lib/pleroma/web/mastodon_api/controllers/poll_controller.ex:37 lib/pleroma/web/mastodon_api/controllers/poll_controller.ex:49 -#: lib/pleroma/web/mastodon_api/controllers/poll_controller.ex:50 lib/pleroma/web/mastodon_api/controllers/status_controller.ex:290 +#: lib/pleroma/web/mastodon_api/controllers/poll_controller.ex:50 lib/pleroma/web/mastodon_api/controllers/status_controller.ex:306 #: lib/pleroma/web/mastodon_api/controllers/subscription_controller.ex:71 msgid "Record not found" msgstr "" #, elixir-format -#: lib/pleroma/web/admin_api/admin_api_controller.ex:1153 -#: lib/pleroma/web/feed/user_controller.ex:79 lib/pleroma/web/mastodon_api/controllers/fallback_controller.ex:32 +#: lib/pleroma/web/admin_api/controllers/fallback_controller.ex:35 +#: lib/pleroma/web/feed/user_controller.ex:77 lib/pleroma/web/mastodon_api/controllers/fallback_controller.ex:36 #: lib/pleroma/web/ostatus/ostatus_controller.ex:149 msgid "Something went wrong" msgstr "" @@ -287,7 +277,7 @@ msgid "The message visibility must be direct" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/utils.ex:566 +#: lib/pleroma/web/common_api/utils.ex:573 msgid "The status is over the character limit" msgstr "" @@ -302,65 +292,65 @@ msgid "Throttled" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:266 +#: lib/pleroma/web/common_api/common_api.ex:356 msgid "Too many choices" msgstr "" #, elixir-format -#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:442 +#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:443 msgid "Unhandled activity type" msgstr "" #, elixir-format -#: lib/pleroma/web/admin_api/admin_api_controller.ex:536 +#: lib/pleroma/web/admin_api/controllers/admin_api_controller.ex:485 msgid "You can't revoke your own admin status." msgstr "" #, elixir-format -#: lib/pleroma/web/oauth/oauth_controller.ex:218 -#: lib/pleroma/web/oauth/oauth_controller.ex:309 +#: lib/pleroma/web/oauth/oauth_controller.ex:221 +#: lib/pleroma/web/oauth/oauth_controller.ex:308 msgid "Your account is currently disabled" msgstr "" #, elixir-format -#: lib/pleroma/web/oauth/oauth_controller.ex:180 -#: lib/pleroma/web/oauth/oauth_controller.ex:332 +#: lib/pleroma/web/oauth/oauth_controller.ex:183 +#: lib/pleroma/web/oauth/oauth_controller.ex:331 msgid "Your login is missing a confirmed e-mail address" msgstr "" #, elixir-format -#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:389 +#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:390 msgid "can't read inbox of %{nickname} as %{as_nickname}" msgstr "" #, elixir-format -#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:472 +#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:473 msgid "can't update outbox of %{nickname} as %{as_nickname}" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:388 +#: lib/pleroma/web/common_api/common_api.ex:471 msgid "conversation is already muted" msgstr "" #, elixir-format -#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:316 -#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:491 +#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:314 +#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:492 msgid "error" msgstr "" #, elixir-format -#: lib/pleroma/web/pleroma_api/controllers/mascot_controller.ex:29 +#: lib/pleroma/web/pleroma_api/controllers/mascot_controller.ex:32 msgid "mascots can only be images" msgstr "" #, elixir-format -#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:60 +#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:62 msgid "not found" msgstr "" #, elixir-format -#: lib/pleroma/web/oauth/oauth_controller.ex:395 +#: lib/pleroma/web/oauth/oauth_controller.ex:394 msgid "Bad OAuth request." msgstr "" @@ -375,17 +365,17 @@ msgid "CAPTCHA expired" msgstr "" #, elixir-format -#: lib/pleroma/plugs/uploaded_media.ex:55 +#: lib/pleroma/plugs/uploaded_media.ex:57 msgid "Failed" msgstr "" #, elixir-format -#: lib/pleroma/web/oauth/oauth_controller.ex:411 +#: lib/pleroma/web/oauth/oauth_controller.ex:410 msgid "Failed to authenticate: %{message}." msgstr "" #, elixir-format -#: lib/pleroma/web/oauth/oauth_controller.ex:442 +#: lib/pleroma/web/oauth/oauth_controller.ex:441 msgid "Failed to set up user account." msgstr "" @@ -395,7 +385,7 @@ msgid "Insufficient permissions: %{permissions}." msgstr "" #, elixir-format -#: lib/pleroma/plugs/uploaded_media.ex:94 +#: lib/pleroma/plugs/uploaded_media.ex:104 msgid "Internal Error" msgstr "" @@ -411,12 +401,12 @@ msgid "Invalid answer data" msgstr "" #, elixir-format -#: lib/pleroma/web/nodeinfo/nodeinfo_controller.ex:128 +#: lib/pleroma/web/nodeinfo/nodeinfo_controller.ex:33 msgid "Nodeinfo schema version not handled" msgstr "" #, elixir-format -#: lib/pleroma/web/oauth/oauth_controller.ex:169 +#: lib/pleroma/web/oauth/oauth_controller.ex:172 msgid "This action is outside the authorized scopes" msgstr "" @@ -426,13 +416,13 @@ msgid "Unknown error, please check the details and try again." msgstr "" #, elixir-format -#: lib/pleroma/web/oauth/oauth_controller.ex:116 -#: lib/pleroma/web/oauth/oauth_controller.ex:155 +#: lib/pleroma/web/oauth/oauth_controller.ex:119 +#: lib/pleroma/web/oauth/oauth_controller.ex:158 msgid "Unlisted redirect_uri." msgstr "" #, elixir-format -#: lib/pleroma/web/oauth/oauth_controller.ex:391 +#: lib/pleroma/web/oauth/oauth_controller.ex:390 msgid "Unsupported OAuth provider: %{provider}." msgstr "" @@ -452,12 +442,12 @@ msgid "CAPTCHA Error" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:200 +#: lib/pleroma/web/common_api/common_api.ex:290 msgid "Could not add reaction emoji" msgstr "" #, elixir-format -#: lib/pleroma/web/common_api/common_api.ex:211 +#: lib/pleroma/web/common_api/common_api.ex:301 msgid "Could not remove reaction emoji" msgstr "" @@ -472,39 +462,45 @@ msgid "List not found" msgstr "" #, elixir-format -#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:124 +#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:123 msgid "Missing parameter: %{name}" msgstr "" #, elixir-format -#: lib/pleroma/web/oauth/oauth_controller.ex:207 -#: lib/pleroma/web/oauth/oauth_controller.ex:322 +#: lib/pleroma/web/oauth/oauth_controller.ex:210 +#: lib/pleroma/web/oauth/oauth_controller.ex:321 msgid "Password reset is required" msgstr "" #, elixir-format #: lib/pleroma/tests/auth_test_controller.ex:9 -#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:6 lib/pleroma/web/admin_api/admin_api_controller.ex:6 -#: lib/pleroma/web/controller_helper.ex:6 lib/pleroma/web/fallback_redirect_controller.ex:6 -#: lib/pleroma/web/feed/tag_controller.ex:6 lib/pleroma/web/feed/user_controller.ex:6 -#: lib/pleroma/web/mailer/subscription_controller.ex:2 lib/pleroma/web/masto_fe_controller.ex:6 -#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/app_controller.ex:6 -#: lib/pleroma/web/mastodon_api/controllers/auth_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/conversation_controller.ex:6 -#: lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/domain_block_controller.ex:6 -#: lib/pleroma/web/mastodon_api/controllers/fallback_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/filter_controller.ex:6 -#: lib/pleroma/web/mastodon_api/controllers/follow_request_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/instance_controller.ex:6 -#: lib/pleroma/web/mastodon_api/controllers/list_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/marker_controller.ex:6 -#: lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex:14 lib/pleroma/web/mastodon_api/controllers/media_controller.ex:6 -#: lib/pleroma/web/mastodon_api/controllers/notification_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/poll_controller.ex:6 -#: lib/pleroma/web/mastodon_api/controllers/report_controller.ex:8 lib/pleroma/web/mastodon_api/controllers/scheduled_activity_controller.ex:6 -#: lib/pleroma/web/mastodon_api/controllers/search_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/status_controller.ex:6 -#: lib/pleroma/web/mastodon_api/controllers/subscription_controller.ex:7 lib/pleroma/web/mastodon_api/controllers/suggestion_controller.ex:6 -#: lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex:6 lib/pleroma/web/media_proxy/media_proxy_controller.ex:6 -#: lib/pleroma/web/mongooseim/mongoose_im_controller.ex:6 lib/pleroma/web/nodeinfo/nodeinfo_controller.ex:6 -#: lib/pleroma/web/oauth/fallback_controller.ex:6 lib/pleroma/web/oauth/mfa_controller.ex:10 -#: lib/pleroma/web/oauth/oauth_controller.ex:6 lib/pleroma/web/ostatus/ostatus_controller.ex:6 -#: lib/pleroma/web/pleroma_api/controllers/account_controller.ex:6 lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex:2 -#: lib/pleroma/web/pleroma_api/controllers/mascot_controller.ex:6 lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex:6 +#: lib/pleroma/web/activity_pub/activity_pub_controller.ex:6 lib/pleroma/web/admin_api/controllers/admin_api_controller.ex:6 +#: lib/pleroma/web/admin_api/controllers/config_controller.ex:6 lib/pleroma/web/admin_api/controllers/fallback_controller.ex:6 +#: lib/pleroma/web/admin_api/controllers/invite_controller.ex:6 lib/pleroma/web/admin_api/controllers/media_proxy_cache_controller.ex:6 +#: lib/pleroma/web/admin_api/controllers/oauth_app_controller.ex:6 lib/pleroma/web/admin_api/controllers/relay_controller.ex:6 +#: lib/pleroma/web/admin_api/controllers/report_controller.ex:6 lib/pleroma/web/admin_api/controllers/status_controller.ex:6 +#: lib/pleroma/web/controller_helper.ex:6 lib/pleroma/web/embed_controller.ex:6 +#: lib/pleroma/web/fallback_redirect_controller.ex:6 lib/pleroma/web/feed/tag_controller.ex:6 +#: lib/pleroma/web/feed/user_controller.ex:6 lib/pleroma/web/mailer/subscription_controller.ex:2 +#: lib/pleroma/web/masto_fe_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/account_controller.ex:6 +#: lib/pleroma/web/mastodon_api/controllers/app_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/auth_controller.ex:6 +#: lib/pleroma/web/mastodon_api/controllers/conversation_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex:6 +#: lib/pleroma/web/mastodon_api/controllers/domain_block_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/fallback_controller.ex:6 +#: lib/pleroma/web/mastodon_api/controllers/filter_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/follow_request_controller.ex:6 +#: lib/pleroma/web/mastodon_api/controllers/instance_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/list_controller.ex:6 +#: lib/pleroma/web/mastodon_api/controllers/marker_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex:14 +#: lib/pleroma/web/mastodon_api/controllers/media_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/notification_controller.ex:6 +#: lib/pleroma/web/mastodon_api/controllers/poll_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/report_controller.ex:8 +#: lib/pleroma/web/mastodon_api/controllers/scheduled_activity_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/search_controller.ex:6 +#: lib/pleroma/web/mastodon_api/controllers/status_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/subscription_controller.ex:7 +#: lib/pleroma/web/mastodon_api/controllers/suggestion_controller.ex:6 lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex:6 +#: lib/pleroma/web/media_proxy/media_proxy_controller.ex:6 lib/pleroma/web/mongooseim/mongoose_im_controller.ex:6 +#: lib/pleroma/web/nodeinfo/nodeinfo_controller.ex:6 lib/pleroma/web/oauth/fallback_controller.ex:6 +#: lib/pleroma/web/oauth/mfa_controller.ex:10 lib/pleroma/web/oauth/oauth_controller.ex:6 +#: lib/pleroma/web/ostatus/ostatus_controller.ex:6 lib/pleroma/web/pleroma_api/controllers/account_controller.ex:6 +#: lib/pleroma/web/pleroma_api/controllers/chat_controller.ex:5 lib/pleroma/web/pleroma_api/controllers/conversation_controller.ex:6 +#: lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex:2 lib/pleroma/web/pleroma_api/controllers/emoji_reaction_controller.ex:6 +#: lib/pleroma/web/pleroma_api/controllers/mascot_controller.ex:6 lib/pleroma/web/pleroma_api/controllers/notification_controller.ex:6 #: lib/pleroma/web/pleroma_api/controllers/scrobble_controller.ex:6 #: lib/pleroma/web/pleroma_api/controllers/two_factor_authentication_controller.ex:7 lib/pleroma/web/static_fe/static_fe_controller.ex:6 #: lib/pleroma/web/twitter_api/controllers/password_controller.ex:10 lib/pleroma/web/twitter_api/controllers/remote_follow_controller.ex:6 @@ -519,46 +515,56 @@ msgid "Two-factor authentication enabled, you must use a access token." msgstr "" #, elixir-format -#: lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex:210 +#: lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex:210 msgid "Unexpected error occurred while adding file to pack." msgstr "" #, elixir-format -#: lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex:138 +#: lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex:138 msgid "Unexpected error occurred while creating pack." msgstr "" #, elixir-format -#: lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex:278 +#: lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex:278 msgid "Unexpected error occurred while removing file from pack." msgstr "" #, elixir-format -#: lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex:250 +#: lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex:250 msgid "Unexpected error occurred while updating file in pack." msgstr "" #, elixir-format -#: lib/pleroma/web/pleroma_api/controllers/emoji_api_controller.ex:179 +#: lib/pleroma/web/pleroma_api/controllers/emoji_pack_controller.ex:179 msgid "Unexpected error occurred while updating pack metadata." msgstr "" -#, elixir-format -#: lib/pleroma/plugs/user_is_admin_plug.ex:40 -msgid "User is not an admin or OAuth admin scope is not granted." -msgstr "" - #, elixir-format #: lib/pleroma/web/mastodon_api/controllers/subscription_controller.ex:61 msgid "Web push subscription is disabled on this Pleroma instance" msgstr "" #, elixir-format -#: lib/pleroma/web/admin_api/admin_api_controller.ex:502 +#: lib/pleroma/web/admin_api/controllers/admin_api_controller.ex:451 msgid "You can't revoke your own admin/moderator status." msgstr "" #, elixir-format -#: lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex:105 +#: lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex:126 msgid "authorization required for timeline view" msgstr "" + +#, elixir-format +#: lib/pleroma/web/mastodon_api/controllers/fallback_controller.ex:24 +msgid "Access denied" +msgstr "" + +#, elixir-format +#: lib/pleroma/web/mastodon_api/controllers/account_controller.ex:282 +msgid "This API requires an authenticated user" +msgstr "" + +#, elixir-format +#: lib/pleroma/plugs/user_is_admin_plug.ex:21 +msgid "User is not an admin." +msgstr "" diff --git a/priv/gettext/it/LC_MESSAGES/errors.po b/priv/gettext/it/LC_MESSAGES/errors.po index 406a297d1..cd0cd6c65 100644 --- a/priv/gettext/it/LC_MESSAGES/errors.po +++ b/priv/gettext/it/LC_MESSAGES/errors.po @@ -562,11 +562,11 @@ msgstr "Errore inaspettato durante l'aggiornamento del file nel pacchetto." msgid "Unexpected error occurred while updating pack metadata." msgstr "Errore inaspettato durante l'aggiornamento dei metadati del pacchetto." -#: lib/pleroma/plugs/user_is_admin_plug.ex:40 +#: lib/pleroma/plugs/user_is_admin_plug.ex:21 #, elixir-format -msgid "User is not an admin or OAuth admin scope is not granted." +msgid "User is not an admin." msgstr "" -"L'utente non è un amministratore o non ha ricevuto questa autorizzazione " +"L'utente non è un amministratore." "OAuth." #: lib/pleroma/web/mastodon_api/controllers/subscription_controller.ex:61 diff --git a/priv/gettext/nl/LC_MESSAGES/errors.po b/priv/gettext/nl/LC_MESSAGES/errors.po index 3118f6b5d..cfcb05fe6 100644 --- a/priv/gettext/nl/LC_MESSAGES/errors.po +++ b/priv/gettext/nl/LC_MESSAGES/errors.po @@ -559,9 +559,9 @@ msgstr "" msgid "Unexpected error occurred while updating pack metadata." msgstr "" -#: lib/pleroma/plugs/user_is_admin_plug.ex:40 +#: lib/pleroma/plugs/user_is_admin_plug.ex:21 #, elixir-format -msgid "User is not an admin or OAuth admin scope is not granted." +msgid "User is not an admin." msgstr "" #: lib/pleroma/web/mastodon_api/controllers/subscription_controller.ex:61 diff --git a/priv/gettext/pl/LC_MESSAGES/errors.po b/priv/gettext/pl/LC_MESSAGES/errors.po index 7241d8a0a..653ea00a1 100644 --- a/priv/gettext/pl/LC_MESSAGES/errors.po +++ b/priv/gettext/pl/LC_MESSAGES/errors.po @@ -566,9 +566,9 @@ msgstr "Nieoczekiwany błąd podczas zmieniania pliku w paczce." msgid "Unexpected error occurred while updating pack metadata." msgstr "Nieoczekiwany błąd podczas zmieniania metadanych paczki." -#: lib/pleroma/plugs/user_is_admin_plug.ex:40 +#: lib/pleroma/plugs/user_is_admin_plug.ex:21 #, elixir-format -msgid "User is not an admin or OAuth admin scope is not granted." +msgid "User is not an admin." msgstr "" #: lib/pleroma/web/mastodon_api/controllers/subscription_controller.ex:61 diff --git a/test/plugs/admin_secret_authentication_plug_test.exs b/test/plugs/admin_secret_authentication_plug_test.exs index 100016c62..b541a7208 100644 --- a/test/plugs/admin_secret_authentication_plug_test.exs +++ b/test/plugs/admin_secret_authentication_plug_test.exs @@ -7,6 +7,8 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlugTest do import Pleroma.Factory alias Pleroma.Plugs.AdminSecretAuthenticationPlug + alias Pleroma.Plugs.OAuthScopesPlug + alias Pleroma.Plugs.PlugHelper test "does nothing if a user is assigned", %{conn: conn} do user = insert(:user) @@ -39,6 +41,7 @@ test "with `admin_token` query parameter", %{conn: conn} do |> AdminSecretAuthenticationPlug.call(%{}) assert conn.assigns[:user].is_admin + assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug) end test "with `x-admin-token` HTTP header", %{conn: conn} do @@ -57,6 +60,7 @@ test "with `x-admin-token` HTTP header", %{conn: conn} do |> AdminSecretAuthenticationPlug.call(%{}) assert conn.assigns[:user].is_admin + assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug) end end end diff --git a/test/plugs/user_is_admin_plug_test.exs b/test/plugs/user_is_admin_plug_test.exs index fd6a50e53..8bc00e444 100644 --- a/test/plugs/user_is_admin_plug_test.exs +++ b/test/plugs/user_is_admin_plug_test.exs @@ -8,112 +8,30 @@ defmodule Pleroma.Plugs.UserIsAdminPlugTest do alias Pleroma.Plugs.UserIsAdminPlug import Pleroma.Factory - describe "unless [:auth, :enforce_oauth_admin_scope_usage]," do - setup do: clear_config([:auth, :enforce_oauth_admin_scope_usage], false) + test "accepts a user that is an admin" do + user = insert(:user, is_admin: true) - test "accepts a user that is an admin" do - user = insert(:user, is_admin: true) + conn = assign(build_conn(), :user, user) - conn = assign(build_conn(), :user, user) + ret_conn = UserIsAdminPlug.call(conn, %{}) - ret_conn = UserIsAdminPlug.call(conn, %{}) - - assert conn == ret_conn - end - - test "denies a user that isn't an admin" do - user = insert(:user) - - conn = - build_conn() - |> assign(:user, user) - |> UserIsAdminPlug.call(%{}) - - assert conn.status == 403 - end - - test "denies when a user isn't set" do - conn = UserIsAdminPlug.call(build_conn(), %{}) - - assert conn.status == 403 - end + assert conn == ret_conn end - describe "with [:auth, :enforce_oauth_admin_scope_usage]," do - setup do: clear_config([:auth, :enforce_oauth_admin_scope_usage], true) + test "denies a user that isn't an admin" do + user = insert(:user) - setup do - admin_user = insert(:user, is_admin: true) - non_admin_user = insert(:user, is_admin: false) - blank_user = nil + conn = + build_conn() + |> assign(:user, user) + |> UserIsAdminPlug.call(%{}) - {:ok, %{users: [admin_user, non_admin_user, blank_user]}} - end + assert conn.status == 403 + end - test "if token has any of admin scopes, accepts a user that is an admin", %{conn: conn} do - user = insert(:user, is_admin: true) - token = insert(:oauth_token, user: user, scopes: ["admin:something"]) + test "denies when a user isn't set" do + conn = UserIsAdminPlug.call(build_conn(), %{}) - conn = - conn - |> assign(:user, user) - |> assign(:token, token) - - ret_conn = UserIsAdminPlug.call(conn, %{}) - - assert conn == ret_conn - end - - test "if token has any of admin scopes, denies a user that isn't an admin", %{conn: conn} do - user = insert(:user, is_admin: false) - token = insert(:oauth_token, user: user, scopes: ["admin:something"]) - - conn = - conn - |> assign(:user, user) - |> assign(:token, token) - |> UserIsAdminPlug.call(%{}) - - assert conn.status == 403 - end - - test "if token has any of admin scopes, denies when a user isn't set", %{conn: conn} do - token = insert(:oauth_token, scopes: ["admin:something"]) - - conn = - conn - |> assign(:user, nil) - |> assign(:token, token) - |> UserIsAdminPlug.call(%{}) - - assert conn.status == 403 - end - - test "if token lacks admin scopes, denies users regardless of is_admin flag", - %{users: users} do - for user <- users do - token = insert(:oauth_token, user: user) - - conn = - build_conn() - |> assign(:user, user) - |> assign(:token, token) - |> UserIsAdminPlug.call(%{}) - - assert conn.status == 403 - end - end - - test "if token is missing, denies users regardless of is_admin flag", %{users: users} do - for user <- users do - conn = - build_conn() - |> assign(:user, user) - |> assign(:token, nil) - |> UserIsAdminPlug.call(%{}) - - assert conn.status == 403 - end - end + assert conn.status == 403 end end diff --git a/test/web/admin_api/controllers/admin_api_controller_test.exs b/test/web/admin_api/controllers/admin_api_controller_test.exs index c2433f23c..da91cd552 100644 --- a/test/web/admin_api/controllers/admin_api_controller_test.exs +++ b/test/web/admin_api/controllers/admin_api_controller_test.exs @@ -41,6 +41,16 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do {:ok, %{admin: admin, token: token, conn: conn}} end + test "with valid `admin_token` query parameter, skips OAuth scopes check" do + clear_config([:admin_token], "password123") + + user = insert(:user) + + conn = get(build_conn(), "/api/pleroma/admin/users/#{user.nickname}?admin_token=password123") + + assert json_response(conn, 200) + end + describe "with [:auth, :enforce_oauth_admin_scope_usage]," do setup do: clear_config([:auth, :enforce_oauth_admin_scope_usage], true) diff --git a/test/web/admin_api/controllers/report_controller_test.exs b/test/web/admin_api/controllers/report_controller_test.exs index 940bce340..f30dc8956 100644 --- a/test/web/admin_api/controllers/report_controller_test.exs +++ b/test/web/admin_api/controllers/report_controller_test.exs @@ -297,7 +297,7 @@ test "returns 403 when requested by a non-admin" do |> get("/api/pleroma/admin/reports") assert json_response(conn, :forbidden) == - %{"error" => "User is not an admin or OAuth admin scope is not granted."} + %{"error" => "User is not an admin."} end test "returns 403 when requested by anonymous" do From 9b225db7d86289fb9d9c51f62e6ec29f6c07f60d Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 14 Jul 2020 11:58:41 +0300 Subject: [PATCH 04/13] [#1940] Applied rate limit for requests with bad `admin_token`. Added doc warnings on `admin_token` setting. --- config/description.exs | 6 ++++-- docs/configuration/cheatsheet.md | 2 ++ .../plugs/admin_secret_authentication_plug.ex | 17 +++++++++++++---- .../admin_secret_authentication_plug_test.exs | 9 +++++++++ 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/config/description.exs b/config/description.exs index 84dcdb87e..8ec4b712f 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2008,13 +2008,15 @@ label: "Pleroma Admin Token", type: :group, description: - "Allows to set a token that can be used to authenticate with the admin api without using an actual user by giving it as the `admin_token` parameter", + "Allows to set a token that can be used to authenticate with the admin api without using an actual user by giving it as the `admin_token` parameter (risky; use HTTP Basic Auth or OAuth-based authentication if possible)", children: [ %{ key: :admin_token, type: :string, description: "Admin token", - suggestions: ["We recommend a secure random string or UUID"] + suggestions: [ + "We recommend NOT setting the value do to increased security risk; if set, use a secure random long string or UUID (and change it as often as possible)" + ] } ] }, diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index f796330f1..24b162ce7 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -815,6 +815,8 @@ or curl -H "X-Admin-Token: somerandomtoken" "http://localhost:4000/api/pleroma/admin/users/invites" ``` +Warning: it's discouraged to use this feature because of the associated security risk: static / rarely changed instance-wide token is much weaker compared to email-password pair of a real admin user; consider using HTTP Basic Auth or OAuth-based authentication instead. + ### :auth * `Pleroma.Web.Auth.PleromaAuthenticator`: default database authenticator. diff --git a/lib/pleroma/plugs/admin_secret_authentication_plug.ex b/lib/pleroma/plugs/admin_secret_authentication_plug.ex index ff0328d4a..2e54df47a 100644 --- a/lib/pleroma/plugs/admin_secret_authentication_plug.ex +++ b/lib/pleroma/plugs/admin_secret_authentication_plug.ex @@ -5,15 +5,19 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug do import Plug.Conn - alias Pleroma.User alias Pleroma.Plugs.OAuthScopesPlug + alias Pleroma.Plugs.RateLimiter + alias Pleroma.User def init(options) do options end def secret_token do - Pleroma.Config.get(:admin_token) + case Pleroma.Config.get(:admin_token) do + blank when blank in [nil, ""] -> nil + token -> token + end end def call(%{assigns: %{user: %User{}}} = conn, _), do: conn @@ -30,7 +34,7 @@ def authenticate(%{params: %{"admin_token" => admin_token}} = conn) do if admin_token == secret_token() do assign_admin_user(conn) else - conn + handle_bad_token(conn) end end @@ -38,8 +42,9 @@ def authenticate(conn) do token = secret_token() case get_req_header(conn, "x-admin-token") do + blank when blank in [[], [""]] -> conn [^token] -> assign_admin_user(conn) - _ -> conn + _ -> handle_bad_token(conn) end end @@ -48,4 +53,8 @@ defp assign_admin_user(conn) do |> assign(:user, %User{is_admin: true}) |> OAuthScopesPlug.skip_plug() end + + defp handle_bad_token(conn) do + RateLimiter.call(conn, name: :authentication) + end end diff --git a/test/plugs/admin_secret_authentication_plug_test.exs b/test/plugs/admin_secret_authentication_plug_test.exs index b541a7208..89df03c4b 100644 --- a/test/plugs/admin_secret_authentication_plug_test.exs +++ b/test/plugs/admin_secret_authentication_plug_test.exs @@ -4,11 +4,14 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlugTest do use Pleroma.Web.ConnCase, async: true + + import Mock import Pleroma.Factory alias Pleroma.Plugs.AdminSecretAuthenticationPlug alias Pleroma.Plugs.OAuthScopesPlug alias Pleroma.Plugs.PlugHelper + alias Pleroma.Plugs.RateLimiter test "does nothing if a user is assigned", %{conn: conn} do user = insert(:user) @@ -27,6 +30,10 @@ test "does nothing if a user is assigned", %{conn: conn} do describe "when secret set it assigns an admin user" do setup do: clear_config([:admin_token]) + setup_with_mocks([{RateLimiter, [:passthrough], []}]) do + :ok + end + test "with `admin_token` query parameter", %{conn: conn} do Pleroma.Config.put(:admin_token, "password123") @@ -35,6 +42,7 @@ test "with `admin_token` query parameter", %{conn: conn} do |> AdminSecretAuthenticationPlug.call(%{}) refute conn.assigns[:user] + assert called(RateLimiter.call(conn, name: :authentication)) conn = %{conn | params: %{"admin_token" => "password123"}} @@ -53,6 +61,7 @@ test "with `x-admin-token` HTTP header", %{conn: conn} do |> AdminSecretAuthenticationPlug.call(%{}) refute conn.assigns[:user] + assert called(RateLimiter.call(conn, name: :authentication)) conn = conn From 858d9fc7e8e722604676c90cf2707f0209f935ec Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Mon, 13 Jul 2020 15:47:13 +0200 Subject: [PATCH 05/13] MRF Policies: Return a {:reject, reason} instead of {:reject, nil} --- .../activity_pub/mrf/anti_followbot_policy.ex | 2 +- .../activity_pub/mrf/anti_link_spam_policy.ex | 7 +++---- .../web/activity_pub/mrf/hellthread_policy.ex | 4 ++-- .../web/activity_pub/mrf/keyword_policy.ex | 7 ++++--- .../web/activity_pub/mrf/mention_policy.ex | 5 +++-- .../web/activity_pub/mrf/object_age_policy.ex | 8 +++----- .../web/activity_pub/mrf/reject_non_public.ex | 2 +- .../web/activity_pub/mrf/simple_policy.ex | 16 ++++++++++------ lib/pleroma/web/activity_pub/mrf/tag_policy.ex | 7 ++++--- .../activity_pub/mrf/user_allow_list_policy.ex | 2 +- .../web/activity_pub/mrf/vocabulary_policy.ex | 18 +++++++++++------- .../mrf/anti_followbot_policy_test.exs | 4 ++-- .../mrf/hellthread_policy_test.exs | 3 ++- .../activity_pub/mrf/keyword_policy_test.exs | 12 ++++++++---- .../activity_pub/mrf/mention_policy_test.exs | 6 ++++-- .../mrf/reject_non_public_test.exs | 4 ++-- .../activity_pub/mrf/simple_policy_test.exs | 16 ++++++++-------- test/web/activity_pub/mrf/tag_policy_test.exs | 6 +++--- .../mrf/user_allowlist_policy_test.exs | 2 +- .../mrf/vocabulary_policy_test.exs | 8 ++++---- 20 files changed, 77 insertions(+), 62 deletions(-) diff --git a/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex b/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex index 0270b96ae..b96388489 100644 --- a/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/anti_followbot_policy.ex @@ -60,7 +60,7 @@ def filter(%{"type" => "Follow", "actor" => actor_id} = message) do if score < 0.8 do {:ok, message} else - {:reject, nil} + {:reject, "[AntiFollowbotPolicy] Scored #{actor_id} as #{score}"} end end diff --git a/lib/pleroma/web/activity_pub/mrf/anti_link_spam_policy.ex b/lib/pleroma/web/activity_pub/mrf/anti_link_spam_policy.ex index a7e187b5e..b22464111 100644 --- a/lib/pleroma/web/activity_pub/mrf/anti_link_spam_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/anti_link_spam_policy.ex @@ -39,14 +39,13 @@ def filter(%{"type" => "Create", "actor" => actor, "object" => object} = message {:ok, message} {:old_user, false} -> - {:reject, nil} + {:reject, "[AntiLinkSpamPolicy] User has no posts nor followers"} {:error, _} -> - {:reject, nil} + {:reject, "[AntiLinkSpamPolicy] Failed to get or fetch user by ap_id"} e -> - Logger.warn("[MRF anti-link-spam] WTF: unhandled error #{inspect(e)}") - {:reject, nil} + {:reject, "[AntiLinkSpamPolicy] Unhandled error #{inspect(e)}"} end end diff --git a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex index f6b2c4415..9ba07b4e3 100644 --- a/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/hellthread_policy.ex @@ -43,7 +43,7 @@ defp delist_message(message, _threshold), do: {:ok, message} defp reject_message(message, threshold) when threshold > 0 do with {_, recipients} <- get_recipient_count(message) do if recipients > threshold do - {:reject, nil} + {:reject, "[HellthreadPolicy] #{recipients} recipients is over the limit of #{threshold}"} else {:ok, message} end @@ -87,7 +87,7 @@ def filter(%{"type" => "Create", "object" => %{"type" => object_type}} = message {:ok, message} <- delist_message(message, delist_threshold) do {:ok, message} else - _e -> {:reject, nil} + e -> e end end diff --git a/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex b/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex index 88b0d2b39..15e09dcf0 100644 --- a/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/keyword_policy.ex @@ -24,7 +24,7 @@ defp check_reject(%{"object" => %{"content" => content, "summary" => summary}} = if Enum.any?(Pleroma.Config.get([:mrf_keyword, :reject]), fn pattern -> string_matches?(content, pattern) or string_matches?(summary, pattern) end) do - {:reject, nil} + {:reject, "[KeywordPolicy] Matches with rejected keyword"} else {:ok, message} end @@ -89,8 +89,9 @@ def filter(%{"type" => "Create", "object" => %{"content" => _content}} = message {:ok, message} <- check_replace(message) do {:ok, message} else - _e -> - {:reject, nil} + {:reject, nil} -> {:reject, "[KeywordPolicy] "} + {:reject, _} = e -> e + _e -> {:reject, "[KeywordPolicy] "} end end diff --git a/lib/pleroma/web/activity_pub/mrf/mention_policy.ex b/lib/pleroma/web/activity_pub/mrf/mention_policy.ex index 06f003921..7910ca131 100644 --- a/lib/pleroma/web/activity_pub/mrf/mention_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/mention_policy.ex @@ -12,8 +12,9 @@ def filter(%{"type" => "Create"} = message) do reject_actors = Pleroma.Config.get([:mrf_mention, :actors], []) recipients = (message["to"] || []) ++ (message["cc"] || []) - if Enum.any?(recipients, fn recipient -> Enum.member?(reject_actors, recipient) end) do - {:reject, nil} + if rejected_mention = + Enum.find(recipients, fn recipient -> Enum.member?(reject_actors, recipient) end) do + {:reject, "[MentionPolicy] Rejected for mention of #{rejected_mention}"} else {:ok, message} end diff --git a/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex b/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex index a62914135..5f111c72f 100644 --- a/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/object_age_policy.ex @@ -28,7 +28,7 @@ defp check_date(%{"object" => %{"published" => published}} = message) do defp check_reject(message, actions) do if :reject in actions do - {:reject, nil} + {:reject, "[ObjectAgePolicy]"} else {:ok, message} end @@ -47,9 +47,8 @@ defp check_delist(message, actions) do {:ok, message} else - # Unhandleable error: somebody is messing around, just drop the message. _e -> - {:reject, nil} + {:reject, "[ObjectAgePolicy] Unhandled error"} end else {:ok, message} @@ -69,9 +68,8 @@ defp check_strip_followers(message, actions) do {:ok, message} else - # Unhandleable error: somebody is messing around, just drop the message. _e -> - {:reject, nil} + {:reject, "[ObjectAgePolicy] Unhandled error"} end else {:ok, message} diff --git a/lib/pleroma/web/activity_pub/mrf/reject_non_public.ex b/lib/pleroma/web/activity_pub/mrf/reject_non_public.ex index 4fd63106d..0b9ed2224 100644 --- a/lib/pleroma/web/activity_pub/mrf/reject_non_public.ex +++ b/lib/pleroma/web/activity_pub/mrf/reject_non_public.ex @@ -38,7 +38,7 @@ def filter(%{"type" => "Create"} = object) do {:ok, object} true -> - {:reject, nil} + {:reject, "[RejectNonPublic] visibility: #{visibility}"} end end diff --git a/lib/pleroma/web/activity_pub/mrf/simple_policy.ex b/lib/pleroma/web/activity_pub/mrf/simple_policy.ex index 70a2ca053..b77b8c7b4 100644 --- a/lib/pleroma/web/activity_pub/mrf/simple_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/simple_policy.ex @@ -21,7 +21,7 @@ defp check_accept(%{host: actor_host} = _actor_info, object) do accepts == [] -> {:ok, object} actor_host == Config.get([Pleroma.Web.Endpoint, :url, :host]) -> {:ok, object} MRF.subdomain_match?(accepts, actor_host) -> {:ok, object} - true -> {:reject, nil} + true -> {:reject, "[SimplePolicy] host not in accept list"} end end @@ -31,7 +31,7 @@ defp check_reject(%{host: actor_host} = _actor_info, object) do |> MRF.subdomains_regex() if MRF.subdomain_match?(rejects, actor_host) do - {:reject, nil} + {:reject, "[SimplePolicy] host in reject list"} else {:ok, object} end @@ -114,7 +114,7 @@ defp check_report_removal(%{host: actor_host} = _actor_info, %{"type" => "Flag"} |> MRF.subdomains_regex() if MRF.subdomain_match?(report_removal, actor_host) do - {:reject, nil} + {:reject, "[SimplePolicy] host in report_removal list"} else {:ok, object} end @@ -159,7 +159,7 @@ def filter(%{"type" => "Delete", "actor" => actor} = object) do |> MRF.subdomains_regex() if MRF.subdomain_match?(reject_deletes, actor_host) do - {:reject, nil} + {:reject, "[SimplePolicy] host in reject_deletes list"} else {:ok, object} end @@ -177,7 +177,9 @@ def filter(%{"actor" => actor} = object) do {:ok, object} <- check_report_removal(actor_info, object) do {:ok, object} else - _e -> {:reject, nil} + {:reject, nil} -> {:reject, "[SimplePolicy]"} + {:reject, _} = e -> e + _ -> {:reject, "[SimplePolicy]"} end end @@ -191,7 +193,9 @@ def filter(%{"id" => actor, "type" => obj_type} = object) {:ok, object} <- check_banner_removal(actor_info, object) do {:ok, object} else - _e -> {:reject, nil} + {:reject, nil} -> {:reject, "[SimplePolicy]"} + {:reject, _} = e -> e + _ -> {:reject, "[SimplePolicy]"} end end diff --git a/lib/pleroma/web/activity_pub/mrf/tag_policy.ex b/lib/pleroma/web/activity_pub/mrf/tag_policy.ex index c310462cb..febabda08 100644 --- a/lib/pleroma/web/activity_pub/mrf/tag_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/tag_policy.ex @@ -134,12 +134,13 @@ defp process_tag( if user.local == true do {:ok, message} else - {:reject, nil} + {:reject, + "[TagPolicy] Follow from #{actor} tagged with mrf_tag:disable-remote-subscription"} end end - defp process_tag("mrf_tag:disable-any-subscription", %{"type" => "Follow"}), - do: {:reject, nil} + defp process_tag("mrf_tag:disable-any-subscription", %{"type" => "Follow", "actor" => actor}), + do: {:reject, "[TagPolicy] Follow from #{actor} tagged with mrf_tag:disable-any-subscription"} defp process_tag(_, message), do: {:ok, message} diff --git a/lib/pleroma/web/activity_pub/mrf/user_allow_list_policy.ex b/lib/pleroma/web/activity_pub/mrf/user_allow_list_policy.ex index 651aed70f..1a28f2ba2 100644 --- a/lib/pleroma/web/activity_pub/mrf/user_allow_list_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/user_allow_list_policy.ex @@ -14,7 +14,7 @@ defp filter_by_list(%{"actor" => actor} = object, allow_list) do if actor in allow_list do {:ok, object} else - {:reject, nil} + {:reject, "[UserAllowListPolicy] #{actor} not in the list"} end end diff --git a/lib/pleroma/web/activity_pub/mrf/vocabulary_policy.ex b/lib/pleroma/web/activity_pub/mrf/vocabulary_policy.ex index 6167a74e2..a6c545570 100644 --- a/lib/pleroma/web/activity_pub/mrf/vocabulary_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/vocabulary_policy.ex @@ -11,22 +11,26 @@ def filter(%{"type" => "Undo", "object" => child_message} = message) do with {:ok, _} <- filter(child_message) do {:ok, message} else - {:reject, nil} -> - {:reject, nil} + {:reject, _} = e -> e end end def filter(%{"type" => message_type} = message) do with accepted_vocabulary <- Pleroma.Config.get([:mrf_vocabulary, :accept]), rejected_vocabulary <- Pleroma.Config.get([:mrf_vocabulary, :reject]), - true <- - Enum.empty?(accepted_vocabulary) || Enum.member?(accepted_vocabulary, message_type), - false <- - length(rejected_vocabulary) > 0 && Enum.member?(rejected_vocabulary, message_type), + {_, true} <- + {:accepted, + Enum.empty?(accepted_vocabulary) || Enum.member?(accepted_vocabulary, message_type)}, + {_, false} <- + {:rejected, + length(rejected_vocabulary) > 0 && Enum.member?(rejected_vocabulary, message_type)}, {:ok, _} <- filter(message["object"]) do {:ok, message} else - _ -> {:reject, nil} + {:reject, _} = e -> e + {:accepted, _} -> {:reject, "[VocabularyPolicy] #{message_type} not in accept list"} + {:rejected, _} -> {:reject, "[VocabularyPolicy] #{message_type} in reject list"} + _ -> {:reject, "[VocabularyPolicy]"} end end diff --git a/test/web/activity_pub/mrf/anti_followbot_policy_test.exs b/test/web/activity_pub/mrf/anti_followbot_policy_test.exs index fca0de7c6..3c795f5ac 100644 --- a/test/web/activity_pub/mrf/anti_followbot_policy_test.exs +++ b/test/web/activity_pub/mrf/anti_followbot_policy_test.exs @@ -21,7 +21,7 @@ test "matches followbots by nickname" do "id" => "https://example.com/activities/1234" } - {:reject, nil} = AntiFollowbotPolicy.filter(message) + assert {:reject, "[AntiFollowbotPolicy]" <> _} = AntiFollowbotPolicy.filter(message) end test "matches followbots by display name" do @@ -36,7 +36,7 @@ test "matches followbots by display name" do "id" => "https://example.com/activities/1234" } - {:reject, nil} = AntiFollowbotPolicy.filter(message) + assert {:reject, "[AntiFollowbotPolicy]" <> _} = AntiFollowbotPolicy.filter(message) end end diff --git a/test/web/activity_pub/mrf/hellthread_policy_test.exs b/test/web/activity_pub/mrf/hellthread_policy_test.exs index 6e9daa7f9..26f5bcdaa 100644 --- a/test/web/activity_pub/mrf/hellthread_policy_test.exs +++ b/test/web/activity_pub/mrf/hellthread_policy_test.exs @@ -50,7 +50,8 @@ test "rejects the message if the recipient count is above reject_threshold", %{ } do Pleroma.Config.put([:mrf_hellthread], %{delist_threshold: 0, reject_threshold: 2}) - {:reject, nil} = filter(message) + assert {:reject, "[HellthreadPolicy] 3 recipients is over the limit of 2"} == + filter(message) end test "does not reject the message if the recipient count is below reject_threshold", %{ diff --git a/test/web/activity_pub/mrf/keyword_policy_test.exs b/test/web/activity_pub/mrf/keyword_policy_test.exs index fd1f7aec8..b3d0f3d90 100644 --- a/test/web/activity_pub/mrf/keyword_policy_test.exs +++ b/test/web/activity_pub/mrf/keyword_policy_test.exs @@ -25,7 +25,8 @@ test "rejects if string matches in content" do } } - assert {:reject, nil} == KeywordPolicy.filter(message) + assert {:reject, "[KeywordPolicy] Matches with rejected keyword"} = + KeywordPolicy.filter(message) end test "rejects if string matches in summary" do @@ -39,7 +40,8 @@ test "rejects if string matches in summary" do } } - assert {:reject, nil} == KeywordPolicy.filter(message) + assert {:reject, "[KeywordPolicy] Matches with rejected keyword"} = + KeywordPolicy.filter(message) end test "rejects if regex matches in content" do @@ -55,7 +57,8 @@ test "rejects if regex matches in content" do } } - {:reject, nil} == KeywordPolicy.filter(message) + {:reject, "[KeywordPolicy] Matches with rejected keyword"} == + KeywordPolicy.filter(message) end) end @@ -72,7 +75,8 @@ test "rejects if regex matches in summary" do } } - {:reject, nil} == KeywordPolicy.filter(message) + {:reject, "[KeywordPolicy] Matches with rejected keyword"} == + KeywordPolicy.filter(message) end) end end diff --git a/test/web/activity_pub/mrf/mention_policy_test.exs b/test/web/activity_pub/mrf/mention_policy_test.exs index aa003bef5..220309cc9 100644 --- a/test/web/activity_pub/mrf/mention_policy_test.exs +++ b/test/web/activity_pub/mrf/mention_policy_test.exs @@ -76,7 +76,8 @@ test "to" do "to" => ["https://example.com/blocked"] } - assert MentionPolicy.filter(message) == {:reject, nil} + assert MentionPolicy.filter(message) == + {:reject, "[MentionPolicy] Rejected for mention of https://example.com/blocked"} end test "cc" do @@ -88,7 +89,8 @@ test "cc" do "cc" => ["https://example.com/blocked"] } - assert MentionPolicy.filter(message) == {:reject, nil} + assert MentionPolicy.filter(message) == + {:reject, "[MentionPolicy] Rejected for mention of https://example.com/blocked"} end end end diff --git a/test/web/activity_pub/mrf/reject_non_public_test.exs b/test/web/activity_pub/mrf/reject_non_public_test.exs index f36299b86..58b46b9a2 100644 --- a/test/web/activity_pub/mrf/reject_non_public_test.exs +++ b/test/web/activity_pub/mrf/reject_non_public_test.exs @@ -64,7 +64,7 @@ test "it's rejected when addrer of message in the follower addresses of user and } Pleroma.Config.put([:mrf_rejectnonpublic, :allow_followersonly], false) - assert {:reject, nil} = RejectNonPublic.filter(message) + assert {:reject, _} = RejectNonPublic.filter(message) end end @@ -94,7 +94,7 @@ test "it's reject when direct messages aren't allow" do } Pleroma.Config.put([:mrf_rejectnonpublic, :allow_direct], false) - assert {:reject, nil} = RejectNonPublic.filter(message) + assert {:reject, _} = RejectNonPublic.filter(message) end end end diff --git a/test/web/activity_pub/mrf/simple_policy_test.exs b/test/web/activity_pub/mrf/simple_policy_test.exs index b7b9bc6a2..e842d8d8d 100644 --- a/test/web/activity_pub/mrf/simple_policy_test.exs +++ b/test/web/activity_pub/mrf/simple_policy_test.exs @@ -124,7 +124,7 @@ test "has a matching host" do report_message = build_report_message() local_message = build_local_message() - assert SimplePolicy.filter(report_message) == {:reject, nil} + assert {:reject, _} = SimplePolicy.filter(report_message) assert SimplePolicy.filter(local_message) == {:ok, local_message} end @@ -133,7 +133,7 @@ test "match with wildcard domain" do report_message = build_report_message() local_message = build_local_message() - assert SimplePolicy.filter(report_message) == {:reject, nil} + assert {:reject, _} = SimplePolicy.filter(report_message) assert SimplePolicy.filter(local_message) == {:ok, local_message} end end @@ -241,7 +241,7 @@ test "activity has a matching host" do remote_message = build_remote_message() - assert SimplePolicy.filter(remote_message) == {:reject, nil} + assert {:reject, _} = SimplePolicy.filter(remote_message) end test "activity matches with wildcard domain" do @@ -249,7 +249,7 @@ test "activity matches with wildcard domain" do remote_message = build_remote_message() - assert SimplePolicy.filter(remote_message) == {:reject, nil} + assert {:reject, _} = SimplePolicy.filter(remote_message) end test "actor has a matching host" do @@ -257,7 +257,7 @@ test "actor has a matching host" do remote_user = build_remote_user() - assert SimplePolicy.filter(remote_user) == {:reject, nil} + assert {:reject, _} = SimplePolicy.filter(remote_user) end end @@ -279,7 +279,7 @@ test "is not empty but activity doesn't have a matching host" do remote_message = build_remote_message() assert SimplePolicy.filter(local_message) == {:ok, local_message} - assert SimplePolicy.filter(remote_message) == {:reject, nil} + assert {:reject, _} = SimplePolicy.filter(remote_message) end test "activity has a matching host" do @@ -429,7 +429,7 @@ test "it accepts deletions even from non-whitelisted servers" do test "it rejects the deletion" do deletion_message = build_remote_deletion_message() - assert SimplePolicy.filter(deletion_message) == {:reject, nil} + assert {:reject, _} = SimplePolicy.filter(deletion_message) end end @@ -439,7 +439,7 @@ test "it rejects the deletion" do test "it rejects the deletion" do deletion_message = build_remote_deletion_message() - assert SimplePolicy.filter(deletion_message) == {:reject, nil} + assert {:reject, _} = SimplePolicy.filter(deletion_message) end end diff --git a/test/web/activity_pub/mrf/tag_policy_test.exs b/test/web/activity_pub/mrf/tag_policy_test.exs index e7793641a..6ff71d640 100644 --- a/test/web/activity_pub/mrf/tag_policy_test.exs +++ b/test/web/activity_pub/mrf/tag_policy_test.exs @@ -12,8 +12,8 @@ defmodule Pleroma.Web.ActivityPub.MRF.TagPolicyTest do describe "mrf_tag:disable-any-subscription" do test "rejects message" do actor = insert(:user, tags: ["mrf_tag:disable-any-subscription"]) - message = %{"object" => actor.ap_id, "type" => "Follow"} - assert {:reject, nil} = TagPolicy.filter(message) + message = %{"object" => actor.ap_id, "type" => "Follow", "actor" => actor.ap_id} + assert {:reject, _} = TagPolicy.filter(message) end end @@ -22,7 +22,7 @@ test "rejects non-local follow requests" do actor = insert(:user, tags: ["mrf_tag:disable-remote-subscription"]) follower = insert(:user, tags: ["mrf_tag:disable-remote-subscription"], local: false) message = %{"object" => actor.ap_id, "type" => "Follow", "actor" => follower.ap_id} - assert {:reject, nil} = TagPolicy.filter(message) + assert {:reject, _} = TagPolicy.filter(message) end test "allows non-local follow requests" do diff --git a/test/web/activity_pub/mrf/user_allowlist_policy_test.exs b/test/web/activity_pub/mrf/user_allowlist_policy_test.exs index ba1b69658..8e1ad5bc8 100644 --- a/test/web/activity_pub/mrf/user_allowlist_policy_test.exs +++ b/test/web/activity_pub/mrf/user_allowlist_policy_test.exs @@ -26,6 +26,6 @@ test "rejected if allow list isn't empty and user not in allow list" do actor = insert(:user) Pleroma.Config.put([:mrf_user_allowlist], %{"localhost" => ["test-ap-id"]}) message = %{"actor" => actor.ap_id} - assert UserAllowListPolicy.filter(message) == {:reject, nil} + assert {:reject, _} = UserAllowListPolicy.filter(message) end end diff --git a/test/web/activity_pub/mrf/vocabulary_policy_test.exs b/test/web/activity_pub/mrf/vocabulary_policy_test.exs index 69f22bb77..2bceb67ee 100644 --- a/test/web/activity_pub/mrf/vocabulary_policy_test.exs +++ b/test/web/activity_pub/mrf/vocabulary_policy_test.exs @@ -46,7 +46,7 @@ test "it does not accept disallowed child objects" do } } - {:reject, nil} = VocabularyPolicy.filter(message) + {:reject, _} = VocabularyPolicy.filter(message) end test "it does not accept disallowed parent types" do @@ -60,7 +60,7 @@ test "it does not accept disallowed parent types" do } } - {:reject, nil} = VocabularyPolicy.filter(message) + {:reject, _} = VocabularyPolicy.filter(message) end end @@ -75,7 +75,7 @@ test "it rejects based on parent activity type" do "object" => "whatever" } - {:reject, nil} = VocabularyPolicy.filter(message) + {:reject, _} = VocabularyPolicy.filter(message) end test "it rejects based on child object type" do @@ -89,7 +89,7 @@ test "it rejects based on child object type" do } } - {:reject, nil} = VocabularyPolicy.filter(message) + {:reject, _} = VocabularyPolicy.filter(message) end test "it passes through objects that aren't disallowed" do From 8d56fb6d223995de3f753eeef9475583e2b1e6ad Mon Sep 17 00:00:00 2001 From: rinpatch Date: Tue, 14 Jul 2020 12:00:53 +0300 Subject: [PATCH 06/13] Migrate in-db config after updating to Oban 2.0 --- docs/configuration/cheatsheet.md | 3 +-- ...20200714081657_oban_2_0_config_changes.exs | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 priv/repo/migrations/20200714081657_oban_2_0_config_changes.exs diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index f796330f1..7b1fd92f3 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -629,8 +629,7 @@ Email notifications settings. Configuration options described in [Oban readme](https://github.com/sorentwo/oban#usage): * `repo` - app's Ecto repo (`Pleroma.Repo`) -* `verbose` - logs verbosity -* `prune` - non-retryable jobs [pruning settings](https://github.com/sorentwo/oban#pruning) (`:disabled` / `{:maxlen, value}` / `{:maxage, value}`) +* `log` - logs verbosity * `queues` - job queues (see below) * `crontab` - periodic jobs, see [`Oban.Cron`](#obancron) diff --git a/priv/repo/migrations/20200714081657_oban_2_0_config_changes.exs b/priv/repo/migrations/20200714081657_oban_2_0_config_changes.exs new file mode 100644 index 000000000..c54bb2511 --- /dev/null +++ b/priv/repo/migrations/20200714081657_oban_2_0_config_changes.exs @@ -0,0 +1,27 @@ +defmodule Elixir.Pleroma.Repo.Migrations.Oban20ConfigChanges do + use Ecto.Migration + import Ecto.Query + alias Pleroma.ConfigDB + alias Pleroma.Repo + + def change do + config_entry = + from(c in ConfigDB, where: c.group == ^":pleroma" and c.key == ^"Oban") + |> select([c], struct(c, [:value, :id])) + |> Repo.one() + + if config_entry do + %{value: value} = config_entry + + value = + case Keyword.fetch(value, :verbose) do + {:ok, log} -> Keyword.put_new(value, :log, log) + _ -> value + end + |> Keyword.drop([:verbose, :prune]) + + Ecto.Changeset.change(config_entry, %{value: value}) + |> Repo.update() + end + end +end From e6ccc2556568f2180c3ce1945bdc7a0cba97e924 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Tue, 14 Jul 2020 11:41:30 +0300 Subject: [PATCH 07/13] Fix in-db configuration in dev environment Previously, in-db configuration only worked when `warnings_as_errors` was disabled because re-compiling scrubbers on application restart created a warning about module conflicts. This patch fixes that by enabling `ignore_module_conflict` option of the compiler at runtime, and enables `warnings_as_errors` in prod since there is no reason to keep it disabled anymore. --- lib/pleroma/application.ex | 4 ++++ mix.exs | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index b68a373a4..3282c6882 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -35,6 +35,10 @@ def user_agent do # See http://elixir-lang.org/docs/stable/elixir/Application.html # for more information on OTP Applications def start(_type, _args) do + # Scrubbers are compiled at runtime and therefore will cause a conflict + # every time the application is restarted, so we disable module + # conflicts at runtime + Code.compiler_options(ignore_module_conflict: true) Config.Holder.save_default() Pleroma.HTML.compile_scrubbers() Config.DeprecationWarnings.warn() diff --git a/mix.exs b/mix.exs index d7992ee37..741f917e6 100644 --- a/mix.exs +++ b/mix.exs @@ -90,8 +90,6 @@ defp elixirc_paths(:test), do: ["lib", "test/support"] defp elixirc_paths(_), do: ["lib"] defp warnings_as_errors(:prod), do: false - # Uncomment this if you need testing configurable_from_database logic - # defp warnings_as_errors(:dev), do: false defp warnings_as_errors(_), do: true # Specifies OAuth dependencies. From ce314e6fe236c7a41535dd8a9a0f097c74c6f1ce Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 14 Jul 2020 11:24:58 -0500 Subject: [PATCH 08/13] Clarify description and suggestion --- config/description.exs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/description.exs b/config/description.exs index 8ec4b712f..2b41e7dac 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2008,14 +2008,14 @@ label: "Pleroma Admin Token", type: :group, description: - "Allows to set a token that can be used to authenticate with the admin api without using an actual user by giving it as the `admin_token` parameter (risky; use HTTP Basic Auth or OAuth-based authentication if possible)", + "Allows setting a token that can be used to authenticate requests with admin privileges without a normal user account token. Append the `admin_token` parameter to requests to utilize it. (Please reconsider using HTTP Basic Auth or OAuth-based authentication if possible)", children: [ %{ key: :admin_token, type: :string, description: "Admin token", suggestions: [ - "We recommend NOT setting the value do to increased security risk; if set, use a secure random long string or UUID (and change it as often as possible)" + "Please use a high entropy string or UUID" ] } ] From 124b4709dcf12a417f5164e53ef3ba67e538d4c7 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 14 Jul 2020 19:31:05 +0300 Subject: [PATCH 09/13] [#1940] Added `admin_token` param (as `admin_api_params/0`) to existing Admin API OpenAPI operations. --- lib/pleroma/web/api_spec/helpers.ex | 4 ++++ .../web/api_spec/operations/admin/config_operation.ex | 3 +++ .../web/api_spec/operations/admin/invite_operation.ex | 4 ++++ .../operations/admin/media_proxy_cache_operation.ex | 3 +++ .../web/api_spec/operations/admin/oauth_app_operation.ex | 6 ++++-- .../web/api_spec/operations/admin/relay_operation.ex | 3 +++ .../web/api_spec/operations/admin/report_operation.ex | 7 +++++-- .../web/api_spec/operations/admin/status_operation.ex | 7 ++++--- test/web/admin_api/controllers/config_controller_test.exs | 8 ++++++++ 9 files changed, 38 insertions(+), 7 deletions(-) diff --git a/lib/pleroma/web/api_spec/helpers.ex b/lib/pleroma/web/api_spec/helpers.ex index a258e8421..2a7f1a706 100644 --- a/lib/pleroma/web/api_spec/helpers.ex +++ b/lib/pleroma/web/api_spec/helpers.ex @@ -29,6 +29,10 @@ def request_body(description, schema_ref, opts \\ []) do } end + def admin_api_params do + [Operation.parameter(:admin_token, :query, :string, "Allows authorization via admin token.")] + end + def pagination_params do [ Operation.parameter(:max_id, :query, :string, "Return items older than this ID"), diff --git a/lib/pleroma/web/api_spec/operations/admin/config_operation.ex b/lib/pleroma/web/api_spec/operations/admin/config_operation.ex index 7b38a2ef4..3a8380797 100644 --- a/lib/pleroma/web/api_spec/operations/admin/config_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/config_operation.ex @@ -26,6 +26,7 @@ def show_operation do %Schema{type: :boolean, default: false}, "Get only saved in database settings" ) + | admin_api_params() ], security: [%{"oAuth" => ["read"]}], responses: %{ @@ -41,6 +42,7 @@ def update_operation do summary: "Update config settings", operationId: "AdminAPI.ConfigController.update", security: [%{"oAuth" => ["write"]}], + parameters: admin_api_params(), requestBody: request_body("Parameters", %Schema{ type: :object, @@ -73,6 +75,7 @@ def descriptions_operation do summary: "Get JSON with config descriptions.", operationId: "AdminAPI.ConfigController.descriptions", security: [%{"oAuth" => ["read"]}], + parameters: admin_api_params(), responses: %{ 200 => Operation.response("Config Descriptions", "application/json", %Schema{ diff --git a/lib/pleroma/web/api_spec/operations/admin/invite_operation.ex b/lib/pleroma/web/api_spec/operations/admin/invite_operation.ex index d3af9db49..801024d75 100644 --- a/lib/pleroma/web/api_spec/operations/admin/invite_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/invite_operation.ex @@ -20,6 +20,7 @@ def index_operation do summary: "Get a list of generated invites", operationId: "AdminAPI.InviteController.index", security: [%{"oAuth" => ["read:invites"]}], + parameters: admin_api_params(), responses: %{ 200 => Operation.response("Invites", "application/json", %Schema{ @@ -51,6 +52,7 @@ def create_operation do summary: "Create an account registration invite token", operationId: "AdminAPI.InviteController.create", security: [%{"oAuth" => ["write:invites"]}], + parameters: admin_api_params(), requestBody: request_body("Parameters", %Schema{ type: :object, @@ -71,6 +73,7 @@ def revoke_operation do summary: "Revoke invite by token", operationId: "AdminAPI.InviteController.revoke", security: [%{"oAuth" => ["write:invites"]}], + parameters: admin_api_params(), requestBody: request_body( "Parameters", @@ -97,6 +100,7 @@ def email_operation do summary: "Sends registration invite via email", operationId: "AdminAPI.InviteController.email", security: [%{"oAuth" => ["write:invites"]}], + parameters: admin_api_params(), requestBody: request_body( "Parameters", diff --git a/lib/pleroma/web/api_spec/operations/admin/media_proxy_cache_operation.ex b/lib/pleroma/web/api_spec/operations/admin/media_proxy_cache_operation.ex index 0358cfbad..20d033f66 100644 --- a/lib/pleroma/web/api_spec/operations/admin/media_proxy_cache_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/media_proxy_cache_operation.ex @@ -33,6 +33,7 @@ def index_operation do %Schema{type: :integer, default: 50}, "Number of statuses to return" ) + | admin_api_params() ], responses: %{ 200 => success_response() @@ -46,6 +47,7 @@ def delete_operation do summary: "Remove a banned MediaProxy URL from Cachex", operationId: "AdminAPI.MediaProxyCacheController.delete", security: [%{"oAuth" => ["write:media_proxy_caches"]}], + parameters: admin_api_params(), requestBody: request_body( "Parameters", @@ -71,6 +73,7 @@ def purge_operation do summary: "Purge and optionally ban a MediaProxy URL", operationId: "AdminAPI.MediaProxyCacheController.purge", security: [%{"oAuth" => ["write:media_proxy_caches"]}], + parameters: admin_api_params(), requestBody: request_body( "Parameters", diff --git a/lib/pleroma/web/api_spec/operations/admin/oauth_app_operation.ex b/lib/pleroma/web/api_spec/operations/admin/oauth_app_operation.ex index fbc9f80d7..a75f3e622 100644 --- a/lib/pleroma/web/api_spec/operations/admin/oauth_app_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/oauth_app_operation.ex @@ -36,6 +36,7 @@ def index_operation do %Schema{type: :integer, default: 50}, "Number of apps to return" ) + | admin_api_params() ], responses: %{ 200 => @@ -72,6 +73,7 @@ def create_operation do summary: "Create OAuth App", operationId: "AdminAPI.OAuthAppController.create", requestBody: request_body("Parameters", create_request()), + parameters: admin_api_params(), security: [%{"oAuth" => ["write"]}], responses: %{ 200 => Operation.response("App", "application/json", oauth_app()), @@ -85,7 +87,7 @@ def update_operation do tags: ["Admin", "oAuth Apps"], summary: "Update OAuth App", operationId: "AdminAPI.OAuthAppController.update", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], security: [%{"oAuth" => ["write"]}], requestBody: request_body("Parameters", update_request()), responses: %{ @@ -103,7 +105,7 @@ def delete_operation do tags: ["Admin", "oAuth Apps"], summary: "Delete OAuth App", operationId: "AdminAPI.OAuthAppController.delete", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], security: [%{"oAuth" => ["write"]}], responses: %{ 204 => no_content_response(), diff --git a/lib/pleroma/web/api_spec/operations/admin/relay_operation.ex b/lib/pleroma/web/api_spec/operations/admin/relay_operation.ex index 7672cb467..67ee5eee0 100644 --- a/lib/pleroma/web/api_spec/operations/admin/relay_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/relay_operation.ex @@ -19,6 +19,7 @@ def index_operation do summary: "List Relays", operationId: "AdminAPI.RelayController.index", security: [%{"oAuth" => ["read"]}], + parameters: admin_api_params(), responses: %{ 200 => Operation.response("Response", "application/json", %Schema{ @@ -41,6 +42,7 @@ def follow_operation do summary: "Follow a Relay", operationId: "AdminAPI.RelayController.follow", security: [%{"oAuth" => ["write:follows"]}], + parameters: admin_api_params(), requestBody: request_body("Parameters", %Schema{ type: :object, @@ -64,6 +66,7 @@ def unfollow_operation do summary: "Unfollow a Relay", operationId: "AdminAPI.RelayController.unfollow", security: [%{"oAuth" => ["write:follows"]}], + parameters: admin_api_params(), requestBody: request_body("Parameters", %Schema{ type: :object, diff --git a/lib/pleroma/web/api_spec/operations/admin/report_operation.ex b/lib/pleroma/web/api_spec/operations/admin/report_operation.ex index 15e78bfaf..3bb7ec49e 100644 --- a/lib/pleroma/web/api_spec/operations/admin/report_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/report_operation.ex @@ -48,6 +48,7 @@ def index_operation do %Schema{type: :integer, default: 50}, "Number number of log entries per page" ) + | admin_api_params() ], responses: %{ 200 => @@ -71,7 +72,7 @@ def show_operation do tags: ["Admin", "Reports"], summary: "Get an individual report", operationId: "AdminAPI.ReportController.show", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], security: [%{"oAuth" => ["read:reports"]}], responses: %{ 200 => Operation.response("Report", "application/json", report()), @@ -86,6 +87,7 @@ def update_operation do summary: "Change the state of one or multiple reports", operationId: "AdminAPI.ReportController.update", security: [%{"oAuth" => ["write:reports"]}], + parameters: admin_api_params(), requestBody: request_body("Parameters", update_request(), required: true), responses: %{ 204 => no_content_response(), @@ -100,7 +102,7 @@ def notes_create_operation do tags: ["Admin", "Reports"], summary: "Create report note", operationId: "AdminAPI.ReportController.notes_create", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], requestBody: request_body("Parameters", %Schema{ type: :object, @@ -124,6 +126,7 @@ def notes_delete_operation do parameters: [ Operation.parameter(:report_id, :path, :string, "Report ID"), Operation.parameter(:id, :path, :string, "Note ID") + | admin_api_params() ], security: [%{"oAuth" => ["write:reports"]}], responses: %{ diff --git a/lib/pleroma/web/api_spec/operations/admin/status_operation.ex b/lib/pleroma/web/api_spec/operations/admin/status_operation.ex index 745399b4b..c105838a4 100644 --- a/lib/pleroma/web/api_spec/operations/admin/status_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/status_operation.ex @@ -55,6 +55,7 @@ def index_operation do %Schema{type: :integer, default: 50}, "Number of statuses to return" ) + | admin_api_params() ], responses: %{ 200 => @@ -71,7 +72,7 @@ def show_operation do tags: ["Admin", "Statuses"], summary: "Show Status", operationId: "AdminAPI.StatusController.show", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], security: [%{"oAuth" => ["read:statuses"]}], responses: %{ 200 => Operation.response("Status", "application/json", status()), @@ -85,7 +86,7 @@ def update_operation do tags: ["Admin", "Statuses"], summary: "Change the scope of an individual reported status", operationId: "AdminAPI.StatusController.update", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], security: [%{"oAuth" => ["write:statuses"]}], requestBody: request_body("Parameters", update_request(), required: true), responses: %{ @@ -100,7 +101,7 @@ def delete_operation do tags: ["Admin", "Statuses"], summary: "Delete an individual reported status", operationId: "AdminAPI.StatusController.delete", - parameters: [id_param()], + parameters: [id_param() | admin_api_params()], security: [%{"oAuth" => ["write:statuses"]}], responses: %{ 200 => empty_object_response(), diff --git a/test/web/admin_api/controllers/config_controller_test.exs b/test/web/admin_api/controllers/config_controller_test.exs index 064ef9bc7..61bc9fd39 100644 --- a/test/web/admin_api/controllers/config_controller_test.exs +++ b/test/web/admin_api/controllers/config_controller_test.exs @@ -152,6 +152,14 @@ test "subkeys with full update right merge", %{conn: conn} do assert emoji_val[:groups] == [a: 1, b: 2] assert assets_val[:mascots] == [a: 1, b: 2] end + + test "with valid `admin_token` query parameter, skips OAuth scopes check" do + clear_config([:admin_token], "password123") + + build_conn() + |> get("/api/pleroma/admin/config?admin_token=password123") + |> json_response_and_validate_schema(200) + end end test "POST /api/pleroma/admin/config error", %{conn: conn} do From 1dd767b8c7b7565ad94ccb85324e97fa9885923e Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Tue, 14 Jul 2020 21:44:08 +0300 Subject: [PATCH 10/13] Include port in host for signatures --- lib/pleroma/web/activity_pub/publisher.ex | 20 +++++++++---- test/web/activity_pub/publisher_test.exs | 34 ++++++++++++++++++++++- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index b70cbd043..d88f7f3ee 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/lib/pleroma/web/activity_pub/publisher.ex @@ -49,7 +49,8 @@ def is_representable?(%Activity{} = activity) do """ def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = params) do Logger.debug("Federating #{id} to #{inbox}") - %{host: host, path: path} = URI.parse(inbox) + + uri = URI.parse(inbox) digest = "SHA-256=" <> (:crypto.hash(:sha256, json) |> Base.encode64()) @@ -57,8 +58,8 @@ def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = pa signature = Pleroma.Signature.sign(actor, %{ - "(request-target)": "post #{path}", - host: host, + "(request-target)": "post #{uri.path}", + host: signature_host(uri), "content-length": byte_size(json), digest: digest, date: date @@ -76,8 +77,9 @@ def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = pa {"digest", digest} ] ) do - if !Map.has_key?(params, :unreachable_since) || params[:unreachable_since], - do: Instances.set_reachable(inbox) + if not Map.has_key?(params, :unreachable_since) || params[:unreachable_since] do + Instances.set_reachable(inbox) + end result else @@ -96,6 +98,14 @@ def publish_one(%{actor_id: actor_id} = params) do |> publish_one() end + defp signature_host(%URI{port: port, scheme: scheme, host: host}) do + if port == URI.default_port(scheme) do + host + else + "#{host}:#{port}" + end + end + defp should_federate?(inbox, public) do if public do true diff --git a/test/web/activity_pub/publisher_test.exs b/test/web/activity_pub/publisher_test.exs index c2bc38d52..b9388b966 100644 --- a/test/web/activity_pub/publisher_test.exs +++ b/test/web/activity_pub/publisher_test.exs @@ -123,6 +123,39 @@ test "it returns inbox for messages involving single recipients in total" do end describe "publish_one/1" do + test "publish to url with with different ports" do + inbox80 = "http://42.site/users/nick1/inbox" + inbox42 = "http://42.site:42/users/nick1/inbox" + + mock(fn + %{method: :post, url: "http://42.site:42/users/nick1/inbox"} -> + {:ok, %Tesla.Env{status: 200, body: "port 42"}} + + %{method: :post, url: "http://42.site/users/nick1/inbox"} -> + {:ok, %Tesla.Env{status: 200, body: "port 80"}} + end) + + actor = insert(:user) + + assert {:ok, %{body: "port 42"}} = + Publisher.publish_one(%{ + inbox: inbox42, + json: "{}", + actor: actor, + id: 1, + unreachable_since: true + }) + + assert {:ok, %{body: "port 80"}} = + Publisher.publish_one(%{ + inbox: inbox80, + json: "{}", + actor: actor, + id: 1, + unreachable_since: true + }) + end + test_with_mock "calls `Instances.set_reachable` on successful federation if `unreachable_since` is not specified", Instances, [:passthrough], @@ -131,7 +164,6 @@ test "it returns inbox for messages involving single recipients in total" do inbox = "http://200.site/users/nick1/inbox" assert {:ok, _} = Publisher.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1}) - assert called(Instances.set_reachable(inbox)) end From 6d8427cca21db0a9250f6ce32fe513c0bef7cddb Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Wed, 15 Jul 2020 09:58:35 +0200 Subject: [PATCH 11/13] AP C2S tests: Make sure you can't use another user's AP id --- .../activity_pub_controller_test.exs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs index e722f7c04..ed900d8f8 100644 --- a/test/web/activity_pub/activity_pub_controller_test.exs +++ b/test/web/activity_pub/activity_pub_controller_test.exs @@ -1082,6 +1082,45 @@ test "it increases like count when receiving a like action", %{conn: conn} do assert object = Object.get_by_ap_id(note_object.data["id"]) assert object.data["like_count"] == 1 end + + test "it doesn't spreads faulty attributedTo or actor fields", %{ + conn: conn, + activity: activity + } do + reimu = insert(:user, nickname: "reimu") + cirno = insert(:user, nickname: "cirno") + + assert reimu.ap_id + assert cirno.ap_id + + activity = + activity + |> put_in(["object", "actor"], reimu.ap_id) + |> put_in(["object", "attributedTo"], reimu.ap_id) + |> put_in(["actor"], reimu.ap_id) + |> put_in(["attributedTo"], reimu.ap_id) + + _reimu_outbox = + conn + |> assign(:user, cirno) + |> put_req_header("content-type", "application/activity+json") + |> post("/users/#{reimu.nickname}/outbox", activity) + |> json_response(403) + + cirno_outbox = + conn + |> assign(:user, cirno) + |> put_req_header("content-type", "application/activity+json") + |> post("/users/#{cirno.nickname}/outbox", activity) + |> json_response(201) + + assert cirno_outbox["attributedTo"] == nil + assert cirno_outbox["actor"] == cirno.ap_id + + assert cirno_object = Object.normalize(cirno_outbox["object"]) + assert cirno_object.data["actor"] == cirno.ap_id + assert cirno_object.data["attributedTo"] == cirno.ap_id + end end describe "/relay/followers" do From c413649a8db26db742ff53c6c09a9a3b96e8cb6a Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 15 Jul 2020 16:20:17 +0300 Subject: [PATCH 12/13] Bring back oban job pruning Closes #1945 --- config/config.exs | 1 + 1 file changed, 1 insertion(+) diff --git a/config/config.exs b/config/config.exs index 6fc84efc2..daeefdca3 100644 --- a/config/config.exs +++ b/config/config.exs @@ -512,6 +512,7 @@ attachments_cleanup: 5, new_users_digest: 1 ], + plugins: [Oban.Plugins.Pruner], crontab: [ {"0 0 * * *", Pleroma.Workers.Cron.ClearOauthTokenWorker}, {"0 * * * *", Pleroma.Workers.Cron.StatsWorker}, From d29b8997f4a3601eac7f2e1e57de27a67df6699c Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Wed, 15 Jul 2020 15:25:33 +0200 Subject: [PATCH 13/13] MastoAPI: fix & test giving MRF reject reasons --- .../mastodon_api/controllers/status_controller.ex | 5 +++++ .../controllers/status_controller_test.exs | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index 12be530c9..9bb2ef117 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -172,6 +172,11 @@ def create(%{assigns: %{user: user}, body_params: %{status: _} = params} = conn, with_direct_conversation_id: true ) else + {:error, {:reject, message}} -> + conn + |> put_status(:unprocessable_entity) + |> json(%{error: message}) + {:error, message} -> conn |> put_status(:unprocessable_entity) diff --git a/test/web/mastodon_api/controllers/status_controller_test.exs b/test/web/mastodon_api/controllers/status_controller_test.exs index fd2de8d80..d34f300da 100644 --- a/test/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/web/mastodon_api/controllers/status_controller_test.exs @@ -22,6 +22,8 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do setup do: clear_config([:instance, :federating]) setup do: clear_config([:instance, :allow_relay]) setup do: clear_config([:rich_media, :enabled]) + setup do: clear_config([:mrf, :policies]) + setup do: clear_config([:mrf_keyword, :reject]) describe "posting statuses" do setup do: oauth_access(["write:statuses"]) @@ -157,6 +159,17 @@ test "it fails to create a status if `expires_in` is less or equal than an hour" |> json_response_and_validate_schema(422) end + test "Get MRF reason when posting a status is rejected by one", %{conn: conn} do + Pleroma.Config.put([:mrf_keyword, :reject], ["GNO"]) + Pleroma.Config.put([:mrf, :policies], [Pleroma.Web.ActivityPub.MRF.KeywordPolicy]) + + assert %{"error" => "[KeywordPolicy] Matches with rejected keyword"} = + conn + |> put_req_header("content-type", "application/json") + |> post("api/v1/statuses", %{"status" => "GNO/Linux"}) + |> json_response_and_validate_schema(422) + end + test "posting an undefined status with an attachment", %{user: user, conn: conn} do file = %Plug.Upload{ content_type: "image/jpg",