From 7f4c22d072a77669a358bcca2847cae72abf1d10 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 20 Jan 2026 18:34:17 +0100 Subject: [PATCH] refactor: fix credo issues --- lib/mv_web/live/member_live/index.ex | 98 +++++++++++++++++++------- test/mv_web/member_live/index_test.exs | 4 +- 2 files changed, 74 insertions(+), 28 deletions(-) diff --git a/lib/mv_web/live/member_live/index.ex b/lib/mv_web/live/member_live/index.ex index 6eba629..8e3d5f1 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -1194,32 +1194,13 @@ defmodule MvWeb.MemberLive.Index do params |> Enum.filter(fn {key, _value} -> String.starts_with?(key, @boolean_filter_prefix) end) |> Enum.reduce(%{}, fn {key, value_str}, acc -> - # Extract custom field ID from parameter name (explicitly remove prefix) - # This is more secure than String.replace_prefix which only removes first occurrence - custom_field_id_str = String.slice(key, prefix_length, String.length(key) - prefix_length) - - # Validate custom field ID length (UUIDs are max @max_uuid_length characters) - # This provides an additional security layer beyond UUID format validation - if String.length(custom_field_id_str) <= @max_uuid_length do - # Validate custom field ID exists and is boolean type - case Ecto.UUID.cast(custom_field_id_str) do - {:ok, _custom_field_id} -> - if Map.has_key?(boolean_custom_fields, custom_field_id_str) do - # Validate filter value - case determine_boolean_filter(value_str) do - nil -> acc - filter_value -> Map.put(acc, custom_field_id_str, filter_value) - end - else - acc - end - - :error -> - acc - end - else + process_boolean_filter_param( + key, + value_str, + prefix_length, + boolean_custom_fields, acc - end + ) end) # Security: Limit number of filters to prevent DoS attacks @@ -1240,6 +1221,73 @@ defmodule MvWeb.MemberLive.Index do assign(socket, :boolean_custom_field_filters, filters) end + # Processes a single boolean filter parameter from URL params. + # + # Validates the parameter and adds it to the accumulator if valid. + # Returns the accumulator unchanged if validation fails. + defp process_boolean_filter_param( + key, + value_str, + prefix_length, + boolean_custom_fields, + acc + ) do + # Extract custom field ID from parameter name (explicitly remove prefix) + # This is more secure than String.replace_prefix which only removes first occurrence + custom_field_id_str = String.slice(key, prefix_length, String.length(key) - prefix_length) + + # Validate custom field ID length (UUIDs are max @max_uuid_length characters) + # This provides an additional security layer beyond UUID format validation + if String.length(custom_field_id_str) > @max_uuid_length do + acc + else + validate_and_add_boolean_filter( + custom_field_id_str, + value_str, + boolean_custom_fields, + acc + ) + end + end + + # Validates UUID format and custom field existence, then adds filter if valid. + defp validate_and_add_boolean_filter( + custom_field_id_str, + value_str, + boolean_custom_fields, + acc + ) do + case Ecto.UUID.cast(custom_field_id_str) do + {:ok, _custom_field_id} -> + add_boolean_filter_if_valid( + custom_field_id_str, + value_str, + boolean_custom_fields, + acc + ) + + :error -> + acc + end + end + + # Adds boolean filter to accumulator if custom field exists and value is valid. + defp add_boolean_filter_if_valid( + custom_field_id_str, + value_str, + boolean_custom_fields, + acc + ) do + if Map.has_key?(boolean_custom_fields, custom_field_id_str) do + case determine_boolean_filter(value_str) do + nil -> acc + filter_value -> Map.put(acc, custom_field_id_str, filter_value) + end + else + acc + end + end + # Determines valid boolean filter value from URL parameter. # # SECURITY: This function whitelists allowed filter values. Only "true" and "false" diff --git a/test/mv_web/member_live/index_test.exs b/test/mv_web/member_live/index_test.exs index e5330da..20cfd68 100644 --- a/test/mv_web/member_live/index_test.exs +++ b/test/mv_web/member_live/index_test.exs @@ -945,9 +945,7 @@ defmodule MvWeb.MemberLive.IndexTest do # Build URL with all 60 filters filter_params = - boolean_fields - |> Enum.map(fn cf -> "bf_#{cf.id}=true" end) - |> Enum.join("&") + Enum.map_join(boolean_fields, "&", fn cf -> "bf_#{cf.id}=true" end) {:ok, view, _html} = live(conn, "/members?#{filter_params}")