Prevent information leakage on registration when using PowEmailConfirmation

CHANGELOG.md

@@ -8,6 +8,7 @@
 * [`PowResetPassword.Phoenix.ResetPasswordController`] Now uses `PowResetPassword.Phoenix.Messages.maybe_email_has_been_sent/1` with a generic response that tells the user the email has been sent only if an account was found
 * [`PowResetPassword.Phoenix.ResetPasswordController`] When a user doesn't exist will now return success message if `PowEmailConfirmation` extension is enabled
 * [`PowResetPassword.Phoenix.Messages`] Added `PowResetPassword.Phoenix.Messages.maybe_email_has_been_sent/1` and let `PowResetPassword.Phoenix.Messages.email_has_been_sent/1` fall back to it
+* [`PowEmailConfirmation.Phoenix.ControllerCallbacks`] When a user tries to sign up and the email has already been taken the default e-mail confirmation required message will be shown
 
 ### Bug fixes
 

lib/extensions/email_confirmation/README.md

@@ -4,6 +4,8 @@ This extension will send an e-mail confirmation link when the user registers, an
 
 Users won't be signed in when they register, and can't sign in until the e-mail has been confirmed. The confirmation e-mail will be sent every time the sign in fails. The user will be redirected to `after_registration_path/1` and `after_sign_in_path/1` accordingly.
 
+To prevent information leak, the user will see the same confirmation required message with no e-mail sent if a user attempts to register with an already taken email.
+
 When users updates their e-mail, it won't be changed until the user has confirmed the new e-mail by clicking the e-mail confirmation link.
 
 ## Installation

lib/extensions/email_confirmation/phoenix/controllers/controller_callbacks.ex

@@ -12,6 +12,15 @@ defmodule PowEmailConfirmation.Phoenix.ControllerCallbacks do
   user redirected back to `Pow.Phoenix.Routes.after_registration_path/1` or
   `Pow.Phoenix.Routes.after_registration_path/1` respectively.
 
+  ### E-mail already taken during registration
+
+  Triggers on `Pow.Phoenix.RegistrationController.create/2`.
+
+  When an error is returned during registration, and the changeset has an error
+  for the `:email` field with the message `has already been taken`, the user
+  will see the same success flow as above, but without any e-mail sent. This
+  will prevent information leak.
+
   ### User updates e-mail
 
   Triggers on `Pow.Phoenix.RegistrationController.update/2` and
@@ -40,6 +49,18 @@ defmodule PowEmailConfirmation.Phoenix.ControllerCallbacks do
 
     halt_unconfirmed(conn, {:ok, user, conn}, return_path)
   end
+  def before_respond(Pow.Phoenix.RegistrationController, :create, {:error, changeset, conn}, _config) do
+    case prevent_information_leak?(conn, changeset) do
+      true ->
+        return_path = routes(conn).after_registration_path(conn)
+        conn        = redirect_and_show_error(conn, return_path)
+
+        {:halt, conn}
+
+      false ->
+        {:error, changeset, conn}
+    end
+  end
   def before_respond(Pow.Phoenix.SessionController, :create, {:ok, conn}, _config) do
     return_path = routes(conn).after_sign_in_path(conn)
 
@@ -60,18 +81,30 @@ defmodule PowEmailConfirmation.Phoenix.ControllerCallbacks do
   defp halt_and_send_confirmation_email(conn, return_path) do
     user        = Plug.current_user(conn)
     {:ok, conn} = Plug.clear_authenticated_user(conn)
-    error       = extension_messages(conn).email_confirmation_required(conn)
 
     send_confirmation_email(user, conn)
 
-    conn =
-      conn
-      |> Phoenix.Controller.put_flash(:error, error)
-      |> Phoenix.Controller.redirect(to: return_path)
+    conn = redirect_and_show_error(conn, return_path)
 
     {:halt, conn}
   end
 
+  defp redirect_and_show_error(conn, return_path) do
+    error = extension_messages(conn).email_confirmation_required(conn)
+
+    conn
+    |> Phoenix.Controller.put_flash(:error, error)
+    |> Phoenix.Controller.redirect(to: return_path)
+  end
+
+  defp prevent_information_leak?(_conn, %{errors: errors}) do
+    Enum.find_value(errors, false, fn
+      {:email, {"has already been taken", _keys}} -> true
+      _any                                        -> false
+    end)
+  end
+  defp prevent_information_leak?(_conn, _changeset), do: false
+
   defp warn_unconfirmed(%{params: %{"user" => %{"email" => email}}} = conn, %{unconfirmed_email: email} = user) do
     case PowEmailConfirmationPlug.pending_email_change?(conn) do
       true  -> warn_and_send_confirmation_email(conn)

test/extensions/email_confirmation/phoenix/controllers/controller_callbacks_test.exs

@@ -57,6 +57,27 @@ defmodule PowEmailConfirmation.Phoenix.ControllerCallbacksTest do
       assert mail.html =~ "<a href=\"http://localhost/confirm-email/#{token}\">"
       assert mail.user.email == "test@example.com"
     end
+
+    @email_taken_invalid_params %{"user" => %{"email" => "taken@example.com", "password" => @password, "confirm_password" => "s"}}
+    test "with invalid params and email taken", %{conn: conn} do
+      conn = post conn, Routes.pow_registration_path(conn, :create, @email_taken_invalid_params)
+
+      assert html = html_response(conn, 200)
+      refute html =~ "<span class=\"help-block\">has already been taken</span>"
+      assert html =~ "<span class=\"help-block\">not same as password</span>"
+    end
+
+    @email_taken_valid_params %{"user" => %{"email" => "taken@example.com", "password" => @password, "confirm_password" => @password}}
+    test "with valid params and email taken", %{conn: conn} do
+      conn = post conn, Routes.pow_registration_path(conn, :create, @email_taken_valid_params)
+
+      assert get_flash(conn, :error) == "You'll need to confirm your e-mail before you can sign in. An e-mail confirmation link has been sent to you."
+      assert redirected_to(conn) == "/after_registration"
+
+      refute Plug.current_user(conn)
+
+      refute_received {:mail_mock, _mail}
+    end
   end
 
   describe "Pow.Phoenix.RegistrationController.update/2" do

test/support/extensions/email_confirmation/repo_mock.ex

@@ -32,7 +32,7 @@ defmodule PowEmailConfirmation.Test.RepoMock do
     %{user() | unconfirmed_email: "new@example.com", email_confirmed_at: DateTime.utc_now()}
   end
 
-  def update(%{changes: %{email: "taken@example.com"}} = changeset, _opts) do
+  def update(%{changes: %{email: "taken@example.com"}, valid?: true} = changeset, _opts) do
     changeset = Ecto.Changeset.add_error(changeset, :email, "has already been taken")
 
     {:error, changeset}
@@ -57,6 +57,12 @@ defmodule PowEmailConfirmation.Test.RepoMock do
     |> Enum.reduce(changeset, & &1.(&2))
   end
 
+  def insert(%{valid?: false} = changeset, _opts), do: {:error, %{changeset | action: :insert}}
+  def insert(%{changes: %{email: "taken@example.com"}, valid?: true} = changeset, _opts) do
+    changeset = Ecto.Changeset.add_error(changeset, :email, "has already been taken")
+
+    {:error, %{changeset | action: :insert}}
+  end
   def insert(%{valid?: true} = changeset, _opts) do
     user =
       changeset