github-personal/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-01-27 10:18:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(portal): Add email as separate column on auth_identities table (#7472)

Browse Source 
Why:

* Currently, when using the API, a user has no way of easily identifying
what identities they are pulling back as the response only includes the
`provider_identifier` which for most of our AuthProviders is an ID for
the IdP and not an email address. Along with that, when adding users to
an OIDC provider within Firezone, there is no check for whether or not
an identity has already been added with a given email address. By
creating a separate email column on the `auth_identities` table, it will
be very straight forward to know whether an email address exists for a
given identity, return it in an API response and allow the admin of a
Firezone account to track users (Identities) by email rather than IdP
identifier.

Fixes #7392
This commit is contained in:
Brian Manifold
2024-12-13 09:26:47 -08:00
committed by GitHub
parent b63061994d
commit f114bc95cd
21 changed files with 415 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
elixir/apps/api/lib/api/controllers/identity_controller.ex
View File

@@ -56,6 +56,8 @@ 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),
@@ -140,4 +142,51 @@ 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

3
elixir/apps/api/lib/api/controllers/identity_json.ex
View File

@@ -24,7 +24,8 @@ defmodule API.IdentityJSON do
id: identity.id,
actor_id: identity.actor_id,
provider_id: identity.provider_id,
provider_identifier: identity.provider_identifier
provider_identifier: identity.provider_identifier,
email: identity.email
}
end
end

20
elixir/apps/api/lib/api/schemas/identity_schema.ex
View File

@@ -13,14 +13,16 @@ defmodule API.Schemas.Identity do
id: %Schema{type: :string, description: "Identity ID"},
actor_id: %Schema{type: :string, description: "Actor ID"},
provider_id: %Schema{type: :string, description: "Identity Provider ID"},
provider_identifier: %Schema{type: :string, description: "Identifier from Provider"}
provider_identifier: %Schema{type: :string, description: "Identifier from Provider"},
email: %Schema{type: :string, description: "Email"}
},
required: [:id, :actor_id, :provider_id, :provider_identifier],
required: [:id, :actor_id, :provider_id, :provider_identifier, :email],
example: %{
"id" => "42a7f82f-831a-4a9d-8f17-c66c2bb6e205",
"actor_id" => "cdfa97e6-cca1-41db-8fc7-864daedb46df",
"provider_id" => "989f9e96-e348-47ec-ba85-869fcd7adb19",
"provider_identifier" => "foo@bar.com"
"provider_identifier" => "2551705710219359",
"email" => "foo@bar.com"
}
})
end
@@ -40,7 +42,8 @@ defmodule API.Schemas.Identity do
required: [:identity],
example: %{
"identity" => %{
"provider_identifier" => "foo@bar.com"
"provider_identifier" => "2551705710219359",
"email" => "foo@bar.com"
}
}
})
@@ -63,7 +66,8 @@ defmodule API.Schemas.Identity do
"id" => "42a7f82f-831a-4a9d-8f17-c66c2bb6e205",
"actor_id" => "cdfa97e6-cca1-41db-8fc7-864daedb46df",
"provider_id" => "989f9e96-e348-47ec-ba85-869fcd7adb19",
"provider_identifier" => "foo@bar.com"
"provider_identifier" => "2551705710219359",
"email" => "foo@bar.com"
}
}
})
@@ -88,13 +92,15 @@ defmodule API.Schemas.Identity do
"id" => "42a7f82f-831a-4a9d-8f17-c66c2bb6e205",
"actor_id" => "8f44a02b-b8eb-406f-8202-4274bf60ebd0",
"provider_id" => "6472d898-5b98-4c3b-b4b9-d3158c1891be",
"provider_identifier" => "foo@bar.com"
"provider_identifier" => "2551705710219359",
"email" => "foo@bar.com"
},
%{
"id" => "8a70eb96-e74b-4cdc-91b8-48c05ef74d4c",
"actor_id" => "38c92cda-1ddb-45b3-9d1a-7efc375e00c1",
"provider_id" => "04f13eed-4845-47c3-833e-fdd869fab96f",
"provider_identifier" => "baz@bar.com"
"provider_identifier" => "2638957392736483",
"email" => "baz@bar.com"
}
],
"metadata" => %{

223
elixir/apps/api/test/api/controllers/identity_controller_test.exs
View File

@@ -49,7 +49,7 @@ defmodule API.IdentityControllerTest do
assert equal_ids?(data_ids, identity_ids)
end
test "lists resources with limit", %{conn: conn, account: account, actor: actor} do
test "lists identities with limit", %{conn: conn, account: account, actor: actor} do
identities =
for _ <- 1..3, do: Fixtures.Auth.create_identity(%{account: account, actor: actor})
@@ -88,7 +88,70 @@ defmodule API.IdentityControllerTest do
assert json_response(conn, 401) == %{"error" => %{"reason" => "Unauthorized"}}
end
test "returns a single resource", %{conn: conn, account: account, actor: actor} do
test "returns a single identity with populated email field", %{
conn: conn,
account: account,
actor: actor
} do
identity =
Fixtures.Auth.create_identity(%{
account: account,
actor: actor,
provider_identifier: "172836495673",
email: "foo@bar.com"
})
conn =
conn
|> authorize_conn(actor)
|> put_req_header("content-type", "application/json")
|> get("/actors/#{actor.id}/identities/#{identity.id}")
assert json_response(conn, 200) == %{
"data" => %{
"id" => identity.id,
"actor_id" => actor.id,
"provider_id" => identity.provider_id,
"provider_identifier" => identity.provider_identifier,
"email" => identity.email
}
}
end
test "returns a single identity with populated email field from provider_identifier", %{
conn: conn,
account: account,
actor: actor
} do
identity =
Fixtures.Auth.create_identity(%{
account: account,
actor: actor,
provider_identifier: "foo@bar.com"
})
conn =
conn
|> authorize_conn(actor)
|> put_req_header("content-type", "application/json")
|> get("/actors/#{actor.id}/identities/#{identity.id}")
assert json_response(conn, 200) == %{
"data" => %{
"id" => identity.id,
"actor_id" => actor.id,
"provider_id" => identity.provider_id,
"provider_identifier" => identity.provider_identifier,
"email" => identity.provider_identifier
}
}
end
test "returns a single identity with empty email field", %{
conn: conn,
account: account,
actor: actor
} do
identity = Fixtures.Auth.create_identity(%{account: account, actor: actor})
conn =
@@ -102,7 +165,8 @@ defmodule API.IdentityControllerTest do
"id" => identity.id,
"actor_id" => actor.id,
"provider_id" => identity.provider_id,
"provider_identifier" => identity.provider_identifier
"provider_identifier" => identity.provider_identifier,
"email" => nil
}
}
end
@@ -158,7 +222,7 @@ defmodule API.IdentityControllerTest do
assert resp == %{"error" => %{"reason" => "Not Found"}}
end
test "returns error on invalid identity attrs", %{
test "returns error on empty identity attrs", %{
conn: conn,
account: account,
actor: api_actor
@@ -189,7 +253,7 @@ defmodule API.IdentityControllerTest do
}
end
test "creates a resource with valid attrs", %{
test "returns error on invalid identity attrs", %{
conn: conn,
account: account,
actor: api_actor
@@ -199,7 +263,37 @@ defmodule API.IdentityControllerTest do
actor = Fixtures.Actors.create_actor(account: account)
attrs = %{"provider_identifier" => "foo@local"}
attrs = %{email: "foo"}
conn =
conn
|> authorize_conn(api_actor)
|> put_req_header("content-type", "application/json")
|> post("/actors/#{actor.id}/providers/#{oidc_provider.id}/identities",
identity: attrs
)
assert resp = json_response(conn, 422)
assert resp == %{
"error" => %{
"reason" => "Unprocessable Entity",
"validation_errors" => %{"provider_identifier" => ["can't be blank"]}
}
}
end
test "creates an identity with provider_identifier attr only and is not an email address", %{
conn: conn,
account: account,
actor: api_actor
} do
{oidc_provider, _bypass} =
Fixtures.Auth.start_and_create_openid_connect_provider(account: account)
actor = Fixtures.Actors.create_actor(account: account)
attrs = %{"provider_identifier" => "128asdf92qrh9joqwefoiu23"}
conn =
conn
@@ -210,10 +304,116 @@ defmodule API.IdentityControllerTest do
)
assert resp = json_response(conn, 201)
assert resp["data"]["provider_identifier"] == attrs["provider_identifier"]
assert resp["data"]["provider_id"] == oidc_provider.id
assert resp["data"]["actor_id"] == actor.id
assert resp["data"]["email"] == nil
end
test "creates an identity with provider_identifier attr only and is an email address", %{
conn: conn,
account: account,
actor: api_actor
} do
{oidc_provider, _bypass} =
Fixtures.Auth.start_and_create_openid_connect_provider(account: account)
actor = Fixtures.Actors.create_actor(account: account)
attrs = %{"provider_identifier" => "foo@localhost.local"}
conn =
conn
|> authorize_conn(api_actor)
|> put_req_header("content-type", "application/json")
|> post("/actors/#{actor.id}/providers/#{oidc_provider.id}/identities",
identity: attrs
)
assert resp = json_response(conn, 201)
assert resp["data"]["provider_identifier"] == attrs["provider_identifier"]
assert resp["data"]["email"] == attrs["provider_identifier"]
end
test "creates an identity with email attr only and populates provider_identifier", %{
conn: conn,
account: account,
actor: api_actor
} do
{oidc_provider, _bypass} =
Fixtures.Auth.start_and_create_openid_connect_provider(account: account)
actor = Fixtures.Actors.create_actor(account: account)
attrs = %{"email" => "foo@localhost.local"}
conn =
conn
|> authorize_conn(api_actor)
|> put_req_header("content-type", "application/json")
|> post("/actors/#{actor.id}/providers/#{oidc_provider.id}/identities",
identity: attrs
)
assert resp = json_response(conn, 201)
assert resp["data"]["provider_identifier"] == attrs["email"]
assert resp["data"]["email"] == attrs["email"]
end
test "creates an identity with provider_identifier attr and email attr being the same value",
%{
conn: conn,
account: account,
actor: api_actor
} do
{oidc_provider, _bypass} =
Fixtures.Auth.start_and_create_openid_connect_provider(account: account)
actor = Fixtures.Actors.create_actor(account: account)
attrs = %{
"provider_identifier" => "foo@localhost.local",
"email" => "foo@localhost.local"
}
conn =
conn
|> authorize_conn(api_actor)
|> put_req_header("content-type", "application/json")
|> post("/actors/#{actor.id}/providers/#{oidc_provider.id}/identities",
identity: attrs
)
assert resp = json_response(conn, 201)
assert resp["data"]["provider_identifier"] == attrs["provider_identifier"]
assert resp["data"]["email"] == attrs["email"]
end
test "creates an identity with provider_identifier attr and email attr being different values",
%{
conn: conn,
account: account,
actor: api_actor
} do
{oidc_provider, _bypass} =
Fixtures.Auth.start_and_create_openid_connect_provider(account: account)
actor = Fixtures.Actors.create_actor(account: account)
attrs = %{
"provider_identifier" => "foo@localhost.local",
"email" => "bar@localhost.local"
}
conn =
conn
|> authorize_conn(api_actor)
|> put_req_header("content-type", "application/json")
|> post("/actors/#{actor.id}/providers/#{oidc_provider.id}/identities",
identity: attrs
)
assert resp = json_response(conn, 201)
assert resp["data"]["provider_identifier"] == attrs["provider_identifier"]
assert resp["data"]["email"] == attrs["email"]
end
end
@@ -224,7 +424,7 @@ defmodule API.IdentityControllerTest do
assert json_response(conn, 401) == %{"error" => %{"reason" => "Unauthorized"}}
end
test "deletes a resource", %{conn: conn, account: account, actor: actor} do
test "deletes an identity", %{conn: conn, account: account, actor: actor} do
identity = Fixtures.Auth.create_identity(%{account: account, actor: actor})
conn =
@@ -238,7 +438,8 @@ defmodule API.IdentityControllerTest do
"id" => identity.id,
"actor_id" => actor.id,
"provider_id" => identity.provider_id,
"provider_identifier" => identity.provider_identifier
"provider_identifier" => identity.provider_identifier,
"email" => nil
}
}

10
elixir/apps/domain/lib/domain/auth.ex
View File

@@ -626,6 +626,7 @@ defmodule Domain.Auth do
{:error, :not_found} -> {:error, :unauthorized}
{:error, :invalid} -> {:error, :unauthorized}
{:error, :expired} -> {:error, :unauthorized}
{:error, :internal_error} -> {:error, :internal_error}
{:error, %Ecto.Changeset{}} -> {:error, :malformed_request}
end
end
@@ -904,4 +905,13 @@ defmodule Domain.Auth do
granted_permissions = fetch_type_permissions!(granted_role)
MapSet.subset?(granted_permissions, subject.permissions)
end
def valid_email?(email) do
to_string(email) =~ email_regex()
end
def email_regex do
# Regex to check if string is in the shape of an email
~r/^[^\s]+@[^\s]+\.[^\s]+$/
end
end

22
elixir/apps/domain/lib/domain/auth/adapters.ex
View File

@@ -101,11 +101,23 @@ defmodule Domain.Auth.Adapters do
adapter = fetch_provider_adapter!(provider)
case adapter.verify_and_update_identity(provider, payload) do
{:ok, %Identity{} = identity, expires_at} -> {:ok, identity, expires_at}
{:error, :not_found} -> {:error, :not_found}
{:error, :invalid} -> {:error, :invalid}
{:error, :expired} -> {:error, :expired}
{:error, :internal_error} -> {:error, :internal_error}
{:ok, %Identity{} = identity, expires_at} ->
{:ok, identity, expires_at}
{:error, :not_found} ->
{:error, :not_found}
{:error, :invalid} ->
{:error, :invalid}
{:error, :expired} ->
{:error, :expired}
{:error, :internal_error} ->
{:error, :internal_error}
{:error, %Ecto.Changeset{} = changeset} ->
{:error, changeset}
end
end

1
elixir/apps/domain/lib/domain/auth/adapters/google_workspace.ex
View File

@@ -38,6 +38,7 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace do
@impl true
def identity_changeset(%Provider{} = _provider, %Ecto.Changeset{} = changeset) do
changeset
|> Domain.Repo.Changeset.trim_change(:email)
|> Domain.Repo.Changeset.trim_change(:provider_identifier)
|> Domain.Repo.Changeset.copy_change(:provider_virtual_state, :provider_state)
|> Ecto.Changeset.put_change(:provider_virtual_state, %{})

1
elixir/apps/domain/lib/domain/auth/adapters/jumpcloud.ex
View File

@@ -36,6 +36,7 @@ defmodule Domain.Auth.Adapters.JumpCloud do
@impl true
def identity_changeset(%Provider{} = _provider, %Ecto.Changeset{} = changeset) do
changeset
|> Domain.Repo.Changeset.trim_change(:email)
|> Domain.Repo.Changeset.trim_change(:provider_identifier)
|> Domain.Repo.Changeset.copy_change(:provider_virtual_state, :provider_state)
|> Ecto.Changeset.put_change(:provider_virtual_state, %{})

1
elixir/apps/domain/lib/domain/auth/adapters/microsoft_entra.ex
View File

@@ -37,6 +37,7 @@ defmodule Domain.Auth.Adapters.MicrosoftEntra do
@impl true
def identity_changeset(%Provider{} = _provider, %Ecto.Changeset{} = changeset) do
changeset
|> Domain.Repo.Changeset.trim_change(:email)
|> Domain.Repo.Changeset.trim_change(:provider_identifier)
|> Domain.Repo.Changeset.copy_change(:provider_virtual_state, :provider_state)
|> Ecto.Changeset.put_change(:provider_virtual_state, %{})

1
elixir/apps/domain/lib/domain/auth/adapters/okta.ex
View File

@@ -37,6 +37,7 @@ defmodule Domain.Auth.Adapters.Okta do
@impl true
def identity_changeset(%Provider{} = _provider, %Ecto.Changeset{} = changeset) do
changeset
|> Domain.Repo.Changeset.trim_change(:email)
|> Domain.Repo.Changeset.trim_change(:provider_identifier)
|> Domain.Repo.Changeset.copy_change(:provider_virtual_state, :provider_state)
|> Ecto.Changeset.put_change(:provider_virtual_state, %{})

14
elixir/apps/domain/lib/domain/auth/adapters/openid_connect.ex
View File

@@ -32,6 +32,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do
@impl true
def identity_changeset(%Provider{} = _provider, %Ecto.Changeset{} = changeset) do
changeset
|> Domain.Repo.Changeset.trim_change(:email)
|> Domain.Repo.Changeset.trim_change(:provider_identifier)
|> Domain.Repo.Changeset.copy_change(:provider_virtual_state, :provider_state)
|> Ecto.Changeset.put_change(:provider_virtual_state, %{})
@@ -119,7 +120,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do
code_verifier: code_verifier
}
with {:ok, provider_identifier, identity_state} <-
with {:ok, provider_identifier, email, identity_state} <-
fetch_state(provider, token_params, identifier_claim) do
Identity.Query.not_disabled()
|> Identity.Query.by_provider_id(provider.id)
@@ -134,6 +135,8 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do
# if an email was used in provider identifier and it's replaced by sub claim
# later, we want to use the ID from sub claim as provider_identifier
|> Ecto.Changeset.put_change(:provider_identifier, provider_identifier)
|> Ecto.Changeset.put_change(:email, email)
|> Identity.Changeset.changeset()
end
)
|> case do
@@ -177,9 +180,10 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do
code_verifier: code_verifier
}
with {:ok, provider_identifier, identity_state} <-
with {:ok, provider_identifier, email, identity_state} <-
fetch_state(provider, token_params, identifier_claim) do
Domain.Auth.upsert_identity(actor, provider, %{
email: email,
provider_identifier: provider_identifier,
provider_virtual_state: identity_state
})
@@ -195,7 +199,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do
refresh_token: provider.adapter_state["refresh_token"]
}
with {:ok, _provider_identifier, adapter_state} <-
with {:ok, _provider_identifier, _email, adapter_state} <-
fetch_state(provider, token_params, identifier_claim) do
Provider.Query.not_deleted()
|> Provider.Query.by_id(provider.id)
@@ -247,7 +251,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do
refresh_token: identity.provider_state["refresh_token"]
}
with {:ok, _provider_identifier, identity_state} <-
with {:ok, _provider_identifier, _email, identity_state} <-
fetch_state(identity.provider, token_params, identifier_claim) do
Identity.Query.not_deleted()
|> Identity.Query.by_id(identity.id)
@@ -281,7 +285,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do
provider_identifier = claims[identifier_claim]
{:ok, provider_identifier,
{:ok, provider_identifier, claims["email"] || userinfo["email"],
%{
"access_token" => tokens["access_token"],
"refresh_token" => tokens["refresh_token"],

1
elixir/apps/domain/lib/domain/auth/identity.ex
View File

@@ -5,6 +5,7 @@ defmodule Domain.Auth.Identity do
belongs_to :actor, Domain.Actors.Actor, on_replace: :update
belongs_to :provider, Domain.Auth.Provider
field :email, :string
field :provider_identifier, :string
field :provider_state, :map, redact: true
field :provider_virtual_state, :map, virtual: true, redact: true

9
elixir/apps/domain/lib/domain/auth/identity/changeset.ex
View File

@@ -21,7 +21,7 @@ defmodule Domain.Auth.Identity.Changeset do
attrs
) do
%Identity{}
|> cast(attrs, ~w[provider_identifier provider_virtual_state]a)
|> cast(attrs, ~w[email provider_identifier provider_virtual_state]a)
|> validate_required(~w[provider_identifier]a)
|> put_change(:actor_id, actor.id)
|> put_change(:provider_id, provider.id)
@@ -35,7 +35,7 @@ defmodule Domain.Auth.Identity.Changeset do
attrs
) do
%Identity{}
|> cast(attrs, ~w[provider_identifier provider_state provider_virtual_state]a)
|> cast(attrs, ~w[email provider_identifier provider_state provider_virtual_state]a)
|> validate_required(~w[provider_identifier]a)
|> cast_assoc(:actor,
with: fn _actor, attrs ->
@@ -51,7 +51,7 @@ defmodule Domain.Auth.Identity.Changeset do
def update_identity_and_actor(%Identity{} = identity, attrs) do
identity
|> cast(attrs, ~w[provider_state]a)
|> cast(attrs, ~w[email provider_state]a)
|> cast_assoc(:actor,
with: fn actor, attrs ->
Actors.Actor.Changeset.sync(actor, attrs)
@@ -66,6 +66,9 @@ defmodule Domain.Auth.Identity.Changeset do
|> unique_constraint(:provider_identifier,
name: :auth_identities_account_id_provider_id_provider_identifier_idx
)
|> unique_constraint(:email,
name: :auth_identities_account_id_provider_id_email_idx
)
end
def update_identity_provider_state(identity_or_changeset, %{} = state, virtual_state \\ %{}) do

23
elixir/apps/domain/lib/domain/auth/identity/sync.ex
View File

@@ -95,7 +95,10 @@ defmodule Domain.Auth.Identity.Sync do
provider_identifiers_to_insert
|> Enum.uniq()
|> Enum.reduce_while({:ok, []}, fn provider_identifier, {:ok, acc} ->
attrs = Map.get(attrs_by_provider_identifier, provider_identifier)
attrs =
Map.get(attrs_by_provider_identifier, provider_identifier)
|> add_email_attr()
changeset = Identity.Changeset.create_identity_and_actor(provider, attrs)
case Repo.insert(changeset) do
@@ -122,7 +125,7 @@ defmodule Domain.Auth.Identity.Sync do
|> Enum.reduce(%{}, fn identity, acc ->
acc_identity = Map.get(acc, identity.provider_identifier)
# make sure that deleted identities are have the least priority in case of conflicts
# make sure that deleted identities have the least priority in case of conflicts
cond do
is_nil(acc_identity) ->
Map.put(acc, identity.provider_identifier, identity)
@@ -139,7 +142,11 @@ defmodule Domain.Auth.Identity.Sync do
|> Enum.uniq()
|> Enum.reduce_while({:ok, []}, fn provider_identifier, {:ok, acc} ->
identity = Map.get(identity_by_provider_identifier, provider_identifier)
attrs = Map.get(attrs_by_provider_identifier, provider_identifier)
attrs =
Map.get(attrs_by_provider_identifier, provider_identifier)
|> add_email_attr()
changeset = Identity.Changeset.update_identity_and_actor(identity, attrs)
case Repo.update(changeset) do
@@ -151,4 +158,14 @@ defmodule Domain.Auth.Identity.Sync do
end
end)
end
defp add_email_attr(attrs) do
email = attrs["provider_state"]["userinfo"]["email"] || ""
if Domain.Auth.valid_email?(email) do
Map.put(attrs, "email", email)
else
attrs
end
end
end

2
elixir/apps/domain/lib/domain/repo/changeset.ex
View File

@@ -206,7 +206,7 @@ defmodule Domain.Repo.Changeset do
def validate_email(%Ecto.Changeset{} = changeset, field) do
changeset
|> validate_format(field, ~r/^[^\s]+@[^\s]+\.[^\s]+$/, message: "is an invalid email address")
|> validate_format(field, Domain.Auth.email_regex(), message: "is an invalid email address")
|> validate_length(field, max: 160)
end

9
elixir/apps/domain/priv/repo/migrations/20241120171334_add_identity_email_column.exs Normal file
View File

@@ -0,0 +1,9 @@
defmodule Domain.Repo.Migrations.AddIdentityEmailColumn do
use Ecto.Migration
def change do
alter table(:auth_identities) do
add(:email, :citext)
end
end
end

13
elixir/apps/domain/priv/repo/migrations/20241126185037_add_identity_email_unique_index.exs Normal file
View File

@@ -0,0 +1,13 @@
defmodule Domain.Repo.Migrations.AddIdentityEmailUniqueIndex do
use Ecto.Migration
def change do
create(
index(:auth_identities, [:account_id, :provider_id, :email],
name: :auth_identities_account_id_provider_id_email_idx,
where: "deleted_at IS NULL",
unique: true
)
)
end
end

22
elixir/apps/domain/priv/repo/migrations/20241205165118_migrate_email_data.exs Normal file
View File

@@ -0,0 +1,22 @@
defmodule Domain.Repo.Migrations.MigrateEmailData do
use Ecto.Migration
def change do
execute("""
UPDATE auth_identities AS ai
SET email = COALESCE(
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

14
elixir/apps/domain/test/domain/auth/adapters/openid_connect_test.exs
View File

@@ -304,7 +304,9 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do
assert DateTime.diff(identity.provider_state["expires_at"], DateTime.utc_now()) in 3595..3605
end
test "prefers existing identities over manually created identities on a conflict", %{
# NOTE: The only time this should happen is if an IdP reuses an email address
# that is already in use in Firezone for a given provider
test "return error on identity conflict", %{
account: account,
provider: provider,
bypass: bypass
@@ -312,7 +314,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do
email = Fixtures.Auth.email()
sub = Ecto.UUID.generate()
existing_identity =
_existing_identity =
Fixtures.Auth.create_identity(
account: account,
provider: provider,
@@ -346,13 +348,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do
redirect_uri = "https://example.com/"
payload = {redirect_uri, code_verifier, "MyFakeCode"}
assert {:ok, identity, _expires_at} = verify_and_update_identity(provider, payload)
assert identity.id == existing_identity.id
assert identity.provider_state["access_token"] == "MY_ACCESS_TOKEN"
assert identity.provider_state["refresh_token"] == "MY_REFRESH_TOKEN"
assert DateTime.diff(identity.provider_state["expires_at"], DateTime.utc_now()) in 3595..3605
assert {:error, %Ecto.Changeset{}} = verify_and_update_identity(provider, payload)
end
test "verifies newly created identities by email profile field", %{

10
elixir/apps/domain/test/support/fixtures/auth.ex
View File

@@ -446,8 +446,18 @@ defmodule Domain.Fixtures.Auth do
|> Fixtures.Actors.create_actor()
end)
{email, attrs} =
Map.pop_lazy(attrs, :email, fn ->
if Domain.Auth.valid_email?(provider_identifier) do
provider_identifier
else
nil
end
end)
attrs = Map.put(attrs, :provider_identifier, provider_identifier)
attrs = Map.put(attrs, :provider_identifier_confirmation, provider_identifier)
attrs = Map.put(attrs, :email, email)
{:ok, identity} = Auth.upsert_identity(actor, provider, attrs)

12
elixir/apps/web/lib/web/live/actors/users/new_identity.ex
View File

@@ -99,6 +99,8 @@ defmodule Web.Actors.Users.NewIdentity do
end
def handle_event("submit", %{"identity" => attrs}, socket) do
attrs = add_email(attrs)
with {:ok, identity} <-
Auth.create_identity(
socket.assigns.actor,
@@ -130,4 +132,14 @@ defmodule Web.Actors.Users.NewIdentity do
_ -> ~p"/#{socket.assigns.account}/actors/#{socket.assigns.actor}"
end
end
defp add_email(attrs) do
identifier = attrs["provider_identifier"]
if Domain.Auth.valid_email?(identifier) do
Map.put(attrs, "email", identifier)
else
Map.put(attrs, "email", nil)
end
end
end