Skip to content

Commit

Permalink
fix(ConfirmationHookChange): use Info.find_strategy/2..3 rather tha…
Browse files Browse the repository at this point in the history
…n 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.
  • Loading branch information
jimsynz committed Jun 18, 2023
1 parent cc3aa41 commit ee4f3e4
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down
36 changes: 36 additions & 0 deletions lib/ash_authentication/info.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ defmodule AshAuthentication.Info do
extension: AshAuthentication,
sections: [:authentication]

alias Ash.{Changeset, Query}
alias AshAuthentication.Strategy
alias Spark.Dsl.Extension

Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

0 comments on commit ee4f3e4

Please sign in to comment.