From b096e30cffc79a4adf12be88da412a290cd0d190 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 18 Dec 2018 17:13:52 +0300 Subject: [PATCH] [#114] Added email confirmation resend action. Added tests for registration, authentication, email confirmation, confirmation resending. Made admin methods create confirmed users. --- lib/mix/tasks/pleroma/user.ex | 4 +- lib/pleroma/user.ex | 28 ++++---- lib/pleroma/web/oauth/oauth_controller.ex | 18 +++++- lib/pleroma/web/router.ex | 2 + .../web/twitter_api/twitter_api_controller.ex | 13 +++- test/user_test.exs | 42 ++++++++++++ test/web/oauth/oauth_controller_test.exs | 50 +++++++++++++++ .../twitter_api_controller_test.exs | 64 +++++++++++++++++++ test/web/twitter_api/twitter_api_test.exs | 25 ++++++++ 9 files changed, 230 insertions(+), 16 deletions(-) diff --git a/lib/mix/tasks/pleroma/user.ex b/lib/mix/tasks/pleroma/user.ex index 3d30e3a81..51086a443 100644 --- a/lib/mix/tasks/pleroma/user.ex +++ b/lib/mix/tasks/pleroma/user.ex @@ -103,8 +103,8 @@ def run(["new", nickname, email | rest]) do bio: bio } - user = User.register_changeset(%User{}, params) - Repo.insert!(user) + changeset = User.register_changeset(%User{}, params, confirmed: true) + {:ok, _user} = User.register(changeset) Mix.shell().info("User #{nickname} created") diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 234617574..0cd7bc463 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -74,13 +74,15 @@ def follow_changeset(struct, params \\ %{}) do def user_info(%User{} = user) do oneself = if user.local, do: 1, else: 0 + user_info = user.info %{ following_count: length(user.following) - oneself, - note_count: user.info.note_count, - follower_count: user.info.follower_count, - locked: user.info.locked, - default_scope: user.info.default_scope + note_count: user_info.note_count, + follower_count: user_info.follower_count, + locked: user_info.locked, + confirmation_pending: user_info.confirmation_pending, + default_scope: user_info.default_scope } end @@ -209,17 +211,21 @@ def register_changeset(struct, params \\ %{}, opts \\ []) do @doc "Inserts provided changeset, performs post-registration actions (confirmation email sending etc.)" def register(%Ecto.Changeset{} = changeset) do with {:ok, user} <- Repo.insert(changeset) do - if user.info.confirmation_pending do - {:ok, _} = - user - |> Pleroma.UserEmail.account_confirmation_email() - |> Pleroma.Mailer.deliver() - end - + {:ok, _} = try_send_confirmation_email(user) {:ok, user} end end + def try_send_confirmation_email(%User{} = user) do + if user.info.confirmation_pending do + user + |> Pleroma.UserEmail.account_confirmation_email() + |> Pleroma.Mailer.deliver() + else + {:ok, :noop} + end + end + def needs_update?(%User{local: true}), do: false def needs_update?(%User{local: false, last_refreshed_at: nil}), do: true diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex index 10158f07e..9a972ee47 100644 --- a/lib/pleroma/web/oauth/oauth_controller.ex +++ b/lib/pleroma/web/oauth/oauth_controller.ex @@ -31,7 +31,7 @@ def create_authorization(conn, %{ }) do with %User{} = user <- User.get_by_nickname_or_email(name), true <- Pbkdf2.checkpw(password, user.password_hash), - true <- User.auth_active?(user), + {:auth_active, true} <- {:auth_active, User.auth_active?(user)}, %App{} = app <- Repo.get_by(App, client_id: client_id), {:ok, auth} <- Authorization.create_authorization(app, user) do # Special case: Local MastodonFE. @@ -64,6 +64,15 @@ def create_authorization(conn, %{ redirect(conn, external: url) end + else + {:auth_active, false} -> + conn + |> put_flash(:error, "Account confirmation pending") + |> put_status(:forbidden) + |> authorize(params) + + error -> + error end end @@ -102,7 +111,7 @@ def token_exchange( with %App{} = app <- get_app_from_request(conn, params), %User{} = user <- User.get_by_nickname_or_email(name), true <- Pbkdf2.checkpw(password, user.password_hash), - true <- User.auth_active?(user), + {:auth_active, true} <- {:auth_active, User.auth_active?(user)}, {:ok, auth} <- Authorization.create_authorization(app, user), {:ok, token} <- Token.exchange_token(app, auth) do response = %{ @@ -115,6 +124,11 @@ def token_exchange( json(conn, response) else + {:auth_active, false} -> + conn + |> put_status(:forbidden) + |> json(%{error: "Account confirmation pending"}) + _error -> put_status(conn, 400) |> json(%{error: "Invalid credentials"}) diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 0e4589116..ca069ab99 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -284,6 +284,8 @@ defmodule Pleroma.Web.Router do get("/account/confirm_email/:token", TwitterAPI.Controller, :confirm_email, as: :confirm_email) + post("/account/resend_confirmation_email", TwitterAPI.Controller, :resend_confirmation_email) + get("/search", TwitterAPI.Controller, :search) get("/statusnet/tags/timeline/:tag", TwitterAPI.Controller, :public_and_external_timeline) end diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index e8a3150e9..7286c153b 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -13,7 +13,7 @@ defmodule Pleroma.Web.TwitterAPI.Controller do require Logger plug(:only_if_public_instance when action in [:public_timeline, :public_and_external_timeline]) - plug(:fetch_flash when action in [:confirm_email]) + plug(:fetch_flash when action in [:confirm_email, :resend_confirmation_email]) action_fallback(:errors) def verify_credentials(%{assigns: %{user: user}} = conn, _params) do @@ -385,6 +385,17 @@ def confirm_email(conn, %{"token" => token}) do end end + def resend_confirmation_email(conn, params) do + nickname_or_email = params["email"] || params["nickname"] + + with %User{} = user <- User.get_by_nickname_or_email(nickname_or_email), + {:ok, _} <- User.try_send_confirmation_email(user) do + conn + |> put_flash(:info, "Email confirmation has been sent.") + |> json_response(:no_content, "") + end + end + def update_avatar(%{assigns: %{user: user}} = conn, params) do {:ok, object} = ActivityPub.upload(params, type: :avatar) change = Changeset.change(user, %{avatar: object.data}) diff --git a/test/user_test.exs b/test/user_test.exs index 1e73770df..b4d8174c6 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -177,6 +177,48 @@ test "it ensures info is not nil" do end end + describe "user registration, with :account_activation_required" do + @full_user_data %{ + bio: "A guy", + name: "my name", + nickname: "nick", + password: "test", + password_confirmation: "test", + email: "email@example.com" + } + + setup do + setting = Pleroma.Config.get([:instance, :account_activation_required]) + + unless setting do + Pleroma.Config.put([:instance, :account_activation_required], true) + on_exit(fn -> Pleroma.Config.put([:instance, :account_activation_required], setting) end) + end + + :ok + end + + test "it creates unconfirmed user" do + changeset = User.register_changeset(%User{}, @full_user_data) + assert changeset.valid? + + {:ok, user} = Repo.insert(changeset) + + assert user.info.confirmation_pending + assert user.info.confirmation_token + end + + test "it creates confirmed user if :confirmed option is given" do + changeset = User.register_changeset(%User{}, @full_user_data, confirmed: true) + assert changeset.valid? + + {:ok, user} = Repo.insert(changeset) + + refute user.info.confirmation_pending + refute user.info.confirmation_token + end + end + describe "get_or_fetch/1" do test "gets an existing user by nickname" do user = insert(:user) diff --git a/test/web/oauth/oauth_controller_test.exs b/test/web/oauth/oauth_controller_test.exs index 3a902f128..55b471d8a 100644 --- a/test/web/oauth/oauth_controller_test.exs +++ b/test/web/oauth/oauth_controller_test.exs @@ -50,6 +50,26 @@ test "issues a token for an all-body request" do assert Repo.get_by(Token, token: token) end + test "issues a token for `password` grant_type with valid credentials" do + password = "testpassword" + user = insert(:user, password_hash: Comeonin.Pbkdf2.hashpwsalt(password)) + + app = insert(:oauth_app) + + conn = + build_conn() + |> post("/oauth/token", %{ + "grant_type" => "password", + "username" => user.nickname, + "password" => password, + "client_id" => app.client_id, + "client_secret" => app.client_secret + }) + + assert %{"access_token" => token} = json_response(conn, 200) + assert Repo.get_by(Token, token: token) + end + test "issues a token for request with HTTP basic auth client credentials" do user = insert(:user) app = insert(:oauth_app) @@ -93,6 +113,36 @@ test "rejects token exchange with invalid client credentials" do refute Map.has_key?(resp, "access_token") end + test "rejects token exchange for valid credentials belonging to unconfirmed user" do + password = "testpassword" + user = insert(:user, password_hash: Comeonin.Pbkdf2.hashpwsalt(password)) + info_change = Pleroma.User.Info.confirmation_update(user.info, :unconfirmed) + + {:ok, user} = + user + |> Ecto.Changeset.change() + |> Ecto.Changeset.put_embed(:info, info_change) + |> Repo.update() + + refute Pleroma.User.auth_active?(user) + + app = insert(:oauth_app) + + conn = + build_conn() + |> post("/oauth/token", %{ + "grant_type" => "password", + "username" => user.nickname, + "password" => password, + "client_id" => app.client_id, + "client_secret" => app.client_secret + }) + + assert resp = json_response(conn, 403) + assert %{"error" => _} = resp + refute Map.has_key?(resp, "access_token") + end + test "rejects an invalid authorization code" do app = insert(:oauth_app) diff --git a/test/web/twitter_api/twitter_api_controller_test.exs b/test/web/twitter_api/twitter_api_controller_test.exs index c16c0cdc0..eb154608c 100644 --- a/test/web/twitter_api/twitter_api_controller_test.exs +++ b/test/web/twitter_api/twitter_api_controller_test.exs @@ -873,6 +873,70 @@ test "it returns 500 when user is not local", %{conn: conn, user: user} do end end + describe "GET /api/account/confirm_email/:token" do + setup do + user = insert(:user) + info_change = User.Info.confirmation_update(user.info, :unconfirmed) + + {:ok, user} = + user + |> Changeset.change() + |> Changeset.put_embed(:info, info_change) + |> Repo.update() + + assert user.info.confirmation_pending + + [user: user] + end + + test "it redirects to root url", %{conn: conn, user: user} do + conn = get(conn, "/api/account/confirm_email/#{user.info.confirmation_token}") + + assert 302 == conn.status + end + + test "it confirms the user account", %{conn: conn, user: user} do + get(conn, "/api/account/confirm_email/#{user.info.confirmation_token}") + + user = Repo.get(User, user.id) + + refute user.info.confirmation_pending + refute user.info.confirmation_token + end + end + + describe "POST /api/account/resend_confirmation_email" do + setup do + user = insert(:user) + info_change = User.Info.confirmation_update(user.info, :unconfirmed) + + {:ok, user} = + user + |> Changeset.change() + |> Changeset.put_embed(:info, info_change) + |> Repo.update() + + assert user.info.confirmation_pending + + [user: user] + end + + test "it returns 204 No Content", %{conn: conn, user: user} do + conn + |> assign(:user, user) + |> post("/api/account/resend_confirmation_email?email=#{user.email}") + |> json_response(:no_content) + end + + test "it sends confirmation email", %{conn: conn, user: user} do + conn + |> assign(:user, user) + |> post("/api/account/resend_confirmation_email?email=#{user.email}") + + Swoosh.TestAssertions.assert_email_sent(Pleroma.UserEmail.account_confirmation_email(user)) + end + end + describe "GET /api/externalprofile/show" do test "it returns the user", %{conn: conn} do user = insert(:user) diff --git a/test/web/twitter_api/twitter_api_test.exs b/test/web/twitter_api/twitter_api_test.exs index 3d3a637b7..b7c89b605 100644 --- a/test/web/twitter_api/twitter_api_test.exs +++ b/test/web/twitter_api/twitter_api_test.exs @@ -275,6 +275,31 @@ test "it registers a new user with empty string in bio and returns the user." do UserView.render("show.json", %{user: fetched_user}) end + @moduletag skip: "needs 'account_activation_required: true' in config" + test "it sends confirmation email if :account_activation_required is specified in instance config" do + setting = Pleroma.Config.get([:instance, :account_activation_required]) + + unless setting do + Pleroma.Config.put([:instance, :account_activation_required], true) + on_exit(fn -> Pleroma.Config.put([:instance, :account_activation_required], setting) end) + end + + data = %{ + "nickname" => "lain", + "email" => "lain@wired.jp", + "fullname" => "lain iwakura", + "bio" => "", + "password" => "bear", + "confirm" => "bear" + } + + {:ok, user} = TwitterAPI.register_user(data) + + assert user.info.confirmation_pending + + Swoosh.TestAssertions.assert_email_sent(Pleroma.UserEmail.account_confirmation_email(user)) + end + test "it registers a new user and parses mentions in the bio" do data1 = %{ "nickname" => "john",