diff --git a/elixir/apps/domain/lib/domain/auth/adapter/openid_connect/directory_sync.ex b/elixir/apps/domain/lib/domain/auth/adapter/openid_connect/directory_sync.ex index 16edc395f..ea9c2cf07 100644 --- a/elixir/apps/domain/lib/domain/auth/adapter/openid_connect/directory_sync.ex +++ b/elixir/apps/domain/lib/domain/auth/adapter/openid_connect/directory_sync.ex @@ -225,7 +225,7 @@ defmodule Domain.Auth.Adapter.OpenIDConnect.DirectorySync do Auth.Provider.Changeset.sync_failed(provider, user_message) |> Domain.Repo.update!() - |> send_sync_error_email() + |> send_rate_limited_sync_error_email() |> log_sync_error(log_message) :error @@ -384,27 +384,30 @@ defmodule Domain.Auth.Adapter.OpenIDConnect.DirectorySync do end) end + defp send_rate_limited_sync_error_email(provider) do + if notification_criteria_met?(provider) do + send_sync_error_email(provider) + else + Logger.debug("Sync error email already sent today") + provider + end + end + defp send_sync_error_email(provider) do provider = Repo.preload(provider, :account) - if notification_criteria_met?(provider) do - Domain.Actors.all_admins_for_account!(provider.account, preload: :identities) - |> Enum.flat_map(fn actor -> - Enum.map(actor.identities, &Domain.Auth.get_identity_email(&1)) - end) - |> Enum.uniq() - |> Enum.each(fn email -> - Domain.Mailer.SyncEmail.sync_error_email(provider, email) - |> Domain.Mailer.deliver() - end) + Domain.Actors.all_admins_for_account!(provider.account, preload: :identities) + |> Enum.flat_map(fn actor -> + Enum.map(actor.identities, &Domain.Auth.get_identity_email(&1)) + end) + |> Enum.uniq() + |> Enum.each(fn email -> + Domain.Mailer.SyncEmail.sync_error_email(provider, email) + |> Domain.Mailer.deliver() + end) - Auth.Provider.Changeset.sync_error_emailed(provider) - |> Domain.Repo.update!() - else - Logger.debug("Sync error email already sent today") - - provider - end + Auth.Provider.Changeset.sync_error_emailed(provider) + |> Domain.Repo.update!() end defp notification_criteria_met?(provider) do diff --git a/elixir/apps/domain/lib/domain/mailer/sync_email/sync_error.html.eex b/elixir/apps/domain/lib/domain/mailer/sync_email/sync_error.html.eex index a6085469e..6f281d639 100644 --- a/elixir/apps/domain/lib/domain/mailer/sync_email/sync_error.html.eex +++ b/elixir/apps/domain/lib/domain/mailer/sync_email/sync_error.html.eex @@ -66,7 +66,7 @@ <%= @provider.name %> Sync Error!

- <%= @provider.name %> has failed to sync <%= @provider.last_syncs_failed %> times. + <%= @provider.name %> has failed to sync <%= @provider.last_syncs_failed %> time(s).

‍

diff --git a/elixir/apps/domain/lib/domain/mailer/sync_email/sync_error.text.eex b/elixir/apps/domain/lib/domain/mailer/sync_email/sync_error.text.eex index 18d80e66e..ee6cfaaa4 100644 --- a/elixir/apps/domain/lib/domain/mailer/sync_email/sync_error.text.eex +++ b/elixir/apps/domain/lib/domain/mailer/sync_email/sync_error.text.eex @@ -1,6 +1,6 @@ Identity Provider Sync Error! -An Identity Provider in your Firezone Account has failed to sync <%= @provider.last_syncs_failed%> times. +An Identity Provider in your Firezone Account has failed to sync <%= @provider.last_syncs_failed%> time(s). The following is the last sync error message: <%= @provider.last_sync_error %> diff --git a/elixir/apps/domain/mix.exs b/elixir/apps/domain/mix.exs index b52edde51..336fae6d6 100644 --- a/elixir/apps/domain/mix.exs +++ b/elixir/apps/domain/mix.exs @@ -59,7 +59,7 @@ defmodule Domain.MixProject do {:openid_connect, github: "firezone/openid_connect", ref: "e4d9dca8ae43c765c00a7d3dfa12d6f24f5b3418"}, {:argon2_elixir, "~> 4.0"}, - {:workos, git: "https://github.com/firezone/workos-elixir.git", branch: "main"}, + {:workos, "~> 1.1"}, # Erlang Clustering {:libcluster, "~> 3.3"}, diff --git a/elixir/apps/domain/test/domain/auth/adapters/google_workspace/jobs/sync_directory_test.exs b/elixir/apps/domain/test/domain/auth/adapters/google_workspace/jobs/sync_directory_test.exs index d03216fdc..d53f62945 100644 --- a/elixir/apps/domain/test/domain/auth/adapters/google_workspace/jobs/sync_directory_test.exs +++ b/elixir/apps/domain/test/domain/auth/adapters/google_workspace/jobs/sync_directory_test.exs @@ -747,10 +747,12 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.Jobs.SyncDirectoryTest do cancel_bypass_expectations_check(bypass) end - test "disables the sync on 401 response code", %{provider: provider} do + test "disables the sync on 401 response code", %{account: account, provider: provider} do bypass = Bypass.open() GoogleWorkspaceDirectory.override_endpoint_url("http://localhost:#{bypass.port}/") GoogleWorkspaceDirectory.mock_token_endpoint(bypass) + actor = Fixtures.Actors.create_actor(type: :account_admin_user, account: account) + _identity = Fixtures.Auth.create_identity(account: account, actor: actor) error_message = "Admin SDK API has not been used in project XXXX before or it is disabled. " <> @@ -813,6 +815,11 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.Jobs.SyncDirectoryTest do assert updated_provider.last_sync_error == error_message assert updated_provider.sync_disabled_at + assert_email_sent(fn email -> + assert email.subject == "Firezone Identity Provider Sync Error" + assert email.text_body =~ "failed to sync 1 time(s)" + end) + cancel_bypass_expectations_check(bypass) end @@ -882,9 +889,14 @@ defmodule Domain.Auth.Adapters.GoogleWorkspace.Jobs.SyncDirectoryTest do assert_email_sent(fn email -> assert email.subject == "Firezone Identity Provider Sync Error" - assert email.text_body =~ "failed to sync 10 times" + assert email.text_body =~ "failed to sync 10 time(s)" end) + {:ok, pid} = Task.Supervisor.start_link() + assert execute(%{task_supervisor: pid}) == :ok + + refute_email_sent() + cancel_bypass_expectations_check(bypass) end end diff --git a/elixir/apps/domain/test/domain/auth/adapters/jumpcloud/jobs/sync_directory_test.exs b/elixir/apps/domain/test/domain/auth/adapters/jumpcloud/jobs/sync_directory_test.exs index d5368021d..08c47ec28 100644 --- a/elixir/apps/domain/test/domain/auth/adapters/jumpcloud/jobs/sync_directory_test.exs +++ b/elixir/apps/domain/test/domain/auth/adapters/jumpcloud/jobs/sync_directory_test.exs @@ -589,9 +589,14 @@ defmodule Domain.Auth.Adapters.JumpCloud.Jobs.SyncDirectoryTest do assert_email_sent(fn email -> assert email.subject == "Firezone Identity Provider Sync Error" - assert email.text_body =~ "failed to sync 10 times" + assert email.text_body =~ "failed to sync 10 time(s)" end) + {:ok, pid} = Task.Supervisor.start_link() + assert execute(%{task_supervisor: pid}) == :ok + + refute_email_sent() + cancel_bypass_expectations_check(bypass) end end diff --git a/elixir/apps/domain/test/domain/auth/adapters/microsoft_entra/jobs/sync_directory_test.exs b/elixir/apps/domain/test/domain/auth/adapters/microsoft_entra/jobs/sync_directory_test.exs index 2fa71eedc..2bbb5544a 100644 --- a/elixir/apps/domain/test/domain/auth/adapters/microsoft_entra/jobs/sync_directory_test.exs +++ b/elixir/apps/domain/test/domain/auth/adapters/microsoft_entra/jobs/sync_directory_test.exs @@ -407,9 +407,14 @@ defmodule Domain.Auth.Adapters.MicrosoftEntra.Jobs.SyncDirectoryTest do refute_received {:expire_flow, _flow_id, _client_id, _resource_id} end - test "stops the sync retires on 401 error on the provider", %{provider: provider} do + test "stops the sync retries on 401 error on the provider", %{ + account: account, + provider: provider + } do bypass = Bypass.open() MicrosoftEntraDirectory.override_endpoint_url("http://localhost:#{bypass.port}/") + actor = Fixtures.Actors.create_actor(type: :account_admin_user, account: account) + _identity = Fixtures.Auth.create_identity(account: account, actor: actor) error_message = "Lifetime validation failed, the token is expired." @@ -441,6 +446,11 @@ defmodule Domain.Auth.Adapters.MicrosoftEntra.Jobs.SyncDirectoryTest do refute updated_provider.last_synced_at assert updated_provider.last_syncs_failed == 1 assert updated_provider.last_sync_error == error_message + + assert_email_sent(fn email -> + assert email.subject == "Firezone Identity Provider Sync Error" + assert email.text_body =~ "failed to sync 1 time(s)" + end) end test "persists the sync error on the provider", %{provider: provider} do @@ -507,9 +517,14 @@ defmodule Domain.Auth.Adapters.MicrosoftEntra.Jobs.SyncDirectoryTest do assert_email_sent(fn email -> assert email.subject == "Firezone Identity Provider Sync Error" - assert email.text_body =~ "failed to sync 10 times" + assert email.text_body =~ "failed to sync 10 time(s)" end) + {:ok, pid} = Task.Supervisor.start_link() + assert execute(%{task_supervisor: pid}) == :ok + + refute_email_sent() + cancel_bypass_expectations_check(bypass) end end diff --git a/elixir/apps/domain/test/domain/auth/adapters/okta/jobs/sync_directory_test.exs b/elixir/apps/domain/test/domain/auth/adapters/okta/jobs/sync_directory_test.exs index fc771cac7..73123e061 100644 --- a/elixir/apps/domain/test/domain/auth/adapters/okta/jobs/sync_directory_test.exs +++ b/elixir/apps/domain/test/domain/auth/adapters/okta/jobs/sync_directory_test.exs @@ -819,9 +819,14 @@ defmodule Domain.Auth.Adapters.Okta.Jobs.SyncDirectoryTest do assert_email_sent(fn email -> assert email.subject == "Firezone Identity Provider Sync Error" - assert email.text_body =~ "failed to sync 10 times" + assert email.text_body =~ "failed to sync 10 time(s)" end) + {:ok, pid} = Task.Supervisor.start_link() + assert execute(%{task_supervisor: pid}) == :ok + + refute_email_sent() + cancel_bypass_expectations_check(bypass) end end diff --git a/elixir/apps/domain/test/domain/mailer/sync_error_email_test.exs b/elixir/apps/domain/test/domain/mailer/sync_error_email_test.exs index 0cbad69fe..2ab05f7c9 100644 --- a/elixir/apps/domain/test/domain/mailer/sync_error_email_test.exs +++ b/elixir/apps/domain/test/domain/mailer/sync_error_email_test.exs @@ -25,7 +25,7 @@ defmodule Domain.Mailer.SyncErrorEmailTest do email_body = sync_error_email(provider, admin_email) - assert email_body.text_body =~ "2 times" + assert email_body.text_body =~ "2 time(s)" assert email_body.text_body =~ expected_msg end end diff --git a/elixir/mix.lock b/elixir/mix.lock index 4e211d657..03a038073 100644 --- a/elixir/mix.lock +++ b/elixir/mix.lock @@ -109,7 +109,7 @@ "web_driver_client": {:hex, :web_driver_client, "0.2.0", "63b76cd9eb3b0716ec5467a0f8bead73d3d9612e63f7560d21357f03ad86e31a", [:mix], [{:hackney, "~> 1.6", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:tesla, "~> 1.3", [hex: :tesla, repo: "hexpm", optional: false]}], "hexpm", "83cc6092bc3e74926d1c8455f0ce927d5d1d36707b74d9a65e38c084aab0350f"}, "websock": {:hex, :websock, "0.5.3", "2f69a6ebe810328555b6fe5c831a851f485e303a7c8ce6c5f675abeb20ebdadc", [:mix], [], "hexpm", "6105453d7fac22c712ad66fab1d45abdf049868f253cf719b625151460b8b453"}, "websock_adapter": {:hex, :websock_adapter, "0.5.7", "65fa74042530064ef0570b75b43f5c49bb8b235d6515671b3d250022cb8a1f9e", [:mix], [{:bandit, ">= 0.6.0", [hex: :bandit, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.6", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "d0f478ee64deddfec64b800673fd6e0c8888b079d9f3444dd96d2a98383bdbd1"}, - "workos": {:git, "https://github.com/firezone/workos-elixir.git", "6d6995e2a765656a6834577837433393fac9b35b", [branch: "main"]}, + "workos": {:hex, :workos, "1.1.0", "b64de032a254ebfb86b90b274e5d8865042afda6389e3c4d2d889ee0f4bd7735", [:mix], [{:hackney, "~> 1.9", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.4.1", [hex: :jason, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:tesla, "~> 1.4", [hex: :tesla, repo: "hexpm", optional: false]}], "hexpm", "88034983748821353cc28660278e0fd1886378bd4888ea77651889c0d126f3d5"}, "yamerl": {:hex, :yamerl, "0.10.0", "4ff81fee2f1f6a46f1700c0d880b24d193ddb74bd14ef42cb0bcf46e81ef2f8e", [:rebar3], [], "hexpm", "346adb2963f1051dc837a2364e4acf6eb7d80097c0f53cbdc3046ec8ec4b4e6e"}, "yaml_elixir": {:hex, :yaml_elixir, "2.11.0", "9e9ccd134e861c66b84825a3542a1c22ba33f338d82c07282f4f1f52d847bd50", [:mix], [{:yamerl, "~> 0.10", [hex: :yamerl, repo: "hexpm", optional: false]}], "hexpm", "53cc28357ee7eb952344995787f4bb8cc3cecbf189652236e9b163e8ce1bc242"}, "ymlr": {:hex, :ymlr, "5.1.3", "a8061add5a378e20272a31905be70209a5680fdbe0ad51f40cb1af4bdd0a010b", [:mix], [], "hexpm", "8663444fa85101a117887c170204d4c5a2182567e5f84767f0071cf15f2efb1e"},