From a05cb10a95901ff0daacfc17a7709f3a277f1cd4 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Fri, 14 Dec 2018 16:38:56 +0300 Subject: [PATCH 01/17] [#114] Email confirmation route, action, node setting, User.Info fields. --- lib/pleroma/user.ex | 4 ++++ lib/pleroma/user/info.ex | 6 ++++++ lib/pleroma/web/nodeinfo/nodeinfo_controller.ex | 1 + lib/pleroma/web/router.ex | 1 + .../web/twitter_api/twitter_api_controller.ex | 13 +++++++++++++ 5 files changed, 25 insertions(+) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 3ad1ab87a..ee0a0dfb9 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -364,6 +364,10 @@ def get_or_fetch_by_nickname(nickname) do end end + def get_by_confirmation_token(token) do + Repo.one(from(u in User, where: fragment("? ->> 'confirmation_token' = ?", u.info, ^token))) + end + def get_followers_query(%User{id: id, follower_address: follower_address}) do from( u in User, diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index a3785447c..f75984038 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -9,6 +9,8 @@ defmodule Pleroma.User.Info do field(:note_count, :integer, default: 0) field(:follower_count, :integer, default: 0) field(:locked, :boolean, default: false) + field(:confirmation_pending, :boolean, default: false) + field(:confirmation_token, :string, default: nil) field(:default_scope, :string, default: "public") field(:blocks, {:array, :string}, default: []) field(:domain_blocks, {:array, :string}, default: []) @@ -141,6 +143,10 @@ def profile_update(info, params) do ]) end + def confirmation_update(info, params) do + cast(info, params, [:confirmation_pending, :confirmation_token]) + end + def mastodon_profile_update(info, params) do info |> cast(params, [ diff --git a/lib/pleroma/web/nodeinfo/nodeinfo_controller.ex b/lib/pleroma/web/nodeinfo/nodeinfo_controller.ex index 44c11f40a..70921605d 100644 --- a/lib/pleroma/web/nodeinfo/nodeinfo_controller.ex +++ b/lib/pleroma/web/nodeinfo/nodeinfo_controller.ex @@ -132,6 +132,7 @@ def nodeinfo(conn, %{"version" => "2.0"}) do banner: Keyword.get(instance, :banner_upload_limit), background: Keyword.get(instance, :background_upload_limit) }, + accountActivationRequired: Keyword.get(instance, :account_activation_required, false), invitesEnabled: Keyword.get(instance, :invites_enabled, false), features: features } diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index dd1985d6e..b2fbc088d 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -281,6 +281,7 @@ defmodule Pleroma.Web.Router do post("/account/register", TwitterAPI.Controller, :register) post("/account/password_reset", TwitterAPI.Controller, :password_reset) + get("/account/confirm_email/:token", TwitterAPI.Controller, :confirm_email) get("/search", TwitterAPI.Controller, :search) get("/statusnet/tags/timeline/:tag", TwitterAPI.Controller, :public_and_external_timeline) diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index 327620302..2680be25f 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -372,6 +372,19 @@ def password_reset(conn, params) do end end + def confirm_email(conn, %{"token" => token}) do + with %User{} = user <- User.get_by_confirmation_token(token), + true <- user.local, + new_info_fields <- %{confirmation_pending: false, confirmation_token: nil}, + info_change <- User.Info.confirmation_update(user.info, new_info_fields), + changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_change), + {:ok, _} <- User.update_and_set_cache(changeset) do + conn + |> put_flash(:info, "Email confirmed. Please sign in.") + |> redirect(to: "/") + 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}) From 1de0aa2f1025d4a860a11e658ce5fed26fe1c4ad Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Mon, 17 Dec 2018 17:28:58 +0300 Subject: [PATCH 02/17] [#114] Account confirmation email, registration as unconfirmed (config-based), auth prevention for unconfirmed. --- lib/pleroma/emails/user_email.ex | 24 ++++++++++++++++++- lib/pleroma/user.ex | 2 ++ lib/pleroma/user/info.ex | 14 +++++++++++ lib/pleroma/web/oauth/oauth_controller.ex | 2 ++ lib/pleroma/web/router.ex | 3 ++- .../controllers/util_controller.ex | 2 ++ lib/pleroma/web/twitter_api/twitter_api.ex | 22 +++++++++++++++-- .../web/twitter_api/twitter_api_controller.ex | 4 ++-- 8 files changed, 67 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/emails/user_email.ex b/lib/pleroma/emails/user_email.ex index 7e3e9b020..856816386 100644 --- a/lib/pleroma/emails/user_email.ex +++ b/lib/pleroma/emails/user_email.ex @@ -15,6 +15,7 @@ defp sender do defp recipient(email, nil), do: email 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 = @@ -32,7 +33,7 @@ def password_reset_email(user, password_reset_token) when is_binary(password_res """ new() - |> to(recipient(user.email, user.name)) + |> to(recipient(user)) |> from(sender()) |> subject("Password reset") |> html_body(html_body) @@ -63,4 +64,25 @@ def user_invitation_email( |> subject("Invitation to #{instance_name()}") |> html_body(html_body) end + + def account_confirmation_email(user) do + confirmation_url = + Router.Helpers.confirm_email_url( + Endpoint, + :confirm_email, + to_string(user.info.confirmation_token) + ) + + html_body = """ +

Welcome to #{instance_name()}!

+

Email confirmation is required to activate the account.

+

Click the following link to proceed: activate your account.

+ """ + + new() + |> to(recipient(user)) + |> from(sender()) + |> subject("#{instance_name()} account confirmation") + |> html_body(html_body) + end end diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index ee0a0dfb9..a38ead81a 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -38,6 +38,8 @@ defmodule Pleroma.User do timestamps() end + def auth_active?(user), do: user.info && !user.info.confirmation_pending + def avatar_url(user) do case user.avatar do %{"url" => [%{"href" => href} | _]} -> href diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index f75984038..9ce9129cd 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -143,6 +143,20 @@ def profile_update(info, params) do ]) end + def confirmation_update(info, :confirmed) do + confirmation_update(info, %{ + confirmation_pending: false, + confirmation_token: nil + }) + end + + def confirmation_update(info, :unconfirmed) do + confirmation_update(info, %{ + confirmation_pending: true, + confirmation_token: :crypto.strong_rand_bytes(32) |> Base.url_encode64() + }) + end + def confirmation_update(info, params) do cast(info, params, [:confirmation_pending, :confirmation_token]) end diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex index 20c2e799b..10158f07e 100644 --- a/lib/pleroma/web/oauth/oauth_controller.ex +++ b/lib/pleroma/web/oauth/oauth_controller.ex @@ -31,6 +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), %App{} = app <- Repo.get_by(App, client_id: client_id), {:ok, auth} <- Authorization.create_authorization(app, user) do # Special case: Local MastodonFE. @@ -101,6 +102,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), {:ok, auth} <- Authorization.create_authorization(app, user), {:ok, token} <- Token.exchange_token(app, auth) do response = %{ diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index b2fbc088d..0e4589116 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -281,7 +281,8 @@ defmodule Pleroma.Web.Router do post("/account/register", TwitterAPI.Controller, :register) post("/account/password_reset", TwitterAPI.Controller, :password_reset) - get("/account/confirm_email/:token", TwitterAPI.Controller, :confirm_email) + + get("/account/confirm_email/:token", TwitterAPI.Controller, :confirm_email, as: :confirm_email) get("/search", TwitterAPI.Controller, :search) get("/statusnet/tags/timeline/:tag", TwitterAPI.Controller, :public_and_external_timeline) diff --git a/lib/pleroma/web/twitter_api/controllers/util_controller.ex b/lib/pleroma/web/twitter_api/controllers/util_controller.ex index 38653f0b8..3baeba619 100644 --- a/lib/pleroma/web/twitter_api/controllers/util_controller.ex +++ b/lib/pleroma/web/twitter_api/controllers/util_controller.ex @@ -174,6 +174,8 @@ def config(conn, _params) do closed: if(Keyword.get(instance, :registrations_open), do: "0", else: "1"), private: if(Keyword.get(instance, :public, true), do: "0", else: "1"), vapidPublicKey: vapid_public_key, + accountActivationRequired: + if(Keyword.get(instance, :account_activation_required, false), do: "1", else: "0"), invitesEnabled: if(Keyword.get(instance, :invites_enabled, false), do: "1", else: "0") } diff --git a/lib/pleroma/web/twitter_api/twitter_api.ex b/lib/pleroma/web/twitter_api/twitter_api.ex index 90b8345c5..b77761aa4 100644 --- a/lib/pleroma/web/twitter_api/twitter_api.ex +++ b/lib/pleroma/web/twitter_api/twitter_api.ex @@ -1,8 +1,10 @@ defmodule Pleroma.Web.TwitterAPI.TwitterAPI do alias Pleroma.{UserInviteToken, User, Activity, Repo, Object} + alias Pleroma.{UserEmail, Mailer} alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.TwitterAPI.UserView alias Pleroma.Web.CommonAPI + import Ecto.Query def create_status(%User{} = user, %{"status" => _} = data) do @@ -165,6 +167,22 @@ def register_user(params) do with {:ok, user} <- Repo.insert(changeset) do !registrations_open && UserInviteToken.mark_as_used(token.token) + + if Pleroma.Config.get([:instance, :account_activation_required]) do + info_change = User.Info.confirmation_update(user.info, :unconfirmed) + + {:ok, unconfirmed_user} = + user + |> Ecto.Changeset.change() + |> Ecto.Changeset.put_embed(:info, info_change) + |> Repo.update() + + {:ok, _} = + unconfirmed_user + |> UserEmail.account_confirmation_email() + |> Mailer.deliver() + end + {:ok, user} else {:error, changeset} -> @@ -189,8 +207,8 @@ def password_reset(nickname_or_email) do %User{local: true} = user <- User.get_by_nickname_or_email(nickname_or_email), {:ok, token_record} <- Pleroma.PasswordResetToken.create_token(user) do user - |> Pleroma.UserEmail.password_reset_email(token_record.token) - |> Pleroma.Mailer.deliver() + |> UserEmail.password_reset_email(token_record.token) + |> Mailer.deliver() else false -> {:error, "bad user identifier"} diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index 2680be25f..e8a3150e9 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -13,6 +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]) action_fallback(:errors) def verify_credentials(%{assigns: %{user: user}} = conn, _params) do @@ -375,8 +376,7 @@ def password_reset(conn, params) do def confirm_email(conn, %{"token" => token}) do with %User{} = user <- User.get_by_confirmation_token(token), true <- user.local, - new_info_fields <- %{confirmation_pending: false, confirmation_token: nil}, - info_change <- User.Info.confirmation_update(user.info, new_info_fields), + info_change <- User.Info.confirmation_update(user.info, :confirmed), changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_change), {:ok, _} <- User.update_and_set_cache(changeset) do conn From b86057cc7f45c79767ff5b83730c2c15ad6bb3bd Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 18 Dec 2018 13:13:57 +0300 Subject: [PATCH 03/17] [#114] Refactored User.register_changeset to init confirmation data. Introduced User.register/1 to encapsulate User record creation and post-registration actions. --- lib/pleroma/user.ex | 25 ++++++++++++- .../web/admin_api/admin_api_controller.ex | 10 ++--- lib/pleroma/web/twitter_api/twitter_api.ex | 37 ++++++------------- 3 files changed, 39 insertions(+), 33 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index a38ead81a..234617574 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -170,7 +170,14 @@ def reset_password(user, data) do update_and_set_cache(password_update_changeset(user, data)) end - def register_changeset(struct, params \\ %{}) do + def register_changeset(struct, params \\ %{}, opts \\ []) do + confirmation_status = + if opts[:confirmed] || !Pleroma.Config.get([:instance, :account_activation_required]) do + :confirmed + else + :unconfirmed + end + changeset = struct |> cast(params, [:bio, :email, :name, :nickname, :password, :password_confirmation]) @@ -182,7 +189,7 @@ def register_changeset(struct, params \\ %{}) do |> validate_format(:email, @email_regex) |> validate_length(:bio, max: 1000) |> validate_length(:name, min: 1, max: 100) - |> put_change(:info, %Pleroma.User.Info{}) + |> put_change(:info, User.Info.confirmation_update(%User.Info{}, confirmation_status)) if changeset.valid? do hashed = Pbkdf2.hashpwsalt(changeset.changes[:password]) @@ -199,6 +206,20 @@ def register_changeset(struct, params \\ %{}) do end end + @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, user} + 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/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 4d73cf219..683310168 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -1,6 +1,6 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do use Pleroma.Web, :controller - alias Pleroma.{User, Repo} + alias Pleroma.User alias Pleroma.Web.ActivityPub.Relay import Pleroma.Web.ControllerHelper, only: [json_response: 3] @@ -26,7 +26,7 @@ def user_create( conn, %{"nickname" => nickname, "email" => email, "password" => password} ) do - new_user = %{ + user_data = %{ nickname: nickname, name: nickname, email: email, @@ -35,11 +35,11 @@ def user_create( bio: "." } - User.register_changeset(%User{}, new_user) - |> Repo.insert!() + changeset = User.register_changeset(%User{}, user_data, confirmed: true) + {:ok, user} = User.register(changeset) conn - |> json(new_user.nickname) + |> json(user.nickname) end def tag_users(conn, %{"nicknames" => nicknames, "tags" => tags}) do diff --git a/lib/pleroma/web/twitter_api/twitter_api.ex b/lib/pleroma/web/twitter_api/twitter_api.ex index b77761aa4..d8dd7dfa8 100644 --- a/lib/pleroma/web/twitter_api/twitter_api.ex +++ b/lib/pleroma/web/twitter_api/twitter_api.ex @@ -161,34 +161,19 @@ def register_user(params) do Repo.get_by(UserInviteToken, %{token: tokenString}) end - cond do - registrations_open || (!is_nil(token) && !token.used) -> - changeset = User.register_changeset(%User{info: %{}}, params) + cond do + registrations_open || (!is_nil(token) && !token.used) -> + changeset = User.register_changeset(%User{}, params) - with {:ok, user} <- Repo.insert(changeset) do - !registrations_open && UserInviteToken.mark_as_used(token.token) + with {:ok, user} <- User.register(changeset) do + !registrations_open && UserInviteToken.mark_as_used(token.token) - if Pleroma.Config.get([:instance, :account_activation_required]) do - info_change = User.Info.confirmation_update(user.info, :unconfirmed) - - {:ok, unconfirmed_user} = - user - |> Ecto.Changeset.change() - |> Ecto.Changeset.put_embed(:info, info_change) - |> Repo.update() - - {:ok, _} = - unconfirmed_user - |> UserEmail.account_confirmation_email() - |> Mailer.deliver() - end - - {:ok, user} - else - {:error, changeset} -> - errors = - Ecto.Changeset.traverse_errors(changeset, fn {msg, _opts} -> msg end) - |> Jason.encode!() + {:ok, user} + else + {:error, changeset} -> + errors = + Ecto.Changeset.traverse_errors(changeset, fn {msg, _opts} -> msg end) + |> Jason.encode!() {:error, %{error: errors}} end From aed0f902871524ecc1db0d8c088ce5939e7c685a Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 18 Dec 2018 14:07:05 +0300 Subject: [PATCH 04/17] [#114] Added `pleroma.confirmation_pending` to user views, adjusted view tests. --- .../web/mastodon_api/views/account_view.ex | 1 + .../web/twitter_api/views/user_view.ex | 1 + test/web/mastodon_api/account_view_test.exs | 10 ++++++++-- test/web/twitter_api/views/user_view_test.exs | 20 +++++++++++++++---- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/views/account_view.ex b/lib/pleroma/web/mastodon_api/views/account_view.ex index ebcf9230b..50df88aca 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -62,6 +62,7 @@ def render("account.json", %{user: user} = opts) do # Pleroma extension pleroma: %{ + confirmation_pending: user_info.confirmation_pending, tags: user.tags } } diff --git a/lib/pleroma/web/twitter_api/views/user_view.ex b/lib/pleroma/web/twitter_api/views/user_view.ex index 8a88d72b1..45b893eda 100644 --- a/lib/pleroma/web/twitter_api/views/user_view.ex +++ b/lib/pleroma/web/twitter_api/views/user_view.ex @@ -81,6 +81,7 @@ def render("user.json", %{user: user = %User{}} = assigns) do # Pleroma extension "pleroma" => %{ + "confirmation_pending" => user_info.confirmation_pending, "tags" => user.tags } } diff --git a/test/web/mastodon_api/account_view_test.exs b/test/web/mastodon_api/account_view_test.exs index 3cb9b9c5b..fec97c700 100644 --- a/test/web/mastodon_api/account_view_test.exs +++ b/test/web/mastodon_api/account_view_test.exs @@ -55,7 +55,10 @@ test "Represent a user account" do privacy: "public", sensitive: false }, - pleroma: %{tags: []} + pleroma: %{ + confirmation_pending: false, + tags: [] + } } assert expected == AccountView.render("account.json", %{user: user}) @@ -93,7 +96,10 @@ test "Represent a Service(bot) account" do privacy: "public", sensitive: false }, - pleroma: %{tags: []} + pleroma: %{ + confirmation_pending: false, + tags: [] + } } assert expected == AccountView.render("account.json", %{user: user}) diff --git a/test/web/twitter_api/views/user_view_test.exs b/test/web/twitter_api/views/user_view_test.exs index 34e6d4e27..0adc69ff9 100644 --- a/test/web/twitter_api/views/user_view_test.exs +++ b/test/web/twitter_api/views/user_view_test.exs @@ -96,7 +96,10 @@ test "A user" do "default_scope" => "public", "no_rich_text" => false, "fields" => [], - "pleroma" => %{"tags" => []} + "pleroma" => %{ + "confirmation_pending" => false, + "tags" => [] + } } assert represented == UserView.render("show.json", %{user: user}) @@ -138,7 +141,10 @@ test "A user for a given other follower", %{user: user} do "default_scope" => "public", "no_rich_text" => false, "fields" => [], - "pleroma" => %{"tags" => []} + "pleroma" => %{ + "confirmation_pending" => false, + "tags" => [] + } } assert represented == UserView.render("show.json", %{user: user, for: follower}) @@ -181,7 +187,10 @@ test "A user that follows you", %{user: user} do "default_scope" => "public", "no_rich_text" => false, "fields" => [], - "pleroma" => %{"tags" => []} + "pleroma" => %{ + "confirmation_pending" => false, + "tags" => [] + } } assert represented == UserView.render("show.json", %{user: follower, for: user}) @@ -231,7 +240,10 @@ test "A blocked user for the blocker" do "default_scope" => "public", "no_rich_text" => false, "fields" => [], - "pleroma" => %{"tags" => []} + "pleroma" => %{ + "confirmation_pending" => false, + "tags" => [] + } } blocker = Repo.get(User, blocker.id) From b096e30cffc79a4adf12be88da412a290cd0d190 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 18 Dec 2018 17:13:52 +0300 Subject: [PATCH 05/17] [#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", From 3371a45884b5d80ff4e8178ea921fe6f6c8d174d Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 18 Dec 2018 17:30:30 +0300 Subject: [PATCH 06/17] [#114] Formatting fix. --- lib/pleroma/web/twitter_api/twitter_api.ex | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/pleroma/web/twitter_api/twitter_api.ex b/lib/pleroma/web/twitter_api/twitter_api.ex index d8dd7dfa8..d816dc3bc 100644 --- a/lib/pleroma/web/twitter_api/twitter_api.ex +++ b/lib/pleroma/web/twitter_api/twitter_api.ex @@ -161,19 +161,19 @@ def register_user(params) do Repo.get_by(UserInviteToken, %{token: tokenString}) end - cond do - registrations_open || (!is_nil(token) && !token.used) -> - changeset = User.register_changeset(%User{}, params) + cond do + registrations_open || (!is_nil(token) && !token.used) -> + changeset = User.register_changeset(%User{}, params) - with {:ok, user} <- User.register(changeset) do - !registrations_open && UserInviteToken.mark_as_used(token.token) + with {:ok, user} <- User.register(changeset) do + !registrations_open && UserInviteToken.mark_as_used(token.token) - {:ok, user} - else - {:error, changeset} -> - errors = - Ecto.Changeset.traverse_errors(changeset, fn {msg, _opts} -> msg end) - |> Jason.encode!() + {:ok, user} + else + {:error, changeset} -> + errors = + Ecto.Changeset.traverse_errors(changeset, fn {msg, _opts} -> msg end) + |> Jason.encode!() {:error, %{error: errors}} end From 468d472d2b703c3f2ffa691c7afc65cae31ee25a Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 18 Dec 2018 17:52:38 +0300 Subject: [PATCH 07/17] [#114] Added info on `account_activation_required` setting to config readme. --- config/config.md | 1 + 1 file changed, 1 insertion(+) diff --git a/config/config.md b/config/config.md index edabd6e0f..8f3f308df 100644 --- a/config/config.md +++ b/config/config.md @@ -69,6 +69,7 @@ config :pleroma, Pleroma.Mailer, * `banner_upload_limit`: File size limit of user’s profile banners * `registrations_open`: Enable registrations for anyone, invitations can be enabled when false. * `invites_enabled`: Enable user invitations for admins (depends on `registrations_open: false`). +* `account_activation_required`: Require users to confirm their emails before signing in. * `federating`: Enable federation with other instances * `allow_relay`: Enable Pleroma’s Relay, which makes it possible to follow a whole instance * `rewrite_policy`: Message Rewrite Policy, either one or a list. Here are the ones available by default: From 59fc5d15dfde0120fb10ec14631ad1115a6087a9 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 19 Dec 2018 16:27:16 +0300 Subject: [PATCH 08/17] [#114] User.Info: renamed `confirmation_update` to `confirmation_change`. --- lib/pleroma/user.ex | 2 +- lib/pleroma/user/info.ex | 10 +++++----- lib/pleroma/web/twitter_api/twitter_api_controller.ex | 2 +- test/web/oauth/oauth_controller_test.exs | 2 +- test/web/twitter_api/twitter_api_controller_test.exs | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 0cd7bc463..41d01cbf5 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -191,7 +191,7 @@ def register_changeset(struct, params \\ %{}, opts \\ []) do |> validate_format(:email, @email_regex) |> validate_length(:bio, max: 1000) |> validate_length(:name, min: 1, max: 100) - |> put_change(:info, User.Info.confirmation_update(%User.Info{}, confirmation_status)) + |> put_change(:info, User.Info.confirmation_change(%User.Info{}, confirmation_status)) if changeset.valid? do hashed = Pbkdf2.hashpwsalt(changeset.changes[:password]) diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index 9ce9129cd..2800e9cff 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -143,21 +143,21 @@ def profile_update(info, params) do ]) end - def confirmation_update(info, :confirmed) do - confirmation_update(info, %{ + def confirmation_change(info, :confirmed) do + confirmation_change(info, %{ confirmation_pending: false, confirmation_token: nil }) end - def confirmation_update(info, :unconfirmed) do - confirmation_update(info, %{ + def confirmation_change(info, :unconfirmed) do + confirmation_change(info, %{ confirmation_pending: true, confirmation_token: :crypto.strong_rand_bytes(32) |> Base.url_encode64() }) end - def confirmation_update(info, params) do + def confirmation_change(info, params) do cast(info, params, [:confirmation_pending, :confirmation_token]) end diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index 7286c153b..4bd729b77 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -376,7 +376,7 @@ def password_reset(conn, params) do def confirm_email(conn, %{"token" => token}) do with %User{} = user <- User.get_by_confirmation_token(token), true <- user.local, - info_change <- User.Info.confirmation_update(user.info, :confirmed), + info_change <- User.Info.confirmation_change(user.info, :confirmed), changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_change), {:ok, _} <- User.update_and_set_cache(changeset) do conn diff --git a/test/web/oauth/oauth_controller_test.exs b/test/web/oauth/oauth_controller_test.exs index 55b471d8a..26505bab7 100644 --- a/test/web/oauth/oauth_controller_test.exs +++ b/test/web/oauth/oauth_controller_test.exs @@ -116,7 +116,7 @@ test "rejects token exchange with invalid client credentials" do 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) + info_change = Pleroma.User.Info.confirmation_change(user.info, :unconfirmed) {:ok, user} = user diff --git a/test/web/twitter_api/twitter_api_controller_test.exs b/test/web/twitter_api/twitter_api_controller_test.exs index eb154608c..a269c9757 100644 --- a/test/web/twitter_api/twitter_api_controller_test.exs +++ b/test/web/twitter_api/twitter_api_controller_test.exs @@ -876,7 +876,7 @@ test "it returns 500 when user is not local", %{conn: conn, user: user} do describe "GET /api/account/confirm_email/:token" do setup do user = insert(:user) - info_change = User.Info.confirmation_update(user.info, :unconfirmed) + info_change = User.Info.confirmation_change(user.info, :unconfirmed) {:ok, user} = user @@ -908,7 +908,7 @@ test "it confirms the user account", %{conn: conn, user: user} do describe "POST /api/account/resend_confirmation_email" do setup do user = insert(:user) - info_change = User.Info.confirmation_update(user.info, :unconfirmed) + info_change = User.Info.confirmation_change(user.info, :unconfirmed) {:ok, user} = user From 968d7490b689ba501a64f350841dc8f9b33b5244 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 19 Dec 2018 16:27:16 +0300 Subject: [PATCH 09/17] [#114] User.Info: renamed `confirmation_update` to `confirmation_changeset`. --- lib/pleroma/user.ex | 2 +- lib/pleroma/user/info.ex | 10 +++++----- lib/pleroma/web/twitter_api/twitter_api_controller.ex | 2 +- test/web/oauth/oauth_controller_test.exs | 2 +- test/web/twitter_api/twitter_api_controller_test.exs | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 41d01cbf5..4e227b08d 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -191,7 +191,7 @@ def register_changeset(struct, params \\ %{}, opts \\ []) do |> validate_format(:email, @email_regex) |> validate_length(:bio, max: 1000) |> validate_length(:name, min: 1, max: 100) - |> put_change(:info, User.Info.confirmation_change(%User.Info{}, confirmation_status)) + |> put_change(:info, User.Info.confirmation_changeset(%User.Info{}, confirmation_status)) if changeset.valid? do hashed = Pbkdf2.hashpwsalt(changeset.changes[:password]) diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index 2800e9cff..ad9fe1bbe 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -143,21 +143,21 @@ def profile_update(info, params) do ]) end - def confirmation_change(info, :confirmed) do - confirmation_change(info, %{ + def confirmation_changeset(info, :confirmed) do + confirmation_changeset(info, %{ confirmation_pending: false, confirmation_token: nil }) end - def confirmation_change(info, :unconfirmed) do - confirmation_change(info, %{ + def confirmation_changeset(info, :unconfirmed) do + confirmation_changeset(info, %{ confirmation_pending: true, confirmation_token: :crypto.strong_rand_bytes(32) |> Base.url_encode64() }) end - def confirmation_change(info, params) do + def confirmation_changeset(info, params) do cast(info, params, [:confirmation_pending, :confirmation_token]) end diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index 4bd729b77..b362f3946 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -376,7 +376,7 @@ def password_reset(conn, params) do def confirm_email(conn, %{"token" => token}) do with %User{} = user <- User.get_by_confirmation_token(token), true <- user.local, - info_change <- User.Info.confirmation_change(user.info, :confirmed), + info_change <- User.Info.confirmation_changeset(user.info, :confirmed), changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_change), {:ok, _} <- User.update_and_set_cache(changeset) do conn diff --git a/test/web/oauth/oauth_controller_test.exs b/test/web/oauth/oauth_controller_test.exs index 26505bab7..0621a8acc 100644 --- a/test/web/oauth/oauth_controller_test.exs +++ b/test/web/oauth/oauth_controller_test.exs @@ -116,7 +116,7 @@ test "rejects token exchange with invalid client credentials" do 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_change(user.info, :unconfirmed) + info_change = Pleroma.User.Info.confirmation_changeset(user.info, :unconfirmed) {:ok, user} = user diff --git a/test/web/twitter_api/twitter_api_controller_test.exs b/test/web/twitter_api/twitter_api_controller_test.exs index a269c9757..53b390793 100644 --- a/test/web/twitter_api/twitter_api_controller_test.exs +++ b/test/web/twitter_api/twitter_api_controller_test.exs @@ -876,7 +876,7 @@ test "it returns 500 when user is not local", %{conn: conn, user: user} do describe "GET /api/account/confirm_email/:token" do setup do user = insert(:user) - info_change = User.Info.confirmation_change(user.info, :unconfirmed) + info_change = User.Info.confirmation_changeset(user.info, :unconfirmed) {:ok, user} = user @@ -908,7 +908,7 @@ test "it confirms the user account", %{conn: conn, user: user} do describe "POST /api/account/resend_confirmation_email" do setup do user = insert(:user) - info_change = User.Info.confirmation_change(user.info, :unconfirmed) + info_change = User.Info.confirmation_changeset(user.info, :unconfirmed) {:ok, user} = user From a532ad5d720cbbe3ef58e09f8ad209bfe15b43c9 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 19 Dec 2018 17:24:55 +0300 Subject: [PATCH 10/17] [#114] User.register/1 tweak. --- lib/pleroma/user.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 4e227b08d..4b8caf65c 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -210,8 +210,8 @@ 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 - {:ok, _} = try_send_confirmation_email(user) + with {:ok, user} <- Repo.insert(changeset), + {:ok, _} = try_send_confirmation_email(user) do {:ok, user} end end From 279096228c8b0113a8ea63a73e011934a3226df7 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 19 Dec 2018 18:56:52 +0300 Subject: [PATCH 11/17] [#114] Made MastodonAPI and TwitterAPI user show actions return 404 for auth-inactive users unless requested by admin or moderator. --- lib/pleroma/user.ex | 4 +++- lib/pleroma/user/info.ex | 2 ++ .../web/mastodon_api/mastodon_api_controller.ex | 3 ++- .../web/twitter_api/twitter_api_controller.ex | 14 +++++++++++--- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 4b8caf65c..7e792cb0c 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -38,7 +38,9 @@ defmodule Pleroma.User do timestamps() end - def auth_active?(user), do: user.info && !user.info.confirmation_pending + def auth_active?(%User{} = user), do: user.info && !user.info.confirmation_pending + + def superuser?(%User{} = user), do: user.info && User.Info.superuser?(user.info) def avatar_url(user) do case user.avatar do diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index ad9fe1bbe..3de4af56c 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -37,6 +37,8 @@ defmodule Pleroma.User.Info do # subject _> Where is this used? end + def superuser?(info), do: info.is_admin || info.is_moderator + def set_activation_status(info, deactivated) do params = %{deactivated: deactivated} diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index 665b75437..c6db89442 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -110,7 +110,8 @@ def verify_credentials(%{assigns: %{user: user}} = conn, _) do end def user(%{assigns: %{user: for_user}} = conn, %{"id" => id}) do - with %User{} = user <- Repo.get(User, id) do + with %User{} = user <- Repo.get(User, id), + true <- User.auth_active?(user) || user.id == for_user.id || User.superuser?(for_user) do account = AccountView.render("account.json", %{user: user, for: for_user}) json(conn, account) else diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index b362f3946..e047ed0ad 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -97,10 +97,13 @@ def friends_timeline(%{assigns: %{user: user}} = conn, params) do end def show_user(conn, params) do - with {:ok, shown} <- TwitterAPI.get_user(params) do + for_user = conn.assigns.user + + with {:ok, shown} <- TwitterAPI.get_user(params), + true <- User.auth_active?(shown) || for_user && (for_user.id == shown.id || User.superuser?(for_user)) do params = - if user = conn.assigns.user do - %{user: shown, for: user} + if for_user do + %{user: shown, for: for_user} else %{user: shown} end @@ -111,6 +114,11 @@ def show_user(conn, params) do else {:error, msg} -> bad_request_reply(conn, msg) + + false -> + conn + |> put_status(404) + |> json(%{error: "Unconfirmed user"}) end end From b520d44b58db3bac7a883c2220d83d05b07784d0 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 19 Dec 2018 19:03:39 +0300 Subject: [PATCH 12/17] [#114] `mix format` --- lib/pleroma/web/twitter_api/twitter_api_controller.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index e047ed0ad..2b0f7135a 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -100,7 +100,9 @@ def show_user(conn, params) do for_user = conn.assigns.user with {:ok, shown} <- TwitterAPI.get_user(params), - true <- User.auth_active?(shown) || for_user && (for_user.id == shown.id || User.superuser?(for_user)) do + true <- + User.auth_active?(shown) || + (for_user && (for_user.id == shown.id || User.superuser?(for_user))) do params = if for_user do %{user: shown, for: for_user} From 501ce34d7fbb74f91e8afcaa491625c5a0152597 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 20 Dec 2018 12:55:12 +0300 Subject: [PATCH 13/17] [#114] Stylistic adjustments. --- lib/pleroma/user.ex | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 7e792cb0c..7e3a342f1 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -76,15 +76,14 @@ 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, - confirmation_pending: user_info.confirmation_pending, - 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 @@ -182,6 +181,8 @@ def register_changeset(struct, params \\ %{}, opts \\ []) do :unconfirmed end + info_change = User.Info.confirmation_changeset(%User.Info{}, confirmation_status) + changeset = struct |> cast(params, [:bio, :email, :name, :nickname, :password, :password_confirmation]) @@ -193,7 +194,7 @@ def register_changeset(struct, params \\ %{}, opts \\ []) do |> validate_format(:email, @email_regex) |> validate_length(:bio, max: 1000) |> validate_length(:name, min: 1, max: 100) - |> put_change(:info, User.Info.confirmation_changeset(%User.Info{}, confirmation_status)) + |> put_change(:info, info_change) if changeset.valid? do hashed = Pbkdf2.hashpwsalt(changeset.changes[:password]) From 8adcd1e80f78cdacd245e9b6aacea4b05cb1a880 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 20 Dec 2018 13:05:42 +0300 Subject: [PATCH 14/17] [#114] Removed flash messages rendering on redirects. --- lib/pleroma/web/twitter_api/twitter_api_controller.ex | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index 2b0f7135a..46dc9b12c 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -13,7 +13,6 @@ 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, :resend_confirmation_email]) action_fallback(:errors) def verify_credentials(%{assigns: %{user: user}} = conn, _params) do @@ -390,7 +389,6 @@ def confirm_email(conn, %{"token" => token}) do changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_change), {:ok, _} <- User.update_and_set_cache(changeset) do conn - |> put_flash(:info, "Email confirmed. Please sign in.") |> redirect(to: "/") end end @@ -401,7 +399,6 @@ def resend_confirmation_email(conn, params) do 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 From f69cbf4755b974de0303731327180bb51ed244fc Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 20 Dec 2018 13:41:30 +0300 Subject: [PATCH 15/17] [#114] Added :user_id component to email confirmation path to improve the security. Added tests for `confirm_email` action. --- lib/pleroma/emails/user_email.ex | 1 + lib/pleroma/user.ex | 4 ---- lib/pleroma/web/router.ex | 7 ++++++- .../web/twitter_api/twitter_api_controller.ex | 6 ++++-- .../twitter_api_controller_test.exs | 18 +++++++++++++++--- 5 files changed, 26 insertions(+), 10 deletions(-) diff --git a/lib/pleroma/emails/user_email.ex b/lib/pleroma/emails/user_email.ex index 856816386..8f916f470 100644 --- a/lib/pleroma/emails/user_email.ex +++ b/lib/pleroma/emails/user_email.ex @@ -70,6 +70,7 @@ def account_confirmation_email(user) do Router.Helpers.confirm_email_url( Endpoint, :confirm_email, + user.id, to_string(user.info.confirmation_token) ) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 7e3a342f1..ad50cf558 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -396,10 +396,6 @@ def get_or_fetch_by_nickname(nickname) do end end - def get_by_confirmation_token(token) do - Repo.one(from(u in User, where: fragment("? ->> 'confirmation_token' = ?", u.info, ^token))) - end - def get_followers_query(%User{id: id, follower_address: follower_address}) do from( u in User, diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index ca069ab99..d1c3b34f6 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -282,7 +282,12 @@ defmodule Pleroma.Web.Router do post("/account/register", TwitterAPI.Controller, :register) post("/account/password_reset", TwitterAPI.Controller, :password_reset) - get("/account/confirm_email/:token", TwitterAPI.Controller, :confirm_email, as: :confirm_email) + get( + "/account/confirm_email/:user_id/:token", + TwitterAPI.Controller, + :confirm_email, + as: :confirm_email + ) post("/account/resend_confirmation_email", TwitterAPI.Controller, :resend_confirmation_email) diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index 46dc9b12c..c644681b0 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -382,9 +382,11 @@ def password_reset(conn, params) do end end - def confirm_email(conn, %{"token" => token}) do - with %User{} = user <- User.get_by_confirmation_token(token), + def confirm_email(conn, %{"user_id" => uid, "token" => token}) do + with %User{} = user <- Repo.get(User, uid), true <- user.local, + true <- user.info.confirmation_pending, + true <- user.info.confirmation_token == token, info_change <- User.Info.confirmation_changeset(user.info, :confirmed), changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_change), {:ok, _} <- User.update_and_set_cache(changeset) do diff --git a/test/web/twitter_api/twitter_api_controller_test.exs b/test/web/twitter_api/twitter_api_controller_test.exs index 53b390793..16422c35a 100644 --- a/test/web/twitter_api/twitter_api_controller_test.exs +++ b/test/web/twitter_api/twitter_api_controller_test.exs @@ -873,7 +873,7 @@ test "it returns 500 when user is not local", %{conn: conn, user: user} do end end - describe "GET /api/account/confirm_email/:token" do + describe "GET /api/account/confirm_email/:id/:token" do setup do user = insert(:user) info_change = User.Info.confirmation_changeset(user.info, :unconfirmed) @@ -890,19 +890,31 @@ test "it returns 500 when user is not local", %{conn: conn, user: user} do end test "it redirects to root url", %{conn: conn, user: user} do - conn = get(conn, "/api/account/confirm_email/#{user.info.confirmation_token}") + conn = get(conn, "/api/account/confirm_email/#{user.id}/#{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}") + get(conn, "/api/account/confirm_email/#{user.id}/#{user.info.confirmation_token}") user = Repo.get(User, user.id) refute user.info.confirmation_pending refute user.info.confirmation_token end + + test "it returns 500 if user cannot be found by id", %{conn: conn, user: user} do + conn = get(conn, "/api/account/confirm_email/0/#{user.info.confirmation_token}") + + assert 500 == conn.status + end + + test "it returns 500 if token is invalid", %{conn: conn, user: user} do + conn = get(conn, "/api/account/confirm_email/#{user.id}/wrong_token") + + assert 500 == conn.status + end end describe "POST /api/account/resend_confirmation_email" do From 7cab7de9ff0432a582cfca0852a4b66fdd124c41 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 20 Dec 2018 14:48:48 +0300 Subject: [PATCH 16/17] [#114] Allowed unconfirmed users to authenticate if :account_activation_required is disabled prior to confirmation. Ensured that no confirmation emails are sent if :account_activation_required is not true. Adjusted tests. --- lib/pleroma/user.ex | 8 ++++++-- test/web/twitter_api/twitter_api_controller_test.exs | 7 +++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index ad50cf558..f8827abec 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -38,7 +38,10 @@ defmodule Pleroma.User do timestamps() end - def auth_active?(%User{} = user), do: user.info && !user.info.confirmation_pending + def auth_active?(%User{} = user) do + (user.info && !user.info.confirmation_pending) || + !Pleroma.Config.get([:instance, :account_activation_required]) + end def superuser?(%User{} = user), do: user.info && User.Info.superuser?(user.info) @@ -220,7 +223,8 @@ def register(%Ecto.Changeset{} = changeset) do end def try_send_confirmation_email(%User{} = user) do - if user.info.confirmation_pending do + if user.info.confirmation_pending && + Pleroma.Config.get([:instance, :account_activation_required]) do user |> Pleroma.UserEmail.account_confirmation_email() |> Pleroma.Mailer.deliver() diff --git a/test/web/twitter_api/twitter_api_controller_test.exs b/test/web/twitter_api/twitter_api_controller_test.exs index 16422c35a..1324bcc71 100644 --- a/test/web/twitter_api/twitter_api_controller_test.exs +++ b/test/web/twitter_api/twitter_api_controller_test.exs @@ -919,6 +919,13 @@ test "it returns 500 if token is invalid", %{conn: conn, user: user} do describe "POST /api/account/resend_confirmation_email" do 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 + user = insert(:user) info_change = User.Info.confirmation_changeset(user.info, :unconfirmed) From 851db74f1ca533f27f72f1341571948b15d2f561 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 20 Dec 2018 15:23:16 +0300 Subject: [PATCH 17/17] [#114] Fixed test. --- test/web/oauth/oauth_controller_test.exs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/web/oauth/oauth_controller_test.exs b/test/web/oauth/oauth_controller_test.exs index 0621a8acc..52441407d 100644 --- a/test/web/oauth/oauth_controller_test.exs +++ b/test/web/oauth/oauth_controller_test.exs @@ -113,7 +113,14 @@ 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 + test "rejects token exchange for valid credentials belonging to unconfirmed user and confirmation is required" 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 + password = "testpassword" user = insert(:user, password_hash: Comeonin.Pbkdf2.hashpwsalt(password)) info_change = Pleroma.User.Info.confirmation_changeset(user.info, :unconfirmed)