silverdr

Phx_gen_auth user creation

I am going through the phx_gen_auth output in more details today…

… and another thing caught a bit of my attention:

  def create(conn, %{"user" => user_params}) do
    case Accounts.register_user(user_params) do
      {:ok, user} ->
        {:ok, _} =
          Accounts.deliver_user_confirmation_instructions(
            user, &Routes.user_confirmation_url(conn, :confirm, &1)
          )

        conn
        |> put_flash(:info, "User created successfully.")
        |> UserAuth.log_in_user(user)

      {:error, %Ecto.Changeset{} = changeset} ->
        render(conn, "new.html", changeset: changeset)
    end
  end

is an action in user_registration_controller.ex. It first creates (inserts) the user via Accounts.register_user() and later on creates plus inserts the “confirmation” token and delivers notification via Accounts.deliver_user_confirmation_instructions(). And here’s the doubt: I can easily imagine situation where inserting the token fails but the user is already inserted. Or the notification fails. And the user cannot be re-registered because the email is already taken, and so on.

So… shouldn’t these three be rather run transactionally?

15 comments

/phoenix #phx_gen_auth

6 1524 15

2021-11-23 21:54:33 UTC

Most Liked

josevalim

Creator of Elixir

Yes, putting it in a transaction could be an improvement. But since the email delivery can fail anyway, we have safe retry mechanisms, and the token creation failing being extremely unlikely, so I don’t worry it.

About emailing new tokens, that’s on purpose, cause the token is encrypted, so we can’t recover old ones, and we don’t want to make them stale (as you can receive a late email first).

People flooding others inbox is a possibility. Not much on confirmation, cause it requires an unconfirmed account, but definitely on password recovery. A large app likely requires rate limiting, not only on those screens but also on login, but it has been out of scope so far. Another idea is to require another information, which is app specific such as date of birth, before resetting the password.

That’s to say, your auditing has been on point. Perhaps improvements for the future. If you do implement rate limiting, then consider writing a blog post, I am sure it will be helpful.

Post #6

silverdr

Noticed that too. The question is more of “why not wrap it in a transaction and have such scenarios covered?” Is there a reason other than simply “nobody felt a need to”?

As for requesting new token and instruction, there are two aspects that raise a bit of concern to my un-phoenix-trained eye too:

said requesting does not seem to require any authorisation (like after authentication/logging-in first for example)
upon requesting new token, all the previous ones remain stored in the database. Until the account is “confirmed” (removing of them is nicely transactional, BTW).

Doesn’t that (the latter especially) mean that even a third party can easily flood the database with tons of records, potentially leading to a DOS in an extreme case?

Post #3

f0rest8

This isn’t really a point-to-be-made but rather sharing my experience (and blog post below) with phx_gen_auth’s design decisions as they relate to my work with Metamorphic.

phx_gen_auth and rate limiting
I implemented a simple session-based rate limiting for log in based on Chris McCord’s post on GenServers and ETS. This also helped me practice and familiarize myself with the whole GenServer/ETS process a bit before employing ETS to handle temporary storage of people’s images on Metamorphic.

Here’s the friend link for free viewing of the post on Medium.

The rate limiting was super easy to implement into my design thanks to phx_gen_auth’s design.

phx_gen_auth and password reset
In regard to password reset, I had a slightly different problem of creating a “zero-knowledge” encrypted system for people’s data.

If someone were to reset their password they would lose access to any data they had previously encrypted. Rather than allow that with ample warnings, I opted to just remove the password reset option altogether (you can still change your password from within your account), because the other options required things that I felt could ultimately lead to an attacker (albeit motivated) creating havoc with a person’s account (one thought was to allow you to reset your password if you use the same machine as when you first registered your account, but all these kinds of solutions come with other security, privacy, and design tradeoffs).

That really didn’t have much to do with phx_gen_auth but I include it because the beauty, in my opinion, of phx_gen_auth is that it understands that people may have quite different needs for their authentication. By laying just the strong foundation, it becomes really easy to quickly customize and adapt the system to fit your needs (be it “more” secure, or remove things altogether).

I think the decisions that were made about phx_gen_auth, and reasoning behind them, were made with a lot of foresight and wisdom from past experiences. And my experience using it has been very positive because of it. Thank you!

Post #7