fix(PasswordReset): Generate the reset token using the target action, not the source action. (#25)

* fix(PasswordReset): Generate the reset token using the target action, not the source action.

Also improve tests.

* improvement(PasswordReset): rework PasswordReset to be a provider in it's own right - this means it has it's own routes, etc.
This commit is contained in:
James Harton 2022-11-04 11:24:33 +13:00 committed by GitHub
parent c5eb02f4b0
commit bab9ec363e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 402 additions and 111 deletions

View file

@ -13,8 +13,8 @@
<h2><%= inspect(config.subject_name) %> - <%= Ash.Api.Info.short_name(config.api) %> / <%= Ash.Resource.Info.short_name(config.resource) %></h2>
<%= for provider <- config.providers do %>
<%= Module.concat(provider, HTML).register(config.resource, action: "/auth/#{config.subject_name}/#{provider.provides()}/callback", legend: "Register") %>
<%= Module.concat(provider, HTML).sign_in(config.resource, action: "/auth/#{config.subject_name}/#{provider.provides()}/callback", legend: "Sign in") %>
<%= Module.concat(provider, Html).request(config.resource, action: "/auth/#{config.subject_name}/#{provider.provides(config.resource)}") %>
<%= Module.concat(provider, Html).callback(config.resource, action: "/auth/#{config.subject_name}/#{provider.provides(config.resource)}/callback") %>
<% end %>
<% end %>

View file

@ -67,7 +67,8 @@ defmodule AshAuthentication.Jwt do
@doc """
Given a record, generate a signed JWT for use while authenticating.
"""
@spec token_for_record(Resource.record()) :: {:ok, token, claims} | :error
@spec token_for_record(Resource.record(), extra_claims :: %{}, options :: keyword) ::
{:ok, token, claims} | :error
def token_for_record(record, extra_claims \\ %{}, opts \\ []) do
resource = record.__struct__

View file

@ -111,15 +111,14 @@ defmodule AshAuthentication.PasswordAuthentication do
#{Spark.Dsl.Extension.doc(@dsl)}
"""
@behaviour AshAuthentication.Provider
use Spark.Dsl.Extension,
sections: @dsl,
transformers: [AshAuthentication.PasswordAuthentication.Transformer]
use AshAuthentication.Provider
alias Ash.Resource
alias AshAuthentication.PasswordAuthentication
alias Plug.Conn
@doc """
Attempt to sign in an user of the provided resource type.
@ -149,40 +148,24 @@ defmodule AshAuthentication.PasswordAuthentication do
to: PasswordAuthentication.Actions,
as: :register
@doc """
Handle the request phase.
The password authentication provider does nothing with the request phase, and just returns
the `conn` unmodified.
"""
@impl true
@spec request_plug(Conn.t(), any) :: Conn.t()
defdelegate request_plug(conn, config), to: PasswordAuthentication.Plug, as: :request
@doc """
Handle the callback phase.
Handles both sign-in and registration actions via the same endpoint.
"""
@impl true
@spec callback_plug(Conn.t(), any) :: Conn.t()
defdelegate callback_plug(conn, config), to: PasswordAuthentication.Plug, as: :callback
defdelegate callback_plug(conn, config), to: PasswordAuthentication.Plug, as: :handle
@doc """
Returns the name of the associated provider.
Handle the request phase.
Handles both sign-in and registration actions via the same endpoint.
"""
@impl true
@spec provides :: String.t()
def provides, do: "password"
defdelegate request_plug(conn, config), to: PasswordAuthentication.Plug, as: :handle
@doc false
@impl true
@spec has_register_step?(any) :: boolean
def has_register_step?(_), do: true
@doc """
Returns whether password authentication is enabled for the resource
"""
@spec enabled?(Resource.t()) :: boolean
def enabled?(resource), do: __MODULE__ in Spark.extensions(resource)
@spec has_register_step?(Resource.t()) :: boolean
def has_register_step?(_resource), do: true
end

View file

@ -1,8 +1,9 @@
defmodule AshAuthentication.PasswordAuthentication.HTML do
defmodule AshAuthentication.PasswordAuthentication.Html do
@moduledoc """
Renders a very basic forms for using password authentication.
Renders a very basic form for using password authentication.
These are mainly used for testing.
These are mainly used for testing, and you should instead write your own or
use the widgets in `ash_authentication_phoenix`.
"""
require EEx
@ -35,7 +36,7 @@ defmodule AshAuthentication.PasswordAuthentication.HTML do
<input type="hidden" name="<%= @subject_name %>[action]" value="<%= @register_action_name %>" />
<fieldset>
<%= if @legend do %><legend><%= @legend %></legend><% end %>
<input type="text" name="<%= @subject_name %>[<%= @identity_field %>]" placeholder="register" />
<input type="text" name="<%= @subject_name %>[<%= @identity_field %>]" placeholder="<%= @identity_field %>" />
<br />
<input type="password" name="<%= @subject_name %>[<%= @password_field %>]" placeholder="Password" />
<br />
@ -69,8 +70,12 @@ defmodule AshAuthentication.PasswordAuthentication.HTML do
@doc """
Render a basic HTML sign-in form.
"""
@spec sign_in(module, options) :: String.t()
def sign_in(resource, options) do
@spec callback(module, options) :: String.t()
def callback(resource, options) do
options =
options
|> Keyword.put_new(:legend, "Sign in")
resource
|> build_assigns(options)
|> render_sign_in()
@ -79,8 +84,12 @@ defmodule AshAuthentication.PasswordAuthentication.HTML do
@doc """
Render a basic HTML registration form.
"""
@spec register(module, options) :: String.t()
def register(resource, options) do
@spec request(module, options) :: String.t()
def request(resource, options) do
options =
options
|> Keyword.put_new(:legend, "Register")
resource
|> build_assigns(options)
|> render_register()

View file

@ -1,38 +1,27 @@
defmodule AshAuthentication.PasswordAuthentication.Plug do
@moduledoc """
Handlers for incoming request and callback HTTP requests.
Handlers for incoming HTTP requests.
AshAuthentication is written with an eye towards OAuth which uses a two-phase
request/callback process which can be used to register and sign in an user in
a single flow. This doesn't really work that well with `PasswordAuthentication` which has
seperate "registration" and "sign-in" actions.
a single flow. This doesn't really work that well with
`PasswordAuthentication` which has seperate "registration" and "sign-in"
actions.
Here we simply ignore the request phase, which will cause an error to be
returned to the remote user if they somehow find themselves there.
We use the "callback" phase to handle both registration and sign in by passing
an "action" parameter along with the form data.
We handle both registration and sign in by passing an "action" parameter along
with the form data.
"""
import AshAuthentication.Plug.Helpers, only: [private_store: 2]
alias AshAuthentication.{PasswordAuthentication, PasswordReset}
alias AshAuthentication.PasswordAuthentication
alias Plug.Conn
@doc """
Handle the request phase.
The password authentication provider does nothing with the request phase, and just returns
the `conn` unmodified.
"""
@spec request(Conn.t(), any) :: Conn.t()
def request(conn, _opts), do: conn
@doc """
Handle the callback phase.
Handles both sign-in and registration actions via the same endpoint.
"""
@spec callback(Conn.t(), any) :: Conn.t()
def callback(%{params: params, private: %{authenticator: config}} = conn, _opts) do
@spec handle(Conn.t(), any) :: Conn.t()
def handle(%{params: params, private: %{authenticator: config}} = conn, _opts) do
params
|> Map.get(to_string(config.subject_name), %{})
|> do_action(config.resource)
@ -45,7 +34,7 @@ defmodule AshAuthentication.PasswordAuthentication.Plug do
end
end
def callback(conn, _opts), do: conn
def handle(conn, _opts), do: conn
defp do_action(%{"action" => "sign_in"} = attrs, resource),
do: PasswordAuthentication.sign_in_action(resource, attrs)
@ -53,8 +42,5 @@ defmodule AshAuthentication.PasswordAuthentication.Plug do
defp do_action(%{"action" => "register"} = attrs, resource),
do: PasswordAuthentication.register_action(resource, attrs)
defp do_action(%{"action" => "reset_password"} = attrs, resource),
do: PasswordReset.reset_password(resource, attrs)
defp do_action(_attrs, _resource), do: {:error, "No action provided"}
end

View file

@ -106,15 +106,11 @@ defmodule AshAuthentication.PasswordReset do
sections: @dsl,
transformers: [AshAuthentication.PasswordReset.Transformer]
use AshAuthentication.Provider
alias Ash.{Changeset, Query, Resource}
alias AshAuthentication.{Jwt, PasswordReset}
@doc """
Returns whether password reset is enabled for the resource
"""
@spec enabled?(Resource.t()) :: boolean
def enabled?(resource), do: __MODULE__ in Spark.extensions(resource)
@doc """
Request a password reset for a user.
@ -177,7 +173,7 @@ defmodule AshAuthentication.PasswordReset do
with true <- enabled?(resource),
{:ok, lifetime} <- PasswordReset.Info.token_lifetime(resource),
{:ok, action} <- PasswordReset.Info.request_password_reset_action_name(resource),
{:ok, action} <- PasswordReset.Info.password_reset_action_name(resource),
{:ok, token, _claims} <-
Jwt.token_for_record(user, %{"act" => action}, token_lifetime: lifetime) do
{:ok, token}
@ -185,4 +181,7 @@ defmodule AshAuthentication.PasswordReset do
_ -> :error
end
end
defdelegate request_plug(conn, any), to: PasswordReset.Plug, as: :request
defdelegate callback_plug(conn, any), to: PasswordReset.Plug, as: :callback
end

View file

@ -0,0 +1,94 @@
defmodule AshAuthentication.PasswordReset.Html do
@moduledoc """
Renders a very basic form for password resetting.
These are mainly used for testing, and you should instead write your own or
use the widgets in `ash_authentication_phoenix`.
"""
require EEx
alias AshAuthentication.{PasswordAuthentication, PasswordReset}
EEx.function_from_string(
:defp,
:render_request,
~s"""
<form method="<%= @method %>" action="<%= @action %>">
<fieldset>
<%= if @legend do %><legend><%= @legend %></legend><% end %>
<input type="text" name="<%= @subject_name %>[<%= @identity_field %>]" placeholder="<%= @identity_field %>" />
<br />
<input type="submit" value="Request password reset" />
</fieldset>
</form>
""",
[:assigns]
)
EEx.function_from_string(
:defp,
:render_reset,
~s"""
<form method="<%= @method %>" action="<%= @action %>">
<fieldset>
<%= if @legend do %><legend><%= @legend %></legend><% end %>
<input type="token" name="<%= @subject_name %>[reset_token]" placeholder="Reset token" />
<br />
<input type="password" name="<%= @subject_name %>[<%= @password_field %>]" placeholder="Password" />
<br />
<%= if @confirmation_required? do %>
<input type="password" name="<%= @subject_name %>[<%= @password_confirmation_field %>]" placeholder="Password confirmation" />
<br />
<% end %>
<input type="submit" value="Reset password" />
</fieldset>
</form>
""",
[:assigns]
)
@defaults [method: "POST", legend: nil]
@type options :: [method_option | action_option]
@typedoc """
The HTTP method used to submit the form.
Defaults to `#{inspect(Keyword.get(@defaults, :method))}`.
"""
@type method_option :: {:method, String.t()}
@typedoc """
The path/URL to which the form should be submitted.
"""
@type action_option :: {:action, String.t()}
@doc """
Render a reset request.
"""
@spec request(module, options) :: String.t()
def request(resource, options) do
resource
|> build_assigns(Keyword.put(options, :legend, "Request password reset"))
|> render_request()
end
@doc """
Render a reset form
"""
@spec callback(module, options) :: String.t()
def callback(resource, options) do
resource
|> build_assigns(Keyword.put(options, :legend, "Reset password"))
|> render_reset()
end
defp build_assigns(resource, options) do
@defaults
|> Keyword.merge(options)
|> Map.new()
|> Map.merge(PasswordAuthentication.Info.password_authentication_options(resource))
|> Map.merge(PasswordReset.Info.options(resource))
|> Map.merge(AshAuthentication.Info.authentication_options(resource))
end
end

View file

@ -0,0 +1,45 @@
defmodule AshAuthentication.PasswordReset.Plug do
@moduledoc """
Handlers for incoming HTTP requests.
"""
import AshAuthentication.Plug.Helpers, only: [private_store: 2]
alias AshAuthentication.PasswordReset
alias Plug.Conn
@doc """
Handle an inbound password reset request.
"""
@spec request(Conn.t(), any) :: Conn.t()
def request(%{params: params, private: %{authenticator: config}} = conn, _opts) do
params =
params
|> Map.get(to_string(config.subject_name), %{})
case PasswordReset.request_password_reset(config.resource, params) do
{:ok, _} ->
private_store(conn, {:success, nil})
{:error, reason} ->
private_store(conn, {:failure, reason})
end
end
@doc """
Handle an inbound password reset.
"""
@spec callback(Conn.t(), any) :: Conn.t()
def callback(%{params: params, private: %{authenticator: config}} = conn, _opts) do
params =
params
|> Map.get(to_string(config.subject_name), %{})
case PasswordReset.reset_password(config.resource, params) do
{:ok, user} when is_struct(user, config.resource) ->
private_store(conn, {:success, user})
{:error, reason} ->
private_store(conn, {:failure, reason})
end
end
end

View file

@ -13,7 +13,7 @@ defmodule AshAuthentication.PasswordReset.ResetTokenValidation do
def validate(changeset, _) do
with token when is_binary(token) <- Changeset.get_argument(changeset, :reset_token),
{:ok, %{"act" => token_action}, _} <- Jwt.verify(token, changeset.resource),
{:ok, resource_action} <- Info.request_password_reset_action_name(changeset.resource),
{:ok, resource_action} <- Info.password_reset_action_name(changeset.resource),
true <- to_string(resource_action) == token_action do
:ok
else

View file

@ -52,6 +52,18 @@ defmodule AshAuthentication.PasswordReset.Transformer do
&build_change_action(&1, change_action_name)
),
:ok <- validate_change_action(dsl_state, change_action_name) do
authentication =
Transformer.get_persisted(dsl_state, :authentication)
|> Map.update(
:providers,
[AshAuthentication.PasswordReset],
&[AshAuthentication.PasswordReset | &1]
)
dsl_state =
dsl_state
|> Transformer.persist(:authentication, authentication)
{:ok, dsl_state}
else
:error -> {:error, "Configuration error"}

View file

@ -19,11 +19,19 @@ defmodule AshAuthentication.Plug.Dispatcher do
"""
@impl true
@spec call(Conn.t(), config | any) :: Conn.t()
def call(
def call(conn, {phase, routes, return_to}) do
conn
|> dispatch(phase, routes)
|> return(return_to)
end
def call(conn, {_phase, _routes, return_to}), do: return_to.handle_failure(conn)
defp dispatch(
%{params: %{"subject_name" => subject_name, "provider" => provider}} = conn,
{phase, routes, return_to}
phase,
routes
) do
conn =
case Map.get(routes, {subject_name, provider}) do
config when is_map(config) ->
conn = Conn.put_private(conn, :authenticator, config)
@ -36,21 +44,29 @@ defmodule AshAuthentication.Plug.Dispatcher do
_ ->
conn
end
case conn do
%{state: :sent} ->
conn
%{private: %{authentication_result: {:success, user}}} ->
return_to.handle_success(conn, user, Map.get(user.__metadata__, :token))
%{private: %{authentication_result: {:failure, reason}}} ->
return_to.handle_failure(conn, reason)
_ ->
return_to.handle_failure(conn, nil)
end
end
def call(conn, {_phase, _routes, return_to}), do: return_to.handle_failure(conn)
defp dispatch(conn, _phase, _routes), do: conn
defp return(%{state: :sent} = conn, _return_to), do: conn
defp return(
%{
private: %{
authentication_result: {:success, user},
authenticator: %{resource: resource}
}
} = conn,
return_to
)
when is_struct(user, resource),
do: return_to.handle_success(conn, user, Map.get(user.__metadata__, :token))
defp return(%{private: %{authentication_result: {:success, nil}}} = conn, return_to),
do: return_to.handle_success(conn, nil, nil)
defp return(%{private: %{authentication_result: {:failure, reason}}} = conn, return_to),
do: return_to.handle_failure(conn, reason)
defp return(conn, return_to), do: return_to.handle_failure(conn, nil)
end

View file

@ -11,13 +11,16 @@ defmodule AshAuthentication.Plug.Helpers do
Store the user in the connections' session.
"""
@spec store_in_session(Conn.t(), Resource.record()) :: Conn.t()
def store_in_session(conn, user) do
def store_in_session(conn, user) when is_struct(user) do
subject_name = AshAuthentication.Info.authentication_subject_name!(user.__struct__)
subject = AshAuthentication.resource_to_subject(user)
Conn.put_session(conn, subject_name, subject)
end
def store_in_session(conn, _), do: conn
@doc """
Given a list of subjects, turn as many as possible into users.
"""
@ -169,15 +172,19 @@ defmodule AshAuthentication.Plug.Helpers do
"""
@spec private_store(
Conn.t(),
{:success, Resource.record()} | {:failure, nil | Changeset.t() | Error.t()}
{:success, nil | Resource.record()} | {:failure, nil | Changeset.t() | Error.t()}
) ::
Conn.t()
def private_store(conn, {:success, nil}),
do: Conn.put_private(conn, :authentication_result, {:success, nil})
def private_store(conn, {:success, record})
when is_struct(record, conn.private.authenticator.resource),
do: Conn.put_private(conn, :authentication_result, {:success, record})
def private_store(conn, {:failure, reason})
when is_nil(reason) or is_map(reason),
when is_nil(reason) or is_binary(reason) or is_map(reason),
do: Conn.put_private(conn, :authentication_result, {:failure, reason})
# Dyanamically generated atoms are generally frowned upon, but in this case

View file

@ -35,7 +35,7 @@ defmodule AshAuthentication.Plug.Router do
|> Map.delete(:providers)
|> Map.put(:provider, provider)
{{subject_name, provider.provides()}, config}
{{subject_name, provider.provides(config.resource)}, config}
end)
end)
|> Map.new()

View file

@ -6,7 +6,7 @@ defmodule AshAuthentication.Provider do
@doc """
The name of the provider for routing purposes, eg: "github".
"""
@callback provides() :: String.t()
@callback provides(Resource.t()) :: String.t()
@doc """
Given some credentials for a potentially existing user, verify the credentials
@ -31,10 +31,98 @@ defmodule AshAuthentication.Provider do
@doc """
A function plug which can handle the callback phase.
"""
@callback callback_plug(Conn.t(), AshAuthentication.resource_config()) :: Conn.t()
@callback callback_plug(Conn.t(), any) :: Conn.t()
@doc """
A function plug which can handle the request phase.
"""
@callback request_plug(Conn.t(), AshAuthentication.resource_config()) :: Conn.t()
@callback request_plug(Conn.t(), any) :: Conn.t()
@doc """
Is this extension enabled for this resource?
"""
@callback enabled?(Resource.t()) :: boolean
defmacro __using__(_) do
quote do
@behaviour AshAuthentication.Provider
@doc """
The name of the provider to be used in routes.
The default implementation derives it from the module name removing any
"Authentication" suffix.
Overridable.
"""
@impl true
@spec provides(Resource.t()) :: String.t()
def provides(_resource) do
__MODULE__
|> Module.split()
|> List.last()
|> String.trim_trailing("Authentication")
|> Macro.underscore()
end
@doc """
Handle a request for this extension to sign in a user.
Defaults to returning an error. Overridable.
"""
@impl true
def sign_in_action(_resource, _attributes),
do: {:error, "Sign in not supported by `#{inspect(__MODULE__)}`"}
@doc """
Handle a request for this extension to register a user.
Defaults to returning an error. Overridable.
"""
@impl true
def register_action(_resource, _attributes),
do: {:error, "Registration not supported by `#{inspect(__MODULE__)}`"}
@doc """
Handle an inbound request to the `request` path.
Defaults to returning the `conn` unchanged. Overridable.
"""
@impl true
def request_plug(conn, _config), do: conn
@doc """
Handle an inbound request to the `callback` path.
Defaults to returning the `conn` unchanged. Overridable.
"""
@impl true
def callback_plug(conn, _config), do: conn
@doc """
Does this extension require a separate register step?
Defaults to `false`. Overridable.
"""
@impl true
def has_register_step?(_resource), do: false
@doc """
Is `resource` supported by this provider?
Defaults to `false`. Overridable.
"""
@impl true
@spec enabled?(Resource.t()) :: boolean
def enabled?(resource), do: __MODULE__ in Spark.extensions(resource)
defoverridable provides: 1,
sign_in_action: 2,
register_action: 2,
request_plug: 2,
callback_plug: 2,
has_register_step?: 1,
enabled?: 1
end
end
end

View file

@ -87,4 +87,20 @@ defmodule AshAuthentication.PasswordResetTest do
assert reloaded_user.hashed_password == user.hashed_password
end
end
describe "reset_token_for/1" do
test "when given a resource which supports password resets, it generates a token" do
assert {:ok, token} =
build_user()
|> PasswordReset.reset_token_for()
assert token =~ ~r/^[\w\.-]+$/
end
test "when given a resource which doesn't support password resets, it returns an error" do
assert :error =
build_token_revocation()
|> PasswordReset.reset_token_for()
end
end
end

View file

@ -53,7 +53,7 @@ defmodule AshAuthentication.PlugTest do
resp = Jason.decode!(resp)
assert status == 401
assert resp["status"] == "failed"
assert resp["status"] == "failure"
assert resp["reason"] =~ ~r/Forbidden/
end
end

View file

@ -8,11 +8,14 @@ defmodule AshAuthenticationTest do
assert [
%{
api: Example,
providers: [AshAuthentication.PasswordAuthentication],
providers: providers,
resource: Example.UserWithUsername,
subject_name: :user_with_username
}
] = AshAuthentication.authenticated_resources(:ash_authentication)
assert AshAuthentication.PasswordAuthentication in providers
assert AshAuthentication.PasswordReset in providers
end
end
end

View file

@ -36,6 +36,7 @@ defmodule AshAuthentication.DataCase do
@doc """
Sets up the sandbox based on the test tags.
"""
@spec setup_sandbox(any) :: :ok
def setup_sandbox(tags) do
pid = Sandbox.start_owner!(Example.Repo, shared: not tags[:async])
on_exit(fn -> Sandbox.stop_owner(pid) end)
@ -49,6 +50,7 @@ defmodule AshAuthentication.DataCase do
assert %{password: ["password is too short"]} = errors_on(changeset)
"""
@spec errors_on(Ecto.Changeset.t()) :: %{atom => [any]}
def errors_on(changeset) do
Ecto.Changeset.traverse_errors(changeset, fn {message, opts} ->
Regex.replace(~r"%{(\w+)}", message, fn _, key ->
@ -57,9 +59,16 @@ defmodule AshAuthentication.DataCase do
end)
end
@doc "Generate a random username using Faker"
@spec username :: String.t()
def username, do: Faker.Internet.user_name()
@doc "Generate a random password using Faker"
@spec password :: String.t()
def password, do: Faker.Lorem.words(4) |> Enum.join(" ")
@doc "User factory"
@spec build_user(keyword) :: Example.UserWithUsername.t() | no_return
def build_user(attrs \\ []) do
password = password()
@ -74,4 +83,16 @@ defmodule AshAuthentication.DataCase do
|> Ash.Changeset.for_create(:register, attrs)
|> Example.create!()
end
@doc "Token revocation factory"
@spec build_token_revocation :: Example.TokenRevocation.t() | no_return
def build_token_revocation do
{:ok, token, _claims} =
build_user()
|> AshAuthentication.Jwt.token_for_record()
Example.TokenRevocation
|> Ash.Changeset.for_create(:revoke_token, %{token: token})
|> Example.create!()
end
end

View file

@ -3,6 +3,16 @@ defmodule Example.AuthPlug do
use AshAuthentication.Plug, otp_app: :ash_authentication
@impl true
def handle_success(conn, nil, nil) do
conn
|> put_resp_header("content-type", "application/json")
|> send_resp(
200,
Jason.encode!(%{status: :success})
)
end
def handle_success(conn, user, token) do
conn
|> store_in_session(user)
@ -10,6 +20,7 @@ defmodule Example.AuthPlug do
|> send_resp(
200,
Jason.encode!(%{
status: :success,
token: token,
user: %{
id: user.id,
@ -26,7 +37,7 @@ defmodule Example.AuthPlug do
|> send_resp(
401,
Jason.encode!(%{
status: "failed",
status: :failure,
reason: inspect(reason)
})
)