From 3ce658b93098551792a69f2455e6e9339a1722e2 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Tue, 25 Aug 2020 19:17:51 +0300 Subject: [PATCH 1/6] schedule expired oauth tokens deletion with Oban --- config/config.exs | 2 +- config/description.exs | 1 - lib/pleroma/web/oauth/token.ex | 24 ++++++++++------ lib/pleroma/web/oauth/token/clean_worker.ex | 2 -- lib/pleroma/web/oauth/token/query.ex | 6 ---- .../web/oauth/token/strategy/refresh_token.ex | 2 +- .../workers/cron/clear_oauth_token_worker.ex | 23 --------------- lib/pleroma/workers/purge_expired_token.ex | 28 +++++++++++++++++++ test/plugs/oauth_plug_test.exs | 2 +- test/web/oauth/token_test.exs | 13 --------- .../twitter_api/password_controller_test.exs | 4 +-- .../cron/clear_oauth_token_worker_test.exs | 22 --------------- .../purge_expired_oauth_token_test.exs | 27 ++++++++++++++++++ 13 files changed, 76 insertions(+), 80 deletions(-) delete mode 100644 lib/pleroma/workers/cron/clear_oauth_token_worker.ex create mode 100644 lib/pleroma/workers/purge_expired_token.ex delete mode 100644 test/workers/cron/clear_oauth_token_worker_test.exs create mode 100644 test/workers/purge_expired_oauth_token_test.exs diff --git a/config/config.exs b/config/config.exs index 1a2b312b5..fa4c96b79 100644 --- a/config/config.exs +++ b/config/config.exs @@ -530,6 +530,7 @@ log: false, queues: [ activity_expiration: 10, + oauth_token_expiration: 1, federator_incoming: 50, federator_outgoing: 50, web_push: 50, @@ -543,7 +544,6 @@ ], plugins: [Oban.Plugins.Pruner], crontab: [ - {"0 0 * * *", Pleroma.Workers.Cron.ClearOauthTokenWorker}, {"* * * * *", Pleroma.Workers.Cron.PurgeExpiredActivitiesWorker}, {"0 0 * * 0", Pleroma.Workers.Cron.DigestEmailsWorker}, {"0 0 * * *", Pleroma.Workers.Cron.NewUsersDigestWorker} diff --git a/config/description.exs b/config/description.exs index eac97ad64..4c4deed30 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2290,7 +2290,6 @@ type: {:list, :tuple}, description: "Settings for cron background jobs", suggestions: [ - {"0 0 * * *", Pleroma.Workers.Cron.ClearOauthTokenWorker}, {"* * * * *", Pleroma.Workers.Cron.PurgeExpiredActivitiesWorker}, {"0 0 * * 0", Pleroma.Workers.Cron.DigestEmailsWorker}, {"0 0 * * *", Pleroma.Workers.Cron.NewUsersDigestWorker} diff --git a/lib/pleroma/web/oauth/token.ex b/lib/pleroma/web/oauth/token.ex index 08bb7326d..4d00fcb1c 100644 --- a/lib/pleroma/web/oauth/token.ex +++ b/lib/pleroma/web/oauth/token.ex @@ -50,7 +50,7 @@ def exchange_token(app, auth) do true <- auth.app_id == app.id do user = if auth.user_id, do: User.get_cached_by_id(auth.user_id), else: %User{} - create_token( + create( app, user, %{scopes: auth.scopes} @@ -83,8 +83,21 @@ defp put_valid_until(changeset, attrs) do |> validate_required([:valid_until]) end - @spec create_token(App.t(), User.t(), map()) :: {:ok, Token} | {:error, Changeset.t()} - def create_token(%App{} = app, %User{} = user, attrs \\ %{}) do + @spec create(App.t(), User.t(), map()) :: {:ok, Token} | {:error, Changeset.t()} + def create(%App{} = app, %User{} = user, attrs \\ %{}) do + with {:ok, token} <- do_create(app, user, attrs) do + if Pleroma.Config.get([:oauth2, :clean_expired_tokens]) do + Pleroma.Workers.PurgeExpiredOAuthToken.enqueue(%{ + token_id: token.id, + valid_until: DateTime.from_naive!(token.valid_until, "Etc/UTC") + }) + end + + {:ok, token} + end + end + + defp do_create(app, user, attrs) do %__MODULE__{user_id: user.id, app_id: app.id} |> cast(%{scopes: attrs[:scopes] || app.scopes}, [:scopes]) |> validate_required([:scopes, :app_id]) @@ -105,11 +118,6 @@ def delete_user_token(%User{id: user_id}, token_id) do |> Repo.delete_all() end - def delete_expired_tokens do - Query.get_expired_tokens() - |> Repo.delete_all() - end - def get_user_tokens(%User{id: user_id}) do Query.get_by_user(user_id) |> Query.preload([:app]) diff --git a/lib/pleroma/web/oauth/token/clean_worker.ex b/lib/pleroma/web/oauth/token/clean_worker.ex index e3aa4eb7e..2f51bdb75 100644 --- a/lib/pleroma/web/oauth/token/clean_worker.ex +++ b/lib/pleroma/web/oauth/token/clean_worker.ex @@ -12,7 +12,6 @@ defmodule Pleroma.Web.OAuth.Token.CleanWorker do @one_day 86_400_000 alias Pleroma.MFA - alias Pleroma.Web.OAuth alias Pleroma.Workers.BackgroundWorker def start_link(_), do: GenServer.start_link(__MODULE__, %{}) @@ -32,7 +31,6 @@ def handle_info(:perform, state) do end def perform(:clean) do - OAuth.Token.delete_expired_tokens() MFA.Token.delete_expired_tokens() end end diff --git a/lib/pleroma/web/oauth/token/query.ex b/lib/pleroma/web/oauth/token/query.ex index 93d6e26ed..fd6d9b112 100644 --- a/lib/pleroma/web/oauth/token/query.ex +++ b/lib/pleroma/web/oauth/token/query.ex @@ -33,12 +33,6 @@ def get_by_id(query \\ Token, id) do from(q in query, where: q.id == ^id) end - @spec get_expired_tokens(query, DateTime.t() | nil) :: query - def get_expired_tokens(query \\ Token, date \\ nil) do - expired_date = date || Timex.now() - from(q in query, where: fragment("?", q.valid_until) < ^expired_date) - end - @spec get_by_user(query, String.t()) :: query def get_by_user(query \\ Token, user_id) do from(q in query, where: q.user_id == ^user_id) diff --git a/lib/pleroma/web/oauth/token/strategy/refresh_token.ex b/lib/pleroma/web/oauth/token/strategy/refresh_token.ex index debc29b0b..625b0fde2 100644 --- a/lib/pleroma/web/oauth/token/strategy/refresh_token.ex +++ b/lib/pleroma/web/oauth/token/strategy/refresh_token.ex @@ -46,7 +46,7 @@ defp revoke_access_token(token) do defp create_access_token({:error, error}, _), do: {:error, error} defp create_access_token({:ok, token}, %{app: app, user: user} = token_params) do - Token.create_token(app, user, add_refresh_token(token_params, token.refresh_token)) + Token.create(app, user, add_refresh_token(token_params, token.refresh_token)) end defp add_refresh_token(params, token) do diff --git a/lib/pleroma/workers/cron/clear_oauth_token_worker.ex b/lib/pleroma/workers/cron/clear_oauth_token_worker.ex deleted file mode 100644 index 276f47efc..000000000 --- a/lib/pleroma/workers/cron/clear_oauth_token_worker.ex +++ /dev/null @@ -1,23 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Workers.Cron.ClearOauthTokenWorker do - @moduledoc """ - The worker to cleanup expired oAuth tokens. - """ - - use Oban.Worker, queue: "background" - - alias Pleroma.Config - alias Pleroma.Web.OAuth.Token - - @impl Oban.Worker - def perform(_job) do - if Config.get([:oauth2, :clean_expired_tokens], false) do - Token.delete_expired_tokens() - end - - :ok - end -end diff --git a/lib/pleroma/workers/purge_expired_token.ex b/lib/pleroma/workers/purge_expired_token.ex new file mode 100644 index 000000000..6068e43bf --- /dev/null +++ b/lib/pleroma/workers/purge_expired_token.ex @@ -0,0 +1,28 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Workers.PurgeExpiredOAuthToken do + @moduledoc """ + Worker which purges expired OAuth tokens + """ + + use Oban.Worker, queue: :oauth_token_expiration, max_attempts: 1 + + @spec enqueue(%{token_id: integer(), valid_until: DateTime.t()}) :: + {:ok, Oban.Job.t()} | {:error, Ecto.Changeset.t()} + def enqueue(args) do + {scheduled_at, args} = Map.pop(args, :valid_until) + + args + |> __MODULE__.new(scheduled_at: scheduled_at) + |> Oban.insert() + end + + @impl true + def perform(%Oban.Job{args: %{"token_id" => id}}) do + Pleroma.Web.OAuth.Token + |> Pleroma.Repo.get(id) + |> Pleroma.Repo.delete() + end +end diff --git a/test/plugs/oauth_plug_test.exs b/test/plugs/oauth_plug_test.exs index f74c068cd..9d39d3153 100644 --- a/test/plugs/oauth_plug_test.exs +++ b/test/plugs/oauth_plug_test.exs @@ -16,7 +16,7 @@ defmodule Pleroma.Plugs.OAuthPlugTest do setup %{conn: conn} do user = insert(:user) - {:ok, %{token: token}} = Pleroma.Web.OAuth.Token.create_token(insert(:oauth_app), user) + {:ok, %{token: token}} = Pleroma.Web.OAuth.Token.create(insert(:oauth_app), user) %{user: user, token: token, conn: conn} end diff --git a/test/web/oauth/token_test.exs b/test/web/oauth/token_test.exs index 40d71eb59..c88b9cc98 100644 --- a/test/web/oauth/token_test.exs +++ b/test/web/oauth/token_test.exs @@ -69,17 +69,4 @@ test "deletes all tokens of a user" do assert tokens == 2 end - - test "deletes expired tokens" do - insert(:oauth_token, valid_until: Timex.shift(Timex.now(), days: -3)) - insert(:oauth_token, valid_until: Timex.shift(Timex.now(), days: -3)) - t3 = insert(:oauth_token) - t4 = insert(:oauth_token, valid_until: Timex.shift(Timex.now(), minutes: 10)) - {tokens, _} = Token.delete_expired_tokens() - assert tokens == 2 - available_tokens = Pleroma.Repo.all(Token) - - token_ids = available_tokens |> Enum.map(& &1.id) - assert token_ids == [t3.id, t4.id] - end end diff --git a/test/web/twitter_api/password_controller_test.exs b/test/web/twitter_api/password_controller_test.exs index 231a46c67..a5e9e2178 100644 --- a/test/web/twitter_api/password_controller_test.exs +++ b/test/web/twitter_api/password_controller_test.exs @@ -37,7 +37,7 @@ test "it shows password reset form", %{conn: conn} do test "it returns HTTP 200", %{conn: conn} do user = insert(:user) {:ok, token} = PasswordResetToken.create_token(user) - {:ok, _access_token} = Token.create_token(insert(:oauth_app), user, %{}) + {:ok, _access_token} = Token.create(insert(:oauth_app), user, %{}) params = %{ "password" => "test", @@ -62,7 +62,7 @@ test "it sets password_reset_pending to false", %{conn: conn} do user = insert(:user, password_reset_pending: true) {:ok, token} = PasswordResetToken.create_token(user) - {:ok, _access_token} = Token.create_token(insert(:oauth_app), user, %{}) + {:ok, _access_token} = Token.create(insert(:oauth_app), user, %{}) params = %{ "password" => "test", diff --git a/test/workers/cron/clear_oauth_token_worker_test.exs b/test/workers/cron/clear_oauth_token_worker_test.exs deleted file mode 100644 index 67836f34f..000000000 --- a/test/workers/cron/clear_oauth_token_worker_test.exs +++ /dev/null @@ -1,22 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Workers.Cron.ClearOauthTokenWorkerTest do - use Pleroma.DataCase - - import Pleroma.Factory - alias Pleroma.Workers.Cron.ClearOauthTokenWorker - - setup do: clear_config([:oauth2, :clean_expired_tokens]) - - test "deletes expired tokens" do - insert(:oauth_token, - valid_until: NaiveDateTime.add(NaiveDateTime.utc_now(), -60 * 10) - ) - - Pleroma.Config.put([:oauth2, :clean_expired_tokens], true) - ClearOauthTokenWorker.perform(%Oban.Job{}) - assert Pleroma.Repo.all(Pleroma.Web.OAuth.Token) == [] - end -end diff --git a/test/workers/purge_expired_oauth_token_test.exs b/test/workers/purge_expired_oauth_token_test.exs new file mode 100644 index 000000000..3bd650d89 --- /dev/null +++ b/test/workers/purge_expired_oauth_token_test.exs @@ -0,0 +1,27 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Workers.PurgeExpiredOAuthTokenTest do + use Pleroma.DataCase, async: true + use Oban.Testing, repo: Pleroma.Repo + + import Pleroma.Factory + + setup do: clear_config([:oauth2, :clean_expired_tokens], true) + + test "purges expired token" do + user = insert(:user) + app = insert(:oauth_app) + + {:ok, %{id: id}} = Pleroma.Web.OAuth.Token.create(app, user) + + assert_enqueued( + worker: Pleroma.Workers.PurgeExpiredOAuthToken, + args: %{token_id: id} + ) + + assert {:ok, %{id: ^id}} = + perform_job(Pleroma.Workers.PurgeExpiredOAuthToken, %{token_id: id}) + end +end From 7dd986a563545cb63e8404d9b107f1d29c499940 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Sat, 5 Sep 2020 18:35:01 +0300 Subject: [PATCH 2/6] expire mfa tokens through Oban --- config/config.exs | 2 +- docs/configuration/cheatsheet.md | 10 +-- lib/pleroma/mfa/token.ex | 71 +++++++++---------- lib/pleroma/web/oauth/oauth_controller.ex | 4 +- lib/pleroma/web/oauth/token.ex | 5 +- lib/pleroma/web/oauth/token/clean_worker.ex | 36 ---------- .../controllers/remote_follow_controller.ex | 2 +- lib/pleroma/workers/purge_expired_token.ex | 11 +-- .../remote_follow_controller_test.exs | 4 +- .../purge_expired_oauth_token_test.exs | 27 ------- test/workers/purge_expired_token_test.exs | 51 +++++++++++++ 11 files changed, 106 insertions(+), 117 deletions(-) delete mode 100644 lib/pleroma/web/oauth/token/clean_worker.ex delete mode 100644 test/workers/purge_expired_oauth_token_test.exs create mode 100644 test/workers/purge_expired_token_test.exs diff --git a/config/config.exs b/config/config.exs index fa4c96b79..95a6ea9db 100644 --- a/config/config.exs +++ b/config/config.exs @@ -530,7 +530,7 @@ log: false, queues: [ activity_expiration: 10, - oauth_token_expiration: 1, + token_expiration: 5, federator_incoming: 50, federator_outgoing: 50, web_push: 50, diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index ec59896ec..d0bebbd45 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -691,9 +691,8 @@ Pleroma has the following queues: Pleroma has these periodic job workers: -`Pleroma.Workers.Cron.ClearOauthTokenWorker` - a job worker to cleanup expired oauth tokens. - -Example: +* `Pleroma.Workers.Cron.DigestEmailsWorker` - digest emails for users with new mentions and follows +* `Pleroma.Workers.Cron.NewUsersDigestWorker` - digest emails for admins with new registrations ```elixir config :pleroma, Oban, @@ -705,7 +704,8 @@ config :pleroma, Oban, federator_outgoing: 50 ], crontab: [ - {"0 0 * * *", Pleroma.Workers.Cron.ClearOauthTokenWorker} + {"0 0 * * 0", Pleroma.Workers.Cron.DigestEmailsWorker}, + {"0 0 * * *", Pleroma.Workers.Cron.NewUsersDigestWorker} ] ``` @@ -972,7 +972,7 @@ Configure OAuth 2 provider capabilities: * `token_expires_in` - The lifetime in seconds of the access token. * `issue_new_refresh_token` - Keeps old refresh token or generate new refresh token when to obtain an access token. -* `clean_expired_tokens` - Enable a background job to clean expired oauth tokens. Defaults to `false`. Interval settings sets in configuration periodic jobs [`Oban.Cron`](#obancron) +* `clean_expired_tokens` - Enable a background job to clean expired oauth tokens. Defaults to `false`. ## Link parsing diff --git a/lib/pleroma/mfa/token.ex b/lib/pleroma/mfa/token.ex index 0b2449971..69b64c0e8 100644 --- a/lib/pleroma/mfa/token.ex +++ b/lib/pleroma/mfa/token.ex @@ -10,10 +10,11 @@ defmodule Pleroma.MFA.Token do alias Pleroma.Repo alias Pleroma.User alias Pleroma.Web.OAuth.Authorization - alias Pleroma.Web.OAuth.Token, as: OAuthToken @expires 300 + @type t() :: %__MODULE__{} + schema "mfa_tokens" do field(:token, :string) field(:valid_until, :naive_datetime_usec) @@ -24,6 +25,7 @@ defmodule Pleroma.MFA.Token do timestamps() end + @spec get_by_token(String.t()) :: {:ok, t()} | {:error, :not_found} def get_by_token(token) do from( t in __MODULE__, @@ -33,33 +35,40 @@ def get_by_token(token) do |> Repo.find_resource() end - def validate(token) do - with {:fetch_token, {:ok, token}} <- {:fetch_token, get_by_token(token)}, - {:expired, false} <- {:expired, is_expired?(token)} do + @spec validate(String.t()) :: {:ok, t()} | {:error, :not_found} | {:error, :expired_token} + def validate(token_str) do + with {:ok, token} <- get_by_token(token_str), + false <- expired?(token) do {:ok, token} - else - {:expired, _} -> {:error, :expired_token} - {:fetch_token, _} -> {:error, :not_found} - error -> {:error, error} end end - def create_token(%User{} = user) do - %__MODULE__{} - |> change - |> assign_user(user) - |> put_token - |> put_valid_until - |> Repo.insert() + defp expired?(%__MODULE__{valid_until: valid_until}) do + with true <- NaiveDateTime.diff(NaiveDateTime.utc_now(), valid_until) > 0 do + {:error, :expired_token} + end end - def create_token(user, authorization) do + @spec create(User.t(), Authorization.t() | nil) :: {:ok, t()} | {:error, Ecto.Changeset.t()} + def create(user, authorization \\ nil) do + with {:ok, token} <- do_create(user, authorization) do + Pleroma.Workers.PurgeExpiredToken.enqueue(%{ + token_id: token.id, + valid_until: DateTime.from_naive!(token.valid_until, "Etc/UTC"), + mod: __MODULE__ + }) + + {:ok, token} + end + end + + defp do_create(user, authorization) do %__MODULE__{} - |> change + |> change() |> assign_user(user) - |> assign_authorization(authorization) - |> put_token - |> put_valid_until + |> maybe_assign_authorization(authorization) + |> put_token() + |> put_valid_until() |> Repo.insert() end @@ -69,15 +78,19 @@ defp assign_user(changeset, user) do |> validate_required([:user]) end - defp assign_authorization(changeset, authorization) do + defp maybe_assign_authorization(changeset, %Authorization{} = authorization) do changeset |> put_assoc(:authorization, authorization) |> validate_required([:authorization]) end + defp maybe_assign_authorization(changeset, _), do: changeset + defp put_token(changeset) do + token = Pleroma.Web.OAuth.Token.Utils.generate_token() + changeset - |> change(%{token: OAuthToken.Utils.generate_token()}) + |> change(%{token: token}) |> validate_required([:token]) |> unique_constraint(:token) end @@ -89,18 +102,4 @@ defp put_valid_until(changeset) do |> change(%{valid_until: expires_in}) |> validate_required([:valid_until]) end - - def is_expired?(%__MODULE__{valid_until: valid_until}) do - NaiveDateTime.diff(NaiveDateTime.utc_now(), valid_until) > 0 - end - - def is_expired?(_), do: false - - def delete_expired_tokens do - from( - q in __MODULE__, - where: fragment("?", q.valid_until) < ^Timex.now() - ) - |> Repo.delete_all() - end end diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex index dd00600ea..bbe7aa8a0 100644 --- a/lib/pleroma/web/oauth/oauth_controller.ex +++ b/lib/pleroma/web/oauth/oauth_controller.ex @@ -197,7 +197,7 @@ defp handle_create_authorization_error( {:mfa_required, user, auth, _}, params ) do - {:ok, token} = MFA.Token.create_token(user, auth) + {:ok, token} = MFA.Token.create(user, auth) data = %{ "mfa_token" => token.token, @@ -579,7 +579,7 @@ defp put_session_registration_id(%Plug.Conn{} = conn, registration_id), do: put_session(conn, :registration_id, registration_id) defp build_and_response_mfa_token(user, auth) do - with {:ok, token} <- MFA.Token.create_token(user, auth) do + with {:ok, token} <- MFA.Token.create(user, auth) do MFAView.render("mfa_response.json", %{token: token, user: user}) end end diff --git a/lib/pleroma/web/oauth/token.ex b/lib/pleroma/web/oauth/token.ex index 4d00fcb1c..de37998f2 100644 --- a/lib/pleroma/web/oauth/token.ex +++ b/lib/pleroma/web/oauth/token.ex @@ -87,9 +87,10 @@ defp put_valid_until(changeset, attrs) do def create(%App{} = app, %User{} = user, attrs \\ %{}) do with {:ok, token} <- do_create(app, user, attrs) do if Pleroma.Config.get([:oauth2, :clean_expired_tokens]) do - Pleroma.Workers.PurgeExpiredOAuthToken.enqueue(%{ + Pleroma.Workers.PurgeExpiredToken.enqueue(%{ token_id: token.id, - valid_until: DateTime.from_naive!(token.valid_until, "Etc/UTC") + valid_until: DateTime.from_naive!(token.valid_until, "Etc/UTC"), + mod: __MODULE__ }) end diff --git a/lib/pleroma/web/oauth/token/clean_worker.ex b/lib/pleroma/web/oauth/token/clean_worker.ex deleted file mode 100644 index 2f51bdb75..000000000 --- a/lib/pleroma/web/oauth/token/clean_worker.ex +++ /dev/null @@ -1,36 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Web.OAuth.Token.CleanWorker do - @moduledoc """ - The module represents functions to clean an expired OAuth and MFA tokens. - """ - use GenServer - - @ten_seconds 10_000 - @one_day 86_400_000 - - alias Pleroma.MFA - alias Pleroma.Workers.BackgroundWorker - - def start_link(_), do: GenServer.start_link(__MODULE__, %{}) - - def init(_) do - Process.send_after(self(), :perform, @ten_seconds) - {:ok, nil} - end - - @doc false - def handle_info(:perform, state) do - BackgroundWorker.enqueue("clean_expired_tokens", %{}) - interval = Pleroma.Config.get([:oauth2, :clean_expired_tokens_interval], @one_day) - - Process.send_after(self(), :perform, interval) - {:noreply, state} - end - - def perform(:clean) do - MFA.Token.delete_expired_tokens() - end -end diff --git a/lib/pleroma/web/twitter_api/controllers/remote_follow_controller.ex b/lib/pleroma/web/twitter_api/controllers/remote_follow_controller.ex index 521dc9322..072d889e2 100644 --- a/lib/pleroma/web/twitter_api/controllers/remote_follow_controller.ex +++ b/lib/pleroma/web/twitter_api/controllers/remote_follow_controller.ex @@ -135,7 +135,7 @@ defp handle_follow_error(conn, {:verify_mfa_code, followee, token, _} = _) do end defp handle_follow_error(conn, {:mfa_required, followee, user, _} = _) do - {:ok, %{token: token}} = MFA.Token.create_token(user) + {:ok, %{token: token}} = MFA.Token.create(user) render(conn, "follow_mfa.html", %{followee: followee, mfa_token: token, error: false}) end diff --git a/lib/pleroma/workers/purge_expired_token.ex b/lib/pleroma/workers/purge_expired_token.ex index 6068e43bf..a81e0cd28 100644 --- a/lib/pleroma/workers/purge_expired_token.ex +++ b/lib/pleroma/workers/purge_expired_token.ex @@ -2,14 +2,14 @@ # Copyright © 2017-2020 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only -defmodule Pleroma.Workers.PurgeExpiredOAuthToken do +defmodule Pleroma.Workers.PurgeExpiredToken do @moduledoc """ Worker which purges expired OAuth tokens """ - use Oban.Worker, queue: :oauth_token_expiration, max_attempts: 1 + use Oban.Worker, queue: :token_expiration, max_attempts: 1 - @spec enqueue(%{token_id: integer(), valid_until: DateTime.t()}) :: + @spec enqueue(%{token_id: integer(), valid_until: DateTime.t(), mod: module()}) :: {:ok, Oban.Job.t()} | {:error, Ecto.Changeset.t()} def enqueue(args) do {scheduled_at, args} = Map.pop(args, :valid_until) @@ -20,8 +20,9 @@ def enqueue(args) do end @impl true - def perform(%Oban.Job{args: %{"token_id" => id}}) do - Pleroma.Web.OAuth.Token + def perform(%Oban.Job{args: %{"token_id" => id, "mod" => module}}) do + module + |> String.to_existing_atom() |> Pleroma.Repo.get(id) |> Pleroma.Repo.delete() end diff --git a/test/web/twitter_api/remote_follow_controller_test.exs b/test/web/twitter_api/remote_follow_controller_test.exs index f7e54c26a..3852c7ce9 100644 --- a/test/web/twitter_api/remote_follow_controller_test.exs +++ b/test/web/twitter_api/remote_follow_controller_test.exs @@ -227,7 +227,7 @@ test "follows", %{conn: conn} do } ) - {:ok, %{token: token}} = MFA.Token.create_token(user) + {:ok, %{token: token}} = MFA.Token.create(user) user2 = insert(:user) otp_token = TOTP.generate_token(otp_secret) @@ -256,7 +256,7 @@ test "returns error when auth code is incorrect", %{conn: conn} do } ) - {:ok, %{token: token}} = MFA.Token.create_token(user) + {:ok, %{token: token}} = MFA.Token.create(user) user2 = insert(:user) otp_token = TOTP.generate_token(TOTP.generate_secret()) diff --git a/test/workers/purge_expired_oauth_token_test.exs b/test/workers/purge_expired_oauth_token_test.exs deleted file mode 100644 index 3bd650d89..000000000 --- a/test/workers/purge_expired_oauth_token_test.exs +++ /dev/null @@ -1,27 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Workers.PurgeExpiredOAuthTokenTest do - use Pleroma.DataCase, async: true - use Oban.Testing, repo: Pleroma.Repo - - import Pleroma.Factory - - setup do: clear_config([:oauth2, :clean_expired_tokens], true) - - test "purges expired token" do - user = insert(:user) - app = insert(:oauth_app) - - {:ok, %{id: id}} = Pleroma.Web.OAuth.Token.create(app, user) - - assert_enqueued( - worker: Pleroma.Workers.PurgeExpiredOAuthToken, - args: %{token_id: id} - ) - - assert {:ok, %{id: ^id}} = - perform_job(Pleroma.Workers.PurgeExpiredOAuthToken, %{token_id: id}) - end -end diff --git a/test/workers/purge_expired_token_test.exs b/test/workers/purge_expired_token_test.exs new file mode 100644 index 000000000..fb7708c3f --- /dev/null +++ b/test/workers/purge_expired_token_test.exs @@ -0,0 +1,51 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Workers.PurgeExpiredTokenTest do + use Pleroma.DataCase, async: true + use Oban.Testing, repo: Pleroma.Repo + + import Pleroma.Factory + + setup do: clear_config([:oauth2, :clean_expired_tokens], true) + + test "purges expired oauth token" do + user = insert(:user) + app = insert(:oauth_app) + + {:ok, %{id: id}} = Pleroma.Web.OAuth.Token.create(app, user) + + assert_enqueued( + worker: Pleroma.Workers.PurgeExpiredToken, + args: %{token_id: id, mod: Pleroma.Web.OAuth.Token} + ) + + assert {:ok, %{id: ^id}} = + perform_job(Pleroma.Workers.PurgeExpiredToken, %{ + token_id: id, + mod: Pleroma.Web.OAuth.Token + }) + + assert Repo.aggregate(Pleroma.Web.OAuth.Token, :count, :id) == 0 + end + + test "purges expired mfa token" do + authorization = insert(:oauth_authorization) + + {:ok, %{id: id}} = Pleroma.MFA.Token.create(authorization.user, authorization) + + assert_enqueued( + worker: Pleroma.Workers.PurgeExpiredToken, + args: %{token_id: id, mod: Pleroma.MFA.Token} + ) + + assert {:ok, %{id: ^id}} = + perform_job(Pleroma.Workers.PurgeExpiredToken, %{ + token_id: id, + mod: Pleroma.MFA.Token + }) + + assert Repo.aggregate(Pleroma.MFA.Token, :count, :id) == 0 + end +end From c6647c08e10a45aedcd77258a0e71c41d213eaa6 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Mon, 7 Sep 2020 11:54:10 +0300 Subject: [PATCH 3/6] migration and changelog --- CHANGELOG.md | 1 + ...ar_oauth_token_worker_from_oban_config.exs | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 priv/repo/migrations/20200907084956_remove_cron_clear_oauth_token_worker_from_oban_config.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 19b2596cc..14c0252f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Removed - **Breaking:** Removed `Pleroma.Workers.Cron.StatsWorker` setting from Oban `:crontab`. +- **Breaking:** Removed `Pleroma.Workers.Cron.ClearOauthTokenWorker` setting from Oban `:crontab` config. ## [2.1.1] - 2020-09-08 diff --git a/priv/repo/migrations/20200907084956_remove_cron_clear_oauth_token_worker_from_oban_config.exs b/priv/repo/migrations/20200907084956_remove_cron_clear_oauth_token_worker_from_oban_config.exs new file mode 100644 index 000000000..d9c972563 --- /dev/null +++ b/priv/repo/migrations/20200907084956_remove_cron_clear_oauth_token_worker_from_oban_config.exs @@ -0,0 +1,20 @@ +defmodule Pleroma.Repo.Migrations.RemoveCronClearOauthTokenWorkerFromObanConfig do + use Ecto.Migration + + def change do + with %Pleroma.ConfigDB{} = config <- + Pleroma.ConfigDB.get_by_params(%{group: :pleroma, key: Oban}), + crontab when is_list(crontab) <- config.value[:crontab], + index when is_integer(index) <- + Enum.find_index(crontab, fn {_, worker} -> + worker == Pleroma.Workers.Cron.ClearOauthTokenWorker + end) do + updated_value = Keyword.put(config.value, :crontab, List.delete_at(crontab, index)) + + config + |> Ecto.Changeset.change(value: updated_value) + |> Pleroma.Repo.update() + end + + end +end From e11fca88d424b394359f50646e4b4ec9b3ae1a8b Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Mon, 7 Sep 2020 13:44:42 +0300 Subject: [PATCH 4/6] migration to move tokens expiration into Oban --- ...92050_move_tokens_expiration_into_oban.exs | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 priv/repo/migrations/20200907092050_move_tokens_expiration_into_oban.exs diff --git a/priv/repo/migrations/20200907092050_move_tokens_expiration_into_oban.exs b/priv/repo/migrations/20200907092050_move_tokens_expiration_into_oban.exs new file mode 100644 index 000000000..832bd02a7 --- /dev/null +++ b/priv/repo/migrations/20200907092050_move_tokens_expiration_into_oban.exs @@ -0,0 +1,36 @@ +defmodule Pleroma.Repo.Migrations.MoveTokensExpirationIntoOban do + use Ecto.Migration + + import Ecto.Query, only: [from: 2] + + def change do + Supervisor.start_link([{Oban, Pleroma.Config.get(Oban)}], + strategy: :one_for_one, + name: Pleroma.Supervisor + ) + + if Pleroma.Config.get([:oauth2, :clean_expired_tokens]) do + from(t in Pleroma.Web.OAuth.Token, where: t.valid_until > ^NaiveDateTime.utc_now()) + |> Pleroma.Repo.stream() + |> Stream.each(fn token -> + Pleroma.Workers.PurgeExpiredToken.enqueue(%{ + token_id: token.id, + valid_until: DateTime.from_naive!(token.valid_until, "Etc/UTC"), + mod: Pleroma.Web.OAuth.Token + }) + end) + |> Stream.run() + end + + from(t in Pleroma.MFA.Token, where: t.valid_until > ^NaiveDateTime.utc_now()) + |> Pleroma.Repo.stream() + |> Stream.each(fn token -> + Pleroma.Workers.PurgeExpiredToken.enqueue(%{ + token_id: token.id, + valid_until: DateTime.from_naive!(token.valid_until, "Etc/UTC"), + mod: Pleroma.MFA.Token + }) + end) + |> Stream.run() + end +end From eca42566ba62ece75b79915f4af0c8a0f0c48a17 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Mon, 7 Sep 2020 13:54:28 +0300 Subject: [PATCH 5/6] formatting --- ...6_remove_cron_clear_oauth_token_worker_from_oban_config.exs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/priv/repo/migrations/20200907084956_remove_cron_clear_oauth_token_worker_from_oban_config.exs b/priv/repo/migrations/20200907084956_remove_cron_clear_oauth_token_worker_from_oban_config.exs index d9c972563..b5a0a0ff6 100644 --- a/priv/repo/migrations/20200907084956_remove_cron_clear_oauth_token_worker_from_oban_config.exs +++ b/priv/repo/migrations/20200907084956_remove_cron_clear_oauth_token_worker_from_oban_config.exs @@ -2,7 +2,7 @@ defmodule Pleroma.Repo.Migrations.RemoveCronClearOauthTokenWorkerFromObanConfig use Ecto.Migration def change do - with %Pleroma.ConfigDB{} = config <- + with %Pleroma.ConfigDB{} = config <- Pleroma.ConfigDB.get_by_params(%{group: :pleroma, key: Oban}), crontab when is_list(crontab) <- config.value[:crontab], index when is_integer(index) <- @@ -15,6 +15,5 @@ def change do |> Ecto.Changeset.change(value: updated_value) |> Pleroma.Repo.update() end - end end From 8af1fd32234df7d0cdb74d78bcca9f68587b70f2 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Mon, 7 Sep 2020 20:06:28 +0300 Subject: [PATCH 6/6] oban warning --- lib/pleroma/config/oban.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/config/oban.ex b/lib/pleroma/config/oban.ex index c2d56ebab..81758c93d 100644 --- a/lib/pleroma/config/oban.ex +++ b/lib/pleroma/config/oban.ex @@ -5,7 +5,7 @@ def warn do oban_config = Pleroma.Config.get(Oban) crontab = - [Pleroma.Workers.Cron.StatsWorker] + [Pleroma.Workers.Cron.StatsWorker, Pleroma.Workers.Cron.ClearOauthTokenWorker] |> Enum.reduce(oban_config[:crontab], fn removed_worker, acc -> with acc when is_list(acc) <- acc, setting when is_tuple(setting) <-