Email registration is not a sign-in; a password needs a confirmed email

CHANGELOG.md

@@ -1,5 +1,9 @@
 # September 2026
 
+- [breaking] **Registering with an email is no longer a sign-in, and a password signs in only once its email is confirmed.** `POST /api/v1/register` answered `201` with a full session while the confirmation email was still in the queue, as if it were device login, and password login worked the same on an address nobody had confirmed, so anyone could register any address and play as it. The two flows are now separate: device login still creates an account and signs it in, while registering creates the account, queues the email and answers `201` with the account under `data` (`Registration`: `user_id`, `username`, `display_name`, `email_confirmed`), never tokens. `POST /api/v1/login` answers `403 email_not_confirmed` until the emailed link is opened, and the browser password form says to confirm first; both only after the right password, so a guesser learns nothing. `Accounts.authenticate_by_password/2` returns `{:error, :email_not_confirmed}`, and `get_user_by_email_and_password/2` nil. The first account is confirmed as it is created (`email_confirmed: true`), on the API as in the browser, so it logs in at once. An emailed login link still confirms, as it proves the inbox; on an account registered with a password it used to crash, and now confirms and removes the password, which whoever registered the address chose before anyone proved they own it (the page the link opens says so; the confirmation email's link keeps it). The SDKs follow: Godot's `authenticate_register` no longer keeps a session, the C++ `Auth::register_email` takes a plain `Callback` and answers the account without touching the session, and Balaur's `client::register_email` is a REST call rather than `gamend::register`.
+
+- [fixed] **A provider sign-in that claims an unconfirmed account no longer keeps its password.** Signing in with a provider asserting a verified email links that provider to the account holding the address, and confirms it. When that account had never been confirmed, it kept the password whoever registered the address had chosen, so someone who registered a player's address first could still sign in with it once the player claimed the account with Google, Discord or any other provider. As with an emailed login link, the password is now removed and every session, access and refresh token the account held is revoked (`token_version` bumped). Linking to a confirmed account is unchanged.
+
 - [fixed] **The Hex packages compile as the version they were published as.** Each package's `mix.exs` took its version from the environment of whoever compiled it: `GAMEND_CONTENT_APP_VERSION` for gamend_core and gamend_web, `APP_VERSION` for gamend_sdk and gamend_plugin_tools. A host whose Dockerfile exports `GAMEND_CONTENT_APP_VERSION=1.0.0` (gamend_starter's did) built the engine as 1.0.0, and the SDK pair, whose `@version` publishing never stamped, built as 1.0.26 wherever `APP_VERSION` was unset. Either one failed a host's `>= 1.0.1266` requirement with "the dependency does not match the requirement". The publish job now writes the release's version into all four `mix.exs` files and removes the env lookup before anything is published; this repository's own image and docs still take the CI version from the environment.
 
 - [fixed] **`GamendWeb.BrotliCompressor` no longer fails the digest when `brotli` is not installed.** Its docs promised that a missing binary keeps the gzip, but `System.cmd/3` raises `:enoent` for a command it cannot find, so a host that lists it in `:phoenix, :static_compressors` had `mix phx.digest`, and with it `mix assets.deploy`, crash on any machine without `brotli` on `PATH`. It now looks the binary up first and returns `:error` when it is missing, so the digest writes only the `.gz` files.

apps/gamend_core/lib/gamend/accounts.ex

@@ -315,8 +315,9 @@ defmodule Gamend.Accounts do
               to: Registration
 
   @doc """
-  Gets a user by email and password. `nil` for a wrong password, and for an
-  address locked by too many failures (`authenticate_by_password/2` says which).
+  Gets a user by email and password. `nil` for a wrong password, for an
+  address locked by too many failures, and for an email not yet confirmed
+  (`authenticate_by_password/2` says which).
 
   ## Examples
 
@@ -336,15 +337,25 @@ defmodule Gamend.Accounts do
     end
   end
 
+  @typedoc "Why `authenticate_by_password/2` signed nobody in."
+  @type password_error() ::
+          :invalid_credentials | :email_not_confirmed | {:locked, pos_integer()}
+
   @doc """
   Checks an email and password, counting failures per address
   (`Gamend.Accounts.LoginLockouts`).
 
   `{:error, {:locked, seconds}}` when the address is locked, before the
   password is looked at, and for the failure that locks it.
+
+  `{:error, :email_not_confirmed}` for the right password on an account whose
+  email was never confirmed. Anyone can register any address with a password,
+  so the password signs nobody in until the inbox's owner has confirmed it.
+  It is answered only after the password matched, so it tells nothing to
+  someone who does not know it.
   """
   @spec authenticate_by_password(String.t(), String.t()) ::
-          {:ok, User.t()} | {:error, :invalid_credentials | {:locked, pos_integer()}}
+          {:ok, User.t()} | {:error, password_error()}
   def authenticate_by_password(email, password)
       when is_binary(email) and is_binary(password) do
     case LoginLockouts.check(email) do
@@ -359,7 +370,8 @@ defmodule Gamend.Accounts do
     if User.valid_password?(user, password) do
       maybe_upgrade_password_hash(user, password)
       LoginLockouts.clear(email)
-      {:ok, user}
+
+      if user.confirmed_at, do: {:ok, user}, else: {:error, :email_not_confirmed}
     else
       case LoginLockouts.record_failure(email) do
         :ok -> {:error, :invalid_credentials}

apps/gamend_core/lib/gamend/accounts/identities.ex

@@ -297,16 +297,7 @@ defmodule Gamend.Accounts.Identities do
   defp link_provider_to_user(user, attrs, provider_id_field, changeset_fn) do
     if Map.get(attrs, :email_verified) == true do
       attrs = scrub_attrs_for_update(user, attrs, provider_id_field)
-
-      case user |> changeset_fn.(attrs) |> drop_device_credential() |> Repo.update() do
-        {:ok, %User{} = updated} = ok ->
-          Accounts.invalidate_user_cache(user)
-          Accounts.invalidate_user_cache(updated)
-          ok
-
-        other ->
-          other
-      end
+      claim(user, user |> changeset_fn.(attrs) |> drop_device_credential())
     else
       changeset =
         user
@@ -320,6 +311,43 @@ defmodule Gamend.Accounts.Identities do
     end
   end
 
+  # The provider's changeset confirms the email, and a provider vouching for
+  # the address proves the inbox as an emailed login link does
+  # (`Gamend.Accounts.Sessions.login_user_by_magic_link/1`). Like that link, it
+  # must not keep what was set on an unconfirmed account before anyone proved
+  # the inbox: whoever registered the address chose its password, and was
+  # handed any token issued to it, and may not be its owner. So the password
+  # goes, and every token is revoked, as the owner claims the account.
+  defp claim(%User{confirmed_at: nil} = user, changeset) do
+    result =
+      changeset
+      |> Ecto.Changeset.put_change(:hashed_password, nil)
+      |> Accounts.update_user_and_delete_all_tokens()
+
+    case result do
+      {:ok, {%User{} = updated, _expired}} ->
+        # Drop what the old struct was cached under (a retired device id among
+        # it), then re-warm with the revoked one last, as the revocation does.
+        Accounts.invalidate_user_cache(user)
+        {:ok, Accounts.cache_user(updated)}
+
+      other ->
+        other
+    end
+  end
+
+  defp claim(%User{} = user, changeset) do
+    case Repo.update(changeset) do
+      {:ok, %User{} = updated} = ok ->
+        Accounts.invalidate_user_cache(user)
+        Accounts.invalidate_user_cache(updated)
+        ok
+
+      other ->
+        other
+    end
+  end
+
   # Linking a provider to an existing account retires that account's device
   # credential. The provider is linked as usual — the account keeps working, and
   # gains a sign-in method — but device auth is no longer one of its methods.

apps/gamend_core/lib/gamend/accounts/registration.ex

@@ -40,6 +40,15 @@ defmodule Gamend.Accounts.Registration do
 
   def maybe_make_first_user_admin(changeset, false), do: changeset
 
+  # The first account is confirmed as it is created: it gets no email to
+  # confirm with (there may be no mail server configured yet), and a password
+  # does not sign in an unconfirmed account.
+  defp maybe_confirm_first_user(changeset, true = _is_first_user) do
+    Ecto.Changeset.put_change(changeset, :confirmed_at, DateTime.utc_now(:second))
+  end
+
+  defp maybe_confirm_first_user(changeset, false), do: changeset
+
   # When account activation is required, new non-admin users start deactivated.
   # The first user (admin) is always activated.
   @doc false
@@ -105,7 +114,7 @@ defmodule Gamend.Accounts.Registration do
   email goes out from the `mailers` queue (`Gamend.Accounts.ConfirmationMailer`),
   enqueued in the transaction that inserts the user: the call returns once
   both are committed, without waiting on SMTP, and a failed send is retried
-  there. The first user becomes the admin and gets no email.
+  there. The first user becomes the admin and is confirmed, with no email.
   """
   @spec register_user_and_deliver(Types.user_registration_attrs(), (String.t() -> String.t())) ::
           {:ok, User.t()} | {:error, Ecto.Changeset.t() | term()}
@@ -126,7 +135,8 @@ defmodule Gamend.Accounts.Registration do
   @doc """
   Register a user with an email and a password and queue the confirmation
   email, as `register_user_and_deliver/3` does for the browser form: how a
-  game client signs up (`POST /api/v1/register`).
+  game client signs up (`POST /api/v1/register`). The password signs in once
+  the email is confirmed (`Gamend.Accounts.authenticate_by_password/2`).
   """
   @spec register_user_with_password_and_deliver(
           Types.user_registration_attrs(),
@@ -154,6 +164,7 @@ defmodule Gamend.Accounts.Registration do
       |> base_changeset.(attrs, opts)
       |> User.username_changeset(attrs)
       |> maybe_make_first_user_admin(is_first_user)
+      |> maybe_confirm_first_user(is_first_user)
       |> maybe_deactivate_new_user(is_first_user)
     end
 

apps/gamend_core/lib/gamend/accounts/sessions.ex

@@ -59,32 +59,25 @@ defmodule Gamend.Accounts.Sessions do
   1. The user has already confirmed their email. They are logged in
      and the magic link is expired.
 
-  2. The user has not confirmed their email and no password is set.
-     In this case, the user gets confirmed, logged in, and all tokens -
-     including session ones - are expired. In theory, no other tokens
-     exist but we delete all of them for best security practices.
-
-  3. The user has not confirmed their email but a password is set.
-     This cannot happen in the default implementation but may be the
-     source of security pitfalls. See the "Mixing magic link and password registration" section of
-     `mix help phx.gen.auth`.
+  2. The user has not confirmed their email. Opening the link proves they
+     own the inbox, so the user gets confirmed, logged in, and all tokens -
+     including session ones - are expired.
+
+  3. As 2, with a password set: registered with one (`POST /api/v1/register`)
+     and never confirmed. The password is removed as the email is confirmed.
+     Whoever registered the address chose it before anyone proved they own
+     the inbox, so it may be someone else's, and kept it would sign them into
+     the account its owner has just claimed (the "Mixing magic link and
+     password registration" section of `mix help phx.gen.auth`). The owner
+     sets a new one in settings; the link in the confirmation email confirms
+     the account and keeps the password.
   """
   @spec login_user_by_magic_link(String.t()) ::
           {:ok, {User.t(), [UserToken.t()]}} | {:error, :not_found | Ecto.Changeset.t() | term()}
   def login_user_by_magic_link(token) do
     {:ok, query} = UserToken.verify_magic_link_token_query(token)
 
     case Repo.one(query) do
-      # Prevent session fixation attacks by disallowing magic links for unconfirmed users with password
-      {%User{confirmed_at: nil, hashed_password: hash}, _token} when hash != nil ->
-        raise """
-        magic link log in is not allowed for unconfirmed users with a password set!
-
-        This cannot happen with the default implementation, which indicates that you
-        might have adapted the code to a different use case. Please make sure to read the
-        "Mixing magic link and password registration" section of `mix help phx.gen.auth`.
-        """
-
       {%User{confirmed_at: nil} = user, _token} ->
         handle_unconfirmed_login(user)
 
@@ -103,10 +96,13 @@ defmodule Gamend.Accounts.Sessions do
     end
   end
 
+  # Dropping the password is what makes confirming safe (case 3 above); a
+  # user without one is unchanged by it.
   defp handle_unconfirmed_login(user) do
     result =
       user
       |> User.confirm_changeset()
+      |> Ecto.Changeset.put_change(:hashed_password, nil)
       |> Accounts.update_user_and_delete_all_tokens()
 
     case result do

apps/gamend_core/test/gamend/accounts/oauth_test.exs

@@ -36,6 +36,62 @@ defmodule Gamend.Accounts.OAuthTest do
       assert returned.discord_id == "d_link"
     end
 
+    # Registered with a password by whoever typed the address, before anyone
+    # proved the inbox: the owner's provider sign-in claims the account, and
+    # nothing the registrant was given may still open it.
+    test "claiming an unconfirmed account drops its password and revokes its tokens" do
+      user =
+        %{email: "[email protected]"}
+        |> AccountsFixtures.unconfirmed_user_fixture()
+        |> AccountsFixtures.set_password()
+
+      session = Accounts.generate_user_session_token(user)
+      version = Accounts.get_user!(user.id).token_version
+
+      {:ok, returned} =
+        Accounts.find_or_create_from_discord(%{
+          discord_id: "d_claim",
+          email: "[email protected]",
+          email_verified: true
+        })
+
+      assert returned.id == user.id
+      assert returned.confirmed_at
+      refute returned.hashed_password
+      assert returned.token_version > version
+      assert %{hashed_password: nil, discord_id: "d_claim"} = Accounts.get_user!(user.id)
+      refute Accounts.get_user_by_session_token(session)
+
+      assert {:error, :invalid_credentials} =
+               Accounts.authenticate_by_password(
+                 "[email protected]",
+                 AccountsFixtures.valid_user_password()
+               )
+    end
+
+    test "a confirmed account keeps its password when a provider links to it" do
+      user =
+        %{email: "[email protected]"}
+        |> AccountsFixtures.user_fixture()
+        |> AccountsFixtures.set_password()
+
+      {:ok, returned} =
+        Accounts.find_or_create_from_discord(%{
+          discord_id: "d_kept",
+          email: "[email protected]",
+          email_verified: true
+        })
+
+      assert returned.id == user.id
+      assert returned.hashed_password
+
+      assert {:ok, _} =
+               Accounts.authenticate_by_password(
+                 "[email protected]",
+                 AccountsFixtures.valid_user_password()
+               )
+    end
+
     test "refuses to link to an existing account when email is unverified" do
       user = AccountsFixtures.unconfirmed_user_fixture(%{email: "[email protected]"})
 

apps/gamend_core/test/gamend/accounts_test.exs

@@ -52,6 +52,35 @@ defmodule Gamend.AccountsTest do
       assert %User{id: ^id} =
                Accounts.get_user_by_email_and_password(user.email, valid_user_password())
     end
+
+    test "does not return a user whose email is not confirmed" do
+      user = unconfirmed_user_fixture() |> set_password()
+      refute Accounts.get_user_by_email_and_password(user.email, valid_user_password())
+    end
+  end
+
+  describe "authenticate_by_password/2" do
+    test "refuses the right password on an unconfirmed email, and says why" do
+      user = unconfirmed_user_fixture() |> set_password()
+
+      assert {:error, :email_not_confirmed} =
+               Accounts.authenticate_by_password(user.email, valid_user_password())
+    end
+
+    test "a wrong password on an unconfirmed email is only invalid" do
+      user = unconfirmed_user_fixture() |> set_password()
+
+      assert {:error, :invalid_credentials} =
+               Accounts.authenticate_by_password(user.email, "wrong password!")
+    end
+
+    test "signs the user in once the email is confirmed" do
+      %{id: id} = user = unconfirmed_user_fixture() |> set_password()
+      {:ok, _} = Accounts.confirm_user(user)
+
+      assert {:ok, %User{id: ^id}} =
+               Accounts.authenticate_by_password(user.email, valid_user_password())
+    end
   end
 
   describe "get_user!/1" do
@@ -186,8 +215,34 @@ defmodule Gamend.AccountsTest do
         )
 
       assert user.is_admin
+      assert user.confirmed_at
       refute_enqueued(worker: ConfirmationMailer)
     end
+
+    test "the first user registered with a password signs in with it at once" do
+      email = unique_user_email()
+
+      {:ok, user} =
+        Accounts.register_user_with_password_and_deliver(
+          %{"email" => email, "password" => valid_user_password()},
+          fn t -> "http://x/#{t}" end
+        )
+
+      assert user.is_admin
+      assert {:ok, _} = Accounts.authenticate_by_password(email, valid_user_password())
+    end
+
+    test "later users start unconfirmed" do
+      _existing = user_fixture()
+
+      {:ok, user} =
+        Accounts.register_user_with_password_and_deliver(
+          valid_user_attributes(%{"password" => valid_user_password()}),
+          fn t -> "http://x/#{t}" end
+        )
+
+      refute user.confirmed_at
+    end
   end
 
   describe "find_or_create_from_device/2" do
@@ -515,14 +570,16 @@ defmodule Gamend.AccountsTest do
       assert {:error, :not_found} = Accounts.login_user_by_magic_link(encoded_token)
     end
 
-    test "raises when unconfirmed user has password set" do
-      user = unconfirmed_user_fixture()
-      {1, nil} = Repo.update_all(User, set: [hashed_password: "hashed"])
+    # The password was chosen before anyone proved they own the inbox, by
+    # whoever registered the address: it must not survive the owner claiming it.
+    test "confirms an unconfirmed user with a password, and removes the password" do
+      user = unconfirmed_user_fixture() |> set_password()
       {encoded_token, _hashed_token} = generate_user_magic_link_token(user)
 
-      assert_raise RuntimeError, ~r/magic link log in is not allowed/, fn ->
-        Accounts.login_user_by_magic_link(encoded_token)
-      end
+      assert {:ok, {user, _expired}} = Accounts.login_user_by_magic_link(encoded_token)
+      assert user.confirmed_at
+      refute user.hashed_password
+      refute Accounts.get_user_by_email_and_password(user.email, valid_user_password())
     end
   end
 

apps/gamend_core/test/gamend/password_hash_test.exs

@@ -55,10 +55,14 @@ defmodule Gamend.Accounts.PasswordHashTest do
       email = "legacy-#{System.unique_integer([:positive])}@example.com"
       {:ok, user} = Accounts.register_user(%{email: email, password: @password})
 
-      # Put the row back the way a pre-Argon2id database would hold it.
+      # Put the row back the way a pre-Argon2id database would hold it, on a
+      # confirmed account: an unconfirmed one does not sign in by password.
       {:ok, user} =
         user
-        |> Ecto.Changeset.change(hashed_password: Bcrypt.hash_pwd_salt(@password))
+        |> Ecto.Changeset.change(
+          hashed_password: Bcrypt.hash_pwd_salt(@password),
+          confirmed_at: DateTime.utc_now(:second)
+        )
         |> Repo.update()
 
       assert String.starts_with?(user.hashed_password, "$2")