From d08c63500b5deca268ebc24833be4cb3279bdaaa Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 4 May 2020 20:16:18 +0400 Subject: [PATCH 1/7] Ignore unexpected query params --- lib/pleroma/web/api_spec/cast_and_validate.ex | 120 ++++++++++++++++++ .../controllers/account_controller.ex | 2 +- .../controllers/app_controller.ex | 2 +- .../controllers/custom_emoji_controller.ex | 2 +- .../controllers/domain_block_controller.ex | 2 +- .../controllers/notification_controller.ex | 2 +- .../controllers/report_controller.ex | 2 +- 7 files changed, 126 insertions(+), 6 deletions(-) create mode 100644 lib/pleroma/web/api_spec/cast_and_validate.ex diff --git a/lib/pleroma/web/api_spec/cast_and_validate.ex b/lib/pleroma/web/api_spec/cast_and_validate.ex new file mode 100644 index 000000000..f36cf7a55 --- /dev/null +++ b/lib/pleroma/web/api_spec/cast_and_validate.ex @@ -0,0 +1,120 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ApiSpec.CastAndValidate do + @moduledoc """ + This plug is based on [`OpenApiSpex.Plug.CastAndValidate`] + (https://github.com/open-api-spex/open_api_spex/blob/master/lib/open_api_spex/plug/cast_and_validate.ex). + The main difference is ignoring unexpected query params + instead of throwing an error. Also, the default rendering + error module is `Pleroma.Web.ApiSpec.RenderError`. + """ + + @behaviour Plug + + alias Plug.Conn + + @impl Plug + def init(opts) do + opts + |> Map.new() + |> Map.put_new(:render_error, Pleroma.Web.ApiSpec.RenderError) + end + + @impl Plug + def call(%{private: %{open_api_spex: private_data}} = conn, %{ + operation_id: operation_id, + render_error: render_error + }) do + spec = private_data.spec + operation = private_data.operation_lookup[operation_id] + + content_type = + case Conn.get_req_header(conn, "content-type") do + [header_value | _] -> + header_value + |> String.split(";") + |> List.first() + + _ -> + nil + end + + private_data = Map.put(private_data, :operation_id, operation_id) + conn = Conn.put_private(conn, :open_api_spex, private_data) + + case cast_and_validate(spec, operation, conn, content_type) do + {:ok, conn} -> + conn + + {:error, reason} -> + opts = render_error.init(reason) + + conn + |> render_error.call(opts) + |> Plug.Conn.halt() + end + end + + def call( + %{ + private: %{ + phoenix_controller: controller, + phoenix_action: action, + open_api_spex: private_data + } + } = conn, + opts + ) do + operation = + case private_data.operation_lookup[{controller, action}] do + nil -> + operation_id = controller.open_api_operation(action).operationId + operation = private_data.operation_lookup[operation_id] + + operation_lookup = + private_data.operation_lookup + |> Map.put({controller, action}, operation) + + OpenApiSpex.Plug.Cache.adapter().put( + private_data.spec_module, + {private_data.spec, operation_lookup} + ) + + operation + + operation -> + operation + end + + if operation.operationId do + call(conn, Map.put(opts, :operation_id, operation.operationId)) + else + raise "operationId was not found in action API spec" + end + end + + def call(conn, opts), do: OpenApiSpex.Plug.CastAndValidate.call(conn, opts) + + defp cast_and_validate(spec, operation, conn, content_type) do + case OpenApiSpex.cast_and_validate(spec, operation, conn, content_type) do + {:ok, conn} -> + {:ok, conn} + + # Remove unexpected query params and cast/validate again + {:error, errors} -> + query_params = + Enum.reduce(errors, conn.query_params, fn + %{reason: :unexpected_field, name: name, path: [name]}, params -> + Map.delete(params, name) + + _, params -> + params + end) + + conn = %Conn{conn | query_params: query_params} + OpenApiSpex.cast_and_validate(spec, operation, conn, content_type) + end + end +end diff --git a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex index 61b0e2f63..8458cbdd5 100644 --- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex @@ -27,7 +27,7 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do alias Pleroma.Web.OAuth.Token alias Pleroma.Web.TwitterAPI.TwitterAPI - plug(OpenApiSpex.Plug.CastAndValidate, render_error: Pleroma.Web.ApiSpec.RenderError) + plug(Pleroma.Web.ApiSpec.CastAndValidate) plug(:skip_plug, [OAuthScopesPlug, EnsurePublicOrAuthenticatedPlug] when action == :create) diff --git a/lib/pleroma/web/mastodon_api/controllers/app_controller.ex b/lib/pleroma/web/mastodon_api/controllers/app_controller.ex index 408e11474..a516b6c20 100644 --- a/lib/pleroma/web/mastodon_api/controllers/app_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/app_controller.ex @@ -22,7 +22,7 @@ defmodule Pleroma.Web.MastodonAPI.AppController do plug(OAuthScopesPlug, %{scopes: ["read"]} when action == :verify_credentials) - plug(OpenApiSpex.Plug.CastAndValidate) + plug(Pleroma.Web.ApiSpec.CastAndValidate) @local_mastodon_name "Mastodon-Local" diff --git a/lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex b/lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex index 000ad743f..c5f47c5df 100644 --- a/lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex @@ -5,7 +5,7 @@ defmodule Pleroma.Web.MastodonAPI.CustomEmojiController do use Pleroma.Web, :controller - plug(OpenApiSpex.Plug.CastAndValidate) + plug(Pleroma.Web.ApiSpec.CastAndValidate) plug( :skip_plug, diff --git a/lib/pleroma/web/mastodon_api/controllers/domain_block_controller.ex b/lib/pleroma/web/mastodon_api/controllers/domain_block_controller.ex index c4fa383f2..825b231ab 100644 --- a/lib/pleroma/web/mastodon_api/controllers/domain_block_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/domain_block_controller.ex @@ -8,7 +8,7 @@ defmodule Pleroma.Web.MastodonAPI.DomainBlockController do alias Pleroma.Plugs.OAuthScopesPlug alias Pleroma.User - plug(OpenApiSpex.Plug.CastAndValidate) + plug(Pleroma.Web.ApiSpec.CastAndValidate) defdelegate open_api_operation(action), to: Pleroma.Web.ApiSpec.DomainBlockOperation plug( diff --git a/lib/pleroma/web/mastodon_api/controllers/notification_controller.ex b/lib/pleroma/web/mastodon_api/controllers/notification_controller.ex index a14c86893..596b85617 100644 --- a/lib/pleroma/web/mastodon_api/controllers/notification_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/notification_controller.ex @@ -13,7 +13,7 @@ defmodule Pleroma.Web.MastodonAPI.NotificationController do @oauth_read_actions [:show, :index] - plug(OpenApiSpex.Plug.CastAndValidate, render_error: Pleroma.Web.ApiSpec.RenderError) + plug(Pleroma.Web.ApiSpec.CastAndValidate) plug( OAuthScopesPlug, diff --git a/lib/pleroma/web/mastodon_api/controllers/report_controller.ex b/lib/pleroma/web/mastodon_api/controllers/report_controller.ex index f65c5c62b..405167108 100644 --- a/lib/pleroma/web/mastodon_api/controllers/report_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/report_controller.ex @@ -9,7 +9,7 @@ defmodule Pleroma.Web.MastodonAPI.ReportController do action_fallback(Pleroma.Web.MastodonAPI.FallbackController) - plug(OpenApiSpex.Plug.CastAndValidate, render_error: Pleroma.Web.ApiSpec.RenderError) + plug(Pleroma.Web.ApiSpec.CastAndValidate) plug(OAuthScopesPlug, %{scopes: ["write:reports"]} when action == :create) defdelegate open_api_operation(action), to: Pleroma.Web.ApiSpec.ReportOperation From bfbff7d82673d6128a18e73dcc91f70ee669c2ac Mon Sep 17 00:00:00 2001 From: minibikini Date: Mon, 4 May 2020 16:38:23 +0000 Subject: [PATCH 2/7] Apply suggestion to lib/pleroma/web/api_spec/cast_and_validate.ex --- lib/pleroma/web/api_spec/cast_and_validate.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/api_spec/cast_and_validate.ex b/lib/pleroma/web/api_spec/cast_and_validate.ex index f36cf7a55..cd02403c1 100644 --- a/lib/pleroma/web/api_spec/cast_and_validate.ex +++ b/lib/pleroma/web/api_spec/cast_and_validate.ex @@ -1,5 +1,6 @@ # Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors +# Copyright © 2019-2020 Moxley Stratton, Mike Buhot , MPL-2.0 +# Copyright © 2020 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.ApiSpec.CastAndValidate do From 4b9ab67aa8bdf7fdf7390080932fee2e5879a5e4 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 4 May 2020 21:46:25 +0400 Subject: [PATCH 3/7] Ignore unexpected ENUM values in query string --- lib/pleroma/web/api_spec/cast_and_validate.ex | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/pleroma/web/api_spec/cast_and_validate.ex b/lib/pleroma/web/api_spec/cast_and_validate.ex index cd02403c1..b94517c52 100644 --- a/lib/pleroma/web/api_spec/cast_and_validate.ex +++ b/lib/pleroma/web/api_spec/cast_and_validate.ex @@ -110,6 +110,10 @@ defp cast_and_validate(spec, operation, conn, content_type) do %{reason: :unexpected_field, name: name, path: [name]}, params -> Map.delete(params, name) + %{reason: :invalid_enum, name: nil, path: path, value: value}, params -> + path = path |> Enum.reverse() |> tl() |> Enum.reverse() |> list_items_to_string() + update_in(params, path, &List.delete(&1, value)) + _, params -> params end) @@ -118,4 +122,11 @@ defp cast_and_validate(spec, operation, conn, content_type) do OpenApiSpex.cast_and_validate(spec, operation, conn, content_type) end end + + defp list_items_to_string(list) do + Enum.map(list, fn + i when is_atom(i) -> to_string(i) + i -> i + end) + end end From f070b5569ca0eafdca79f1f3e3b6b5025f3f8fc9 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 4 May 2020 22:33:05 +0400 Subject: [PATCH 4/7] Add a config option to enable strict validation --- config/config.exs | 2 ++ lib/pleroma/web/api_spec/cast_and_validate.ex | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/config/config.exs b/config/config.exs index a6c6d6f99..ca9bbab64 100644 --- a/config/config.exs +++ b/config/config.exs @@ -653,6 +653,8 @@ profiles: %{local: false, remote: false}, activities: %{local: false, remote: false} +config :pleroma, Pleroma.Web.ApiSpec.CastAndValidate, strict: false + # Import environment specific config. This must remain at the bottom # of this file so it overrides the configuration defined above. import_config "#{Mix.env()}.exs" diff --git a/lib/pleroma/web/api_spec/cast_and_validate.ex b/lib/pleroma/web/api_spec/cast_and_validate.ex index b94517c52..bd9026237 100644 --- a/lib/pleroma/web/api_spec/cast_and_validate.ex +++ b/lib/pleroma/web/api_spec/cast_and_validate.ex @@ -7,9 +7,10 @@ defmodule Pleroma.Web.ApiSpec.CastAndValidate do @moduledoc """ This plug is based on [`OpenApiSpex.Plug.CastAndValidate`] (https://github.com/open-api-spex/open_api_spex/blob/master/lib/open_api_spex/plug/cast_and_validate.ex). - The main difference is ignoring unexpected query params - instead of throwing an error. Also, the default rendering - error module is `Pleroma.Web.ApiSpec.RenderError`. + The main difference is ignoring unexpected query params instead of throwing + an error and a config option (`[Pleroma.Web.ApiSpec.CastAndValidate, :strict]`) + to disable this behavior. Also, the default rendering error module + is `Pleroma.Web.ApiSpec.RenderError`. """ @behaviour Plug @@ -45,7 +46,7 @@ def call(%{private: %{open_api_spex: private_data}} = conn, %{ private_data = Map.put(private_data, :operation_id, operation_id) conn = Conn.put_private(conn, :open_api_spex, private_data) - case cast_and_validate(spec, operation, conn, content_type) do + case cast_and_validate(spec, operation, conn, content_type, strict?()) do {:ok, conn} -> conn @@ -98,7 +99,11 @@ def call( def call(conn, opts), do: OpenApiSpex.Plug.CastAndValidate.call(conn, opts) - defp cast_and_validate(spec, operation, conn, content_type) do + defp cast_and_validate(spec, operation, conn, content_type, true = _strict) do + OpenApiSpex.cast_and_validate(spec, operation, conn, content_type) + end + + defp cast_and_validate(spec, operation, conn, content_type, false = _strict) do case OpenApiSpex.cast_and_validate(spec, operation, conn, content_type) do {:ok, conn} -> {:ok, conn} @@ -129,4 +134,6 @@ defp list_items_to_string(list) do i -> i end) end + + defp strict?, do: Pleroma.Config.get([__MODULE__, :strict], false) end From e55fd530bc9a6ab42e475efe689e239963906928 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 4 May 2020 22:33:34 +0400 Subject: [PATCH 5/7] Render better errors for ENUM validation --- lib/pleroma/web/api_spec/render_error.ex | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/pleroma/web/api_spec/render_error.ex b/lib/pleroma/web/api_spec/render_error.ex index b5877ca9c..d476b8ef3 100644 --- a/lib/pleroma/web/api_spec/render_error.ex +++ b/lib/pleroma/web/api_spec/render_error.ex @@ -17,6 +17,9 @@ def init(opts), do: opts def call(conn, errors) do errors = Enum.map(errors, fn + %{name: nil, reason: :invalid_enum} = err -> + %OpenApiSpex.Cast.Error{err | name: err.value} + %{name: nil} = err -> %OpenApiSpex.Cast.Error{err | name: List.last(err.path)} From 1cb89aac1eef7711aa7950fe03e02e24bc665317 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 4 May 2020 22:35:28 +0400 Subject: [PATCH 6/7] Enable strict validation mode in dev and test environments --- config/dev.exs | 2 ++ config/test.exs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/config/dev.exs b/config/dev.exs index 7e1e3b4be..4faaeff5b 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -52,6 +52,8 @@ hostname: "localhost", pool_size: 10 +config :pleroma, Pleroma.Web.ApiSpec.CastAndValidate, strict: true + if File.exists?("./config/dev.secret.exs") do import_config "dev.secret.exs" else diff --git a/config/test.exs b/config/test.exs index 040e67e4a..cbf775109 100644 --- a/config/test.exs +++ b/config/test.exs @@ -96,6 +96,8 @@ config :pleroma, Pleroma.Plugs.RemoteIp, enabled: false +config :pleroma, Pleroma.Web.ApiSpec.CastAndValidate, strict: true + if File.exists?("./config/test.secret.exs") do import_config "test.secret.exs" else From cc922e7d8ccbf22a0f7e0898a6ff4639123f0c7f Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Tue, 5 May 2020 14:44:29 +0400 Subject: [PATCH 7/7] Document configuration for Pleroma.Web.ApiSpec.CastAndValidate --- config/description.exs | 14 ++++++++++++++ docs/configuration/cheatsheet.md | 6 +++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/config/description.exs b/config/description.exs index 9d8e3b93c..72bb4d436 100644 --- a/config/description.exs +++ b/config/description.exs @@ -3194,5 +3194,19 @@ ] } ] + }, + %{ + group: :pleroma, + key: Pleroma.Web.ApiSpec.CastAndValidate, + type: :group, + children: [ + %{ + key: :strict, + type: :boolean, + description: + "Enables strict input validation (useful in development, not recommended in production)", + suggestions: [false] + } + ] } ] diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index 681ab6b93..705c4c15e 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -924,4 +924,8 @@ Restrict access for unauthenticated users to timelines (public and federate), us * `remote` * `activities` - statuses * `local` - * `remote` \ No newline at end of file + * `remote` + +## Pleroma.Web.ApiSpec.CastAndValidate + +* `:strict` a boolean, enables strict input validation (useful in development, not recommended in production). Defaults to `false`.