fix(portal): Update IDP sync error email threshold (#7757)

Why:

* An IdP sync can fail for different reasons and because of this we
previously put a threshold on when to send the first 'IdP sync failed'
email, which was set at 10 failed sync attempts. One thing that was
accidentally overlooked was that on one specific failure type (i.e. 401
- Unauthorized) the Firezone sync was automatically disabled and not
tried from that point forward. Unfortunately, that meant an email did
not get sent out because the threshold was not met. This PR resolves
that by making sure the 401 error will send out an email immediately,
while keeping the 10 failed sync threshold for all other errors.

Closes: #7725
This commit is contained in:
Brian Manifold
2025-01-15 12:03:12 -05:00
committed by GitHub
parent 55485c71e6
commit 430b32324a
10 changed files with 69 additions and 29 deletions

View File

@@ -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

View File

@@ -66,7 +66,7 @@
<%= @provider.name %> Sync Error!
</h1>
<p style="margin: 0; line-height: 24px">
<%= @provider.name %> has failed to sync <%= @provider.last_syncs_failed %> times.
<%= @provider.name %> has failed to sync <%= @provider.last_syncs_failed %> time(s).
</p>
<div role="separator" style="line-height: 16px">&zwj;</div>
<p style="margin: 0; line-height: 24px;">

View File

@@ -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 %>

View File

@@ -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"},

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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"},