From 91a06aff1bab42041122edca31b3bff8c1be9dfb Mon Sep 17 00:00:00 2001 From: Pim Kunis Date: Mon, 30 Aug 2021 22:36:01 +0200 Subject: [PATCH] Change primary keys for accounts and devices table --- lib/matrix_server/room_server.ex | 26 +++++++++----- lib/matrix_server/schema/account.ex | 28 ++++++++------- lib/matrix_server/schema/device.ex | 34 +++++-------------- lib/matrix_server/schema/joined_room.ex | 8 ++--- .../client/controllers/login_controller.ex | 12 ++++--- .../client/controllers/register_controller.ex | 14 +++++--- .../client/controllers/room_controller.ex | 10 ++++++ lib/matrix_server_web/router.ex | 1 + .../20210830160818_create_initial_tables.exs | 25 ++++++-------- 9 files changed, 81 insertions(+), 77 deletions(-) diff --git a/lib/matrix_server/room_server.ex b/lib/matrix_server/room_server.ex index 4d489f3..9be1464 100644 --- a/lib/matrix_server/room_server.ex +++ b/lib/matrix_server/room_server.ex @@ -530,6 +530,7 @@ defmodule MatrixServer.RoomServer do room = Room.update_forward_extremities(event, room) event = Repo.insert!(event) state_set = StateResolution.resolve_forward_extremities(event) + # TODO: Do this as a background job, and not after every insert... _ = update_joined_rooms(room, state_set) {:ok, state_set, room} @@ -539,7 +540,6 @@ defmodule MatrixServer.RoomServer do end # Update local accounts' room membership if applicable. - # TODO: Could be a background job. @spec update_joined_rooms(Room.t(), t()) :: JoinedRoom.t() | nil defp update_joined_rooms(%Room{id: room_id}, state_set) do server_name = MatrixServer.server_name() @@ -553,19 +553,27 @@ defmodule MatrixServer.RoomServer do membership == "join" end) - joined_assocs = - Enum.map(joined, fn {{_, state_key}, _} -> - %{localpart: MatrixServer.get_localpart(state_key), room_id: room_id} - end) + map_localparts = + &Enum.map(&1, fn {{_, state_key}, _} -> MatrixServer.get_localpart(state_key) end) - not_joined_localparts = - Enum.map(not_joined, fn {{_, state_key}, _} -> MatrixServer.get_localpart(state_key) end) + joined_localparts = map_localparts.(joined) + not_joined_localparts = map_localparts.(not_joined) - _ = Repo.insert_all(JoinedRoom, joined_assocs, on_conflict: :nothing) + _ = + Repo.insert_all( + JoinedRoom, + from(a in Account, + where: a.localpart in ^joined_localparts, + select: %{account_id: a.id, room_id: ^room_id} + ), + on_conflict: :nothing + ) Repo.delete_all( from jr in JoinedRoom, - where: jr.room_id == ^room_id and jr.localpart in ^not_joined_localparts + join: a in Account, + on: a.id == jr.account_id, + where: jr.room_id == ^room_id and a.localpart in ^not_joined_localparts ) end end diff --git a/lib/matrix_server/schema/account.ex b/lib/matrix_server/schema/account.ex index b91f2bf..9cf703e 100644 --- a/lib/matrix_server/schema/account.ex +++ b/lib/matrix_server/schema/account.ex @@ -13,14 +13,14 @@ defmodule MatrixServer.Account do @max_mxid_length 255 - @primary_key {:localpart, :string, []} schema "accounts" do + field :localpart, :string field :password_hash, :string, redact: true - has_many :devices, Device, foreign_key: :localpart + has_many :devices, Device many_to_many :joined_rooms, Room, join_through: JoinedRoom, - join_keys: [localpart: :localpart, room_id: :id] + join_keys: [account_id: :id, room_id: :id] timestamps(updated_at: false) end @@ -50,31 +50,35 @@ defmodule MatrixServer.Account do Return an multi to register a new user. """ @spec register(Register.t()) :: Multi.t() - def register(%Register{} = input) do + def register(input) do + localpart = input.username || MatrixServer.random_string(10, ?a..?z) + account_params = %{ - localpart: input.username || MatrixServer.random_string(10, ?a..?z), + localpart: localpart, password_hash: Bcrypt.hash_pwd_salt(input.password) } Multi.new() |> Multi.insert(:account, changeset(%Account{}, account_params)) |> Multi.insert(:device, fn %{account: account} -> + device_id = input.device_id || Device.generate_device_id(account.localpart) + access_token = Device.generate_access_token(localpart, device_id) + device_params = %{ display_name: input.initial_device_display_name, - device_id: input.device_id || Device.generate_device_id(account.localpart) + device_id: device_id } - Ecto.build_assoc(account, :devices) + Ecto.build_assoc(account, :devices, access_token: access_token) |> Device.changeset(device_params) end) - |> Multi.run(:device_with_access_token, &Device.insert_new_access_token/2) end @doc """ Return a function to log a user in. """ - @spec login(Login.t()) :: (Ecto.Repo.t() -> {:error, any()} | {:ok, Device.t()}) - def login(%Login{} = input) do + @spec login(Login.t()) :: (Ecto.Repo.t() -> {:error, any()} | {:ok, {Account.t(), Device.t()}}) + def login(input) do localpart = try_get_localpart(input.identifier.user) fn repo -> @@ -83,7 +87,7 @@ defmodule MatrixServer.Account do if Bcrypt.verify_pass(input.password, hash) do case Device.login(input, account) do {:ok, device} -> - device + {account, device} {:error, _cs} -> repo.rollback(:forbidden) @@ -119,7 +123,7 @@ defmodule MatrixServer.Account do |> validate_length(:password_hash, max: 60) |> validate_format(:localpart, MatrixServer.localpart_regex()) |> validate_length(:localpart, max: localpart_length()) - |> unique_constraint(:localpart, name: :accounts_pkey) + |> unique_constraint(:localpart, name: :accounts_localpart_index) end @spec localpart_length :: integer() diff --git a/lib/matrix_server/schema/device.ex b/lib/matrix_server/schema/device.ex index 1099f17..8d41a3c 100644 --- a/lib/matrix_server/schema/device.ex +++ b/lib/matrix_server/schema/device.ex @@ -4,43 +4,27 @@ defmodule MatrixServer.Device do import Ecto.{Changeset, Query} alias MatrixServer.{Account, Device, Repo} - alias MatrixServerWeb.Client.Request.Login @type t :: %__MODULE__{ device_id: String.t(), access_token: String.t(), display_name: String.t(), - localpart: String.t() + account_id: integer() } - @primary_key false schema "devices" do - field :device_id, :string, primary_key: true + field :device_id, :string field :access_token, :string, redact: true field :display_name, :string - belongs_to :account, Account, - foreign_key: :localpart, - references: :localpart, - type: :string, - primary_key: true + belongs_to :account, Account end def changeset(device, params \\ %{}) do device |> cast(params, [:display_name, :device_id]) - |> validate_required([:localpart, :device_id]) - |> unique_constraint([:localpart, :device_id], name: :devices_pkey) - end - - def insert_new_access_token(repo, %{ - device: %Device{localpart: localpart, device_id: device_id} = device - }) do - access_token = generate_access_token(localpart, device_id) - - device - |> change(%{access_token: access_token}) - |> repo.update() + |> validate_required([:device_id]) + |> unique_constraint([:device_id, :account_id], name: :devices_device_id_account_id_index) end def generate_access_token(localpart, device_id) do @@ -52,9 +36,9 @@ defmodule MatrixServer.Device do "#{localpart}_#{System.os_time(:millisecond)}" end - def login(%Login{} = input, account) do - device_id = input.device_id || generate_device_id(account.localpart) - access_token = generate_access_token(account.localpart, device_id) + def login(input, %Account{localpart: localpart} = account) do + device_id = input.device_id || generate_device_id(localpart) + access_token = generate_access_token(localpart, device_id) update_query = from(d in Device) @@ -75,6 +59,6 @@ defmodule MatrixServer.Device do Ecto.build_assoc(account, :devices) |> Device.changeset(device_params) |> put_change(:access_token, access_token) - |> Repo.insert(on_conflict: update_query, conflict_target: [:localpart, :device_id]) + |> Repo.insert(on_conflict: update_query, conflict_target: [:account_id, :device_id]) end end diff --git a/lib/matrix_server/schema/joined_room.ex b/lib/matrix_server/schema/joined_room.ex index b2e3584..aef884b 100644 --- a/lib/matrix_server/schema/joined_room.ex +++ b/lib/matrix_server/schema/joined_room.ex @@ -4,17 +4,13 @@ defmodule MatrixServer.JoinedRoom do alias MatrixServer.{Account, Room} @type t :: %__MODULE__{ - localpart: String.t(), + account_id: integer(), room_id: String.t() } @primary_key false schema "joined_rooms" do - belongs_to :account, Account, - foreign_key: :localpart, - references: :localpart, - type: :string, - primary_key: true + belongs_to :account, Account, primary_key: true belongs_to :room, Room, primary_key: true, type: :string end diff --git a/lib/matrix_server_web/client/controllers/login_controller.ex b/lib/matrix_server_web/client/controllers/login_controller.ex index da47d51..eb6460a 100644 --- a/lib/matrix_server_web/client/controllers/login_controller.ex +++ b/lib/matrix_server_web/client/controllers/login_controller.ex @@ -4,7 +4,7 @@ defmodule MatrixServerWeb.Client.LoginController do import MatrixServerWeb.Error import Ecto.Changeset - alias MatrixServer.{Repo, Account} + alias MatrixServer.{Repo, Account, Device} alias MatrixServerWeb.Client.Request.Login alias Ecto.Changeset @@ -38,11 +38,13 @@ defmodule MatrixServerWeb.Client.LoginController do input = apply_changes(cs) case Account.login(input) |> Repo.transaction() do - {:ok, device} -> + {:ok, + {%Account{localpart: localpart}, + %Device{access_token: access_token, device_id: device_id}}} -> data = %{ - user_id: MatrixServer.get_mxid(device.localpart), - access_token: device.access_token, - device_id: device.device_id + user_id: MatrixServer.get_mxid(localpart), + access_token: access_token, + device_id: device_id } conn diff --git a/lib/matrix_server_web/client/controllers/register_controller.ex b/lib/matrix_server_web/client/controllers/register_controller.ex index 48bc5a6..8411c78 100644 --- a/lib/matrix_server_web/client/controllers/register_controller.ex +++ b/lib/matrix_server_web/client/controllers/register_controller.ex @@ -4,7 +4,7 @@ defmodule MatrixServerWeb.Client.RegisterController do import MatrixServerWeb.Error import Ecto.Changeset - alias MatrixServer.{Repo, Account} + alias MatrixServer.{Repo, Account, Device} alias MatrixServerWeb.Client.Request.Register alias Ecto.Changeset @@ -21,14 +21,18 @@ defmodule MatrixServerWeb.Client.RegisterController do input = apply_changes(cs) case Account.register(input) |> Repo.transaction() do - {:ok, %{device_with_access_token: device}} -> - data = %{user_id: MatrixServer.get_mxid(device.localpart)} + {:ok, + %{ + account: %Account{localpart: localpart}, + device: %Device{device_id: device_id, access_token: access_token} + }} -> + data = %{user_id: MatrixServer.get_mxid(localpart)} data = if not input.inhibit_login do data - |> Map.put(:device_id, device.device_id) - |> Map.put(:access_token, device.access_token) + |> Map.put(:device_id, device_id) + |> Map.put(:access_token, access_token) else data end diff --git a/lib/matrix_server_web/client/controllers/room_controller.ex b/lib/matrix_server_web/client/controllers/room_controller.ex index 4b68336..38f5a35 100644 --- a/lib/matrix_server_web/client/controllers/room_controller.ex +++ b/lib/matrix_server_web/client/controllers/room_controller.ex @@ -207,4 +207,14 @@ defmodule MatrixServerWeb.Client.RoomController do end def unban(conn, _), do: put_error(conn, :missing_param) + + def send_message(%Conn{assigns: %{account: account}, body_params: body_params} = conn, %{ + "room_id" => room_id, + "event_type" => event_type, + "txn_id" => txn_id + }) do + conn + |> send_resp(200, []) + |> halt() + end end diff --git a/lib/matrix_server_web/router.ex b/lib/matrix_server_web/router.ex index 1908588..7d37853 100644 --- a/lib/matrix_server_web/router.ex +++ b/lib/matrix_server_web/router.ex @@ -66,6 +66,7 @@ defmodule MatrixServerWeb.Router do post "/kick", RoomController, :kick post "/ban", RoomController, :ban post "/unban", RoomController, :unban + put "/send/:event_type/:txn_id", RoomController, :send_message end end end diff --git a/priv/repo/migrations/20210830160818_create_initial_tables.exs b/priv/repo/migrations/20210830160818_create_initial_tables.exs index cb2e2e8..b67cd55 100644 --- a/priv/repo/migrations/20210830160818_create_initial_tables.exs +++ b/priv/repo/migrations/20210830160818_create_initial_tables.exs @@ -2,12 +2,14 @@ defmodule MatrixServer.Repo.Migrations.CreateInitialTables do use Ecto.Migration def change do - create table(:accounts, primary_key: false) do - add :localpart, :string, primary_key: true, null: false + create table(:accounts) do + add :localpart, :string, null: false add :password_hash, :string, size: 60, null: false timestamps(updated_at: false) end + create index(:accounts, [:localpart], unique: true) + create table(:rooms, primary_key: false) do add :state, {:array, {:array, :string}}, default: [], null: false add :forward_extremities, {:array, :string}, default: [], null: false @@ -16,10 +18,7 @@ defmodule MatrixServer.Repo.Migrations.CreateInitialTables do end create table(:joined_rooms, primary_key: false) do - add :localpart, - references(:accounts, column: :localpart, type: :string), - primary_key: true, - null: false + add :account_id, references(:accounts), primary_key: true, null: false add :room_id, references(:rooms, type: :string), primary_key: true, @@ -62,20 +61,16 @@ defmodule MatrixServer.Repo.Migrations.CreateInitialTables do create index(:aliases, [:room_id]) - create table(:devices, primary_key: false) do - add :device_id, :string, primary_key: true, null: false + create table(:devices) do + add :device_id, :string, null: false add :access_token, :string add :display_name, :string - add :localpart, - references(:accounts, column: :localpart, on_delete: :delete_all, type: :string), - primary_key: true, - null: false + add :account_id, references(:accounts, on_delete: :delete_all), null: false end - # Compound primary already indexes device_id. - create index(:devices, [:localpart]) - + create index(:devices, [:device_id, :account_id], unique: true) + create index(:devices, [:account_id]) create index(:devices, [:access_token], unique: true) end end