improvement(PasswordReset): A reset request is actually a query, not an update. (#23)

lib/ash_authentication/password_reset.ex

@@ -106,7 +106,7 @@ defmodule AshAuthentication.PasswordReset do
     sections: @dsl,
     transformers: [AshAuthentication.PasswordReset.Transformer]
 
-  alias Ash.{Changeset, Resource}
+  alias Ash.{Changeset, Query, Resource}
   alias AshAuthentication.{Jwt, PasswordReset}
 
   @doc """
@@ -122,19 +122,16 @@ defmodule AshAuthentication.PasswordReset do
 
   ## Example
 
-      iex> user = MyApp.Accounts.get(MyApp.Accounts.User, email: "marty@mcfly.me")
-      ...> request_password_reset(user)
+      iex> request_password_reset(MyApp.Accounts.User, %{"email" => "marty@mcfly.me"})
       :ok
   """
-  def request_password_reset(user) do
-    resource = user.__struct__
-
+  def request_password_reset(resource, params) do
     with true <- enabled?(resource),
          {:ok, action} <- PasswordReset.Info.request_password_reset_action_name(resource),
          {:ok, api} <- AshAuthentication.Info.authentication_api(resource) do
-      user
-      |> Changeset.for_update(action, %{})
-      |> api.update()
+      resource
+      |> Query.for_read(action, params)
+      |> api.read()
     else
       {:error, reason} -> {:error, reason}
       _ -> {:error, "Password resets not supported by resource `#{inspect(resource)}`"}
@@ -146,11 +143,17 @@ defmodule AshAuthentication.PasswordReset do
 
   Given a reset token, password and _maybe_ password confirmation, validate and
   change the user's password.
+
+  ## Example
+
+      iex> reset_password(MyApp.Accounts.User, params)
+      {:ok, %MyApp.Accounts.User{}}
   """
   @spec reset_password(Resource.t(), params) :: {:ok, Resource.record()} | {:error, Changeset.t()}
         when params: %{required(String.t()) => String.t()}
   def reset_password(resource, params) do
-    with {:ok, token} <- Map.fetch(params, "reset_token"),
+    with true <- enabled?(resource),
+         {:ok, token} <- Map.fetch(params, "reset_token"),
          {:ok, %{"sub" => subject}, config} <- Jwt.verify(token, resource),
          {:ok, user} <- AshAuthentication.subject_to_resource(subject, config),
          {:ok, action} <- PasswordReset.Info.password_reset_action_name(config.resource),
@@ -159,8 +162,27 @@ defmodule AshAuthentication.PasswordReset do
       |> Changeset.for_update(action, params)
       |> api.update()
     else
+      false -> {:error, "Password resets not supported by resource `#{inspect(resource)}`"}
       :error -> {:error, "Invalid reset token"}
       {:error, reason} -> {:error, reason}
     end
   end
+
+  @doc """
+  Generate a reset token for a user.
+  """
+  @spec reset_token_for(Resource.record()) :: {:ok, String.t()} | :error
+  def reset_token_for(user) do
+    resource = user.__struct__
+
+    with true <- enabled?(resource),
+         {:ok, lifetime} <- PasswordReset.Info.token_lifetime(resource),
+         {:ok, action} <- PasswordReset.Info.request_password_reset_action_name(resource),
+         {:ok, token, _claims} <-
+           Jwt.token_for_record(user, %{"act" => action}, token_lifetime: lifetime) do
+      {:ok, token}
+    else
+      _ -> :error
+    end
+  end
 end

lib/ash_authentication/password_reset/notifier.ex

@@ -1,21 +0,0 @@
-defmodule AshAuthentication.PasswordReset.Notifier do
-  @moduledoc """
-  This is a moduledoc
-  """
-  use Ash.Notifier
-  alias AshAuthentication.{PasswordReset, PasswordReset.Info}
-
-  @doc false
-  @impl true
-  def notify(notification) do
-    with true <- PasswordReset.enabled?(notification.resource),
-         {:ok, action} <- Info.request_password_reset_action_name(notification.resource),
-         true <- notification.action.name == action,
-         {:ok, {sender, send_opts}} <- Info.sender(notification.resource),
-         {:ok, reset_token} <- Map.fetch(notification.data.__metadata__, :reset_token) do
-      sender.send(notification.data, reset_token, send_opts)
-    end
-
-    :ok
-  end
-end

lib/ash_authentication/password_reset/request_password_reset_action.ex

@@ -1,35 +0,0 @@
-defmodule AshAuthentication.PasswordReset.RequestPasswordResetAction do
-  @moduledoc """
-  A manually implemented action which generates a reset token for a user.
-  """
-  use Ash.Resource.ManualUpdate
-  alias Ash.{Changeset, Resource, Resource.ManualUpdate}
-  alias AshAuthentication.{Jwt, PasswordReset.Info}
-
-  @doc false
-  @impl true
-  @spec update(Changeset.t(), keyword, ManualUpdate.context()) ::
-          {:ok, Resource.record()} | {:error, any}
-  def update(changeset, _opts, _context) do
-    lifetime = Info.token_lifetime!(changeset.resource)
-
-    action =
-      changeset.action
-      |> Map.fetch!(:name)
-      |> to_string()
-
-    {:ok, token, _claims} =
-      changeset.data
-      |> Jwt.token_for_record(%{"act" => action}, token_lifetime: lifetime)
-
-    metadata =
-      changeset.data.__metadata__
-      |> Map.put(:reset_token, token)
-
-    data =
-      changeset.data
-      |> Map.put(:__metadata__, metadata)
-
-    {:ok, data}
-  end
-end

lib/ash_authentication/password_reset/request_password_reset_preparation.ex

@@ -0,0 +1,45 @@
+defmodule AshAuthentication.PasswordReset.RequestPasswordResetPreparation do
+  @moduledoc """
+  Prepare a query for a password reset request.
+
+  This preparation performs three jobs, one before the query executes and two
+  after.
+
+  Firstly, it constraints the query to match the identity field passed to the
+  action.
+
+  Secondly, if there is a user returned by the query, then generate a reset
+  token and publish a notification.  Always returns an empty result.
+  """
+  use Ash.Resource.Preparation
+  alias Ash.{Query, Resource.Preparation}
+  alias AshAuthentication.{PasswordAuthentication, PasswordReset}
+  require Ash.Query
+
+  @doc false
+  @impl true
+  @spec prepare(Query.t(), keyword, Preparation.context()) :: Query.t()
+  def prepare(query, _opts, _context) do
+    {:ok, identity_field} =
+      PasswordAuthentication.Info.password_authentication_identity_field(query.resource)
+
+    {:ok, {sender, send_opts}} = PasswordReset.Info.sender(query.resource)
+
+    identity = Query.get_argument(query, identity_field)
+
+    query
+    |> Query.filter(ref(^identity_field) == ^identity)
+    |> Query.after_action(fn
+      _query, [user] ->
+        case PasswordReset.reset_token_for(user) do
+          {:ok, token} -> sender.send(user, token, send_opts)
+          _ -> nil
+        end
+
+        {:ok, []}
+
+      _, _ ->
+        {:ok, []}
+    end)
+  end
+end

lib/ash_authentication/password_reset/transformer.ex

@@ -10,8 +10,7 @@ defmodule AshAuthentication.PasswordReset.Transformer do
 
   alias AshAuthentication.PasswordReset.{
     Info,
-    Notifier,
-    RequestPasswordResetAction,
+    RequestPasswordResetPreparation,
     ResetTokenValidation,
     Sender
   }
@@ -52,8 +51,7 @@ defmodule AshAuthentication.PasswordReset.Transformer do
              change_action_name,
              &build_change_action(&1, change_action_name)
            ),
-         :ok <- validate_change_action(dsl_state, change_action_name),
-         {:ok, dsl_state} <- maybe_add_notifier(dsl_state, Notifier) do
+         :ok <- validate_change_action(dsl_state, change_action_name) do
       {:ok, dsl_state}
     else
       :error -> {:error, "Configuration error"}
@@ -128,12 +126,30 @@ defmodule AshAuthentication.PasswordReset.Transformer do
     end
   end
 
-  defp build_request_action(_dsl_state, action_name) do
-    Transformer.build_entity(Resource.Dsl, [:actions], :update,
-      name: action_name,
-      manual: RequestPasswordResetAction,
-      accept: []
-    )
+  defp build_request_action(dsl_state, action_name) do
+    with {:ok, identity_field} <- PA.Info.password_authentication_identity_field(dsl_state) do
+      identity_attribute = Resource.Info.attribute(dsl_state, identity_field)
+
+      arguments = [
+        Transformer.build_entity!(Resource.Dsl, [:actions, :read], :argument,
+          name: identity_field,
+          type: identity_attribute.type,
+          allow_nil?: false
+        )
+      ]
+
+      preparations = [
+        Transformer.build_entity!(Resource.Dsl, [:actions, :read], :prepare,
+          preparation: RequestPasswordResetPreparation
+        )
+      ]
+
+      Transformer.build_entity(Resource.Dsl, [:actions], :read,
+        name: action_name,
+        arguments: arguments,
+        preparations: preparations
+      )
+    end
   end
 
   defp build_change_action(dsl_state, action_name) do
@@ -204,8 +220,10 @@ defmodule AshAuthentication.PasswordReset.Transformer do
   end
 
   defp validate_request_action(dsl_state, action_name) do
-    with {:ok, action} <- validate_action_exists(dsl_state, action_name) do
-      validate_action_has_manual(action, RequestPasswordResetAction)
+    with {:ok, action} <- validate_action_exists(dsl_state, action_name),
+         {:ok, identity_field} <- PA.Info.password_authentication_identity_field(dsl_state),
+         :ok <- PA.UserValidations.validate_identity_argument(dsl_state, action, identity_field) do
+      validate_action_has_preparation(action, RequestPasswordResetPreparation)
     end
   end
 
@@ -232,15 +250,4 @@ defmodule AshAuthentication.PasswordReset.Transformer do
       )
     end
   end
-
-  defp maybe_add_notifier(dsl_state, notifier) do
-    notifiers =
-      dsl_state
-      |> Transformer.get_persisted(:notifiers, [])
-      |> MapSet.new()
-      |> MapSet.put(notifier)
-      |> Enum.to_list()
-
-    {:ok, Transformer.persist(dsl_state, :notifiers, notifiers)}
-  end
 end

test/ash_authentication/password_reset_test.exs

@@ -15,33 +15,52 @@ defmodule AshAuthentication.PasswordResetTest do
   end
 
   describe "reset_password_request/1" do
-    test "it generates a password reset token" do
-      {:ok, user} =
-        build_user()
-        |> PasswordReset.request_password_reset()
+    test "when the user is found, it returns an empty list" do
+      user = build_user()
 
-      assert user.__metadata__.reset_token =~ ~r/[\w.]/i
+      assert {:ok, []} =
+               PasswordReset.request_password_reset(Example.UserWithUsername, %{
+                 "username" => user.username
+               })
     end
 
-    test "it sends the reset instructions" do
-      assert capture_log(fn ->
-               {:ok, _} =
-                 build_user()
-                 |> PasswordReset.request_password_reset()
+    test "when the user is not found, it returns an empty list" do
+      assert {:ok, []} =
+               PasswordReset.request_password_reset(Example.UserWithUsername, %{
+                 "username" => username()
+               })
+    end
+
+    test "when the user is found it sends the reset instructions" do
+      user = build_user()
+
+      log =
+        capture_log(fn ->
+          PasswordReset.request_password_reset(Example.UserWithUsername, %{
+            "username" => user.username
+          })
+        end)
+
+      assert log =~ ~r/Password reset request/i
+    end
+
+    test "when the user is not found, it doesn't send reset instructions" do
+      refute capture_log(fn ->
+               PasswordReset.request_password_reset(Example.UserWithUsername, %{
+                 "username" => username()
+               })
              end) =~ ~r/Password reset request/i
     end
   end
 
   describe "reset_password/2" do
     test "when the reset token is valid, it can change the password" do
-      {:ok, user} =
-        build_user()
-        |> PasswordReset.request_password_reset()
-
+      user = build_user()
+      {:ok, token} = PasswordReset.reset_token_for(user)
       password = password()
 
       attrs = %{
-        "reset_token" => user.__metadata__.reset_token,
+        "reset_token" => token,
         "password" => password,
         "password_confirmation" => password
       }