Repository: phoenixframework/phoenix
Pull request #4419 (2021-08-21 07:03:04)

Two step confirmation
Instead of confirming when the link in the email is clicked,
we explicitly require the user to press a button, that is to
avoid crawlers from automatically visiting emails and confirming
accounts.


Patch:
diff --git a/lib/mix/tasks/phx.gen.auth.ex b/lib/mix/tasks/phx.gen.auth.ex
index 18bf8ee7c..185363040 100644
--- a/lib/mix/tasks/phx.gen.auth.ex
+++ b/lib/mix/tasks/phx.gen.auth.ex
@@ -226,6 +226,7 @@ defmodule Mix.Tasks.Phx.Gen.Auth do
       {:eex, "auth_test.exs", Path.join([web_test_prefix, "controllers", web_path, "#{schema.singular}_auth_test.exs"])},
       {:eex, "confirmation_view.ex", Path.join([web_prefix, "views", web_path, "#{schema.singular}_confirmation_view.ex"])},
       {:eex, "confirmation_new.html.eex", Path.join([web_prefix, "templates", web_path, "#{schema.singular}_confirmation", "new.html.eex"])},
+      {:eex, "confirmation_edit.html.eex", Path.join([web_prefix, "templates", web_path, "#{schema.singular}_confirmation", "edit.html.eex"])},
       {:eex, "confirmation_controller.ex", Path.join([web_prefix, "controllers", web_path, "#{schema.singular}_confirmation_controller.ex"])},
       {:eex, "confirmation_controller_test.exs", Path.join([web_test_prefix, "controllers", web_path, "#{schema.singular}_confirmation_controller_test.exs"])},
       {:eex, "_menu.html.eex", Path.join([web_prefix, "templates", "layout", "_#{schema.singular}_menu.html.eex"])},
@@ -431,6 +432,12 @@ defmodule Mix.Tasks.Phx.Gen.Auth do
       menu_name = Injector.app_layout_menu_template_name(schema)
       inject = Injector.app_layout_menu_code_to_inject(schema)
 
+      missing =
+        context
+        |> potential_layout_file_paths()
+        |> Enum.map(&"  * #{&1}")
+        |> Enum.join("\n")
+
       Mix.shell().error("""
 
       Unable to find an application layout file to inject a render
@@ -438,12 +445,7 @@ defmodule Mix.Tasks.Phx.Gen.Auth do
 
       Missing files:
 
-      #{
-        context
-        |> potential_layout_file_paths()
-        |> Enum.map(&"  * #{&1}")
-        |> Enum.join("\n")
-      }
+      #{missing}
 
       Please ensure this phoenix app was not generated with
       --no-html. If you have changed the name of your application
@@ -511,14 +513,15 @@ defmodule Mix.Tasks.Phx.Gen.Auth do
 
     Please re-fetch your dependencies with the following command:
 
-        mix deps.get
-    """)
-
-    Mix.shell().info("""
+        $ mix deps.get
 
     Remember to update your repository by running migrations:
 
-      $ mix ecto.migrate
+        $ mix ecto.migrate
+
+    Once you are ready, visit "/#{context.schema.plural}/register"
+    to create your account and then access to "/dev/mailbox" to
+    see the account confirmation email.
     """)
 
     context
diff --git a/priv/templates/phx.gen.auth/confirmation_controller.ex b/priv/templates/phx.gen.auth/confirmation_controller.ex
index e7d00ff3e..191ceaefd 100644
--- a/priv/templates/phx.gen.auth/confirmation_controller.ex
+++ b/priv/templates/phx.gen.auth/confirmation_controller.ex
@@ -11,7 +11,7 @@ defmodule <%= inspect context.web_module %>.<%= inspect Module.concat(schema.web
     if <%= schema.singular %> = <%= inspect context.alias %>.get_<%= schema.singular %>_by_email(email) do
       <%= inspect context.alias %>.deliver_<%= schema.singular %>_confirmation_instructions(
         <%= schema.singular %>,
-        &Routes.<%= schema.route_helper %>_confirmation_url(conn, :confirm, &1)
+        &Routes.<%= schema.route_helper %>_confirmation_url(conn, :edit, &1)
       )
     end
 
@@ -25,9 +25,13 @@ defmodule <%= inspect context.web_module %>.<%= inspect Module.concat(schema.web
     |> redirect(to: "/")
   end
 
+  def edit(conn, %{"token" => token}) do
+    render(conn, "edit.html", token: token)
+  end
+
   # Do not log in the <%= schema.singular %> after confirmation to avoid a
   # leaked token giving the <%= schema.singular %> access to the account.
-  def confirm(conn, %{"token" => token}) do
+  def update(conn, %{"token" => token}) do
     case <%= inspect context.alias %>.confirm_<%= schema.singular %>(token) do
       {:ok, _} ->
         conn
diff --git a/priv/templates/phx.gen.auth/confirmation_edit.html.eex b/priv/templates/phx.gen.auth/confirmation_edit.html.eex
new file mode 100644
index 000000000..47da0ad1c
--- /dev/null
+++ b/priv/templates/phx.gen.auth/confirmation_edit.html.eex
@@ -0,0 +1,12 @@
+<h1>Confirm account</h1>
+
+<%%= form_for :<%= schema.singular %>, Routes.<%= schema.route_helper %>_confirmation_path(@conn, :update, @token), fn _f -> %>
+  <div>
+    <%%= submit "Confirm my account" %>
+  </div>
+<%% end %>
+
+<p>
+  <%%= link "Register", to: Routes.<%= schema.route_helper %>_registration_path(@conn, :new) %> |
+  <%%= link "Log in", to: Routes.<%= schema.route_helper %>_session_path(@conn, :new) %>
+</p>
diff --git a/priv/templates/phx.gen.auth/context_functions.ex b/priv/templates/phx.gen.auth/context_functions.ex
index 6478dbcf9..c8d0afe6d 100644
--- a/priv/templates/phx.gen.auth/context_functions.ex
+++ b/priv/templates/phx.gen.auth/context_functions.ex
@@ -238,10 +238,10 @@
 
   ## Examples
 
-      iex> deliver_<%= schema.singular %>_confirmation_instructions(<%= schema.singular %>, &Routes.<%= schema.singular %>_confirmation_url(conn, :confirm, &1))
+      iex> deliver_<%= schema.singular %>_confirmation_instructions(<%= schema.singular %>, &Routes.<%= schema.singular %>_confirmation_url(conn, :edit, &1))
       {:ok, %{to: ..., body: ...}}
 
-      iex> deliver_<%= schema.singular %>_confirmation_instructions(confirmed_<%= schema.singular %>, &Routes.<%= schema.singular %>_confirmation_url(conn, :confirm, &1))
+      iex> deliver_<%= schema.singular %>_confirmation_instructions(confirmed_<%= schema.singular %>, &Routes.<%= schema.singular %>_confirmation_url(conn, :edit, &1))
       {:error, :already_confirmed}
 
   """
diff --git a/priv/templates/phx.gen.auth/registration_controller.ex b/priv/templates/phx.gen.auth/registration_controller.ex
index 61021493f..1ff67138f 100644
--- a/priv/templates/phx.gen.auth/registration_controller.ex
+++ b/priv/templates/phx.gen.auth/registration_controller.ex
@@ -16,7 +16,7 @@ defmodule <%= inspect context.web_module %>.<%= inspect Module.concat(schema.web
         {:ok, _} =
           <%= inspect context.alias %>.deliver_<%= schema.singular %>_confirmation_instructions(
             <%= schema.singular %>,
-            &Routes.<%= schema.route_helper %>_confirmation_url(conn, :confirm, &1)
+            &Routes.<%= schema.route_helper %>_confirmation_url(conn, :edit, &1)
           )
 
         conn
diff --git a/priv/templates/phx.gen.auth/routes.ex b/priv/templates/phx.gen.auth/routes.ex
index 3c205d3d9..45bfda743 100644
--- a/priv/templates/phx.gen.auth/routes.ex
+++ b/priv/templates/phx.gen.auth/routes.ex
@@ -28,5 +28,6 @@
     delete "/<%= schema.plural %>/log_out", <%= inspect schema.alias %>SessionController, :delete
     get "/<%= schema.plural %>/confirm", <%= inspect schema.alias %>ConfirmationController, :new
     post "/<%= schema.plural %>/confirm", <%= inspect schema.alias %>ConfirmationController, :create
-    get "/<%= schema.plural %>/confirm/:token", <%= inspect schema.alias %>ConfirmationController, :confirm
+    get "/<%= schema.plural %>/confirm/:token", <%= inspect schema.alias %>ConfirmationController, :edit
+    post "/<%= schema.plural %>/confirm/:token", <%= inspect schema.alias %>ConfirmationController, :update
   end


Test patch:
diff --git a/priv/templates/phx.gen.auth/confirmation_controller_test.exs b/priv/templates/phx.gen.auth/confirmation_controller_test.exs
index c9809ed00..4faa9fde1 100644
--- a/priv/templates/phx.gen.auth/confirmation_controller_test.exs
+++ b/priv/templates/phx.gen.auth/confirmation_controller_test.exs
@@ -10,7 +10,7 @@ defmodule <%= inspect context.web_module %>.<%= inspect Module.concat(schema.web
   end
 
   describe "GET /<%= schema.singular %>s/confirm" do
-    test "renders the confirmation page", %{conn: conn} do
+    test "renders the resend confirmation page", %{conn: conn} do
       conn = get(conn, Routes.<%= schema.route_helper %>_confirmation_path(conn, :new))
       response = html_response(conn, 200)
       assert response =~ "<h1>Resend confirmation instructions</h1>"
@@ -55,14 +55,25 @@ defmodule <%= inspect context.web_module %>.<%= inspect Module.concat(schema.web
     end
   end
 
-  describe "GET /<%= schema.singular %>s/confirm/:token" do
+  describe "GET /users/confirm/:token" do
+    test "renders the confirmation page", %{conn: conn} do
+      conn = get(conn, Routes.<%= schema.route_helper %>_confirmation_path(conn, :edit, "some-token"))
+      response = html_response(conn, 200)
+      assert response =~ "<h1>Confirm account</h1>"
+
+      form_action = Routes.<%= schema.route_helper %>_confirmation_path(conn, :update, "some-token")
+      assert response =~ "action=\"#{form_action}\""
+    end
+  end
+
+  describe "POST /<%= schema.singular %>s/confirm/:token" do
     test "confirms the given token once", %{conn: conn, <%= schema.singular %>: <%= schema.singular %>} do
       token =
         extract_<%= schema.singular %>_token(fn url ->
           <%= inspect context.alias %>.deliver_<%= schema.singular %>_confirmation_instructions(<%= schema.singular %>, url)
         end)
 
-      conn = get(conn, Routes.<%= schema.route_helper %>_confirmation_path(conn, :confirm, token))
+      conn = post(conn, Routes.<%= schema.route_helper %>_confirmation_path(conn, :update, token))
       assert redirected_to(conn) == "/"
       assert get_flash(conn, :info) =~ "<%= schema.human_singular %> confirmed successfully"
       assert <%= inspect context.alias %>.get_<%= schema.singular %>!(<%= schema.singular %>.id).confirmed_at
@@ -70,7 +81,7 @@ defmodule <%= inspect context.web_module %>.<%= inspect Module.concat(schema.web
       assert Repo.all(<%= inspect context.alias %>.<%= inspect schema.alias %>Token) == []
 
       # When not logged in
-      conn = get(conn, Routes.<%= schema.route_helper %>_confirmation_path(conn, :confirm, token))
+      conn = post(conn, Routes.<%= schema.route_helper %>_confirmation_path(conn, :update, token))
       assert redirected_to(conn) == "/"
       assert get_flash(conn, :error) =~ "<%= schema.human_singular %> confirmation link is invalid or it has expired"
 
@@ -78,14 +89,14 @@ defmodule <%= inspect context.web_module %>.<%= inspect Module.concat(schema.web
       conn =
         build_conn()
         |> log_in_<%= schema.singular %>(<%= schema.singular %>)
-        |> get(Routes.<%= schema.route_helper %>_confirmation_path(conn, :confirm, token))
+        |> post(Routes.<%= schema.route_helper %>_confirmation_path(conn, :update, token))
 
       assert redirected_to(conn) == "/"
       refute get_flash(conn, :error)
     end
 
     test "does not confirm email with invalid token", %{conn: conn, <%= schema.singular %>: <%= schema.singular %>} do
-      conn = get(conn, Routes.<%= schema.route_helper %>_confirmation_path(conn, :confirm, "oops"))
+      conn = post(conn, Routes.<%= schema.route_helper %>_confirmation_path(conn, :update, "oops"))
       assert redirected_to(conn) == "/"
       assert get_flash(conn, :error) =~ "<%= schema.human_singular %> confirmation link is invalid or it has expired"
       refute <%= inspect context.alias %>.get_<%= schema.singular %>!(<%= schema.singular %>.id).confirmed_at
diff --git a/test/mix/tasks/phx.gen.auth_test.exs b/test/mix/tasks/phx.gen.auth_test.exs
index 4e8441562..6a80db75e 100644
--- a/test/mix/tasks/phx.gen.auth_test.exs
+++ b/test/mix/tasks/phx.gen.auth_test.exs
@@ -167,7 +167,8 @@ defmodule Mix.Tasks.Phx.Gen.AuthTest do
             delete "/users/log_out", UserSessionController, :delete
             get "/users/confirm", UserConfirmationController, :new
             post "/users/confirm", UserConfirmationController, :create
-            get "/users/confirm/:token", UserConfirmationController, :confirm
+            get "/users/confirm/:token", UserConfirmationController, :edit
+            post "/users/confirm/:token", UserConfirmationController, :update
           end
         """
       end
@@ -181,20 +182,6 @@ defmodule Mix.Tasks.Phx.Gen.AuthTest do
         assert file =~ "def log_in_user(conn, user)"
       end
 
-      assert_received {:mix_shell, :info, ["""
-
-      Please re-fetch your dependencies with the following command:
-
-          mix deps.get
-      """]}
-
-      assert_received {:mix_shell, :info, ["""
-
-      Remember to update your repository by running migrations:
-
-        $ mix ecto.migrate
-      """]}
-
       assert_received {:mix_shell, :info, ["Unable to find the \"MyApp.Mailer\"" <> mailer_notice]}
       assert mailer_notice =~ ~s(A mailer module like the following is expected to be defined)
       assert mailer_notice =~ ~s(in your application in order to send emails.)
@@ -383,7 +370,8 @@ defmodule Mix.Tasks.Phx.Gen.AuthTest do
             delete "/users/log_out", UserSessionController, :delete
             get "/users/confirm", UserConfirmationController, :new
             post "/users/confirm", UserConfirmationController, :create
-            get "/users/confirm/:token", UserConfirmationController, :confirm
+            get "/users/confirm/:token", UserConfirmationController, :edit
+            post "/users/confirm/:token", UserConfirmationController, :update
           end
         """
       end