From c07ba3a5d19c419ef8aaf3ea9ca6e7f48e4f4487 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Fri, 24 Mar 2023 12:13:42 +0100 Subject: [PATCH] Add rate-limiting on queries with Hammer Closes #67 Signed-off-by: Thomas Citharel --- .sobelow-skips | 3 +- config/config.exs | 3 + lib/graphql/authorization.ex | 3 + lib/graphql/schema/event.ex | 8 ++ lib/graphql/schema/media.ex | 8 ++ lib/graphql/schema/user.ex | 23 +++- lib/service/auth/applications.ex | 3 + lib/web/controllers/application_controller.ex | 110 +++++++++++++----- mix.exs | 3 +- mix.lock | 4 +- test/graphql/resolvers/user_test.exs | 1 + .../application_controller_test.exs | 50 +++++++- 12 files changed, 183 insertions(+), 36 deletions(-) diff --git a/.sobelow-skips b/.sobelow-skips index 3e438cb3..840e346b 100644 --- a/.sobelow-skips +++ b/.sobelow-skips @@ -39,4 +39,5 @@ FE1EEB91EA633570F703B251AE2D4D4E 759F752FA0768CCC7871895DC2A5CD51 7EEC79571F3F7CEEB04A8B86D908382A E7967805C1EA5301F2722C7BDB2F25F3 -BDFB0FB1AAF69C18212CBCFD42F8B717 \ No newline at end of file +BDFB0FB1AAF69C18212CBCFD42F8B717 +40220A533CCACB3A1CE9DBF1A8A430A1 \ No newline at end of file diff --git a/config/config.exs b/config/config.exs index ede8c017..67536ba9 100644 --- a/config/config.exs +++ b/config/config.exs @@ -321,6 +321,9 @@ config :mobilizon, Oban, {Oban.Plugins.Pruner, max_age: 300} ] +config :hammer, + backend: {Hammer.Backend.ETS, [expiry_ms: 60_000 * 60 * 4, cleanup_interval_ms: 60_000 * 10]} + config :mobilizon, :rich_media, parsers: [ Mobilizon.Service.RichMedia.Parsers.OEmbed, diff --git a/lib/graphql/authorization.ex b/lib/graphql/authorization.ex index c0bf7822..c30675dd 100644 --- a/lib/graphql/authorization.ex +++ b/lib/graphql/authorization.ex @@ -44,6 +44,9 @@ defmodule Mobilizon.GraphQL.Authorization do def get_user_role(%{role: role}), do: role def get_user_role(nil), do: nil + @impl true + def get_ip(%{ip: ip}), do: ip + @impl true def unauthorized_message(resolution) do case Map.get(resolution.context, :current_user) do diff --git a/lib/graphql/schema/event.ex b/lib/graphql/schema/event.ex index efea1715..a44294ef 100644 --- a/lib/graphql/schema/event.ex +++ b/lib/graphql/schema/event.ex @@ -15,6 +15,9 @@ defmodule Mobilizon.GraphQL.Schema.EventType do import_types(Schema.Events.ParticipantType) import_types(Schema.TagType) + @env Application.compile_env(:mobilizon, :env) + @event_rate_limiting 60 + @desc "An event" object :event do meta(:authorize, :all) @@ -437,6 +440,8 @@ defmodule Mobilizon.GraphQL.Schema.EventType do args: %{organizer_actor_id: :organizer_actor_id} ) + middleware(Rajska.RateLimiter, limit: event_rate_limiting(@env)) + resolve(&Event.create_event/3) end @@ -505,4 +510,7 @@ defmodule Mobilizon.GraphQL.Schema.EventType do resolve(&Event.delete_event/3) end end + + defp event_rate_limiting(:test), do: @event_rate_limiting * 1000 + defp event_rate_limiting(_), do: @event_rate_limiting end diff --git a/lib/graphql/schema/media.ex b/lib/graphql/schema/media.ex index 72a1f5e4..7e77b9fc 100644 --- a/lib/graphql/schema/media.ex +++ b/lib/graphql/schema/media.ex @@ -6,6 +6,9 @@ defmodule Mobilizon.GraphQL.Schema.MediaType do alias Mobilizon.GraphQL.Resolvers.Media + @env Application.compile_env(:mobilizon, :env) + @media_rate_limiting 60 + @desc "A media" object :media do meta(:authorize, :all) @@ -77,6 +80,8 @@ defmodule Mobilizon.GraphQL.Schema.MediaType do args: %{} ) + middleware(Rajska.RateLimiter, limit: media_rate_limiting(@env)) + resolve(&Media.upload_media/3) end @@ -95,4 +100,7 @@ defmodule Mobilizon.GraphQL.Schema.MediaType do resolve(&Media.remove_media/3) end end + + defp media_rate_limiting(:test), do: @media_rate_limiting * 1000 + defp media_rate_limiting(_), do: @media_rate_limiting end diff --git a/lib/graphql/schema/user.ex b/lib/graphql/schema/user.ex index 47f927da..9fc64053 100644 --- a/lib/graphql/schema/user.ex +++ b/lib/graphql/schema/user.ex @@ -7,12 +7,17 @@ defmodule Mobilizon.GraphQL.Schema.UserType do import Absinthe.Resolution.Helpers, only: [dataloader: 2] alias Mobilizon.Events - alias Mobilizon.GraphQL.Resolvers.{Application, Media, User} + alias Mobilizon.GraphQL.Resolvers.Application, as: ApplicationResolver + alias Mobilizon.GraphQL.Resolvers.{Media, User} alias Mobilizon.GraphQL.Resolvers.Users.ActivitySettings alias Mobilizon.GraphQL.Schema import_types(Schema.SortType) + @env Application.compile_env(:mobilizon, :env) + @user_ip_limit 10 + @user_email_limit 5 + @desc "A local user of Mobilizon" object :user do meta(:authorize, :all) @@ -180,7 +185,7 @@ defmodule Mobilizon.GraphQL.Schema.UserType do description: "The user's authorized authentication apps", meta: [private: true, rule: :forbid_app_access] ) do - resolve(&Application.get_user_applications/3) + resolve(&ApplicationResolver.get_user_applications/3) end end @@ -331,6 +336,8 @@ defmodule Mobilizon.GraphQL.Schema.UserType do arg(:password, non_null(:string), description: "The new user's password") arg(:locale, :string, description: "The new user's locale") middleware(Rajska.QueryAuthorization, permit: :all) + middleware(Rajska.RateLimiter, limit: user_ip_limiter(@env)) + middleware(Rajska.RateLimiter, keys: :email, limit: user_email_limiter(@env)) resolve(&User.create_user/3) end @@ -349,6 +356,8 @@ defmodule Mobilizon.GraphQL.Schema.UserType do arg(:email, non_null(:string), description: "The email used to register") arg(:locale, :string, description: "The user's locale") middleware(Rajska.QueryAuthorization, permit: :all) + middleware(Rajska.RateLimiter, limit: user_ip_limiter(@env)) + middleware(Rajska.RateLimiter, keys: :email, limit: user_email_limiter(@env)) resolve(&User.resend_confirmation_email/3) end @@ -357,6 +366,8 @@ defmodule Mobilizon.GraphQL.Schema.UserType do arg(:email, non_null(:string), description: "The user's email") arg(:locale, :string, description: "The user's locale") middleware(Rajska.QueryAuthorization, permit: :all) + middleware(Rajska.RateLimiter, limit: user_ip_limiter(@env)) + middleware(Rajska.RateLimiter, keys: :email, limit: user_email_limiter(@env)) resolve(&User.send_reset_password/3) end @@ -377,6 +388,8 @@ defmodule Mobilizon.GraphQL.Schema.UserType do arg(:email, non_null(:string), description: "The user's email") arg(:password, non_null(:string), description: "The user's password") middleware(Rajska.QueryAuthorization, permit: :all) + middleware(Rajska.RateLimiter, limit: user_ip_limiter(@env)) + middleware(Rajska.RateLimiter, keys: :email, limit: user_email_limiter(@env)) resolve(&User.login_user/3) end @@ -480,4 +493,10 @@ defmodule Mobilizon.GraphQL.Schema.UserType do resolve(&User.update_locale/3) end end + + defp user_ip_limiter(:test), do: @user_ip_limit * 1000 + defp user_ip_limiter(_), do: @user_ip_limit + + defp user_email_limiter(:test), do: @user_email_limit * 1000 + defp user_email_limiter(_), do: @user_email_limit end diff --git a/lib/service/auth/applications.ex b/lib/service/auth/applications.ex index c5031c1e..52c22092 100644 --- a/lib/service/auth/applications.ex +++ b/lib/service/auth/applications.ex @@ -230,6 +230,9 @@ defmodule Mobilizon.Service.Auth.Applications do %ApplicationDeviceActivation{status: :access_denied} -> {:error, :access_denied} + %ApplicationDeviceActivation{status: :pending} -> + {:error, :pending, @device_code_interval} + nil -> {:error, :incorrect_device_code} diff --git a/lib/web/controllers/application_controller.ex b/lib/web/controllers/application_controller.ex index cf695fa7..ef5067da 100644 --- a/lib/web/controllers/application_controller.ex +++ b/lib/web/controllers/application_controller.ex @@ -15,42 +15,63 @@ defmodule Mobilizon.Web.ApplicationController do conn, %{"name" => name, "redirect_uris" => redirect_uris, "scope" => scope} = args ) do - case Applications.create( - name, - String.split(redirect_uris, "\n"), - scope, - Map.get(args, "website") + ip = conn.remote_ip |> :inet.ntoa() |> to_string() + + case Hammer.check_rate( + "create_application:#{ip}", + 60_000, + 10 ) do - {:ok, %Application{} = app} -> - conn - |> Plug.Conn.put_resp_header("cache-control", "no-store") - |> json( - Map.take(app, [:name, :website, :redirect_uris, :client_id, :client_secret, :scope]) - ) - - {:error, :invalid_scope} -> - conn - |> Plug.Conn.put_status(400) - |> json(%{ - "error" => "invalid_scope", - "error_description" => - dgettext( - "errors", - "The scope parameter is not a space separated list of valid scopes" + {:allow, _} -> + case Applications.create( + name, + String.split(redirect_uris, "\n"), + scope, + Map.get(args, "website") + ) do + {:ok, %Application{} = app} -> + conn + |> Plug.Conn.put_resp_header("cache-control", "no-store") + |> json( + Map.take(app, [:name, :website, :redirect_uris, :client_id, :client_secret, :scope]) ) - }) - {:error, error} -> - Logger.error(inspect(error)) + {:error, :invalid_scope} -> + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "invalid_scope", + "error_description" => + dgettext( + "errors", + "The scope parameter is not a space separated list of valid scopes" + ) + }) + {:error, error} -> + Logger.error(inspect(error)) + + conn + |> Plug.Conn.put_status(500) + |> json(%{ + "error" => "server_error", + "error_description" => + dgettext( + "errors", + "Impossible to create application." + ) + }) + end + + {:deny, _} -> conn - |> Plug.Conn.put_status(500) + |> Plug.Conn.put_status(429) |> json(%{ - "error" => "server_error", + "error" => "slow_down", "error_description" => dgettext( "errors", - "Impossible to create application." + "Too many requests" ) }) end @@ -148,7 +169,7 @@ defmodule Mobilizon.Web.ApplicationController do "error_description" => dgettext( "errors", - "No application with this client_id was found" + "No application was found with this client_id" ) }) @@ -231,6 +252,37 @@ defmodule Mobilizon.Web.ApplicationController do ) }) + {:error, :pending, interval} -> + case Hammer.check_rate( + "generate_device_access_token:#{client_id}:#{device_code}", + interval * 1_000, + 1 + ) do + {:allow, _} -> + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "authorization_pending", + "error_description" => + dgettext( + "errors", + "The authorization request is still pending" + ) + }) + + {:deny, _} -> + conn + |> Plug.Conn.put_status(400) + |> json(%{ + "error" => "slow_down", + "error_description" => + dgettext( + "errors", + "Please slow down the rate of your requests" + ) + }) + end + {:error, :access_denied} -> conn |> Plug.Conn.put_status(400) @@ -247,7 +299,7 @@ defmodule Mobilizon.Web.ApplicationController do conn |> Plug.Conn.put_status(400) |> json(%{ - "error" => "invalid_grant", + "error" => "expired_token", "error_description" => dgettext( "errors", diff --git a/mix.exs b/mix.exs index a81301ee..4d44498e 100644 --- a/mix.exs +++ b/mix.exs @@ -210,7 +210,8 @@ defmodule Mobilizon.Mixfile do {:unplug, "~> 1.0.0"}, {:replug, "~> 0.1.0"}, {:exkismet, github: "tcitworld/exkismet"}, - {:rajska, github: "churcho/rajska", branch: "fix/update-absinthe"}, + {:rajska, github: "tcitworld/rajska", branch: "mobilizon"}, + {:hammer, "~> 6.1"}, # Dev and test dependencies {:phoenix_live_reload, "~> 1.2", only: [:dev, :e2e]}, {:ex_machina, "~> 2.3", only: [:dev, :test]}, diff --git a/mix.lock b/mix.lock index ae55d543..dfbabde3 100644 --- a/mix.lock +++ b/mix.lock @@ -71,6 +71,7 @@ "guardian_db": {:hex, :guardian_db, "2.1.0", "ec95a9d99cdd1e550555d09a7bb4a340d8887aad0697f594590c2fd74be02426", [:mix], [{:ecto, "~> 3.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:ecto_sql, "~> 3.1", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:guardian, "~> 1.0 or ~> 2.0", [hex: :guardian, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.13", [hex: :postgrex, repo: "hexpm", optional: true]}], "hexpm", "f8e7d543ac92c395f3a7fd5acbe6829faeade57d688f7562e2f0fca8f94a0d70"}, "guardian_phoenix": {:hex, :guardian_phoenix, "2.0.1", "89a817265af09a6ddf7cb1e77f17ffca90cea2db10ff888375ef34502b2731b1", [:mix], [{:guardian, "~> 2.0", [hex: :guardian, repo: "hexpm", optional: false]}, {:phoenix, "~> 1.3", [hex: :phoenix, repo: "hexpm", optional: false]}], "hexpm", "21f439246715192b231f228680465d1ed5fbdf01555a4a3b17165532f5f9a08c"}, "hackney": {:hex, :hackney, "1.18.1", "f48bf88f521f2a229fc7bae88cf4f85adc9cd9bcf23b5dc8eb6a1788c662c4f6", [:rebar3], [{:certifi, "~> 2.9.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "~> 6.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "~> 1.0.0", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~> 1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.3.1", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~> 1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "a4ecdaff44297e9b5894ae499e9a070ea1888c84afdd1fd9b7b2bc384950128e"}, + "hammer": {:hex, :hammer, "6.1.0", "f263e3c3e9946bd410ea0336b2abe0cb6260af4afb3a221e1027540706e76c55", [:make, :mix], [{:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: false]}], "hexpm", "b47e415a562a6d072392deabcd58090d8a41182cf9044cdd6b0d0faaaf68ba57"}, "haversine": {:hex, :haversine, "0.1.0", "14240e90dae07c9459f538d12a811492f655d95fc68f999403503b4f6c4ec522", [:mix], [], "hexpm", "54dc48e895bc18a59437a37026c873634e17b648a64cb87bfafb96f64d607060"}, "html_entities": {:hex, :html_entities, "0.5.2", "9e47e70598da7de2a9ff6af8758399251db6dbb7eebe2b013f2bbd2515895c3c", [:mix], [], "hexpm", "c53ba390403485615623b9531e97696f076ed415e8d8058b1dbaa28181f4fdcc"}, "http_signatures": {:hex, :http_signatures, "0.1.1", "ca7ebc1b61542b163644c8c3b1f0e0f41037d35f2395940d3c6c7deceab41fd8", [:mix], [], "hexpm", "cc3b8a007322cc7b624c0c15eec49ee58ac977254ff529a3c482f681465942a3"}, @@ -117,9 +118,10 @@ "plug": {:hex, :plug, "1.14.1", "3148623796853ae96c628960b833bf6b6a894d6bdc8c199ef7160c41149b71f2", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "a0e789be21a576b11ec55a0983e4e8f7c7b07d88dfb3b8da9e97767132271d40"}, "plug_cowboy": {:hex, :plug_cowboy, "2.6.1", "9a3bbfceeb65eff5f39dab529e5cd79137ac36e913c02067dba3963a26efe9b2", [:mix], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:cowboy_telemetry, "~> 0.3", [hex: :cowboy_telemetry, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "de36e1a21f451a18b790f37765db198075c25875c64834bcc82d90b309eb6613"}, "plug_crypto": {:hex, :plug_crypto, "1.2.5", "918772575e48e81e455818229bf719d4ab4181fcbf7f85b68a35620f78d89ced", [:mix], [], "hexpm", "26549a1d6345e2172eb1c233866756ae44a9609bd33ee6f99147ab3fd87fd842"}, + "poolboy": {:hex, :poolboy, "1.5.2", "392b007a1693a64540cead79830443abf5762f5d30cf50bc95cb2c1aaafa006b", [:rebar3], [], "hexpm", "dad79704ce5440f3d5a3681c8590b9dc25d1a561e8f5a9c995281012860901e3"}, "postgrex": {:hex, :postgrex, "0.16.5", "fcc4035cc90e23933c5d69a9cd686e329469446ef7abba2cf70f08e2c4b69810", [:mix], [{:connection, "~> 1.1", [hex: :connection, repo: "hexpm", optional: false]}, {:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "edead639dc6e882618c01d8fc891214c481ab9a3788dfe38dd5e37fd1d5fb2e8"}, "progress_bar": {:hex, :progress_bar, "2.0.1", "7b40200112ae533d5adceb80ff75fbe66dc753bca5f6c55c073bfc122d71896d", [:mix], [{:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}], "hexpm", "2519eb58a2f149a3a094e729378256d8cb6d96a259ec94841bd69fdc71f18f87"}, - "rajska": {:git, "https://github.com/churcho/rajska.git", "5da424969d5f40dcab690d3a25b248f85f712823", [branch: "fix/update-absinthe"]}, + "rajska": {:git, "https://github.com/tcitworld/rajska.git", "0c036448e261e8be6a512581c592fadf48982d84", [branch: "mobilizon"]}, "ranch": {:hex, :ranch, "1.8.0", "8c7a100a139fd57f17327b6413e4167ac559fbc04ca7448e9be9057311597a1d", [:make, :rebar3], [], "hexpm", "49fbcfd3682fab1f5d109351b61257676da1a2fdbe295904176d5e521a2ddfe5"}, "remote_ip": {:hex, :remote_ip, "1.1.0", "cb308841595d15df3f9073b7c39243a1dd6ca56e5020295cb012c76fbec50f2d", [:mix], [{:combine, "~> 0.10", [hex: :combine, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "616ffdf66aaad6a72fc546dabf42eed87e2a99e97b09cbd92b10cc180d02ed74"}, "replug": {:hex, :replug, "0.1.0", "61d35f8c873c0078a23c49579a48f36e45789414b1ec0daee3fd5f4e34221f23", [:mix], [{:plug, "~> 1.8", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "f71f7a57e944e854fe4946060c6964098e53958074c69fb844b96e0bd58cfa60"}, diff --git a/test/graphql/resolvers/user_test.exs b/test/graphql/resolvers/user_test.exs index 0dd1affb..5581ff5b 100644 --- a/test/graphql/resolvers/user_test.exs +++ b/test/graphql/resolvers/user_test.exs @@ -384,6 +384,7 @@ defmodule Mobilizon.GraphQL.Resolvers.UserTest do variables: @user_creation ) + assert res["errors"] == nil assert res["data"]["createUser"]["email"] == @user_creation.email res = diff --git a/test/web/controllers/application_controller_test.exs b/test/web/controllers/application_controller_test.exs index 6e12c661..196791ad 100644 --- a/test/web/controllers/application_controller_test.exs +++ b/test/web/controllers/application_controller_test.exs @@ -108,7 +108,7 @@ defmodule Mobilizon.Web.ApplicationControllerTest do assert error = json_response(conn, 400) assert error["error"] == "invalid_client" - assert error["error_description"] == "No application with this client_id was found" + assert error["error_description"] == "No application was found with this client_id" end test "with a scope not matching app registered scopes", %{conn: conn} do @@ -277,12 +277,58 @@ defmodule Mobilizon.Web.ApplicationControllerTest do ) assert error = json_response(conn, 400) - assert error["error"] == "invalid_grant" + assert error["error"] == "expired_token" assert error["error_description"] == "The given device_code has expired" end + test "with a pending authorization", %{conn: conn} do + user = insert(:user) + + {:ok, app} = + Applications.create("My app", ["hello"], "write:event:create write:event:update") + + assert {:ok, _res} = + Mobilizon.Applications.create_application_device_activation(%{ + device_code: "hello", + user_code: "world", + expires_in: 600, + application_id: app.id, + scope: "write:event:create write:event:update", + status: "pending", + user_id: user.id + }) + + conn = + conn + |> Plug.Conn.put_req_header("accept", "application/json") + |> post("/oauth/token", + grant_type: "urn:ietf:params:oauth:grant-type:device_code", + client_id: app.client_id, + device_code: "hello" + ) + + error = json_response(conn, 400) + + assert error["error"] == "authorization_pending" + assert error["error_description"] == "The authorization request is still pending" + + conn = + Phoenix.ConnTest.build_conn() + |> Plug.Conn.put_req_header("accept", "application/json") + |> post("/oauth/token", + grant_type: "urn:ietf:params:oauth:grant-type:device_code", + client_id: app.client_id, + device_code: "hello" + ) + + error = json_response(conn, 400) + + assert error["error"] == "slow_down" + assert error["error_description"] == "Please slow down the rate of your requests" + end + test "with valid params as JSON", %{conn: conn} do user = insert(:user)