From ee4f3e46a6544d8e6ec65f6ca313ad59ccec0afa Mon Sep 17 00:00:00 2001 From: James Harton Date: Mon, 19 Jun 2023 09:28:12 +1200 Subject: [PATCH] fix(ConfirmationHookChange): use `Info.find_strategy/2..3` rather than a hard coded strategy name. Changes: 1. Extracted `find_strategy/2..3` from a bunch of places across the codebase and moved into `Info`. 2. Updated `ConfirmationHookChange` to use `find_strategy/2..3`. Closes #334. --- .../confirmation/confirmation_hook_change.ex | 33 +++++++++++++++-- lib/ash_authentication/info.ex | 36 +++++++++++++++++++ .../password/hash_password_change.ex | 17 +-------- .../password_confirmation_validation.ex | 16 +-------- .../password/sign_in_preparation.ex | 17 +-------- .../sign_in_with_token_preparation.ex | 17 +-------- 6 files changed, 71 insertions(+), 65 deletions(-) diff --git a/lib/ash_authentication/add_ons/confirmation/confirmation_hook_change.ex b/lib/ash_authentication/add_ons/confirmation/confirmation_hook_change.ex index 6af0ba9e..9406d651 100644 --- a/lib/ash_authentication/add_ons/confirmation/confirmation_hook_change.ex +++ b/lib/ash_authentication/add_ons/confirmation/confirmation_hook_change.ex @@ -3,6 +3,35 @@ defmodule AshAuthentication.AddOn.Confirmation.ConfirmationHookChange do Triggers a confirmation flow when one of the monitored fields is changed. Optionally inhibits changes to monitored fields on update. + + You can use this change in your actions where you want to send the user a + confirmation (or inhibit changes after confirmation). If you're not using one + of the actions generated by the confirmation add-on then you'll need to + manually pass the strategy name in the changeset context. Eg: + + ```elixir + Changeset.new(user, %{}) + |> Changeset.set_context(%{strategy_name: :confirm}) + |> Changeset.for_update(:update, params) + |> Accounts.update() + ``` + + or by adding it statically to your action definition: + + ```elixir + update :change_email do + change set_context(%{strategy_name: :confirm}) + change AshAuthentication.AddOn.Confirmation.ConfirmationHookChange + end + ``` + + or by adding it as an option to the change definition: + + ```elixir + update :change_email do + change {AshAuthentication.AddOn.Confirmation.ConfirmationHookChange, strategy_name: :confirm} + end + ``` """ use Ash.Resource.Change @@ -12,8 +41,8 @@ defmodule AshAuthentication.AddOn.Confirmation.ConfirmationHookChange do @doc false @impl true @spec change(Changeset.t(), keyword, Change.context()) :: Changeset.t() - def change(changeset, _opts, _context) do - case Info.strategy(changeset.resource, :confirm) do + def change(changeset, options, context) do + case Info.find_strategy(changeset, context, options) do {:ok, strategy} -> do_change(changeset, strategy) diff --git a/lib/ash_authentication/info.ex b/lib/ash_authentication/info.ex index 61641280..80747fbc 100644 --- a/lib/ash_authentication/info.ex +++ b/lib/ash_authentication/info.ex @@ -7,6 +7,7 @@ defmodule AshAuthentication.Info do extension: AshAuthentication, sections: [:authentication] + alias Ash.{Changeset, Query} alias AshAuthentication.Strategy alias Spark.Dsl.Extension @@ -67,4 +68,39 @@ defmodule AshAuthentication.Info do raise "No strategy action named `#{inspect(action_name)}` found on resource `#{inspect(dsl_or_resource)}`" end end + + @doc """ + Find the underlying strategy that required a change/preparation to be used. + + This is because the `strategy_name` can be passed on the change options, eg: + + ```elixir + change {AshAuthentication.Strategy.Password.HashPasswordChange, strategy_name: :banana_custard} + ``` + + Or via the action context, eg: + + ```elixir + prepare set_context(%{strategy_name: :banana_custard}) + prepare AshAuthentication.Strategy.Password.SignInPreparation + ``` + + Or via the passed-in context on calling the action. + """ + @spec find_strategy(Query.t() | Changeset.t(), context, options) :: {:ok, Strategy.t()} | :error + when context: map, options: Keyword.t() + def find_strategy(queryset, context \\ %{}, options) do + with :error <- strategy_for_action(queryset.resource, queryset.action.name), + :error <- Map.fetch(queryset.context, :strategy_name), + :error <- Map.fetch(context, :strategy_name), + :error <- Keyword.fetch(options, :strategy_name) do + :error + else + {:ok, strategy_name} when is_atom(strategy_name) -> + strategy(queryset.resource, strategy_name) + + {:ok, strategy} -> + {:ok, strategy} + end + end end diff --git a/lib/ash_authentication/strategies/password/hash_password_change.ex b/lib/ash_authentication/strategies/password/hash_password_change.ex index 9693f8a7..1699f7e7 100644 --- a/lib/ash_authentication/strategies/password/hash_password_change.ex +++ b/lib/ash_authentication/strategies/password/hash_password_change.ex @@ -45,7 +45,7 @@ defmodule AshAuthentication.Strategy.Password.HashPasswordChange do def change(changeset, options, context) do changeset |> Changeset.before_action(fn changeset -> - with {:ok, strategy} <- find_strategy(changeset, context, options), + with {:ok, strategy} <- Info.find_strategy(changeset, context, options), value when is_binary(value) <- Changeset.get_argument(changeset, strategy.password_field), {:ok, hash} <- strategy.hash_provider.hash(value) do @@ -59,19 +59,4 @@ defmodule AshAuthentication.Strategy.Password.HashPasswordChange do end end) end - - defp find_strategy(changeset, context, options) do - with :error <- Info.strategy_for_action(changeset.resource, changeset.action.name), - :error <- Map.fetch(changeset.context, :strategy_name), - :error <- Map.fetch(context, :strategy_name), - :error <- Keyword.fetch(options, :strategy_name) do - :error - else - {:ok, strategy_name} when is_atom(strategy_name) -> - Info.strategy(changeset.resource, strategy_name) - - {:ok, strategy} -> - {:ok, strategy} - end - end end diff --git a/lib/ash_authentication/strategies/password/password_confirmation_validation.ex b/lib/ash_authentication/strategies/password/password_confirmation_validation.ex index 9084681d..052cf107 100644 --- a/lib/ash_authentication/strategies/password/password_confirmation_validation.ex +++ b/lib/ash_authentication/strategies/password/password_confirmation_validation.ex @@ -38,7 +38,7 @@ defmodule AshAuthentication.Strategy.Password.PasswordConfirmationValidation do @impl true @spec validate(Changeset.t(), keyword) :: :ok | {:error, String.t() | Exception.t()} def validate(changeset, options) do - case find_strategy(changeset, options) do + case Info.find_strategy(changeset, options) do {:ok, %{confirmation_required?: true} = strategy} -> validate_password_confirmation(changeset, strategy) @@ -67,18 +67,4 @@ defmodule AshAuthentication.Strategy.Password.PasswordConfirmationValidation do )} end end - - defp find_strategy(changeset, options) do - with :error <- Info.strategy_for_action(changeset.resource, changeset.action.name), - :error <- Map.fetch(changeset.context, :strategy_name), - :error <- Keyword.fetch(options, :strategy_name) do - :error - else - {:ok, strategy_name} when is_atom(strategy_name) -> - Info.strategy(changeset.resource, strategy_name) - - {:ok, strategy} -> - {:ok, strategy} - end - end end diff --git a/lib/ash_authentication/strategies/password/sign_in_preparation.ex b/lib/ash_authentication/strategies/password/sign_in_preparation.ex index 8695a0f4..1f449311 100644 --- a/lib/ash_authentication/strategies/password/sign_in_preparation.ex +++ b/lib/ash_authentication/strategies/password/sign_in_preparation.ex @@ -21,7 +21,7 @@ defmodule AshAuthentication.Strategy.Password.SignInPreparation do @impl true @spec prepare(Query.t(), keyword, Preparation.context()) :: Query.t() def prepare(query, options, context) do - {:ok, strategy} = find_strategy(query, context, options) + {:ok, strategy} = Info.find_strategy(query, context, options) identity_field = strategy.identity_field identity = Query.get_argument(query, identity_field) @@ -122,19 +122,4 @@ defmodule AshAuthentication.Strategy.Password.SignInPreparation do record end end - - defp find_strategy(query, context, options) do - with :error <- Info.strategy_for_action(query.resource, query.action.name), - :error <- Map.fetch(query.context, :strategy_name), - :error <- Map.fetch(context, :strategy_name), - :error <- Keyword.fetch(options, :strategy_name) do - :error - else - {:ok, strategy_name} when is_atom(strategy_name) -> - Info.strategy(query.resource, strategy_name) - - {:ok, strategy} -> - {:ok, strategy} - end - end end diff --git a/lib/ash_authentication/strategies/password/sign_in_with_token_preparation.ex b/lib/ash_authentication/strategies/password/sign_in_with_token_preparation.ex index 5f406f7f..e769e959 100644 --- a/lib/ash_authentication/strategies/password/sign_in_with_token_preparation.ex +++ b/lib/ash_authentication/strategies/password/sign_in_with_token_preparation.ex @@ -14,7 +14,7 @@ defmodule AshAuthentication.Strategy.Password.SignInWithTokenPreparation do @impl true @spec prepare(Query.t(), keyword, Preparation.context()) :: Query.t() def prepare(query, options, context) do - {:ok, strategy} = find_strategy(query, context, options) + {:ok, strategy} = Info.find_strategy(query, context, options) query |> check_sign_in_token_configuration(strategy) @@ -156,19 +156,4 @@ defmodule AshAuthentication.Strategy.Password.SignInWithTokenPreparation do defp extract_primary_keys_from_subject(_, _), do: {:error, "The token does not contain a subject"} - - defp find_strategy(query, context, options) do - with :error <- Info.strategy_for_action(query.resource, query.action.name), - :error <- Map.fetch(query.context, :strategy_name), - :error <- Map.fetch(context, :strategy_name), - :error <- Keyword.fetch(options, :strategy_name) do - :error - else - {:ok, strategy_name} when is_atom(strategy_name) -> - Info.strategy(query.resource, strategy_name) - - {:ok, strategy} -> - {:ok, strategy} - end - end end