From a0c4ebb4d73f43a9c567c5309f0e8d1b88995481 Mon Sep 17 00:00:00 2001 From: Maksim Date: Mon, 24 Jun 2019 19:01:56 +0000 Subject: [PATCH] [#184] small refactoring reset password --- lib/mix/tasks/pleroma/user.ex | 4 +- lib/pleroma/emails/user_email.ex | 9 +-- ...dResetToken.ex => password_reset_token.ex} | 1 + lib/pleroma/user.ex | 47 +++++++++------- lib/pleroma/web/oauth/authorization.ex | 12 ++-- lib/pleroma/web/router.ex | 4 +- .../{util => password}/invalid_token.html.eex | 0 .../reset.html.eex} | 2 +- .../reset_failed.html.eex} | 0 .../reset_success.html.eex} | 0 .../controllers/password_controller.ex | 37 ++++++++++++ .../controllers/util_controller.ex | 22 -------- .../web/twitter_api/views/password_view.ex | 8 +++ .../twitter_api/password_controller_test.exs | 56 +++++++++++++++++++ 14 files changed, 142 insertions(+), 60 deletions(-) rename lib/pleroma/{PasswordResetToken.ex => password_reset_token.ex} (93%) rename lib/pleroma/web/templates/twitter_api/{util => password}/invalid_token.html.eex (100%) rename lib/pleroma/web/templates/twitter_api/{util/password_reset.html.eex => password/reset.html.eex} (82%) rename lib/pleroma/web/templates/twitter_api/{util/password_reset_failed.html.eex => password/reset_failed.html.eex} (100%) rename lib/pleroma/web/templates/twitter_api/{util/password_reset_success.html.eex => password/reset_success.html.eex} (100%) create mode 100644 lib/pleroma/web/twitter_api/controllers/password_controller.ex create mode 100644 lib/pleroma/web/twitter_api/views/password_view.ex create mode 100644 test/web/twitter_api/password_controller_test.exs diff --git a/lib/mix/tasks/pleroma/user.ex b/lib/mix/tasks/pleroma/user.ex index ab158f57e..8a78b4fe6 100644 --- a/lib/mix/tasks/pleroma/user.ex +++ b/lib/mix/tasks/pleroma/user.ex @@ -204,9 +204,9 @@ defmodule Mix.Tasks.Pleroma.User do IO.puts( "URL: #{ - Pleroma.Web.Router.Helpers.util_url( + Pleroma.Web.Router.Helpers.reset_password_url( Pleroma.Web.Endpoint, - :show_password_reset, + :reset, token.token ) }" diff --git a/lib/pleroma/emails/user_email.ex b/lib/pleroma/emails/user_email.ex index 8502a0d0c..934620765 100644 --- a/lib/pleroma/emails/user_email.ex +++ b/lib/pleroma/emails/user_email.ex @@ -23,13 +23,8 @@ defmodule Pleroma.Emails.UserEmail do defp recipient(email, name), do: {name, email} defp recipient(%Pleroma.User{} = user), do: recipient(user.email, user.name) - def password_reset_email(user, password_reset_token) when is_binary(password_reset_token) do - password_reset_url = - Router.Helpers.util_url( - Endpoint, - :show_password_reset, - password_reset_token - ) + def password_reset_email(user, token) when is_binary(token) do + password_reset_url = Router.Helpers.reset_password_url(Endpoint, :reset, token) html_body = """

Reset your password at #{instance_name()}

diff --git a/lib/pleroma/PasswordResetToken.ex b/lib/pleroma/password_reset_token.ex similarity index 93% rename from lib/pleroma/PasswordResetToken.ex rename to lib/pleroma/password_reset_token.ex index f31ea5bc5..4a833f6a5 100644 --- a/lib/pleroma/PasswordResetToken.ex +++ b/lib/pleroma/password_reset_token.ex @@ -37,6 +37,7 @@ defmodule Pleroma.PasswordResetToken do |> put_change(:used, true) end + @spec reset_password(binary(), map()) :: {:ok, User.t()} | {:error, binary()} def reset_password(token, data) do with %{used: false} = token <- Repo.get_by(PasswordResetToken, %{token: token}), %User{} = user <- User.get_cached_by_id(token.user_id), diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 1e59a4121..f7191762f 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -9,6 +9,7 @@ defmodule Pleroma.User do import Ecto.Query alias Comeonin.Pbkdf2 + alias Ecto.Multi alias Pleroma.Activity alias Pleroma.Keys alias Pleroma.Notification @@ -194,27 +195,24 @@ defmodule Pleroma.User do end def password_update_changeset(struct, params) do - changeset = - struct - |> cast(params, [:password, :password_confirmation]) - |> validate_required([:password, :password_confirmation]) - |> validate_confirmation(:password) - - OAuth.Token.delete_user_tokens(struct) - OAuth.Authorization.delete_user_authorizations(struct) - - if changeset.valid? do - hashed = Pbkdf2.hashpwsalt(changeset.changes[:password]) - - changeset - |> put_change(:password_hash, hashed) - else - changeset - end + struct + |> cast(params, [:password, :password_confirmation]) + |> validate_required([:password, :password_confirmation]) + |> validate_confirmation(:password) + |> put_password_hash end - def reset_password(user, data) do - update_and_set_cache(password_update_changeset(user, data)) + def reset_password(%User{id: user_id} = user, data) do + multi = + Multi.new() + |> Multi.update(:user, password_update_changeset(user, data)) + |> Multi.delete_all(:tokens, OAuth.Token.Query.get_by_user(user_id)) + |> Multi.delete_all(:auth, OAuth.Authorization.delete_by_user_query(user)) + + case Repo.transaction(multi) do + {:ok, %{user: user} = _} -> set_cache(user) + {:error, _, changeset, _} -> {:error, changeset} + end end def register_changeset(struct, params \\ %{}, opts \\ []) do @@ -250,12 +248,11 @@ defmodule Pleroma.User do end if changeset.valid? do - hashed = Pbkdf2.hashpwsalt(changeset.changes[:password]) ap_id = User.ap_id(%User{nickname: changeset.changes[:nickname]}) followers = User.ap_followers(%User{nickname: changeset.changes[:nickname]}) changeset - |> put_change(:password_hash, hashed) + |> put_password_hash |> put_change(:ap_id, ap_id) |> unique_constraint(:ap_id) |> put_change(:following, [followers]) @@ -1349,4 +1346,12 @@ defmodule Pleroma.User do end defdelegate search(query, opts \\ []), to: User.Search + + defp put_password_hash( + %Ecto.Changeset{valid?: true, changes: %{password: password}} = changeset + ) do + change(changeset, password_hash: Pbkdf2.hashpwsalt(password)) + end + + defp put_password_hash(changeset), do: changeset end diff --git a/lib/pleroma/web/oauth/authorization.ex b/lib/pleroma/web/oauth/authorization.ex index 18973413e..d53e20d12 100644 --- a/lib/pleroma/web/oauth/authorization.ex +++ b/lib/pleroma/web/oauth/authorization.ex @@ -76,14 +76,16 @@ defmodule Pleroma.Web.OAuth.Authorization do def use_token(%Authorization{used: true}), do: {:error, "already used"} @spec delete_user_authorizations(User.t()) :: {integer(), any()} - def delete_user_authorizations(%User{id: user_id}) do - from( - a in Pleroma.Web.OAuth.Authorization, - where: a.user_id == ^user_id - ) + def delete_user_authorizations(%User{} = user) do + user + |> delete_by_user_query |> Repo.delete_all() end + def delete_by_user_query(%User{id: user_id}) do + from(a in __MODULE__, where: a.user_id == ^user_id) + end + @doc "gets auth for app by token" @spec get_by_token(App.t(), String.t()) :: {:ok, t()} | {:error, :not_found} def get_by_token(%App{id: app_id} = _app, token) do diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 837153ed4..c504116b6 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -133,8 +133,8 @@ defmodule Pleroma.Web.Router do scope "/api/pleroma", Pleroma.Web.TwitterAPI do pipe_through(:pleroma_api) - get("/password_reset/:token", UtilController, :show_password_reset) - post("/password_reset", UtilController, :password_reset) + get("/password_reset/:token", PasswordController, :reset, as: :reset_password) + post("/password_reset", PasswordController, :do_reset, as: :reset_password) get("/emoji", UtilController, :emoji) get("/captcha", UtilController, :captcha) get("/healthcheck", UtilController, :healthcheck) diff --git a/lib/pleroma/web/templates/twitter_api/util/invalid_token.html.eex b/lib/pleroma/web/templates/twitter_api/password/invalid_token.html.eex similarity index 100% rename from lib/pleroma/web/templates/twitter_api/util/invalid_token.html.eex rename to lib/pleroma/web/templates/twitter_api/password/invalid_token.html.eex diff --git a/lib/pleroma/web/templates/twitter_api/util/password_reset.html.eex b/lib/pleroma/web/templates/twitter_api/password/reset.html.eex similarity index 82% rename from lib/pleroma/web/templates/twitter_api/util/password_reset.html.eex rename to lib/pleroma/web/templates/twitter_api/password/reset.html.eex index a3facf017..7d3ef6b0d 100644 --- a/lib/pleroma/web/templates/twitter_api/util/password_reset.html.eex +++ b/lib/pleroma/web/templates/twitter_api/password/reset.html.eex @@ -1,5 +1,5 @@

Password Reset for <%= @user.nickname %>

-<%= form_for @conn, util_path(@conn, :password_reset), [as: "data"], fn f -> %> +<%= form_for @conn, reset_password_path(@conn, :do_reset), [as: "data"], fn f -> %>
<%= label f, :password, "Password" %> <%= password_input f, :password %> diff --git a/lib/pleroma/web/templates/twitter_api/util/password_reset_failed.html.eex b/lib/pleroma/web/templates/twitter_api/password/reset_failed.html.eex similarity index 100% rename from lib/pleroma/web/templates/twitter_api/util/password_reset_failed.html.eex rename to lib/pleroma/web/templates/twitter_api/password/reset_failed.html.eex diff --git a/lib/pleroma/web/templates/twitter_api/util/password_reset_success.html.eex b/lib/pleroma/web/templates/twitter_api/password/reset_success.html.eex similarity index 100% rename from lib/pleroma/web/templates/twitter_api/util/password_reset_success.html.eex rename to lib/pleroma/web/templates/twitter_api/password/reset_success.html.eex diff --git a/lib/pleroma/web/twitter_api/controllers/password_controller.ex b/lib/pleroma/web/twitter_api/controllers/password_controller.ex new file mode 100644 index 000000000..1941e6143 --- /dev/null +++ b/lib/pleroma/web/twitter_api/controllers/password_controller.ex @@ -0,0 +1,37 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.TwitterAPI.PasswordController do + @moduledoc """ + The module containts functions for reset password. + """ + + use Pleroma.Web, :controller + + require Logger + + alias Pleroma.PasswordResetToken + alias Pleroma.Repo + alias Pleroma.User + + def reset(conn, %{"token" => token}) do + with %{used: false} = token <- Repo.get_by(PasswordResetToken, %{token: token}), + %User{} = user <- User.get_cached_by_id(token.user_id) do + render(conn, "reset.html", %{ + token: token, + user: user + }) + else + _e -> render(conn, "invalid_token.html") + end + end + + def do_reset(conn, %{"data" => data}) do + with {:ok, _} <- PasswordResetToken.reset_password(data["token"], data) do + render(conn, "reset_success.html") + else + _e -> render(conn, "reset_failed.html") + end + end +end diff --git a/lib/pleroma/web/twitter_api/controllers/util_controller.ex b/lib/pleroma/web/twitter_api/controllers/util_controller.ex index 489170d80..b1863528f 100644 --- a/lib/pleroma/web/twitter_api/controllers/util_controller.ex +++ b/lib/pleroma/web/twitter_api/controllers/util_controller.ex @@ -11,8 +11,6 @@ defmodule Pleroma.Web.TwitterAPI.UtilController do alias Pleroma.Activity alias Pleroma.Emoji alias Pleroma.Notification - alias Pleroma.PasswordResetToken - alias Pleroma.Repo alias Pleroma.User alias Pleroma.Web alias Pleroma.Web.ActivityPub.ActivityPub @@ -20,26 +18,6 @@ defmodule Pleroma.Web.TwitterAPI.UtilController do alias Pleroma.Web.OStatus alias Pleroma.Web.WebFinger - def show_password_reset(conn, %{"token" => token}) do - with %{used: false} = token <- Repo.get_by(PasswordResetToken, %{token: token}), - %User{} = user <- User.get_cached_by_id(token.user_id) do - render(conn, "password_reset.html", %{ - token: token, - user: user - }) - else - _e -> render(conn, "invalid_token.html") - end - end - - def password_reset(conn, %{"data" => data}) do - with {:ok, _} <- PasswordResetToken.reset_password(data["token"], data) do - render(conn, "password_reset_success.html") - else - _e -> render(conn, "password_reset_failed.html") - end - end - def help_test(conn, _params) do json(conn, "ok") end diff --git a/lib/pleroma/web/twitter_api/views/password_view.ex b/lib/pleroma/web/twitter_api/views/password_view.ex new file mode 100644 index 000000000..b166b925d --- /dev/null +++ b/lib/pleroma/web/twitter_api/views/password_view.ex @@ -0,0 +1,8 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.TwitterAPI.PasswordView do + use Pleroma.Web, :view + import Phoenix.HTML.Form +end diff --git a/test/web/twitter_api/password_controller_test.exs b/test/web/twitter_api/password_controller_test.exs new file mode 100644 index 000000000..6b9da8204 --- /dev/null +++ b/test/web/twitter_api/password_controller_test.exs @@ -0,0 +1,56 @@ +defmodule Pleroma.Web.TwitterAPI.PasswordControllerTest do + use Pleroma.Web.ConnCase + + alias Pleroma.PasswordResetToken + alias Pleroma.Web.OAuth.Token + import Pleroma.Factory + + describe "GET /api/pleroma/password_reset/token" do + test "it returns error when token invalid", %{conn: conn} do + response = + conn + |> get("/api/pleroma/password_reset/token") + |> html_response(:ok) + + assert response =~ "

Invalid Token

" + end + + test "it shows password reset form", %{conn: conn} do + user = insert(:user) + {:ok, token} = PasswordResetToken.create_token(user) + + response = + conn + |> get("/api/pleroma/password_reset/#{token.token}") + |> html_response(:ok) + + assert response =~ "

Password Reset for #{user.nickname}

" + end + end + + describe "POST /api/pleroma/password_reset" 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, %{}) + + params = %{ + "password" => "test", + password_confirmation: "test", + token: token.token + } + + response = + conn + |> assign(:user, user) + |> post("/api/pleroma/password_reset", %{data: params}) + |> html_response(:ok) + + assert response =~ "

Password changed!

" + + user = refresh_record(user) + assert Comeonin.Pbkdf2.checkpw("test", user.password_hash) + assert length(Token.get_user_tokens(user)) == 0 + end + end +end