diff --git a/elixir/apps/domain/lib/domain/actors/membership/sync.ex b/elixir/apps/domain/lib/domain/actors/membership/sync.ex index 8b9104634..465b044f9 100644 --- a/elixir/apps/domain/lib/domain/actors/membership/sync.ex +++ b/elixir/apps/domain/lib/domain/actors/membership/sync.ex @@ -17,9 +17,15 @@ defmodule Domain.Actors.Membership.Sync do memberships: memberships } -> tuples = - Enum.map(tuples, fn {group_provider_identifier, actor_provider_identifier} -> - {Map.fetch!(group_ids_by_provider_identifier, group_provider_identifier), - Map.fetch!(actor_ids_by_provider_identifier, actor_provider_identifier)} + Enum.flat_map(tuples, fn {group_provider_identifier, actor_provider_identifier} -> + group_id = Map.get(group_ids_by_provider_identifier, group_provider_identifier) + actor_id = Map.get(actor_ids_by_provider_identifier, actor_provider_identifier) + + if is_nil(group_id) or is_nil(actor_id) do + [] + else + [{group_id, actor_id}] + end end) plan_memberships_update(tuples, memberships) diff --git a/elixir/apps/domain/lib/domain/auth/adapters/google_workspace/api_client.ex b/elixir/apps/domain/lib/domain/auth/adapters/google_workspace/api_client.ex index 066d856ca..223a275d2 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/google_workspace/api_client.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/google_workspace/api_client.ex @@ -1,8 +1,14 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.APIClient do + @moduledoc """ + Warning: DO NOT use `fields` parameter with Google API's, + or they will not return you pagination cursor 🫠. + """ use Supervisor @pool_name __MODULE__.Finch + @max_results 350 + def start_link(_init_arg) do Supervisor.start_link(__MODULE__, nil, name: __MODULE__) end @@ -40,19 +46,7 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.APIClient do "customer" => "my_customer", "showDeleted" => false, "query" => "isSuspended=false isArchived=false", - "fields" => - Enum.join( - ~w[ - users/id - users/primaryEmail - users/name/fullName - users/orgUnitPath - users/creationTime - users/isEnforcedIn2Sv - users/isEnrolledIn2Sv - ], - "," - ) + "maxResults" => @max_results }) ) @@ -68,7 +62,8 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.APIClient do URI.parse("#{endpoint}/admin/directory/v1/groups") |> URI.append_query( URI.encode_query(%{ - "customer" => "my_customer" + "customer" => "my_customer", + "maxResults" => @max_results }) ) @@ -81,7 +76,14 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.APIClient do Domain.Config.fetch_env!(:domain, __MODULE__) |> Keyword.fetch!(:endpoint) - uri = URI.parse("#{endpoint}/admin/directory/v1/customer/my_customer/orgunits") + uri = + URI.parse("#{endpoint}/admin/directory/v1/customer/my_customer/orgunits") + |> URI.append_query( + URI.encode_query(%{ + "maxResults" => @max_results + }) + ) + list_all(uri, api_token, "organizationUnits") end @@ -90,9 +92,14 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.APIClient do Domain.Config.fetch_env!(:domain, __MODULE__) |> Keyword.fetch!(:endpoint) - params = %{"includeDerivedMembership" => true} - uri = URI.parse("#{endpoint}/admin/directory/v1/groups/#{group_id}/members") - uri = URI.append_query(uri, URI.encode_query(params)) + uri = + URI.parse("#{endpoint}/admin/directory/v1/groups/#{group_id}/members") + |> URI.append_query( + URI.encode_query(%{ + "includeDerivedMembership" => true, + "maxResults" => @max_results + }) + ) with {:ok, members} <- list_all(uri, api_token, "members") do members = diff --git a/elixir/apps/domain/lib/domain/auth/adapters/google_workspace/jobs.ex b/elixir/apps/domain/lib/domain/auth/adapters/google_workspace/jobs.ex index fc5fca393..74ef6d4e6 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/google_workspace/jobs.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/google_workspace/jobs.ex @@ -42,119 +42,142 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.Jobs do |> Domain.Repo.preload(:account) |> Enum.chunk_every(5) |> Enum.each(fn providers -> - Enum.map(providers, fn provider -> - Logger.debug("Syncing provider", provider_id: provider.id) - - access_token = provider.adapter_state["access_token"] - - with true <- Domain.Accounts.idp_sync_enabled?(provider.account), - {:ok, users} <- GoogleWorkspace.APIClient.list_users(access_token), - {:ok, organization_units} <- - GoogleWorkspace.APIClient.list_organization_units(access_token), - {:ok, groups} <- GoogleWorkspace.APIClient.list_groups(access_token), - {:ok, tuples} <- - list_membership_tuples(access_token, groups) do - identities_attrs = - Enum.map(users, fn user -> - %{ - "provider_identifier" => user["id"], - "provider_state" => %{ - "userinfo" => %{ - "email" => user["primaryEmail"] - } - }, - "actor" => %{ - "type" => :account_user, - "name" => user["name"]["fullName"] - } - } - end) - - actor_groups_attrs = - Enum.map(groups, fn group -> - %{ - "name" => "Group:" <> group["name"], - "provider_identifier" => "G:" <> group["id"] - } - end) ++ - Enum.map(organization_units, fn organization_unit -> - %{ - "name" => "OrgUnit:" <> organization_unit["name"], - "provider_identifier" => "OU:" <> organization_unit["orgUnitId"] - } - end) - - tuples = - Enum.flat_map(users, fn user -> - organization_unit = - Enum.find(organization_units, fn organization_unit -> - organization_unit["orgUnitPath"] == user["orgUnitPath"] - end) - - if organization_unit["orgUnitId"] do - [{"OU:" <> organization_unit["orgUnitId"], user["id"]}] - else - [] - end - end) ++ tuples - - Ecto.Multi.new() - |> Ecto.Multi.append(Auth.sync_provider_identities_multi(provider, identities_attrs)) - |> Ecto.Multi.append(Actors.sync_provider_groups_multi(provider, actor_groups_attrs)) - |> Actors.sync_provider_memberships_multi(provider, tuples) - |> Ecto.Multi.update(:save_last_updated_at, fn _effects_so_far -> - Auth.Provider.Changeset.sync_finished(provider) - end) - |> Domain.Repo.transaction() - |> case do - {:ok, effects} -> - SyncLogger.log_effects(provider, effects) - - {:error, reason} -> - Logger.error("Failed to sync provider", - provider_id: provider.id, - account_id: provider.account_id, - reason: inspect(reason) - ) - end - else - false -> - Auth.Provider.Changeset.sync_failed( - provider, - "IdP sync is not enabled in your subscription plan" - ) - |> Domain.Repo.update!() - - :ok - - {:error, {status, %{"error" => %{"message" => message}}}} -> - provider = - Auth.Provider.Changeset.sync_failed(provider, message) - |> Domain.Repo.update!() - - log_sync_error(provider, "Google API returned #{status}: #{message}") - - {:error, :retry_later} -> - message = "Google API is temporarily unavailable" - - provider = - Auth.Provider.Changeset.sync_failed(provider, message) - |> Domain.Repo.update!() - - log_sync_error(provider, message) - - {:error, reason} -> - Logger.error("Failed syncing provider", - account_id: provider.account_id, - provider_id: provider.id, - reason: inspect(reason) - ) - end - end) + Enum.map(providers, &sync_provider/1) end) end end + def sync_provider(provider) do + Logger.debug("Syncing provider", + account_id: provider.account_id, + provider_id: provider.id + ) + + access_token = provider.adapter_state["access_token"] + + with true <- Domain.Accounts.idp_sync_enabled?(provider.account), + {:ok, users} <- GoogleWorkspace.APIClient.list_users(access_token), + {:ok, organization_units} <- + GoogleWorkspace.APIClient.list_organization_units(access_token), + {:ok, groups} <- GoogleWorkspace.APIClient.list_groups(access_token), + {:ok, tuples} <- + list_membership_tuples(access_token, groups) do + identities_attrs = + Enum.map(users, fn user -> + %{ + "provider_identifier" => user["id"], + "provider_state" => %{ + "userinfo" => %{ + "email" => user["primaryEmail"] + } + }, + "actor" => %{ + "type" => :account_user, + "name" => user["name"]["fullName"] + } + } + end) + + actor_groups_attrs = + Enum.map(groups, fn group -> + %{ + "name" => "Group:" <> group["name"], + "provider_identifier" => "G:" <> group["id"] + } + end) ++ + Enum.map(organization_units, fn organization_unit -> + %{ + "name" => "OrgUnit:" <> organization_unit["name"], + "provider_identifier" => "OU:" <> organization_unit["orgUnitId"] + } + end) + + tuples = + Enum.flat_map(users, fn user -> + organization_unit = + Enum.find(organization_units, fn organization_unit -> + organization_unit["orgUnitPath"] == user["orgUnitPath"] + end) + + if organization_unit["orgUnitId"] do + [{"OU:" <> organization_unit["orgUnitId"], user["id"]}] + else + [] + end + end) ++ tuples + + Ecto.Multi.new() + |> Ecto.Multi.append(Auth.sync_provider_identities_multi(provider, identities_attrs)) + |> Ecto.Multi.append(Actors.sync_provider_groups_multi(provider, actor_groups_attrs)) + |> Actors.sync_provider_memberships_multi(provider, tuples) + |> Ecto.Multi.update(:save_last_updated_at, fn _effects_so_far -> + Auth.Provider.Changeset.sync_finished(provider) + end) + |> Domain.Repo.transaction(timeout: :timer.minutes(15)) + |> case do + {:ok, effects} -> + SyncLogger.log_effects(provider, effects) + + {:error, reason} -> + Logger.error("Failed to sync provider", + provider_id: provider.id, + account_id: provider.account_id, + reason: inspect(reason) + ) + + {:error, reason} + + {:error, step, reason, _effects_so_far} -> + Logger.error("Failed to sync provider", + provider_id: provider.id, + account_id: provider.account_id, + step: inspect(step), + reason: inspect(reason) + ) + + {:error, reason} + end + else + false -> + Auth.Provider.Changeset.sync_failed( + provider, + "IdP sync is not enabled in your subscription plan" + ) + |> Domain.Repo.update!() + + :ok + + {:error, {401, %{"error" => %{"message" => message}}}} -> + Auth.Provider.Changeset.sync_requires_manual_intervention(provider, message) + |> Domain.Repo.update!() + + :ok + + {:error, {status, %{"error" => %{"message" => message}}}} -> + provider = + Auth.Provider.Changeset.sync_failed(provider, message) + |> Domain.Repo.update!() + + log_sync_error(provider, "Google API returned #{status}: #{message}") + + {:error, :retry_later} -> + message = "Google API is temporarily unavailable" + + provider = + Auth.Provider.Changeset.sync_failed(provider, message) + |> Domain.Repo.update!() + + log_sync_error(provider, message) + + {:error, reason} -> + Logger.error("Failed to sync provider", + account_id: provider.account_id, + provider_id: provider.id, + reason: inspect(reason) + ) + end + end + defp log_sync_error(provider, message) do metadata = [ account_id: provider.account_id, @@ -163,9 +186,9 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.Jobs do ] if provider.last_syncs_failed >= 3 do - Logger.warning("Failed syncing provider", metadata) + Logger.warning("Failed to sync provider", metadata) else - Logger.info("Failed syncing provider", metadata) + Logger.info("Failed to sync provider", metadata) end end diff --git a/elixir/apps/domain/lib/domain/auth/adapters/microsoft_entra/jobs.ex b/elixir/apps/domain/lib/domain/auth/adapters/microsoft_entra/jobs.ex index d3e38cfe0..7f6eae446 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/microsoft_entra/jobs.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/microsoft_entra/jobs.ex @@ -68,7 +68,7 @@ defmodule Domain.Auth.Adapters.MicrosoftEntra.Jobs do |> Ecto.Multi.update(:save_last_updated_at, fn _effects_so_far -> Auth.Provider.Changeset.sync_finished(provider) end) - |> Domain.Repo.transaction() + |> Domain.Repo.transaction(timeout: :timer.minutes(15)) |> case do {:ok, effects} -> SyncLogger.log_effects(provider, effects) @@ -80,14 +80,15 @@ defmodule Domain.Auth.Adapters.MicrosoftEntra.Jobs do reason: inspect(reason) ) - {:error, op, value, changes_so_far} -> + {:error, step, reason, _effects_so_far} -> Logger.error("Failed to sync provider", provider_id: provider.id, account_id: provider.account_id, - op: op, - value: inspect(value), - changes_so_far: inspect(changes_so_far) + step: inspect(step), + reason: inspect(reason) ) + + {:error, reason} end else false -> @@ -116,7 +117,7 @@ defmodule Domain.Auth.Adapters.MicrosoftEntra.Jobs do log_sync_error(provider, message) {:error, reason} -> - Logger.error("Failed syncing provider", + Logger.error("Failed to sync provider", account_id: provider.account_id, provider_id: provider.id, reason: inspect(reason) @@ -132,9 +133,9 @@ defmodule Domain.Auth.Adapters.MicrosoftEntra.Jobs do ] if provider.last_syncs_failed >= 3 do - Logger.warning("Failed syncing provider", metadata) + Logger.warning("Failed to sync provider", metadata) else - Logger.info("Failed syncing provider", metadata) + Logger.info("Failed to sync provider", metadata) end end diff --git a/elixir/apps/domain/lib/domain/auth/adapters/okta/jobs.ex b/elixir/apps/domain/lib/domain/auth/adapters/okta/jobs.ex index 40b831573..d0429abdd 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/okta/jobs.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/okta/jobs.ex @@ -78,7 +78,7 @@ defmodule Domain.Auth.Adapters.Okta.Jobs do |> Ecto.Multi.update(:save_last_updated_at, fn _effects_so_far -> Auth.Provider.Changeset.sync_finished(provider) end) - |> Domain.Repo.transaction() + |> Domain.Repo.transaction(timeout: :timer.minutes(15)) |> case do {:ok, effects} -> SyncLogger.log_effects(provider, effects) @@ -90,14 +90,15 @@ defmodule Domain.Auth.Adapters.Okta.Jobs do reason: inspect(reason) ) - {:error, op, value, changes_so_far} -> + {:error, step, reason, _effects_so_far} -> Logger.error("Failed to sync provider", provider_id: provider.id, account_id: provider.account_id, - op: op, - value: inspect(value), - changes_so_far: inspect(changes_so_far) + step: inspect(step), + reason: inspect(reason) ) + + {:error, reason} end else {:error, {status, %{"errorCode" => error_code, "errorSummary" => error_summary}}} -> @@ -119,7 +120,7 @@ defmodule Domain.Auth.Adapters.Okta.Jobs do log_sync_error(provider, message) {:error, reason} -> - Logger.error("Failed syncing provider", + Logger.error("Failed to sync provider", account_id: provider.account_id, provider_id: provider.id, reason: inspect(reason) @@ -135,9 +136,9 @@ defmodule Domain.Auth.Adapters.Okta.Jobs do ] if provider.last_syncs_failed >= 3 do - Logger.warning("Failed syncing provider", metadata) + Logger.warning("Failed to sync provider", metadata) else - Logger.info("Failed syncing provider", metadata) + Logger.info("Failed to sync provider", metadata) end end diff --git a/elixir/apps/domain/lib/domain/auth/identity/sync.ex b/elixir/apps/domain/lib/domain/auth/identity/sync.ex index d55c78c80..5c76c5521 100644 --- a/elixir/apps/domain/lib/domain/auth/identity/sync.ex +++ b/elixir/apps/domain/lib/domain/auth/identity/sync.ex @@ -25,7 +25,7 @@ defmodule Domain.Auth.Identity.Sync do |> Ecto.Multi.run( :insert_identities, fn repo, %{plan_identities: {insert, _update, _delete}} -> - upsert_identities(repo, provider, attrs_by_provider_identifier, insert) + insert_identities(repo, provider, attrs_by_provider_identifier, insert) end ) |> Ecto.Multi.run( @@ -89,6 +89,8 @@ defmodule Domain.Auth.Identity.Sync do end defp delete_identities(repo, provider, provider_identifiers_to_delete) do + provider_identifiers_to_delete = Enum.uniq(provider_identifiers_to_delete) + {_count, identities} = Identity.Query.by_account_id(provider.account_id) |> Identity.Query.by_provider_id(provider.id) @@ -104,13 +106,14 @@ defmodule Domain.Auth.Identity.Sync do {:ok, identities} end - defp upsert_identities( + defp insert_identities( repo, provider, attrs_by_provider_identifier, provider_identifiers_to_insert ) 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) changeset = Identity.Changeset.create_identity_and_actor(provider, attrs) @@ -136,10 +139,13 @@ defmodule Domain.Auth.Identity.Sync do |> Enum.filter(fn identity -> identity.provider_identifier in provider_identifiers_to_update end) + # make sure that deleted identities are in the end in case of conflicts + |> Enum.sort_by(& &1.deleted_at, :desc) |> repo.preload(:actor) |> Map.new(&{&1.provider_identifier, &1}) provider_identifiers_to_update + |> 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) diff --git a/elixir/apps/domain/lib/domain/auth/provider.ex b/elixir/apps/domain/lib/domain/auth/provider.ex index 4703e558d..698d95bb8 100644 --- a/elixir/apps/domain/lib/domain/auth/provider.ex +++ b/elixir/apps/domain/lib/domain/auth/provider.ex @@ -22,6 +22,7 @@ defmodule Domain.Auth.Provider do field :last_syncs_failed, :integer field :last_sync_error, :string field :last_synced_at, :utc_datetime_usec + field :sync_disabled_at, :utc_datetime_usec field :disabled_at, :utc_datetime_usec field :deleted_at, :utc_datetime_usec diff --git a/elixir/apps/domain/lib/domain/auth/provider/changeset.ex b/elixir/apps/domain/lib/domain/auth/provider/changeset.ex index 8042a49c6..2b3165eb0 100644 --- a/elixir/apps/domain/lib/domain/auth/provider/changeset.ex +++ b/elixir/apps/domain/lib/domain/auth/provider/changeset.ex @@ -4,7 +4,9 @@ defmodule Domain.Auth.Provider.Changeset do alias Domain.Auth.{Subject, Provider, Adapters} @create_fields ~w[id name adapter provisioner adapter_config adapter_state disabled_at]a - @update_fields ~w[name adapter_config last_syncs_failed last_sync_error adapter_state provisioner disabled_at deleted_at]a + @update_fields ~w[name adapter_config + last_syncs_failed last_sync_error sync_disabled_at + adapter_state provisioner disabled_at deleted_at]a @required_fields ~w[name adapter adapter_config provisioner]a def create(account, attrs, %Subject{} = subject) do @@ -47,6 +49,7 @@ defmodule Domain.Auth.Provider.Changeset do |> put_change(:last_synced_at, DateTime.utc_now()) |> put_change(:last_sync_error, nil) |> put_change(:last_syncs_failed, 0) + |> put_change(:sync_disabled_at, nil) end def sync_failed(%Provider{} = provider, error) do @@ -59,6 +62,11 @@ defmodule Domain.Auth.Provider.Changeset do |> put_change(:last_syncs_failed, last_syncs_failed + 1) end + def sync_requires_manual_intervention(%Provider{} = provider, error) do + sync_failed(provider, error) + |> put_change(:sync_disabled_at, DateTime.utc_now()) + end + defp changeset(changeset) do changeset |> validate_length(:name, min: 1, max: 255) diff --git a/elixir/apps/domain/lib/domain/auth/provider/query.ex b/elixir/apps/domain/lib/domain/auth/provider/query.ex index 4de822fd5..d279b4cd2 100644 --- a/elixir/apps/domain/lib/domain/auth/provider/query.ex +++ b/elixir/apps/domain/lib/domain/auth/provider/query.ex @@ -14,10 +14,6 @@ defmodule Domain.Auth.Provider.Query do where(queryable, [provider: provider], is_nil(provider.disabled_at)) end - def not_exceeded_attempts(queryable \\ not_deleted()) do - where(queryable, [provider: provider], provider.last_syncs_failed <= 10) - end - def by_id(queryable \\ not_deleted(), id) def by_id(queryable, {:not, id}) do @@ -47,8 +43,8 @@ defmodule Domain.Auth.Provider.Query do end def only_ready_to_be_synced(queryable \\ not_deleted()) do - where( - queryable, + queryable + |> where( [provider: provider], is_nil(provider.last_synced_at) or fragment( @@ -57,6 +53,7 @@ defmodule Domain.Auth.Provider.Query do provider.last_syncs_failed ) ) + |> where([provider: provider], is_nil(provider.sync_disabled_at)) end def by_non_empty_refresh_token(queryable \\ not_deleted()) do diff --git a/elixir/apps/domain/priv/repo/migrations/20240229204649_add_auth_providers_sync_disabled_at.exs b/elixir/apps/domain/priv/repo/migrations/20240229204649_add_auth_providers_sync_disabled_at.exs new file mode 100644 index 000000000..df7df677b --- /dev/null +++ b/elixir/apps/domain/priv/repo/migrations/20240229204649_add_auth_providers_sync_disabled_at.exs @@ -0,0 +1,9 @@ +defmodule Domain.Repo.Migrations.AddAuthProvidersSyncDisabledAt do + use Ecto.Migration + + def change do + alter table(:auth_providers) do + add(:sync_disabled_at, :utc_datetime_usec) + end + end +end diff --git a/elixir/apps/domain/test/domain/actors_test.exs b/elixir/apps/domain/test/domain/actors_test.exs index d913aef1c..8469cd3c1 100644 --- a/elixir/apps/domain/test/domain/actors_test.exs +++ b/elixir/apps/domain/test/domain/actors_test.exs @@ -957,6 +957,104 @@ defmodule Domain.ActorsTest do insert_memberships: [] }} = Repo.transaction(multi) end + + test "deletes actors that are not processed by identity sync", %{ + account: account, + provider: provider, + group1: group1, + group2: group2, + identity1: identity1, + identity2: identity2 + } do + Fixtures.Actors.create_membership( + account: account, + group: group1, + actor_id: identity1.actor_id + ) + + Fixtures.Actors.create_membership( + account: account, + group: group2, + actor_id: identity2.actor_id + ) + + tuples_list = [ + {group1.provider_identifier, identity1.provider_identifier}, + {group2.provider_identifier, identity2.provider_identifier} + ] + + actor_ids_by_provider_identifier = %{} + + group_ids_by_provider_identifier = %{ + group1.provider_identifier => group1.id, + group2.provider_identifier => group2.id + } + + multi = + Ecto.Multi.new() + |> Ecto.Multi.put(:actor_ids_by_provider_identifier, actor_ids_by_provider_identifier) + |> Ecto.Multi.put(:group_ids_by_provider_identifier, group_ids_by_provider_identifier) + |> sync_provider_memberships_multi(provider, tuples_list) + + assert {:ok, + %{ + plan_memberships: {[], delete}, + delete_memberships: {2, nil}, + insert_memberships: [] + }} = Repo.transaction(multi) + + assert {group1.id, identity1.actor_id} in delete + assert {group2.id, identity2.actor_id} in delete + end + + test "deletes groups that are not processed by groups sync", %{ + account: account, + provider: provider, + group1: group1, + group2: group2, + identity1: identity1, + identity2: identity2 + } do + Fixtures.Actors.create_membership( + account: account, + group: group1, + actor_id: identity1.actor_id + ) + + Fixtures.Actors.create_membership( + account: account, + group: group2, + actor_id: identity2.actor_id + ) + + tuples_list = [ + {group1.provider_identifier, identity1.provider_identifier}, + {group2.provider_identifier, identity2.provider_identifier} + ] + + actor_ids_by_provider_identifier = %{ + identity1.provider_identifier => identity1.actor_id, + identity2.provider_identifier => identity2.actor_id + } + + group_ids_by_provider_identifier = %{} + + multi = + Ecto.Multi.new() + |> Ecto.Multi.put(:actor_ids_by_provider_identifier, actor_ids_by_provider_identifier) + |> Ecto.Multi.put(:group_ids_by_provider_identifier, group_ids_by_provider_identifier) + |> sync_provider_memberships_multi(provider, tuples_list) + + assert {:ok, + %{ + plan_memberships: {[], delete}, + delete_memberships: {2, nil}, + insert_memberships: [] + }} = Repo.transaction(multi) + + assert {group1.id, identity1.actor_id} in delete + assert {group2.id, identity2.actor_id} in delete + end end describe "new_group/0" do diff --git a/elixir/apps/domain/test/domain/auth/adapters/google_workspace/api_client_test.exs b/elixir/apps/domain/test/domain/auth/adapters/google_workspace/api_client_test.exs index fa8a96bb9..b0b9f06bb 100644 --- a/elixir/apps/domain/test/domain/auth/adapters/google_workspace/api_client_test.exs +++ b/elixir/apps/domain/test/domain/auth/adapters/google_workspace/api_client_test.exs @@ -32,21 +32,9 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.APIClientTest do assert conn.params == %{ "customer" => "my_customer", - "fields" => - Enum.join( - ~w[ - users/id - users/primaryEmail - users/name/fullName - users/orgUnitPath - users/creationTime - users/isEnforcedIn2Sv - users/isEnrolledIn2Sv - ], - "," - ), "query" => "isSuspended=false isArchived=false", - "showDeleted" => "false" + "showDeleted" => "false", + "maxResults" => "350" } assert Plug.Conn.get_req_header(conn, "authorization") == ["Bearer #{api_token}"] @@ -77,7 +65,7 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.APIClientTest do end assert_receive {:bypass_request, conn} - assert conn.params == %{} + assert conn.params == %{"maxResults" => "350"} assert Plug.Conn.get_req_header(conn, "authorization") == ["Bearer #{api_token}"] end @@ -109,7 +97,8 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.APIClientTest do assert_receive {:bypass_request, conn} assert conn.params == %{ - "customer" => "my_customer" + "customer" => "my_customer", + "maxResults" => "350" } assert Plug.Conn.get_req_header(conn, "authorization") == ["Bearer #{api_token}"] @@ -141,7 +130,7 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.APIClientTest do end assert_receive {:bypass_request, conn} - assert conn.params == %{"includeDerivedMembership" => "true"} + assert conn.params == %{"includeDerivedMembership" => "true", "maxResults" => "350"} assert Plug.Conn.get_req_header(conn, "authorization") == ["Bearer #{api_token}"] end diff --git a/elixir/apps/domain/test/domain/auth/adapters/google_workspace/jobs_test.exs b/elixir/apps/domain/test/domain/auth/adapters/google_workspace/jobs_test.exs index 7a65a9f03..7ed5c2db9 100644 --- a/elixir/apps/domain/test/domain/auth/adapters/google_workspace/jobs_test.exs +++ b/elixir/apps/domain/test/domain/auth/adapters/google_workspace/jobs_test.exs @@ -704,6 +704,7 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.JobsTest do refute updated_provider.last_synced_at assert updated_provider.last_syncs_failed == 1 assert updated_provider.last_sync_error == error_message + refute updated_provider.sync_disabled_at Bypass.expect_once(bypass, "GET", "/admin/directory/v1/users", fn conn -> Plug.Conn.send_resp(conn, 500, "") @@ -716,5 +717,64 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.JobsTest do assert updated_provider.last_syncs_failed == 2 assert updated_provider.last_sync_error == "Google API is temporarily unavailable" end + + test "disables the sync on 401 response code", %{provider: provider} do + bypass = Bypass.open() + GoogleWorkspaceDirectory.override_endpoint_url("http://localhost:#{bypass.port}/") + + error_message = + "Admin SDK API has not been used in project XXXX before or it is disabled. " <> + "Enable it by visiting https://console.developers.google.com/apis/api/admin.googleapis.com/overview?project=XXXX " <> + "then retry. If you enabled this API recently, wait a few minutes for the action to propagate to our systems and retry." + + response = %{ + "error" => %{ + "code" => 401, + "message" => error_message, + "errors" => [ + %{ + "message" => error_message, + "domain" => "usageLimits", + "reason" => "accessNotConfigured", + "extendedHelp" => "https://console.developers.google.com" + } + ], + "status" => "PERMISSION_DENIED", + "details" => [ + %{ + "@type" => "type.googleapis.com/google.rpc.Help", + "links" => [ + %{ + "description" => "Google developers console API activation", + "url" => + "https://console.developers.google.com/apis/api/admin.googleapis.com/overview?project=100421656358" + } + ] + }, + %{ + "@type" => "type.googleapis.com/google.rpc.ErrorInfo", + "reason" => "SERVICE_DISABLED", + "domain" => "googleapis.com", + "metadata" => %{ + "service" => "admin.googleapis.com", + "consumer" => "projects/100421656358" + } + } + ] + } + } + + Bypass.expect_once(bypass, "GET", "/admin/directory/v1/users", fn conn -> + Plug.Conn.send_resp(conn, 401, Jason.encode!(response)) + end) + + assert sync_directory(%{}) == :ok + + assert updated_provider = Repo.get(Domain.Auth.Provider, provider.id) + refute updated_provider.last_synced_at + assert updated_provider.last_syncs_failed == 1 + assert updated_provider.last_sync_error == error_message + assert updated_provider.sync_disabled_at + end end end diff --git a/elixir/apps/domain/test/domain/auth_test.exs b/elixir/apps/domain/test/domain/auth_test.exs index fb8f3afed..4948f2e12 100644 --- a/elixir/apps/domain/test/domain/auth_test.exs +++ b/elixir/apps/domain/test/domain/auth_test.exs @@ -10,20 +10,20 @@ defmodule Domain.AuthTest do test "returns list of enabled adapters for an account" do account = Fixtures.Accounts.create_account(features: %{idp_sync: true}) - assert list_user_provisioned_provider_adapters!(account) == [ - openid_connect: [enabled: true], + assert Enum.sort(list_user_provisioned_provider_adapters!(account)) == [ google_workspace: [enabled: true], microsoft_entra: [enabled: true], - okta: [enabled: true] + okta: [enabled: true], + openid_connect: [enabled: true] ] account = Fixtures.Accounts.create_account(features: %{idp_sync: false}) - assert list_user_provisioned_provider_adapters!(account) == [ - openid_connect: [enabled: true], + assert Enum.sort(list_user_provisioned_provider_adapters!(account)) == [ google_workspace: [enabled: false], microsoft_entra: [enabled: false], - okta: [enabled: false] + okta: [enabled: false], + openid_connect: [enabled: true] ] end end @@ -482,6 +482,19 @@ defmodule Domain.AuthTest do Domain.Fixture.update!(provider, %{last_synced_at: four_hours_one_minute_ago}) assert {:ok, [_provider]} = list_providers_pending_sync_by_adapter(:google_workspace) end + + test "ignores providers with disabled sync" do + {provider, _bypass} = Fixtures.Auth.start_and_create_google_workspace_provider() + + eleven_minutes_ago = DateTime.utc_now() |> DateTime.add(-11, :minute) + + Domain.Fixture.update!(provider, %{ + last_synced_at: eleven_minutes_ago, + sync_disabled_at: DateTime.utc_now() + }) + + assert list_providers_pending_sync_by_adapter(:google_workspace) == {:ok, []} + end end describe "new_provider/2" do @@ -1806,6 +1819,65 @@ defmodule Domain.AuthTest do assert Repo.aggregate(Auth.Identity, :count) == 0 assert Repo.aggregate(Domain.Actors.Actor, :count) == 0 end + + test "resolves provider identifier conflicts across actors", %{ + account: account, + provider: provider + } do + identity1 = + Fixtures.Auth.create_identity( + account: account, + provider: provider, + provider_identifier: "USER_ID1", + actor: [type: :account_admin_user] + ) + |> Fixtures.Auth.delete_identity() + + identity2 = + Fixtures.Auth.create_identity( + account: account, + provider: provider, + provider_identifier: "USER_ID1", + actor: [type: :account_admin_user] + ) + + attrs_list = [ + %{ + "actor" => %{ + "name" => "Brian Manifold", + "type" => "account_user" + }, + "provider_identifier" => "USER_ID1" + } + ] + + multi = sync_provider_identities_multi(provider, attrs_list) + + assert {:ok, + %{ + identities: [_identity1, _identity2], + plan_identities: {[], update, []}, + delete_identities: [], + insert_identities: [], + actor_ids_by_provider_identifier: actor_ids_by_provider_identifier + }} = Repo.transaction(multi) + + assert length(update) == 2 + assert update == ["USER_ID1", "USER_ID1"] + + identity1 = Repo.get(Domain.Auth.Identity, identity1.id) |> Repo.preload(:actor) + assert identity1.deleted_at + assert identity1.actor.name != "Brian Manifold" + + identity2 = Repo.get(Domain.Auth.Identity, identity2.id) |> Repo.preload(:actor) + refute identity2.deleted_at + assert identity2.actor.name == "Brian Manifold" + + assert Map.get(actor_ids_by_provider_identifier, identity2.provider_identifier) == + identity2.actor.id + + assert Enum.count(actor_ids_by_provider_identifier) == 1 + end end describe "upsert_identity/3" do diff --git a/elixir/apps/domain/test/support/fixtures/auth.ex b/elixir/apps/domain/test/support/fixtures/auth.ex index ddbd35ac2..1693d4930 100644 --- a/elixir/apps/domain/test/support/fixtures/auth.ex +++ b/elixir/apps/domain/test/support/fixtures/auth.ex @@ -280,7 +280,11 @@ defmodule Domain.Fixtures.Auth do end def fail_provider_sync(provider) do - update!(provider, last_sync_error: "Message from fixture", last_syncs_failed: 3) + update!(provider, + last_sync_error: "Message from fixture", + last_syncs_failed: 3, + sync_disabled_at: DateTime.utc_now() + ) end def finish_provider_sync(provider) do diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/components.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/components.ex index a9f684025..f92ffeb5c 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/components.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/components.ex @@ -260,7 +260,8 @@ defmodule Web.Settings.IdentityProviders.Components do
3 && "bg-red-500") || "bg-green-500" + @provider.last_syncs_failed > 3 or (not is_nil(@provider.sync_disabled_at) && "bg-red-500") || + "bg-green-500" ]}> diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/google_workspace/connect.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/google_workspace/connect.ex index 051c8065e..9dfb1906b 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/google_workspace/connect.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/google_workspace/connect.ex @@ -49,7 +49,8 @@ defmodule Web.Settings.IdentityProviders.GoogleWorkspace.Connect do adapter_state: identity.provider_state, disabled_at: nil, last_syncs_failed: 0, - last_sync_error: nil + last_sync_error: nil, + sync_disabled_at: nil }, {:ok, _provider} <- Domain.Auth.update_provider(provider, attrs, subject) do redirect(conn, diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/google_workspace/show.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/google_workspace/show.ex index 9bdc18f47..2398f4d44 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/google_workspace/show.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/google_workspace/show.ex @@ -115,6 +115,7 @@ defmodule Web.Settings.IdentityProviders.GoogleWorkspace.Show do
3 and not is_nil(@provider.last_sync_error)) } class="p-3 mt-2 border-l-4 border-red-500 bg-red-100 rounded-md" diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/microsoft_entra/connect.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/microsoft_entra/connect.ex index ab1f7dea4..ca6f1d9b2 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/microsoft_entra/connect.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/microsoft_entra/connect.ex @@ -49,7 +49,8 @@ defmodule Web.Settings.IdentityProviders.MicrosoftEntra.Connect do adapter_state: identity.provider_state, disabled_at: nil, last_syncs_failed: 0, - last_sync_error: nil + last_sync_error: nil, + sync_disabled_at: nil }, {:ok, _provider} <- Domain.Auth.update_provider(provider, attrs, subject) do redirect(conn, diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/microsoft_entra/show.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/microsoft_entra/show.ex index e88d22edd..5b26a0e24 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/microsoft_entra/show.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/microsoft_entra/show.ex @@ -115,6 +115,7 @@ defmodule Web.Settings.IdentityProviders.MicrosoftEntra.Show do
3 and not is_nil(@provider.last_sync_error)) } class="p-3 mt-2 border-l-4 border-red-500 bg-red-100 rounded-md" diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/okta/connect.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/okta/connect.ex index 7a26bbd10..6e943cdec 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/okta/connect.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/okta/connect.ex @@ -49,7 +49,8 @@ defmodule Web.Settings.IdentityProviders.Okta.Connect do adapter_state: identity.provider_state, disabled_at: nil, last_syncs_failed: 0, - last_sync_error: nil + last_sync_error: nil, + sync_disabled_at: nil }, {:ok, _provider} <- Domain.Auth.update_provider(provider, attrs, subject) do redirect(conn, diff --git a/elixir/apps/web/lib/web/live/settings/identity_providers/okta/show.ex b/elixir/apps/web/lib/web/live/settings/identity_providers/okta/show.ex index bb65f3c88..8ef74786e 100644 --- a/elixir/apps/web/lib/web/live/settings/identity_providers/okta/show.ex +++ b/elixir/apps/web/lib/web/live/settings/identity_providers/okta/show.ex @@ -113,6 +113,7 @@ defmodule Web.Settings.IdentityProviders.Okta.Show do
3 and not is_nil(@provider.last_sync_error)) } class="p-3 mt-2 border-l-4 border-red-500 bg-red-100 rounded-md" diff --git a/elixir/apps/web/test/web/live/settings/identity_providers/google_workspace/connect_test.exs b/elixir/apps/web/test/web/live/settings/identity_providers/google_workspace/connect_test.exs index ad7435f76..1e136921b 100644 --- a/elixir/apps/web/test/web/live/settings/identity_providers/google_workspace/connect_test.exs +++ b/elixir/apps/web/test/web/live/settings/identity_providers/google_workspace/connect_test.exs @@ -207,6 +207,7 @@ defmodule Web.Live.Settings.IdentityProviders.GoogleWorkspace.Connect do assert provider = Repo.get(Domain.Auth.Provider, provider.id) assert provider.last_sync_error == nil assert provider.last_syncs_failed == 0 + assert provider.sync_disabled_at == nil end test "redirects to the actors index when credentials are valid and return path is empty", %{ diff --git a/elixir/apps/web/test/web/live/settings/identity_providers/microsoft_entra/connect_test.exs b/elixir/apps/web/test/web/live/settings/identity_providers/microsoft_entra/connect_test.exs index d00f922df..0dbb71e3c 100644 --- a/elixir/apps/web/test/web/live/settings/identity_providers/microsoft_entra/connect_test.exs +++ b/elixir/apps/web/test/web/live/settings/identity_providers/microsoft_entra/connect_test.exs @@ -218,6 +218,7 @@ defmodule Web.Live.Settings.IdentityProviders.MicrosoftEntra.Connect do assert provider = Repo.get(Domain.Auth.Provider, provider.id) assert provider.last_sync_error == nil assert provider.last_syncs_failed == 0 + assert provider.sync_disabled_at == nil end test "redirects to the actors index when credentials are valid and return path is empty", %{ diff --git a/elixir/apps/web/test/web/live/settings/identity_providers/okta/connect_test.exs b/elixir/apps/web/test/web/live/settings/identity_providers/okta/connect_test.exs index 2924a1df3..22c2fb644 100644 --- a/elixir/apps/web/test/web/live/settings/identity_providers/okta/connect_test.exs +++ b/elixir/apps/web/test/web/live/settings/identity_providers/okta/connect_test.exs @@ -210,6 +210,7 @@ defmodule Web.Live.Settings.IdentityProviders.Okta.Connect do assert provider = Repo.get(Domain.Auth.Provider, provider.id) assert provider.last_sync_error == nil assert provider.last_syncs_failed == 0 + assert provider.sync_disabled_at == nil end test "redirects to the actors index when credentials are valid and return path is empty", %{