fix(secrethub): race condition in jwt configurator (#190)

## 📝 Description
 Fixing the race condition in JWT configurator.

For more details, see [the
task](renderedtext/tasks#7524).

## ✅ Checklist
- [x] I have tested this change
- [x] ~This change requires documentation update~ - N/A

Author: dexyk

secrethub/lib/secrethub/open_id_connect/jwt_configuration.ex

@@ -48,22 +48,14 @@ defmodule Secrethub.OpenIDConnect.JWTConfiguration do
         is_active: true
       }
 
-      # First try to find existing config
-      query =
-        from c in __MODULE__,
-          where: c.org_id == ^org_id and is_nil(c.project_id)
-
-      case Repo.one(query) do
-        nil ->
-          %__MODULE__{}
-          |> changeset(attrs)
-          |> Repo.insert()
-
-        config ->
-          config
-          |> changeset(attrs)
-          |> Repo.update()
-      end
+      # Use upsert with conditional conflict target
+      %__MODULE__{}
+      |> changeset(attrs)
+      |> Repo.insert(
+        on_conflict: {:replace, [:claims, :is_active, :updated_at]},
+        conflict_target: {:unsafe_fragment, ~s<("org_id") WHERE project_id IS NULL>},
+        returning: true
+      )
     else
       {:error, :invalid_claims}
     end
@@ -93,24 +85,15 @@ defmodule Secrethub.OpenIDConnect.JWTConfiguration do
         is_active: true
       }
 
-      # First try to find existing config
-      query =
-        from c in __MODULE__,
-          where: c.org_id == ^org_id and c.project_id == ^project_id
-
-      case Repo.one(query) do
-        nil ->
-          # No existing config, create new one
-          %__MODULE__{}
-          |> changeset(attrs)
-          |> Repo.insert()
-
-        existing ->
-          # Update existing config
-          existing
-          |> changeset(%{claims: claims})
-          |> Repo.update()
-      end
+      # Use upsert with conditional conflict target for project-level config
+      %__MODULE__{}
+      |> changeset(attrs)
+      |> Repo.insert(
+        on_conflict: {:replace, [:claims, :is_active, :updated_at]},
+        conflict_target:
+          {:unsafe_fragment, ~s<("org_id", "project_id") WHERE project_id IS NOT NULL>},
+        returning: true
+      )
     else
       {:error, :invalid_claims}
     end
@@ -190,13 +173,9 @@ defmodule Secrethub.OpenIDConnect.JWTConfiguration do
   def delete_org_config(org_id) do
     query = from(c in __MODULE__, where: c.org_id == ^org_id)
 
-    case Repo.all(query) do
-      [] ->
-        {:error, :not_found}
-
-      configs ->
-        Enum.each(configs, &Repo.delete/1)
-        {:ok, :deleted}
+    case Repo.delete_all(query) do
+      {0, _} -> {:error, :not_found}
+      {_count, _} -> {:ok, :deleted}
     end
   end
 

secrethub/lib/secrethub/open_id_connect/jwt_filter.ex

@@ -10,6 +10,9 @@ defmodule Secrethub.OpenIDConnect.JWTFilter do
   def filter_enabled?(org_id),
     do: FeatureProvider.feature_enabled?(:open_id_connect_filter, param: org_id)
 
+  def project_filter_enabled?(org_id),
+    do: FeatureProvider.feature_enabled?(:open_id_connect_project_filter, param: org_id)
+
   @doc """
   Filters JWT claims based on organization or project settings.
   Uses JWT configuration if available, otherwise falls back to essential claims.
@@ -67,18 +70,32 @@ defmodule Secrethub.OpenIDConnect.JWTFilter do
   Returns a list of active claim names or error tuple.
   """
   def get_allowed_claims(org_id, project_id) do
-    case JWTConfiguration.get_project_config(org_id, project_id) do
-      {:ok, config} ->
-        active_claims =
-          config.claims
-          |> Enum.filter(fn claim -> claim["is_active"] == true end)
-          |> Enum.map(fn claim -> claim["name"] end)
-          |> Enum.sort()
+    if project_filter_enabled?(org_id) do
+      get_project_allowed_claims(org_id, project_id)
+    else
+      get_org_allowed_claims(org_id)
+    end
+  end
 
-        {:ok, active_claims}
+  defp get_project_allowed_claims(org_id, project_id) do
+    JWTConfiguration.get_project_config(org_id, project_id)
+    |> handle_get_config_result()
+  end
 
-      err ->
-        err
-    end
+  defp get_org_allowed_claims(org_id) do
+    JWTConfiguration.get_org_config(org_id)
+    |> handle_get_config_result()
+  end
+
+  defp handle_get_config_result({:ok, config}) do
+    active_claims =
+      config.claims
+      |> Enum.filter(fn claim -> claim["is_active"] == true end)
+      |> Enum.map(fn claim -> claim["name"] end)
+      |> Enum.sort()
+
+    {:ok, active_claims}
   end
+
+  defp handle_get_config_result(err), do: err
 end

secrethub/priv/repo/migrations/20250217100229_add_unique_indexes_to_jwt_configurations.exs

@@ -0,0 +1,37 @@
+defmodule Secrethub.Repo.Migrations.AddUniqueIndexesToJwtConfigurations do
+  use Ecto.Migration
+
+  def up do
+    # Drop any existing duplicate records before adding unique constraints
+    execute """
+    DELETE FROM jwt_configurations
+    WHERE id::text NOT IN (
+      SELECT MIN(id::text)
+      FROM jwt_configurations
+      GROUP BY org_id, project_id
+    );
+    """
+
+    create_if_not_exists unique_index(:jwt_configurations, [:org_id],
+      name: :jwt_configurations_org_unique_index,
+      where: "project_id IS NULL"
+    )
+
+    create_if_not_exists unique_index(:jwt_configurations, [:org_id, :project_id],
+      name: :jwt_configurations_org_project_unique_index,
+      where: "project_id IS NOT NULL"
+    )
+
+    drop_if_exists index(:jwt_configurations, [:org_id, :project_id])
+  end
+
+  def down do
+    drop_if_exists index(:jwt_configurations, [:org_id],
+      name: :jwt_configurations_org_unique_index
+    )
+
+    drop_if_exists index(:jwt_configurations, [:org_id, :project_id],
+      name: :jwt_configurations_org_project_unique_index
+    )
+  end
+end

secrethub/test/secrethub/open_id_connect/jwt_configuration_test.exs

@@ -668,7 +668,95 @@ defmodule Secrethub.OpenIDConnect.JWTConfigurationTest do
         }
       ]
 
-      {:ok, %{org_id: org_id, project_id: project_id, claims: claims}}
+      # Different claims for concurrent tests
+      concurrent_claims = [
+        %{
+          "name" => "sub",
+          "is_active" => true,
+          "description" => "Concurrent update"
+        }
+      ]
+
+      {:ok,
+       %{
+         org_id: org_id,
+         project_id: project_id,
+         claims: claims,
+         concurrent_claims: concurrent_claims
+       }}
+    end
+
+    test "handles concurrent org config creations", %{
+      org_id: org_id,
+      claims: claims,
+      concurrent_claims: concurrent_claims
+    } do
+      # Start multiple concurrent operations
+      tasks =
+        for _ <- 1..20 do
+          Task.async(fn ->
+            JWTConfiguration.create_or_update_org_config(org_id, claims)
+          end)
+        end
+
+      # Add a concurrent update with different claims
+      update_task =
+        Task.async(fn ->
+          JWTConfiguration.create_or_update_org_config(org_id, concurrent_claims)
+        end)
+
+      # Wait for all tasks to complete
+      results = Task.await_many([update_task | tasks], 5000)
+
+      # All operations should succeed
+      assert Enum.all?(results, fn {:ok, _} -> true end)
+
+      # Verify only one configuration exists
+      configs = Repo.all(JWTConfiguration)
+      org_configs = Enum.filter(configs, &is_nil(&1.project_id))
+      assert length(org_configs) == 1
+
+      # The final state should be consistent
+      {:ok, final_config} = JWTConfiguration.get_org_config(org_id)
+      assert final_config.org_id == org_id
+      assert is_nil(final_config.project_id)
+    end
+
+    test "handles concurrent project config creations", %{
+      org_id: org_id,
+      project_id: project_id,
+      claims: claims,
+      concurrent_claims: concurrent_claims
+    } do
+      # Start multiple concurrent operations
+      tasks =
+        for _ <- 1..10 do
+          Task.async(fn ->
+            JWTConfiguration.create_or_update_project_config(org_id, project_id, claims)
+          end)
+        end
+
+      # Add a concurrent update with different claims
+      update_task =
+        Task.async(fn ->
+          JWTConfiguration.create_or_update_project_config(org_id, project_id, concurrent_claims)
+        end)
+
+      # Wait for all tasks to complete
+      results = Task.await_many([update_task | tasks], 5000)
+
+      # All operations should succeed
+      assert Enum.all?(results, fn {:ok, _} -> true end)
+
+      # Verify only one configuration exists for this project
+      configs = Repo.all(JWTConfiguration)
+      project_configs = Enum.filter(configs, &(&1.project_id == project_id))
+      assert length(project_configs) == 1
+
+      # The final state should be consistent
+      {:ok, final_config} = JWTConfiguration.get_project_config(org_id, project_id)
+      assert final_config.org_id == org_id
+      assert final_config.project_id == project_id
     end
 
     test "allows updating existing org config", %{org_id: org_id, claims: claims} do

secrethub/test/secrethub/open_id_connect/jwt_filter_test.exs

@@ -43,7 +43,7 @@ defmodule Secrethub.OpenIDConnect.JWTFilterTest do
       }
 
       with_mock(JWTConfiguration, [],
-        get_project_config: fn "org_id", "project_id" ->
+        get_org_config: fn "org_id" ->
           {:ok,
            %{
              claims: [
@@ -85,7 +85,7 @@ defmodule Secrethub.OpenIDConnect.JWTFilterTest do
       }
 
       with_mock(JWTConfiguration, [],
-        get_project_config: fn "org_id", "project_id" ->
+        get_org_config: fn "org_id" ->
           {:ok,
            %{
              claims: [
@@ -116,7 +116,7 @@ defmodule Secrethub.OpenIDConnect.JWTFilterTest do
       }
 
       with_mock(JWTConfiguration, [],
-        get_project_config: fn "org_id", "project_id" ->
+        get_org_config: fn "org_id" ->
           {:ok,
            %{
              claims: [
@@ -131,15 +131,15 @@ defmodule Secrethub.OpenIDConnect.JWTFilterTest do
     end
 
     test "returns error for invalid claims" do
-      FakeServices.enable_features(["open_id_connect_filter"])
+      FakeServices.enable_features(["open_id_connect_filter", "open_id_connect_project_filter"])
       assert JWTFilter.filter_claims(nil, "org_id", "project_id") == {:error, :invalid_claims}
     end
 
     test "propagates JWT configuration errors" do
       FakeServices.enable_features(["open_id_connect_filter"])
 
       with_mock(JWTConfiguration, [],
-        get_project_config: fn "org_id", "project_id" ->
+        get_org_config: fn "org_id" ->
           {:error, :not_found}
         end
       ) do
@@ -151,7 +151,7 @@ defmodule Secrethub.OpenIDConnect.JWTFilterTest do
 
   describe "get_allowed_claims/2" do
     test "returns active claims from JWT configuration" do
-      FakeServices.enable_features(["open_id_connect_filter"])
+      FakeServices.enable_features(["open_id_connect_filter", "open_id_connect_project_filter"])
 
       with_mock(JWTConfiguration, [],
         get_project_config: fn "org_id", "project_id" ->
@@ -170,7 +170,7 @@ defmodule Secrethub.OpenIDConnect.JWTFilterTest do
     end
 
     test "propagates JWT configuration errors" do
-      FakeServices.enable_features(["open_id_connect_filter"])
+      FakeServices.enable_features(["open_id_connect_filter", "open_id_connect_project_filter"])
 
       with_mock(JWTConfiguration, [],
         get_project_config: fn "org_id", "project_id" ->
@@ -259,7 +259,7 @@ defmodule Secrethub.OpenIDConnect.JWTFilterTest do
     end
 
     test "project config overrides org config", %{org_id: org_id, project_id: project_id} do
-      FakeServices.enable_features(["open_id_connect_filter"])
+      FakeServices.enable_features(["open_id_connect_filter", "open_id_connect_project_filter"])
       # Create org configuration with certain claims active
       {:ok, org_config} = JWTConfiguration.get_org_config(org_id)
 
@@ -293,7 +293,7 @@ defmodule Secrethub.OpenIDConnect.JWTFilterTest do
     end
 
     test "handles non-existent project configuration", %{org_id: org_id, project_id: project_id} do
-      FakeServices.enable_features(["open_id_connect_filter"])
+      FakeServices.enable_features(["open_id_connect_filter", "open_id_connect_project_filter"])
       # Create org configuration with active claims
       {:ok, org_config} = JWTConfiguration.get_org_config(org_id)
 
@@ -355,7 +355,7 @@ defmodule Secrethub.OpenIDConnect.JWTFilterTest do
       org_id: org_id,
       project_id: project_id
     } do
-      FakeServices.enable_features(["open_id_connect_filter"])
+      FakeServices.enable_features(["open_id_connect_filter", "open_id_connect_project_filter"])
 
       project_claims = [
         %{"name" => "sub", "is_active" => true},