local-it/mitgliederverwaltung
2
3
Fork 0
Code Issues 67 Pull requests 7 Projects 3 Releases Packages 1 Activity Actions

Security: Require actor parameter in CSV import

Browse source 
Remove fallback to system_actor in process_chunk to prevent
unauthorized access. Actor must now be explicitly provided.
This commit is contained in:
Moritz 2026-01-25 18:33:25 +01:00 • committed by Simon
parent 33dc8307c8
commit 64952d4ff4
Signed by: simon
GPG key ID: 40E7A58C4AA1EDB2
2 changed files with 48 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

2
lib/mv/membership/import/member_csv.ex
View file

@ -299,7 +299,7 @@ defmodule Mv.Membership.Import.MemberCSV do
custom_field_lookup = Keyword.get(opts, :custom_field_lookup, %{}) custom_field_lookup = Keyword.get(opts, :custom_field_lookup, %{})
existing_error_count = Keyword.get(opts, :existing_error_count, 0) existing_error_count = Keyword.get(opts, :existing_error_count, 0)
max_errors = Keyword.get(opts, :max_errors, @default_max_errors) max_errors = Keyword.get(opts, :max_errors, @default_max_errors)
actor = Keyword.get(opts, :actor) actor = Keyword.fetch!(opts, :actor)
{inserted, failed, errors, _collected_error_count, truncated?} = {inserted, failed, errors, _collected_error_count, truncated?} =
Enum.reduce(chunk_rows_with_lines, {0, 0, [], 0, false}, fn {line_number, row_map}, Enum.reduce(chunk_rows_with_lines, {0, 0, [], 0, false}, fn {line_number, row_map},

89
test/mv/membership/import/member_csv_test.exs
View file

@ -73,25 +73,33 @@ defmodule Mv.Membership.Import.MemberCSVTest do
end end
describe "process_chunk/4" do describe "process_chunk/4" do
test "function exists and accepts chunk_rows_with_lines, column_map, custom_field_map, and opts" do setup do
system_actor = Mv.Helpers.SystemActor.get_system_actor()
%{actor: system_actor}
end
test "function exists and accepts chunk_rows_with_lines, column_map, custom_field_map, and opts",
%{
actor: actor
} do
chunk_rows_with_lines = [{2, %{member: %{email: "john@example.com"}, custom: %{}}}] chunk_rows_with_lines = [{2, %{member: %{email: "john@example.com"}, custom: %{}}}]
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{} custom_field_map = %{}
opts = [] opts = [actor: actor]
# This will fail until the function is implemented # This will fail until the function is implemented
result = MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) result = MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
assert match?({:ok, _}, result) or match?({:error, _}, result) assert match?({:ok, _}, result) or match?({:error, _}, result)
end end
test "creates member successfully with valid data" do test "creates member successfully with valid data", %{actor: actor} do
chunk_rows_with_lines = [ chunk_rows_with_lines = [
{2, %{member: %{email: "john@example.com", first_name: "John"}, custom: %{}}} {2, %{member: %{email: "john@example.com", first_name: "John"}, custom: %{}}}
] ]
column_map = %{email: 0, first_name: 1} column_map = %{email: 0, first_name: 1}
custom_field_map = %{} custom_field_map = %{}
opts = [] opts = [actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -106,14 +114,14 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert Enum.any?(members, &(&1.email == "john@example.com")) assert Enum.any?(members, &(&1.email == "john@example.com"))
end end
test "returns error for invalid email" do test "returns error for invalid email", %{actor: actor} do
chunk_rows_with_lines = [ chunk_rows_with_lines = [
{2, %{member: %{email: "invalid-email"}, custom: %{}}} {2, %{member: %{email: "invalid-email"}, custom: %{}}}
] ]
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{} custom_field_map = %{}
opts = [] opts = [actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -130,14 +138,14 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert error.message != "" assert error.message != ""
end end
test "returns error for missing email" do test "returns error for missing email", %{actor: actor} do
chunk_rows_with_lines = [ chunk_rows_with_lines = [
{2, %{member: %{}, custom: %{}}} {2, %{member: %{}, custom: %{}}}
] ]
column_map = %{} column_map = %{}
custom_field_map = %{} custom_field_map = %{}
opts = [] opts = [actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -152,14 +160,14 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert is_binary(error.message) assert is_binary(error.message)
end end
test "returns error for whitespace-only email" do test "returns error for whitespace-only email", %{actor: actor} do
chunk_rows_with_lines = [ chunk_rows_with_lines = [
{3, %{member: %{email: " "}, custom: %{}}} {3, %{member: %{email: " "}, custom: %{}}}
] ]
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{} custom_field_map = %{}
opts = [] opts = [actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -173,13 +181,11 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert error.field == :email assert error.field == :email
end end
test "returns error for duplicate email" do test "returns error for duplicate email", %{actor: actor} do
# Create existing member first # Create existing member first
system_actor = Mv.Helpers.SystemActor.get_system_actor()
{:ok, _existing} = {:ok, _existing} =
Mv.Membership.create_member(%{email: "duplicate@example.com", first_name: "Existing"}, Mv.Membership.create_member(%{email: "duplicate@example.com", first_name: "Existing"},
actor: system_actor actor: actor
) )
chunk_rows_with_lines = [ chunk_rows_with_lines = [
@ -188,7 +194,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
column_map = %{email: 0, first_name: 1} column_map = %{email: 0, first_name: 1}
custom_field_map = %{} custom_field_map = %{}
opts = [] opts = [actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -203,9 +209,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert error.message =~ "email" or error.message =~ "duplicate" or error.message =~ "unique" assert error.message =~ "email" or error.message =~ "duplicate" or error.message =~ "unique"
end end
test "creates member with custom field values" do test "creates member with custom field values", %{actor: actor} do
system_actor = Mv.Helpers.SystemActor.get_system_actor()
# Create custom field first # Create custom field first
{:ok, custom_field} = {:ok, custom_field} =
Mv.Membership.CustomField Mv.Membership.CustomField
@ -213,7 +217,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
name: "Phone", name: "Phone",
value_type: :string value_type: :string
}) })
|> Ash.create(actor: system_actor) |> Ash.create(actor: actor)
chunk_rows_with_lines = [ chunk_rows_with_lines = [
{2, {2,
@ -230,7 +234,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
to_string(custom_field.id) => %{id: custom_field.id, value_type: custom_field.value_type} to_string(custom_field.id) => %{id: custom_field.id, value_type: custom_field.value_type}
} }
opts = [custom_field_lookup: custom_field_lookup] opts = [custom_field_lookup: custom_field_lookup, actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -239,8 +243,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert chunk_result.failed == 0 assert chunk_result.failed == 0
# Verify member and custom field value were created # Verify member and custom field value were created
system_actor = Mv.Helpers.SystemActor.get_system_actor() members = Mv.Membership.list_members!(actor: actor)
members = Mv.Membership.list_members!(actor: system_actor)
member = Enum.find(members, &(&1.email == "withcustom@example.com")) member = Enum.find(members, &(&1.email == "withcustom@example.com"))
assert member != nil assert member != nil
@ -251,7 +254,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert cfv.value.value == "123-456-7890" assert cfv.value.value == "123-456-7890"
end end
test "handles multiple rows with mixed success and failure" do test "handles multiple rows with mixed success and failure", %{actor: actor} do
chunk_rows_with_lines = [ chunk_rows_with_lines = [
{2, %{member: %{email: "valid1@example.com"}, custom: %{}}}, {2, %{member: %{email: "valid1@example.com"}, custom: %{}}},
{3, %{member: %{email: "invalid-email"}, custom: %{}}}, {3, %{member: %{email: "invalid-email"}, custom: %{}}},
@ -260,7 +263,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{} custom_field_map = %{}
opts = [] opts = [actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -276,7 +279,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert is_binary(error.message) assert is_binary(error.message)
end end
test "preserves CSV line numbers in errors" do test "preserves CSV line numbers in errors", %{actor: actor} do
chunk_rows_with_lines = [ chunk_rows_with_lines = [
{5, %{member: %{email: "invalid"}, custom: %{}}}, {5, %{member: %{email: "invalid"}, custom: %{}}},
{10, %{member: %{email: "also-invalid"}, custom: %{}}} {10, %{member: %{email: "also-invalid"}, custom: %{}}}
@ -284,7 +287,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{} custom_field_map = %{}
opts = [] opts = [actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -297,11 +300,11 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert 10 in line_numbers assert 10 in line_numbers
end end
test "returns {:ok, chunk_result} on success" do test "returns {:ok, chunk_result} on success", %{actor: actor} do
chunk_rows_with_lines = [{2, %{member: %{email: "test@example.com"}, custom: %{}}}] chunk_rows_with_lines = [{2, %{member: %{email: "test@example.com"}, custom: %{}}}]
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{} custom_field_map = %{}
opts = [] opts = [actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -315,11 +318,11 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert is_list(chunk_result.errors) assert is_list(chunk_result.errors)
end end
test "returns {:ok, _} with zero counts for empty chunk" do test "returns {:ok, _} with zero counts for empty chunk", %{actor: actor} do
chunk_rows_with_lines = [] chunk_rows_with_lines = []
column_map = %{} column_map = %{}
custom_field_map = %{} custom_field_map = %{}
opts = [] opts = [actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -334,7 +337,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert function_exported?(MemberCSV, :process_chunk, 4) assert function_exported?(MemberCSV, :process_chunk, 4)
end end
test "error capping collects exactly 50 errors" do test "error capping collects exactly 50 errors", %{actor: actor} do
# Create 50 rows with invalid emails # Create 50 rows with invalid emails
chunk_rows_with_lines = chunk_rows_with_lines =
1..50 1..50
@ -344,7 +347,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{} custom_field_map = %{}
opts = [existing_error_count: 0, max_errors: 50] opts = [existing_error_count: 0, max_errors: 50, actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -354,7 +357,9 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert length(chunk_result.errors) == 50 assert length(chunk_result.errors) == 50
end end
test "error capping collects only first 50 errors when more than 50 errors occur" do test "error capping collects only first 50 errors when more than 50 errors occur", %{
actor: actor
} do
# Create 60 rows with invalid emails # Create 60 rows with invalid emails
chunk_rows_with_lines = chunk_rows_with_lines =
1..60 1..60
@ -364,7 +369,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{} custom_field_map = %{}
opts = [existing_error_count: 0, max_errors: 50] opts = [existing_error_count: 0, max_errors: 50, actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -374,7 +379,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert length(chunk_result.errors) == 50 assert length(chunk_result.errors) == 50
end end
test "error capping respects existing_error_count" do test "error capping respects existing_error_count", %{actor: actor} do
# Create 30 rows with invalid emails # Create 30 rows with invalid emails
chunk_rows_with_lines = chunk_rows_with_lines =
1..30 1..30
@ -384,7 +389,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{} custom_field_map = %{}
opts = [existing_error_count: 25, max_errors: 50] opts = [existing_error_count: 25, max_errors: 50, actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -395,7 +400,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert length(chunk_result.errors) == 25 assert length(chunk_result.errors) == 25
end end
test "error capping collects no errors when limit already reached" do test "error capping collects no errors when limit already reached", %{actor: actor} do
# Create 10 rows with invalid emails # Create 10 rows with invalid emails
chunk_rows_with_lines = chunk_rows_with_lines =
1..10 1..10
@ -405,7 +410,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{} custom_field_map = %{}
opts = [existing_error_count: 50, max_errors: 50] opts = [existing_error_count: 50, max_errors: 50, actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -415,7 +420,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert chunk_result.errors == [] assert chunk_result.errors == []
end end
test "error capping with mixed success and failure" do test "error capping with mixed success and failure", %{actor: actor} do
# Create 100 rows: 30 valid, 70 invalid # Create 100 rows: 30 valid, 70 invalid
valid_rows = valid_rows =
1..30 1..30
@ -433,7 +438,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{} custom_field_map = %{}
opts = [existing_error_count: 0, max_errors: 50] opts = [existing_error_count: 0, max_errors: 50, actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)
@ -444,7 +449,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
assert length(chunk_result.errors) == 50 assert length(chunk_result.errors) == 50
end end
test "error capping with custom max_errors" do test "error capping with custom max_errors", %{actor: actor} do
# Create 20 rows with invalid emails # Create 20 rows with invalid emails
chunk_rows_with_lines = chunk_rows_with_lines =
1..20 1..20
@ -454,7 +459,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{} custom_field_map = %{}
opts = [existing_error_count: 0, max_errors: 10] opts = [existing_error_count: 0, max_errors: 10, actor: actor]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)