diff --git a/elixir/apps/api/lib/api/sockets.ex b/elixir/apps/api/lib/api/sockets.ex index 8575e53a8..342ca2496 100644 --- a/elixir/apps/api/lib/api/sockets.ex +++ b/elixir/apps/api/lib/api/sockets.ex @@ -3,6 +3,7 @@ defmodule API.Sockets do This module provides a set of helper function for Phoenix sockets and error handling around them. """ + require Logger def options do [ @@ -25,12 +26,28 @@ defmodule API.Sockets do def handle_error(conn, :unauthenticated), do: Plug.Conn.send_resp(conn, 403, "Forbidden") - def handle_error(conn, %Ecto.Changeset{}), - do: Plug.Conn.send_resp(conn, 422, "Invalid or missing connection parameters") + def handle_error(conn, %Ecto.Changeset{} = changeset) do + Logger.error("Invalid connection request", changeset: inspect(changeset)) + errors = changeset_error_to_string(changeset) + Plug.Conn.send_resp(conn, 422, "Invalid or missing connection parameters: #{errors}") + end def handle_error(conn, :rate_limit), do: Plug.Conn.send_resp(conn, 429, "Too many requests") + @doc false + def changeset_error_to_string(changeset) do + Ecto.Changeset.traverse_errors(changeset, fn {msg, opts} -> + Enum.reduce(opts, msg, fn {key, value}, acc -> + String.replace(acc, "%{#{key}}", to_string(value)) + end) + end) + |> Enum.reduce("", fn {k, v}, acc -> + joined_errors = Enum.join(v, "; ") + "#{acc}#{k}: #{joined_errors}\n" + end) + end + def real_ip(x_headers, peer_data) do real_ip = if is_list(x_headers) and length(x_headers) > 0 do diff --git a/elixir/apps/api/test/api/client/socket_test.exs b/elixir/apps/api/test/api/client/socket_test.exs index d5d6ed7e9..9d1fdce43 100644 --- a/elixir/apps/api/test/api/client/socket_test.exs +++ b/elixir/apps/api/test/api/client/socket_test.exs @@ -30,6 +30,19 @@ defmodule API.Client.SocketTest do assert connect(Socket, attrs, connect_info: @connect_info) == {:error, :invalid_token} end + test "renders error on invalid attrs" do + subject = Fixtures.Auth.create_subject() + {:ok, token} = Auth.create_session_token_from_subject(subject) + + attrs = %{token: token} + + assert {:error, changeset} = connect(Socket, attrs, connect_info: connect_info(subject)) + + errors = API.Sockets.changeset_error_to_string(changeset) + assert errors =~ "public_key: can't be blank" + assert errors =~ "external_id: can't be blank" + end + test "creates a new client" do subject = Fixtures.Auth.create_subject() {:ok, token} = Auth.create_session_token_from_subject(subject) diff --git a/elixir/apps/domain/lib/domain/clients/client/changeset.ex b/elixir/apps/domain/lib/domain/clients/client/changeset.ex index 48a9e3a22..dcebe2819 100644 --- a/elixir/apps/domain/lib/domain/clients/client/changeset.ex +++ b/elixir/apps/domain/lib/domain/clients/client/changeset.ex @@ -4,7 +4,8 @@ defmodule Domain.Clients.Client.Changeset do alias Domain.Clients @upsert_fields ~w[external_id name public_key]a - @conflict_replace_fields ~w[public_key + @conflict_replace_fields ~w[name + public_key last_seen_user_agent last_seen_remote_ip last_seen_remote_ip_location_region @@ -78,7 +79,6 @@ defmodule Domain.Clients.Client.Changeset do |> unique_constraint([:actor_id, :name]) |> unique_constraint([:actor_id, :public_key]) |> unique_constraint(:external_id) - |> unique_constraint(:name, name: :clients_account_id_actor_id_name_index) end defp put_client_version(changeset) do diff --git a/elixir/apps/domain/priv/repo/migrations/20231221160102_drop_clients_name_unique_constraint.exs b/elixir/apps/domain/priv/repo/migrations/20231221160102_drop_clients_name_unique_constraint.exs new file mode 100644 index 000000000..e359ab1da --- /dev/null +++ b/elixir/apps/domain/priv/repo/migrations/20231221160102_drop_clients_name_unique_constraint.exs @@ -0,0 +1,7 @@ +defmodule Domain.Repo.Migrations.DropClientsNameUniqueConstraint do + use Ecto.Migration + + def change do + execute("DROP INDEX clients_account_id_actor_id_name_index") + end +end diff --git a/elixir/apps/domain/test/domain/clients_test.exs b/elixir/apps/domain/test/domain/clients_test.exs index be94943d0..8aa3b426b 100644 --- a/elixir/apps/domain/test/domain/clients_test.exs +++ b/elixir/apps/domain/test/domain/clients_test.exs @@ -395,7 +395,7 @@ defmodule Domain.ClientsTest do assert Repo.aggregate(Clients.Client, :count, :id) == 1 - assert updated_client.name + assert updated_client.name != client.name assert updated_client.last_seen_remote_ip.address == subject.context.remote_ip assert updated_client.last_seen_remote_ip != client.last_seen_remote_ip assert updated_client.last_seen_user_agent == subject.context.user_agent diff --git a/elixir/apps/web/test/web/live/clients/edit_test.exs b/elixir/apps/web/test/web/live/clients/edit_test.exs index b770a8a3d..586da6a6d 100644 --- a/elixir/apps/web/test/web/live/clients/edit_test.exs +++ b/elixir/apps/web/test/web/live/clients/edit_test.exs @@ -110,13 +110,11 @@ defmodule Web.Live.Clients.EditTest do test "renders changeset errors on submit", %{ account: account, - actor: actor, identity: identity, client: client, conn: conn } do - other_client = Fixtures.Clients.create_client(account: account, actor: actor) - attrs = %{name: other_client.name} + attrs = %{name: String.duplicate("a", 256)} {:ok, lv, _html} = conn @@ -127,7 +125,7 @@ defmodule Web.Live.Clients.EditTest do |> form("form", client: attrs) |> render_submit() |> form_validation_errors() == %{ - "client[name]" => ["has already been taken"] + "client[name]" => ["should be at most 255 character(s)"] } end