Potential improvements to identifying customers by external_id (WIP)

lib/chat_api/customers.ex

@@ -41,9 +41,27 @@ defmodule ChatApi.Customers do
     Customer |> Repo.get!(id) |> Repo.preload(:tags)
   end
 
+  @spec list_by_external_id(binary(), binary(), map()) :: [Customer.t()]
+  def list_by_external_id(external_id, account_id, filters \\ %{})
+
+  def list_by_external_id(external_id, account_id, filters) when is_binary(external_id) do
+    Customer
+    |> where(account_id: ^account_id, external_id: ^external_id)
+    |> where(^filter_where(filters))
+    |> order_by(desc: :updated_at)
+    |> Repo.all()
+  end
+
+  @spec list_by_external_id(integer(), binary(), map()) :: [Customer.t()]
+  def list_by_external_id(external_id, account_id, filters) when is_integer(external_id) do
+    external_id |> to_string() |> list_by_external_id(account_id, filters)
+  end
+
   @spec find_by_external_id(binary(), binary(), map()) :: Customer.t() | nil
   def find_by_external_id(external_id, account_id, filters \\ %{})
 
+  # TODO: return list rather than just one, and then ignore situations where there are multiple with the same ID
+  # TODO: factor in IP address as well? eh, maybe not... but it might be better to just make this super restrictive?
   def find_by_external_id(external_id, account_id, filters) when is_binary(external_id) do
     Customer
     |> where(account_id: ^account_id, external_id: ^external_id)
@@ -250,6 +268,15 @@ defmodule ChatApi.Customers do
     |> Repo.delete()
   end
 
+  @spec is_secure_external_id?(any()) :: boolean()
+  def is_secure_external_id?(external_id) when is_binary(external_id) do
+    String.length(external_id) >= 8 &&
+      !String.match?(external_id, ~r/^[[:alpha:]]+$/) &&
+      !String.match?(external_id, ~r/^[[:digit:]]+$/)
+  end
+
+  def is_secure_external_id?(_external_id), do: false
+
   # Pulled from https://hexdocs.pm/ecto/dynamic-queries.html#building-dynamic-queries
   @spec filter_where(map) :: Ecto.Query.DynamicExpr.t()
   def filter_where(params) do

lib/chat_api_web/controllers/customer_controller.ex

@@ -6,7 +6,7 @@ defmodule ChatApiWeb.CustomerController do
   alias ChatApi.{Accounts, Customers}
   alias ChatApi.Customers.Customer
 
-  action_fallback ChatApiWeb.FallbackController
+  action_fallback(ChatApiWeb.FallbackController)
 
   @spec index(Plug.Conn.t(), map) :: Plug.Conn.t()
   def index(conn, params) do
@@ -48,29 +48,31 @@ defmodule ChatApiWeb.CustomerController do
           "account_id" => account_id
         } = params
       ) do
-    # TODO: support whitelisting urls for an account so we only enable this and
-    # other chat widget-related APIs for incoming requests from supported urls?
-    if Accounts.exists?(account_id) do
-      # TODO: make "host" a required param? (but would have to ignore on mobile...)
-      filters =
-        params
-        |> Map.take(["email", "host"])
-        |> Enum.reject(fn {_k, v} -> blank?(v) end)
-        |> Map.new()
-
-      case Customers.find_by_external_id(external_id, account_id, filters) do
-        %{id: customer_id} ->
-          json(conn, %{
-            data: %{
-              customer_id: customer_id
-            }
-          })
-
-        _ ->
-          json(conn, %{data: %{customer_id: nil}})
-      end
-    else
-      send_account_not_found_error(conn, account_id)
+    cond do
+      !Customers.is_secure_external_id?(external_id) ->
+        json(conn, %{data: %{customer_id: nil}})
+
+      Accounts.exists?(account_id) ->
+        filters =
+          params
+          |> Map.take(["email", "host"])
+          |> Enum.reject(fn {_k, v} -> blank?(v) end)
+          |> Map.new()
+
+        case Customers.list_by_external_id(external_id, account_id, filters) do
+          [%Customer{id: customer_id}] ->
+            json(conn, %{
+              data: %{
+                customer_id: customer_id
+              }
+            })
+
+          _ ->
+            json(conn, %{data: %{customer_id: nil}})
+        end
+
+      true ->
+        send_account_not_found_error(conn, account_id)
     end
   end