Update phx.gen.auth with sudo mode and magic links (#6081)

* Update phx.gen.auth with sudo mode and magic links Closes #6041. Still todo: documentation. * remove failing test assertions * datetime_module * log in page -> login page * format phx.gen.auth_test.exs * format phx.gen.auth.ex * security considerations section for docs * update security considerations Co-Authored-By: José Valim <[email protected]> * small adjustments to phx.gen.auth guide --------- Co-authored-by: José Valim <[email protected]>
phoenixframework · Feb 20, 2025 · 01df1dc · 01df1dc
1 parent 95c653f
commit 01df1dc
Show file tree

Hide file tree

Showing 49 changed files with 1,444 additions and 2,297 deletions.
diff --git a/guides/authentication/mix_phx_gen_auth.md b/guides/authentication/mix_phx_gen_auth.md
@@ -2,7 +2,7 @@
 
 > This guide assumes that you have gone through the [introductory guides](overview.html) and have a Phoenix application [up and running](up_and_running.html).
 
-The `mix phx.gen.auth` command generates a flexible, pre-built authentication system into your Phoenix app. This generator allows you to quickly move past the task of adding authentication to your codebase and stay focused on the real-world problem your application is trying to solve.
+The `mix phx.gen.auth` command generates a flexible, pre-built authentication system based on magic links into your Phoenix app. This generator allows you to quickly move past the task of adding authentication to your codebase and stay focused on the real-world problem your application is trying to solve.
 
 ## Getting started
 
@@ -76,11 +76,10 @@ The generated code ships with an authentication module with a handful of plugs t
 
   * `fetch_current_user` - fetches the current user information if available
   * `require_authenticated_user` - must be invoked after `fetch_current_user` and requires that a current user exists and is authenticated
-  * `redirect_if_user_is_authenticated` - used for the few pages that must not be available to authenticated users
+  * `redirect_if_user_is_authenticated` - used for the few pages that must not be available to authenticated users (only generated for controller based authentication)
+  * `require_sudo_mode` - used for pages that contain sensitive operations and enforces recent authentication
 
-### Confirmation
-
-The generated functionality ships with an account confirmation mechanism, where users have to confirm their account, typically by email. However, the generated code does not forbid users from using the application if their accounts have not yet been confirmed. You can add this functionality by customizing the `require_authenticated_user` in the `Auth` module to check for the `confirmed_at` field (and any other property you desire).
+There are similar `:on_mount` hooks for LiveView based authentication.
 
 ### Notifiers
 
@@ -102,6 +101,10 @@ If your application is sensitive to enumeration attacks, you need to implement y
 
 Furthermore, if you are concerned about enumeration attacks, beware of timing attacks too. For example, registering a new account typically involves additional work (such as writing to the database, sending emails, etc) compared to when an account already exists. Someone could measure the time taken to execute those additional tasks to enumerate emails. This applies to all endpoints (registration, confirmation, password recovery, etc.) that may send email, in-app notifications, etc.
 
+### Confirmation and credential pre-stuffing attacks
+
+The generated functionality ships with an account confirmation mechanism, where users have to confirm their account, typically by email. Furthermore, to prevent security issues, the generated code does forbid users from using the application if their accounts have not yet been confirmed. If you want to change this behavior, please refer to the ["Mixing magic link and password registration" section](Mix.Tasks.Phx.Gen.Auth.html#module-mixing-magic-link-and-password-registration) of `mix phx.gen.auth`.
+
 ### Case sensitiveness
 
 The email lookup is made to be case-insensitive. Case-insensitive lookups are the default in MySQL and MSSQL. In SQLite3 we use [`COLLATE NOCASE`](https://www.sqlite.org/datatype3.html#collating_sequences) in the column definition to support it. In PostgreSQL, we use the [`citext` extension](https://www.postgresql.org/docs/current/citext.html).
@@ -125,6 +128,8 @@ The following links have more information regarding the motivation and design of
   * The [original `phx_gen_auth` repo][phx_gen_auth repo] (for Phoenix 1.5 applications) - This is a great resource to see discussions around decisions that have been made in earlier versions of the project.
   * [Original pull request on bare Phoenix app][auth PR]
   * [Original design spec](https://github.com/dashbitco/mix_phx_gen_auth_demo/blob/auth/README.md)
+  * [Pull request for migrating LiveView based Phoenix 1.7 `phx.gen.auth` to magic links](https://github.com/SteffenDE/phoenix_gen_auth_magic_link/pull/1)
+  * [Pull request for migrating controller based Phoenix 1.7 `phx.gen.auth` to magic links](https://github.com/SteffenDE/phoenix_gen_auth_magic_link/pull/2)
 
 [phx_gen_auth repo]: https://github.com/aaronrenner/phx_gen_auth
 [auth PR]: https://github.com/dashbitco/mix_phx_gen_auth_demo/pull/1
diff --git a/lib/mix/tasks/phx.gen.auth.ex b/lib/mix/tasks/phx.gen.auth.ex
@@ -26,6 +26,30 @@ defmodule Mix.Tasks.Phx.Gen.Auth do
   to authentication views without necessarily triggering a new HTTP request
   each time (which would result in a full page load).
 
+  ## Mixing magic link and password registration
+
+  `mix phx.gen.auth` generates email based authentication, which assumes the user who
+  owns the email address has control over the account. Therefore, it is extremely
+  important to void all access tokens once the user confirms their account for the first
+  time, and we do so by revoking all tokens upon confirmation.
+
+  However, if you allow users to create an account with password, you must also
+  require them to be logged in by the time of confirmation, otherwise you may be
+  vulnerable to credential pre-stuffing, as the following attack is possible:
+
+  1. An attacker registers a new account with the email address of their target, anticipating
+     that the target creates an account at a later point in time.
+  2. The attacker sets a password when registering.
+  3. The target registers an account and sees that their email address is already in use.
+  4. The target logs in by magic link, but does not change the existing password.
+  5. The attacker maintains access using the password they previously set.
+
+  This is why the default implementation raises whenever a user tries to log in for the first
+  time by magic link and there is a password set. If you add registration with email and
+  password, then you must require the user to be logged in to confirm their account.
+  If they don't have a password (because it was set by the attacker), then they can set one
+  via a "Forgot your password?"-like workflow.
+
   ## Password hashing
 
   The password hashing mechanism defaults to `bcrypt` for
@@ -167,7 +191,8 @@ defmodule Mix.Tasks.Phx.Gen.Auth do
       router_scope: router_scope(context),
       web_path_prefix: web_path_prefix(schema),
       test_case_options: test_case_options(ecto_adapter),
-      live?: Keyword.fetch!(context.opts, :live)
+      live?: Keyword.fetch!(context.opts, :live),
+      datetime_module: datetime_module(schema)
     ]
 
     paths = Mix.Phoenix.generator_paths()
@@ -298,34 +323,6 @@ defmodule Mix.Tasks.Phx.Gen.Auth do
             "#{singular}_live",
             "login_test.exs"
           ],
-          "reset_password_live.ex": [
-            web_pre,
-            "live",
-            web_path,
-            "#{singular}_live",
-            "reset_password.ex"
-          ],
-          "reset_password_live_test.exs": [
-            web_test_pre,
-            "live",
-            web_path,
-            "#{singular}_live",
-            "reset_password_test.exs"
-          ],
-          "forgot_password_live.ex": [
-            web_pre,
-            "live",
-            web_path,
-            "#{singular}_live",
-            "forgot_password.ex"
-          ],
-          "forgot_password_live_test.exs": [
-            web_test_pre,
-            "live",
-            web_path,
-            "#{singular}_live",
-            "forgot_password_test.exs"
-          ],
           "settings_live.ex": [web_pre, "live", web_path, "#{singular}_live", "settings.ex"],
           "settings_live_test.exs": [
             web_test_pre,
@@ -347,45 +344,13 @@ defmodule Mix.Tasks.Phx.Gen.Auth do
             web_path,
             "#{singular}_live",
             "confirmation_test.exs"
-          ],
-          "confirmation_instructions_live.ex": [
-            web_pre,
-            "live",
-            web_path,
-            "#{singular}_live",
-            "confirmation_instructions.ex"
-          ],
-          "confirmation_instructions_live_test.exs": [
-            web_test_pre,
-            "live",
-            web_path,
-            "#{singular}_live",
-            "confirmation_instructions_test.exs"
           ]
         ]
 
         remap_files(default_files ++ live_files)
 
       _ ->
         non_live_files = [
-          "confirmation_html.ex": [controller_pre, "#{singular}_confirmation_html.ex"],
-          "confirmation_new.html.heex": [
-            controller_pre,
-            "#{singular}_confirmation_html",
-            "new.html.heex"
-          ],
-          "confirmation_edit.html.heex": [
-            controller_pre,
-            "#{singular}_confirmation_html",
-            "edit.html.heex"
-          ],
-          "confirmation_controller.ex": [controller_pre, "#{singular}_confirmation_controller.ex"],
-          "confirmation_controller_test.exs": [
-            web_test_pre,
-            "controllers",
-            web_path,
-            "#{singular}_confirmation_controller_test.exs"
-          ],
           "registration_new.html.heex": [
             controller_pre,
             "#{singular}_registration_html",
@@ -399,29 +364,13 @@ defmodule Mix.Tasks.Phx.Gen.Auth do
             "#{singular}_registration_controller_test.exs"
           ],
           "registration_html.ex": [controller_pre, "#{singular}_registration_html.ex"],
-          "reset_password_html.ex": [controller_pre, "#{singular}_reset_password_html.ex"],
-          "reset_password_controller.ex": [
-            controller_pre,
-            "#{singular}_reset_password_controller.ex"
-          ],
-          "reset_password_controller_test.exs": [
-            web_test_pre,
-            "controllers",
-            web_path,
-            "#{singular}_reset_password_controller_test.exs"
-          ],
-          "reset_password_edit.html.heex": [
-            controller_pre,
-            "#{singular}_reset_password_html",
-            "edit.html.heex"
-          ],
-          "reset_password_new.html.heex": [
-            controller_pre,
-            "#{singular}_reset_password_html",
-            "new.html.heex"
-          ],
           "session_html.ex": [controller_pre, "#{singular}_session_html.ex"],
           "session_new.html.heex": [controller_pre, "#{singular}_session_html", "new.html.heex"],
+          "session_confirm.html.heex": [
+            controller_pre,
+            "#{singular}_session_html",
+            "confirm.html.heex"
+          ],
           "settings_html.ex": [web_pre, "controllers", web_path, "#{singular}_settings_html.ex"],
           "settings_controller.ex": [controller_pre, "#{singular}_settings_controller.ex"],
           "settings_edit.html.heex": [
@@ -871,6 +820,10 @@ defmodule Mix.Tasks.Phx.Gen.Auth do
   defp test_case_options(Ecto.Adapters.Postgres), do: ", async: true"
   defp test_case_options(adapter) when is_atom(adapter), do: ""
 
+  defp datetime_module(%{timestamp_type: :naive_datetime}), do: NaiveDateTime
+  defp datetime_module(%{timestamp_type: :utc_datetime}), do: DateTime
+  defp datetime_module(%{timestamp_type: :utc_datetime_usec}), do: DateTime
+
   defp put_live_option(schema) do
     opts =
       case Keyword.fetch(schema.opts, :live) do

diff --git a/priv/templates/phx.gen.auth/auth.ex b/priv/templates/phx.gen.auth/auth.ex
@@ -24,24 +24,34 @@ defmodule <%= inspect auth_module %> do
   so LiveView sessions are identified and automatically
   disconnected on log out. The line can be safely removed
   if you are not using LiveView.
+
+  In case the <%= schema.singular %> re-authenticates for sudo mode,
+  the existing remember_me setting is kept, writing a new remember_me cookie.
   """
   def log_in_<%= schema.singular %>(conn, <%= schema.singular %>, params \\ %{}) do
     token = <%= inspect context.alias %>.generate_<%= schema.singular %>_session_token(<%= schema.singular %>)
     <%= schema.singular %>_return_to = get_session(conn, :<%= schema.singular %>_return_to)
+    remember_me = get_session(conn, :<%= schema.singular %>_remember_me)
 
     conn
     |> renew_session()
     |> put_token_in_session(token)
-    |> maybe_write_remember_me_cookie(token, params)
+    |> maybe_write_remember_me_cookie(token, params, remember_me)
     |> redirect(to: <%= schema.singular %>_return_to || signed_in_path(conn))
   end
 
-  defp maybe_write_remember_me_cookie(conn, token, %{"remember_me" => "true"}) do
-    put_resp_cookie(conn, @remember_me_cookie, token, @remember_me_options)
-  end
+  defp maybe_write_remember_me_cookie(conn, token, %{"remember_me" => "true"}, _),
+    do: write_remember_me_cookie(conn, token)
+
+  defp maybe_write_remember_me_cookie(conn, token, _params, true),
+    do: write_remember_me_cookie(conn, token)
 
-  defp maybe_write_remember_me_cookie(conn, _token, _params) do
+  defp maybe_write_remember_me_cookie(conn, _token, _params, _), do: conn
+
+  defp write_remember_me_cookie(conn, token) do
     conn
+    |> put_session(:<%= schema.singular %>_remember_me, true)
+    |> put_resp_cookie(@remember_me_cookie, token, @remember_me_options)
   end
 
   # This function renews the session ID and erases the whole
@@ -124,9 +134,6 @@ defmodule <%= inspect auth_module %> do
       on <%= schema.singular %>_token.
       Redirects to login page if there's no logged <%= schema.singular %>.
 
-    * `:redirect_if_<%= schema.singular %>_is_authenticated` - Authenticates the <%= schema.singular %> from the session.
-      Redirects to signed_in_path if there's a logged <%= schema.singular %>.
-
   ## Examples
 
   Use the `on_mount` lifecycle macro in LiveViews to mount or authenticate
@@ -164,13 +171,18 @@ defmodule <%= inspect auth_module %> do
     end
   end
 
-  def on_mount(:redirect_if_<%= schema.singular %>_is_authenticated, _params, session, socket) do
+  def on_mount(:ensure_sudo_mode, _params, session, socket) do
     socket = mount_current_<%= schema.singular %>(socket, session)
 
-    if socket.assigns.current_<%= schema.singular %> do
-      {:halt, Phoenix.LiveView.redirect(socket, to: signed_in_path(socket))}
-    else
+    if <%= inspect context.alias %>.sudo_mode?(socket.assigns.current_<%= schema.singular %>, -10) do
       {:cont, socket}
+    else
+      socket =
+        socket
+        |> Phoenix.LiveView.put_flash(:error, "You must re-authenticate to access this page.")
+        |> Phoenix.LiveView.redirect(to: ~p"<%= schema.route_prefix %>/log-in")
+
+      {:halt, socket}
     end
   end
 
@@ -182,7 +194,22 @@ defmodule <%= inspect auth_module %> do
     end)
   end
 
-  <% end %>@doc """
+  <% else %>@doc """
+  Used for routes that require sudo mode.
+  """
+  def require_sudo_mode(conn, _opts) do
+    if <%= inspect context.alias %>.sudo_mode?(conn.assigns.current_<%= schema.singular %>, -10) do
+      conn
+    else
+      conn
+      |> put_flash(:error, "You must re-authenticate to access this page.")
+      |> maybe_store_return_to()
+      |> redirect(to: ~p"<%= schema.route_prefix %>/log-in")
+      |> halt()
+    end
+  end
+
+  @doc """
   Used for routes that require the <%= schema.singular %> to not be authenticated.
   """
   def redirect_if_<%= schema.singular %>_is_authenticated(conn, _opts) do
@@ -195,7 +222,7 @@ defmodule <%= inspect auth_module %> do
     end
   end
 
-  @doc """
+  <% end %>@doc """
   Used for routes that require the <%= schema.singular %> to be authenticated.
 
   If you want to enforce the <%= schema.singular %> email is confirmed before
@@ -213,17 +240,38 @@ defmodule <%= inspect auth_module %> do
     end
   end
 
-  defp put_token_in_session(conn, token) do
+  <%= if live? do %>defp put_token_in_session(conn, token) do
     conn
     |> put_session(:<%= schema.singular %>_token, token)
-    |> put_session(:live_socket_id, "<%= schema.plural %>_sessions:#{Base.url_encode64(token)}")
+    |> put_session(:live_socket_id, <%= schema.singular %>_session_topic(token))
+  end
+
+  @doc """
+  Disconnects existing sockets for the given tokens.
+  """
+  def disconnect_sessions(tokens) do
+    Enum.each(tokens, fn %{token: token} ->
+      <%= inspect endpoint_module %>.broadcast(<%= schema.singular %>_session_topic(token), "disconnect", %{})
+    end)
   end
 
-  defp maybe_store_return_to(%{method: "GET"} = conn) do
+  defp <%= schema.singular %>_session_topic(token), do: "<%= schema.plural %>_sessions:#{Base.url_encode64(token)}"
+
+  <% else %>defp put_token_in_session(conn, token) do
+    put_session(conn, :<%= schema.singular %>_token, token)
+  end
+
+  <% end %>defp maybe_store_return_to(%{method: "GET"} = conn) do
     put_session(conn, :<%= schema.singular %>_return_to, current_path(conn))
   end
 
   defp maybe_store_return_to(conn), do: conn
 
-  defp signed_in_path(_conn), do: ~p"/"
+  <%= if live? do %>@doc "Returns the path to redirect to after log in."
+  # the <%= schema.singular %> was already logged in, redirect to settings
+  def signed_in_path(%Plug.Conn{assigns: %{current_<%= schema.singular %>: %<%= inspect context.alias %>.<%= inspect schema.alias %>{}}}) do
+    ~p"<%= schema.route_prefix %>/settings"
+  end
+
+  def signed_in_path(_), do: ~p"/"<% else %>defp signed_in_path(_conn), do: ~p"/"<% end %>
 end