team-alembic / ash_authentication

The Ash Authentication framework
MIT License
96 stars 52 forks source link

Igniters generate invalid content for non-Phoenix sender modules #814

Open sevenseacat opened 1 month ago

sevenseacat commented 1 month ago

When not using Phoenix, igniters such as ash_authentication.add_strategy password will generate email sender modules like this:

defmodule TestAaEmails.Accounts.User.Senders.SendPasswordResetEmail do
  @moduledoc """
  Sends a password reset email
  """

  use AshAuthentication.Sender

  @impl true
  def send(_user, token, _) do
    IO.puts("""
    Click this link to reset your password:

    #{url(~p"/password-reset/#{token}")}
    """)
  end
end

This is invalid because the url function and p sigil are both provided by Phoenix.VerifiedRoutes.

The p sigil can be removed for the non-Phoenix version of the module, leaving a plain string, but how to fully-qualify the URL properly?

zachdaniel commented 1 month ago

So, I've been thinking about this, and I think there is a design flaw. This was actually pointed out by a customer of Alembic's as well. These senders are "web-aware". In phx.gen.auth, that is not the case. They pass down a url which the internals then use to send.

The reason this isn't great for us is because we define the controllers. Certain setups get factually more difficult because of this choice (like if the URL is not static but dependent on a subdomain for tenancy, for example).

@jimsynz had originally used route helpers for this, which allowed it to be derived from the routes in the router. But that was soft deprecated in Phoenix in favor of verified routes, but in this case verified routes is actually crossing the boundary of app -> web in the wrong direction.

We probably need to come up with a backwards compatible idiomatic solution. Probably some kind of additional callback in the auth controller, and if that is defined we pass that down to the senders. like

def url_for(conn, :password, {:reset, token}) do
    ~p"/reset_password/#{token}"
end

def url_for(_, _, _), do: :default

Where default will just be the current host + a standard route for that thing.

Then, when calling actions with senders, they look for a context that is set called ash_authentication: %{url: <url>}, then pass that into the senders.

It sounds a bit involved, but I think phx.gen.auth has the right of it in going through the hoops that avoid this dependency.

@jimsynz what do you think?

zachdaniel commented 1 month ago

I think the simplest answer here is to actually generate the actions with optional url arguments, which then get passed to the senders as opts. Old implementations will continue to work, but can upgrade to use the provided URL. Then, we add def url_for/3 for generating URLs.