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

fix(portal): Resurrect deleted identities and groups (#8615)

Browse Source 
When syncing identities from an identity, we have logic in place that
resurrects any soft-deleted identities in order to maintain their
session history, group memberships and any other relevant data. Users
can be temporarily suspended from their identity provider and then
resumed.

Groups, however, based on cursory research, can never be temporarily
suspended at the identity provider. However, this doesn't mean that we
can't see the group disappear and reappear at a later point in time.
This can happen due to a temporary sync issue, or in the upcoming Group
Filters PR: #8381.

This PR adds more robust testing to ensure we can in fact resurrect
identities as expected.

It also updates the group sync logic to similarly resurrect soft-deleted
groups if they are seen again in a subsequent sync.

To achieve this, we need to update the `UNIQUE CONSTRAINT` used in the
upsert clause during the sync. Before, it was possible for two (or more)
groups to exist with the same provider_identifier and provider_id, if
`deleted_at IS NOT NULL`. Now, we need to ensure that only one group
with the same `account_id, provider_id, provider_identifier` can exist,
since we want to resurrect and not recreate these.

To do this, we use a migration that does the following:

1. Ensures any potentially problematic data is permanently deleted
2. Drops the existing unique constraint
3. Recreates it, omitting `WHERE DELETED_AT IS NULL` from the partial
index.

Based on exploring the production DB data, this should not cause any
issues, but it would be a good idea to double-check before rolling this
out to prod.


Lastly, the final missing piece to the resurrection story is Policies.
This is saved for a future PR since we need to first define the
difference between a policy that was soft-deleted via a sync job, and a
policy that was "perma" deleted by a user.

Related: #8187
This commit is contained in:
Jamil
2025-04-02 14:12:44 -07:00
committed by GitHub
parent 3c99040c86
commit f275bf70d9
7 changed files with 609 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
elixir/apps/domain/lib/domain/actors/group/changeset.ex
View File

@@ -6,10 +6,10 @@ defmodule Domain.Actors.Group.Changeset do
def upsert_conflict_target do
{:unsafe_fragment,
"(account_id, provider_id, provider_identifier) " <>
"WHERE deleted_at IS NULL AND provider_id IS NOT NULL AND provider_identifier IS NOT NULL"}
"WHERE provider_id IS NOT NULL AND provider_identifier IS NOT NULL"}
end
def upsert_on_conflict, do: {:replace, ~w[name updated_at]a}
def upsert_on_conflict, do: {:replace, ~w[name updated_at deleted_at]a}
def create(%Accounts.Account{} = account, attrs, %Auth.Subject{} = subject) do
%Actors.Group{memberships: []}
@@ -41,6 +41,8 @@ defmodule Domain.Actors.Group.Changeset do
|> changeset()
|> put_change(:provider_id, provider.id)
|> put_change(:account_id, provider.account_id)
# resurrect synced groups
|> put_change(:deleted_at, nil)
|> put_change(:created_by, :provider)
end

2
elixir/apps/domain/lib/domain/actors/group/sync.ex
View File

@@ -36,7 +36,7 @@ defmodule Domain.Actors.Group.Sync do
defp all_provider_groups(provider) do
groups =
Group.Query.not_deleted()
Group.Query.all()
|> Group.Query.by_account_id(provider.account_id)
|> Group.Query.by_provider_id(provider.id)
|> Repo.all()

43
elixir/apps/domain/priv/repo/migrations/20250402071836_remove_duplicate_groups.exs Normal file
View File

@@ -0,0 +1,43 @@
defmodule Domain.Repo.Migrations.RemoveDuplicateGroups do
use Ecto.Migration
def change do
# Due to a bug we had where we mistakenly returned an empty list for
# group API fetches, we ended up deleting all groups for a particular customer.
# We need to clean these up and fix the index such that it won't happen again.
# Step 1: Remove all duplicate deleted groups
execute("""
DELETE FROM actor_groups
WHERE id IN (
SELECT a.id
FROM actor_groups a
INNER JOIN actor_groups b
ON a.account_id = b.account_id
AND a.provider_id = b.provider_id
AND a.provider_identifier = b.provider_identifier
WHERE a.deleted_at IS NOT NULL
AND b.deleted_at IS NULL
AND a.provider_id IS NOT NULL
AND a.provider_identifier IS NOT NULL
)
""")
# Step 2: Drop existing index
drop(
index(:actor_groups, [:account_id, :provider_id, :provider_identifier],
unique: true,
where:
"deleted_at IS NULL AND provider_id IS NOT NULL AND provider_identifier IS NOT NULL"
)
)
# Step 3: Create new index
create(
index(:actor_groups, [:account_id, :provider_id, :provider_identifier],
unique: true,
where: "provider_id IS NOT NULL AND provider_identifier IS NOT NULL"
)
)
end
end

163
elixir/apps/domain/test/domain/auth/adapters/google_workspace/jobs/sync_directory_test.exs
View File

@@ -750,6 +750,169 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.Jobs.SyncDirectoryTest do
refute_received {:expire_flow, _flow_id, _client_id, _resource_id}
end
test "resurrects deleted identities that reappear on the next sync", %{
account: account,
provider: provider
} do
actor = Fixtures.Actors.create_actor(account: account)
provider_identifier = "USER_ID1"
identity =
Fixtures.Auth.create_identity(
account: account,
provider: provider,
actor: actor,
provider_identifier: provider_identifier
)
inserted_at = identity.inserted_at
id = identity.id
# Soft delete the identity
Repo.update_all(Domain.Auth.Identity, set: [deleted_at: DateTime.utc_now()])
assert Domain.Auth.all_identities_for(actor) == []
# Simulate a sync
bypass = Bypass.open()
users = [
%{
"agreedToTerms" => true,
"archived" => false,
"creationTime" => "2023-06-10T17:32:06.000Z",
"id" => "USER_ID1",
"kind" => "admin#directory#user",
"lastLoginTime" => "2023-06-26T13:53:30.000Z",
"name" => %{
"familyName" => "Manifold",
"fullName" => "Brian Manifold",
"givenName" => "Brian"
},
"orgUnitPath" => "/Engineering",
"organizations" => [],
"phones" => [],
"primaryEmail" => "b@firez.xxx"
}
]
GoogleWorkspaceDirectory.override_endpoint_url("http://localhost:#{bypass.port}/")
GoogleWorkspaceDirectory.mock_groups_list_endpoint(
bypass,
200,
Jason.encode!(%{"groups" => []})
)
GoogleWorkspaceDirectory.mock_organization_units_list_endpoint(
bypass,
200,
Jason.encode!(%{"organizationUnits" => []})
)
GoogleWorkspaceDirectory.mock_users_list_endpoint(
bypass,
200,
Jason.encode!(%{"users" => users})
)
GoogleWorkspaceDirectory.mock_token_endpoint(bypass)
{:ok, pid} = Task.Supervisor.start_link()
assert execute(%{task_supervisor: pid}) == :ok
# Assert that the identity has been resurrected
assert resurrected_identity = Repo.get(Domain.Auth.Identity, id)
assert resurrected_identity.inserted_at == inserted_at
assert resurrected_identity.id == id
assert resurrected_identity.deleted_at == nil
assert Domain.Auth.all_identities_for(actor) == [resurrected_identity]
end
test "resurrects deleted groups that reappear on the next sync", %{
account: account,
provider: provider
} do
actor_group =
Fixtures.Actors.create_group(
account: account,
provider: provider,
provider_identifier: "G:GROUP_ID1"
)
inserted_at = actor_group.inserted_at
id = actor_group.id
# Soft delete the group
Repo.update_all(Domain.Actors.Group, set: [deleted_at: DateTime.utc_now()])
# Assert that the group and associated policy has been soft-deleted
assert Domain.Actors.Group.Query.not_deleted() |> Repo.all() == []
# Simulate a sync
bypass = Bypass.open()
groups = [
%{
"kind" => "admin#directory#group",
"id" => "GROUP_ID1",
"etag" => "\"ET\"",
"email" => "i@fiez.xxx",
"name" => "Infrastructure",
"directMembersCount" => "5",
"description" => "Group to handle infrastructure alerts and management",
"adminCreated" => true,
"aliases" => [
"pnr@firez.one"
],
"nonEditableAliases" => [
"i@ext.fiez.xxx"
]
}
]
GoogleWorkspaceDirectory.override_endpoint_url("http://localhost:#{bypass.port}/")
GoogleWorkspaceDirectory.mock_groups_list_endpoint(
bypass,
200,
Jason.encode!(%{"groups" => groups})
)
GoogleWorkspaceDirectory.mock_organization_units_list_endpoint(
bypass,
200,
Jason.encode!(%{"organizationUnits" => []})
)
GoogleWorkspaceDirectory.mock_users_list_endpoint(
bypass,
200,
Jason.encode!(%{"users" => []})
)
GoogleWorkspaceDirectory.mock_group_members_list_endpoint(
bypass,
"GROUP_ID1",
200,
Jason.encode!(%{"members" => []})
)
GoogleWorkspaceDirectory.mock_token_endpoint(bypass)
{:ok, pid} = Task.Supervisor.start_link()
assert execute(%{task_supervisor: pid}) == :ok
# Assert that the group has been resurrected
assert resurrected_group = Repo.get(Domain.Actors.Group, id)
assert resurrected_group.inserted_at == inserted_at
assert resurrected_group.id == id
assert resurrected_group.deleted_at == nil
assert Domain.Actors.Group.Query.not_deleted() |> Repo.all() == [resurrected_group]
# TODO:: Test that associated policies are also resurrected as part of https://github.com/firezone/firezone/issues/8187
end
test "persists the sync error on the provider", %{provider: provider} do
error_message =
"Admin SDK API has not been used in project XXXX before or it is disabled. " <>

124
elixir/apps/domain/test/domain/auth/adapters/jumpcloud/jobs/sync_directory_test.exs
View File

@@ -485,6 +485,130 @@ defmodule Domain.Auth.Adapters.JumpCloud.Jobs.SyncDirectoryTest do
refute_received {:expire_flow, _flow_id, _client_id, _resource_id}
end
test "resurrects deleted identities that reappear on the next sync", %{
bypass: bypass,
account: account,
provider: provider
} do
actor = Fixtures.Actors.create_actor(account: account)
provider_identifier = "USER_JDOE_ID"
identity =
Fixtures.Auth.create_identity(
account: account,
provider: provider,
actor: actor,
provider_identifier: provider_identifier
)
inserted_at = identity.inserted_at
id = identity.id
# Soft delete the identity
Repo.update_all(Domain.Auth.Identity, set: [deleted_at: DateTime.utc_now()])
assert Domain.Auth.all_identities_for(actor) == []
# Simulate a sync
users = [
%{
"id" => "workos_user_jdoe_id",
"object" => "directory_user",
"custom_attributes" => %{},
"directory_id" => "dir_123",
"organization_id" => "org_123",
"emails" => [
%{
"primary" => true,
"type" => "type",
"value" => "jdoe@example.local"
}
],
"groups" => [],
"idp_id" => "USER_JDOE_ID",
"first_name" => "John",
"last_name" => "Doe",
"job_title" => "Software Eng",
"raw_attributes" => %{},
"state" => "active",
"username" => "jdoe@example.local",
"created_at" => "2023-07-17T20:07:20.055Z",
"updated_at" => "2023-07-17T20:07:20.055Z"
}
]
WorkOSDirectory.override_base_url("http://localhost:#{bypass.port}/")
WorkOSDirectory.mock_list_directories_endpoint(bypass)
WorkOSDirectory.mock_list_groups_endpoint(bypass, [])
WorkOSDirectory.mock_list_users_endpoint(bypass, users)
{:ok, pid} = Task.Supervisor.start_link()
assert execute(%{task_supervisor: pid}) == :ok
# Assert that the identity has been resurrected
assert resurrected_identity = Repo.get(Domain.Auth.Identity, id)
assert resurrected_identity.inserted_at == inserted_at
assert resurrected_identity.id == id
assert resurrected_identity.deleted_at == nil
assert Domain.Auth.all_identities_for(actor) == [resurrected_identity]
end
test "resurrects deleted groups that reappear on the next sync", %{
bypass: bypass,
account: account,
provider: provider
} do
actor_group =
Fixtures.Actors.create_group(
account: account,
provider: provider,
provider_identifier: "G:GROUP_ENGINEERING_ID"
)
inserted_at = actor_group.inserted_at
id = actor_group.id
# Soft delete the group
Repo.update_all(Domain.Actors.Group, set: [deleted_at: DateTime.utc_now()])
# Assert that the group and associated policy has been soft-deleted
assert Domain.Actors.Group.Query.not_deleted() |> Repo.all() == []
# Simulate a sync
groups = [
%{
"id" => "GROUP_ENGINEERING_ID",
"object" => "directory_group",
"idp_id" => "engineering",
"directory_id" => "dir_123",
"organization_id" => "org_123",
"name" => "Engineering",
"created_at" => "2021-10-27 15:21:50.640958",
"updated_at" => "2021-12-13 12:15:45.531847",
"raw_attributes" => %{}
}
]
WorkOSDirectory.override_base_url("http://localhost:#{bypass.port}/")
WorkOSDirectory.mock_list_directories_endpoint(bypass)
WorkOSDirectory.mock_list_groups_endpoint(bypass, groups)
WorkOSDirectory.mock_list_users_endpoint(bypass, [])
{:ok, pid} = Task.Supervisor.start_link()
assert execute(%{task_supervisor: pid}) == :ok
# Assert that the group has been resurrected
assert resurrected_group = Repo.get(Domain.Actors.Group, id)
assert resurrected_group.inserted_at == inserted_at
assert resurrected_group.id == id
assert resurrected_group.deleted_at == nil
assert Domain.Actors.Group.Query.not_deleted() |> Repo.all() == [resurrected_group]
# TODO:: Test that associated policies are also resurrected as part of https://github.com/firezone/firezone/issues/8187
end
test "stops the sync retires on 401 error from WorkOS", %{provider: provider} do
bypass = Bypass.open()
WorkOSDirectory.override_base_url("http://localhost:#{bypass.port}")

124
elixir/apps/domain/test/domain/auth/adapters/microsoft_entra/jobs/sync_directory_test.exs
View File

@@ -141,6 +141,130 @@ defmodule Domain.Auth.Adapters.MicrosoftEntra.Jobs.SyncDirectoryTest do
assert updated_provider.last_synced_at != provider.last_synced_at
end
test "resurrects deleted identities that reappear on the next sync", %{
account: account,
provider: provider
} do
actor = Fixtures.Actors.create_actor(account: account)
provider_identifier = "USER_JDOE_ID"
identity =
Fixtures.Auth.create_identity(
account: account,
provider: provider,
actor: actor,
provider_identifier: provider_identifier
)
inserted_at = identity.inserted_at
id = identity.id
# Soft delete the identity
Repo.update_all(Domain.Auth.Identity, set: [deleted_at: DateTime.utc_now()])
assert Domain.Auth.all_identities_for(actor) == []
# Simulate a sync
bypass = Bypass.open()
users = [
%{
"id" => "USER_JDOE_ID",
"displayName" => "John Doe",
"givenName" => "John",
"surname" => "Doe",
"userPrincipalName" => "jdoe@example.local",
"mail" => "jdoe@example.local",
"accountEnabled" => true
}
]
MicrosoftEntraDirectory.override_endpoint_url("http://localhost:#{bypass.port}/")
MicrosoftEntraDirectory.mock_groups_list_endpoint(
bypass,
200,
Jason.encode!(%{"value" => []})
)
MicrosoftEntraDirectory.mock_users_list_endpoint(
bypass,
200,
Jason.encode!(%{"value" => users})
)
{:ok, pid} = Task.Supervisor.start_link()
assert execute(%{task_supervisor: pid}) == :ok
# Assert that the identity has been resurrected
assert resurrected_identity = Repo.get(Domain.Auth.Identity, id)
assert resurrected_identity.inserted_at == inserted_at
assert resurrected_identity.id == id
assert resurrected_identity.deleted_at == nil
assert Domain.Auth.all_identities_for(actor) == [resurrected_identity]
end
test "resurrects deleted groups that reappear on the next sync", %{
account: account,
provider: provider
} do
actor_group =
Fixtures.Actors.create_group(
account: account,
provider: provider,
provider_identifier: "G:GROUP_ALL_ID"
)
inserted_at = actor_group.inserted_at
id = actor_group.id
# Soft delete the group
Repo.update_all(Domain.Actors.Group, set: [deleted_at: DateTime.utc_now()])
# Assert that the group and associated policy has been soft-deleted
assert Domain.Actors.Group.Query.not_deleted() |> Repo.all() == []
# Simulate a sync
bypass = Bypass.open()
groups = [
%{"id" => "GROUP_ALL_ID", "displayName" => "All"}
]
MicrosoftEntraDirectory.override_endpoint_url("http://localhost:#{bypass.port}/")
MicrosoftEntraDirectory.mock_groups_list_endpoint(
bypass,
200,
Jason.encode!(%{"value" => groups})
)
MicrosoftEntraDirectory.mock_group_members_list_endpoint(
bypass,
"GROUP_ALL_ID",
200,
Jason.encode!(%{"value" => []})
)
MicrosoftEntraDirectory.mock_users_list_endpoint(
bypass,
200,
Jason.encode!(%{"value" => []})
)
{:ok, pid} = Task.Supervisor.start_link()
assert execute(%{task_supervisor: pid}) == :ok
# Assert that the group has been resurrected
assert resurrected_group = Repo.get(Domain.Actors.Group, id)
assert resurrected_group.inserted_at == inserted_at
assert resurrected_group.id == id
assert resurrected_group.deleted_at == nil
assert Domain.Actors.Group.Query.not_deleted() |> Repo.all() == [resurrected_group]
# TODO:: Test that associated policies are also resurrected as part of https://github.com/firezone/firezone/issues/8187
end
test "does not crash on endpoint errors" do
bypass = Bypass.open()
Bypass.down(bypass)

150
elixir/apps/domain/test/domain/auth/adapters/okta/jobs/sync_directory_test.exs
View File

@@ -747,6 +747,156 @@ defmodule Domain.Auth.Adapters.Okta.Jobs.SyncDirectoryTest do
refute_received {:expire_flow, _flow_id, _client_id, _resource_id}
end
test "resurrects deleted identities that reappear on the next sync", %{
bypass: bypass,
account: account,
provider: provider
} do
actor = Fixtures.Actors.create_actor(account: account)
provider_identifier = "USER_JDOE_ID"
identity =
Fixtures.Auth.create_identity(
account: account,
provider: provider,
actor: actor,
provider_identifier: provider_identifier
)
inserted_at = identity.inserted_at
id = identity.id
# Soft delete the identity
Repo.update_all(Domain.Auth.Identity, set: [deleted_at: DateTime.utc_now()])
assert Domain.Auth.all_identities_for(actor) == []
# Simulate a sync
users = [
%{
"id" => "USER_JDOE_ID",
"status" => "ACTIVE",
"created" => "2023-12-21T18:30:05.000Z",
"activated" => nil,
"statusChanged" => "2023-12-21T20:04:06.000Z",
"lastLogin" => "2024-02-08T05:14:25.000Z",
"lastUpdated" => "2023-12-21T20:04:06.000Z",
"passwordChanged" => "2023-12-21T20:04:06.000Z",
"type" => %{"id" => "otye1rmouoEfu7KCV5d7"},
"profile" => %{
"firstName" => "John",
"lastName" => "Doe",
"mobilePhone" => nil,
"secondEmail" => nil,
"login" => "jdoe@example.com",
"email" => "jdoe@example.com"
},
"_links" => %{
"self" => %{
"href" => "http://localhost:#{bypass.port}/api/v1/users/OT6AZkcmzkDXwkXcjTHY"
}
}
}
]
OktaDirectory.mock_groups_list_endpoint(bypass, 200, Jason.encode!([]))
OktaDirectory.mock_users_list_endpoint(bypass, 200, Jason.encode!(users))
{:ok, pid} = Task.Supervisor.start_link()
assert execute(%{task_supervisor: pid}) == :ok
# Assert that the identity has been resurrected
assert resurrected_identity = Repo.get(Domain.Auth.Identity, id)
assert resurrected_identity.inserted_at == inserted_at
assert resurrected_identity.id == id
assert resurrected_identity.deleted_at == nil
assert Domain.Auth.all_identities_for(actor) == [resurrected_identity]
end
test "resurrects deleted groups that reappear on the next sync", %{
bypass: bypass,
account: account,
provider: provider
} do
actor_group =
Fixtures.Actors.create_group(
account: account,
provider: provider,
provider_identifier: "G:GROUP_DEVOPS_ID"
)
inserted_at = actor_group.inserted_at
id = actor_group.id
# Soft delete the group
Repo.update_all(Domain.Actors.Group, set: [deleted_at: DateTime.utc_now()])
# Assert that the group and associated policy has been soft-deleted
assert Domain.Actors.Group.Query.not_deleted() |> Repo.all() == []
# Simulate a sync
groups = [
%{
"id" => "GROUP_DEVOPS_ID",
"created" => "2024-02-07T04:32:03.000Z",
"lastUpdated" => "2024-02-07T04:32:03.000Z",
"lastMembershipUpdated" => "2024-02-07T04:32:38.000Z",
"objectClass" => [
"okta:user_group"
],
"type" => "OKTA_GROUP",
"profile" => %{
"name" => "DevOps",
"description" => ""
},
"_links" => %{
"logo" => [
%{
"name" => "medium",
"href" => "http://localhost/md/image.png",
"type" => "image/png"
},
%{
"name" => "large",
"href" => "http://localhost/lg/image.png",
"type" => "image/png"
}
],
"users" => %{
"href" => "http://localhost:#{bypass.port}/api/v1/groups/00gezqhvv4IFj2Avg5d7/users"
},
"apps" => %{
"href" => "http://localhost:#{bypass.port}/api/v1/groups/00gezqhvv4IFj2Avg5d7/apps"
}
}
}
]
OktaDirectory.mock_users_list_endpoint(bypass, 200, Jason.encode!([]))
OktaDirectory.mock_groups_list_endpoint(bypass, 200, Jason.encode!(groups))
OktaDirectory.mock_group_members_list_endpoint(
bypass,
"GROUP_DEVOPS_ID",
200,
Jason.encode!([])
)
{:ok, pid} = Task.Supervisor.start_link()
assert execute(%{task_supervisor: pid}) == :ok
# Assert that the group has been resurrected
assert resurrected_group = Repo.get(Domain.Actors.Group, id)
assert resurrected_group.inserted_at == inserted_at
assert resurrected_group.id == id
assert resurrected_group.deleted_at == nil
assert Domain.Actors.Group.Query.not_deleted() |> Repo.all() == [resurrected_group]
# TODO:: Test that associated policies are also resurrected as part of https://github.com/firezone/firezone/issues/8187
end
test "persists the sync error on the provider", %{provider: provider, bypass: bypass} do
response = %{
"errorCode" => "E0000011",