From 4d43d2cb77f12b3056ba5d32e0d1a3a8f79e34e6 Mon Sep 17 00:00:00 2001
From: Jamil <jamilbk@users.noreply.github.com>
Date: Thu, 16 Oct 2025 08:10:04 -0700
Subject: [PATCH] fix(portal): trigger email for Okta 4xx errors (#10578)

When Okta returned a 4xx status code from the API, we had updated error
handler to grab the errors from body or headers and return these.

However, the caller was expecting an explicit empty string for 401 and
403 errors in order to trigger the email send behavior.

Since that wasn't being matched, we were logging the error internally
only, and continuing to retry the sync indefinitely without sending the
user an email.


Fixes #8744
Fixes #9825
---
 .../auth/adapters/okta/jobs/sync_directory.ex | 12 ++---
 .../okta/jobs/sync_directory_test.exs         | 51 ++++++++++++++++++-
 2 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/elixir/apps/domain/lib/domain/auth/adapters/okta/jobs/sync_directory.ex b/elixir/apps/domain/lib/domain/auth/adapters/okta/jobs/sync_directory.ex
index a8588a42e..6689a68e7 100644
--- a/elixir/apps/domain/lib/domain/auth/adapters/okta/jobs/sync_directory.ex
+++ b/elixir/apps/domain/lib/domain/auth/adapters/okta/jobs/sync_directory.ex
@@ -44,15 +44,15 @@ defmodule Domain.Auth.Adapters.Okta.Jobs.SyncDirectory do
         message = "#{error_code} => #{error_summary}"
         {:error, message, "Okta API returned #{status}: #{message}"}
 
-      # TODO: Okta API client needs to be updated to pull message from header
-      {:error, {401, ""}} ->
+      {:error, {401, error_map}} ->
+        user_message = inspect(error_map, pretty: true)
         message = "401 - Unauthorized"
-        {:error, message, message}
+        {:error, user_message, message}
 
-      # TODO: Okta API client needs to be updated to pull message from header
-      {:error, {403, ""}} ->
+      {:error, {403, error_map}} ->
+        user_message = inspect(error_map, pretty: true)
         message = "403 - Forbidden"
-        {:error, message, message}
+        {:error, user_message, message}
 
       {:error, :retry_later} ->
         message = "Okta API is temporarily unavailable"
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 94dfcf4ee..88758c185 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
@@ -941,7 +941,7 @@ defmodule Domain.Auth.Adapters.Okta.Jobs.SyncDirectoryTest do
       cancel_bypass_expectations_check(bypass)
     end
 
-    test "sends email on failed directory sync", %{
+    test "sends email on failed directory sync when errors are in body", %{
       account: account,
       provider: provider,
       bypass: bypass
@@ -986,5 +986,54 @@ defmodule Domain.Auth.Adapters.Okta.Jobs.SyncDirectoryTest do
 
       cancel_bypass_expectations_check(bypass)
     end
+
+    test "sends email on failed directory sync when errors are in header", %{
+      account: account,
+      provider: provider,
+      bypass: bypass
+    } do
+      actor = Fixtures.Actors.create_actor(type: :account_admin_user, account: account)
+      _identity = Fixtures.Auth.create_identity(account: account, actor: actor)
+
+      # Okta returns 401 with error info in www-authenticate header
+      www_authenticate_header =
+        "Bearer authorization_uri=\"http://example.okta.com/oauth2/v1/authorize\", " <>
+          "realm=\"http://example.okta.com\", " <>
+          "scope=\"okta.groups.read\", " <>
+          "error=\"invalid_token\", " <>
+          "error_description=\"The token has expired.\", " <>
+          "resource=\"/api/v1/groups\""
+
+      for path <- [
+            "api/v1/users",
+            "api/v1/groups"
+          ] do
+        Bypass.stub(bypass, "GET", path, fn conn ->
+          conn
+          |> Plug.Conn.put_resp_header("www-authenticate", www_authenticate_header)
+          |> Plug.Conn.send_resp(401, "")
+        end)
+      end
+
+      {:ok, pid} = Task.Supervisor.start_link()
+
+      provider
+      |> Ecto.Changeset.change(last_syncs_failed: 9)
+      |> Repo.update!()
+
+      assert execute(%{task_supervisor: pid}) == :ok
+
+      assert_email_sent(fn email ->
+        assert email.subject == "Firezone Identity Provider Sync Error"
+        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
 end