From 3ce658b93098551792a69f2455e6e9339a1722e2 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Tue, 25 Aug 2020 19:17:51 +0300 Subject: [PATCH] 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