fix(portal): Fixing a few edge cases for identity email (#7532)

elixir/apps/api/lib/api/controllers/identity_controller.ex

@@ -56,8 +56,6 @@ defmodule API.IdentityController do
         "provider_identifier_confirmation",
         Map.get(params, "provider_identifier")
       )
-      |> maybe_put_email()
-      |> maybe_put_identifier()
 
     with {:ok, actor} <- Domain.Actors.fetch_actor_by_id(actor_id, subject),
          {:ok, provider} <- Auth.fetch_provider_by_id(provider_id, subject),
@@ -142,51 +140,4 @@ defmodule API.IdentityController do
 
     {:provider_check, valid?}
   end
-
-  defp maybe_put_email(params) do
-    email =
-      params["email"]
-      |> to_string
-      |> String.trim()
-
-    identifier =
-      params["provider_identifier"]
-      |> to_string()
-      |> String.trim()
-
-    cond do
-      Domain.Auth.valid_email?(email) ->
-        params
-
-      Domain.Auth.valid_email?(identifier) ->
-        Map.put(params, "email", identifier)
-
-      true ->
-        params
-    end
-  end
-
-  defp maybe_put_identifier(params) do
-    email =
-      params["email"]
-      |> to_string()
-      |> String.trim()
-
-    identifier =
-      params["provider_identifier"]
-      |> to_string()
-      |> String.trim()
-
-    cond do
-      identifier != "" ->
-        params
-
-      Domain.Auth.valid_email?(email) ->
-        Map.put(params, "provider_identifier", email)
-        |> Map.put("provider_identifier_confirmation", email)
-
-      true ->
-        params
-    end
-  end
 end

elixir/apps/domain/lib/domain/auth.ex

@@ -435,6 +435,11 @@ defmodule Domain.Auth do
         %Provider{account_id: account_id} = provider,
         attrs
       ) do
+    attrs =
+      attrs
+      |> maybe_put_email()
+      |> maybe_put_identifier()
+
     Identity.Changeset.create_identity(actor, provider, attrs)
     |> Adapters.identity_changeset(provider)
     |> Repo.insert()
@@ -914,4 +919,51 @@ defmodule Domain.Auth do
     # Regex to check if string is in the shape of an email
     ~r/^[^\s]+@[^\s]+\.[^\s]+$/
   end
+
+  defp maybe_put_email(params) do
+    email =
+      params["email"]
+      |> to_string
+      |> String.trim()
+
+    identifier =
+      params["provider_identifier"]
+      |> to_string()
+      |> String.trim()
+
+    cond do
+      valid_email?(email) ->
+        params
+
+      valid_email?(identifier) ->
+        Map.put(params, "email", identifier)
+
+      true ->
+        params
+    end
+  end
+
+  defp maybe_put_identifier(params) do
+    email =
+      params["email"]
+      |> to_string()
+      |> String.trim()
+
+    identifier =
+      params["provider_identifier"]
+      |> to_string()
+      |> String.trim()
+
+    cond do
+      identifier != "" ->
+        params
+
+      valid_email?(email) ->
+        Map.put(params, "provider_identifier", email)
+        |> Map.put("provider_identifier_confirmation", email)
+
+      true ->
+        params
+    end
+  end
 end

elixir/apps/domain/lib/domain/auth/identity/changeset.ex

@@ -67,7 +67,7 @@ defmodule Domain.Auth.Identity.Changeset do
       name: :auth_identities_account_id_provider_id_provider_identifier_idx
     )
     |> unique_constraint(:email,
-      name: :auth_identities_account_id_provider_id_email_idx
+      name: :auth_identities_acct_id_provider_id_email_prov_ident_unique_idx
     )
   end
 

elixir/apps/domain/priv/repo/migrations/20241216183520_migrate_email_data_again.exs

@@ -0,0 +1,21 @@
+defmodule Domain.Repo.Migrations.MigrateEmailDataAgain do
+  use Ecto.Migration
+
+  def change do
+    execute("""
+      UPDATE auth_identities AS ai
+      SET email =
+        CASE
+          WHEN p.adapter = 'email' OR p.adapter = 'userpass' THEN ai.provider_identifier
+          ELSE COALESCE(
+            provider_state #>> '{claims,email}',
+            provider_state #>> '{userinfo,email}'
+          )
+        END
+      FROM auth_providers AS p
+      WHERE ai.provider_id = p.id
+        AND ai.email IS NULL
+        AND ai.deleted_at IS NULL
+    """)
+  end
+end

elixir/apps/domain/test/domain/auth_test.exs

@@ -2358,6 +2358,41 @@ defmodule Domain.AuthTest do
       assert identity.provider_id == provider.id
       assert identity.provider_identifier == provider_identifier
       assert identity.actor_id == actor.id
+      assert identity.email == nil
+
+      assert %Ecto.Changeset{} = identity.provider_virtual_state
+
+      assert %{"password_hash" => _} = identity.provider_state
+      assert %{password_hash: _} = identity.provider_virtual_state.changes
+      assert identity.account_id == provider.account_id
+      assert is_nil(identity.deleted_at)
+    end
+
+    test "creates an identity when provider_identifier is an email address" do
+      account = Fixtures.Accounts.create_account()
+      provider = Fixtures.Auth.create_userpass_provider(account: account)
+      provider_identifier = Fixtures.Auth.email()
+
+      actor =
+        Fixtures.Actors.create_actor(
+          type: :account_admin_user,
+          account: account,
+          provider: provider
+        )
+
+      password = "Firezone1234"
+
+      attrs = %{
+        "provider_identifier" => provider_identifier,
+        "provider_virtual_state" => %{"password" => password, "password_confirmation" => password}
+      }
+
+      assert {:ok, identity} = create_identity(actor, provider, attrs)
+
+      assert identity.provider_id == provider.id
+      assert identity.provider_identifier == provider_identifier
+      assert identity.actor_id == actor.id
+      assert identity.email == provider_identifier
 
       assert %Ecto.Changeset{} = identity.provider_virtual_state