From 585de861d6662d000f8c039a5d562c59039d930a Mon Sep 17 00:00:00 2001 From: Pim Kunis Date: Tue, 13 Jul 2021 23:16:56 +0200 Subject: [PATCH] Add tests for login --- lib/matrix_server/account.ex | 28 ++++++++--- lib/matrix_server/device.ex | 28 +++++++---- .../controllers/auth_controller.ex | 22 +++----- test/controllers/auth_controller_test.exs | 50 +++++++++++++++++++ 4 files changed, 94 insertions(+), 34 deletions(-) diff --git a/lib/matrix_server/account.ex b/lib/matrix_server/account.ex index 7cba00d..e594cd9 100644 --- a/lib/matrix_server/account.ex +++ b/lib/matrix_server/account.ex @@ -4,7 +4,7 @@ defmodule MatrixServer.Account do import Ecto.{Changeset, Query} alias MatrixServer.{Repo, Account, Device} - alias MatrixServerWeb.API.Register + alias MatrixServerWeb.API.{Register, Login} alias Ecto.Multi @max_mxid_length 255 @@ -53,17 +53,20 @@ defmodule MatrixServer.Account do |> Multi.run(:device_with_access_token, &Device.insert_new_access_token/2) end - def login(%{localpart: localpart, password: password} = params) do + def login(%Login{} = api) do + localpart = try_get_localpart(api.identifier.user) + fn repo -> case repo.one(from a in Account, where: a.localpart == ^localpart) do %Account{password_hash: hash} = account -> - if Bcrypt.verify_pass(password, hash) do - device_id = Map.get(params, :device_id, Device.generate_device_id(localpart)) - access_token = Device.generate_access_token(localpart, device_id) + if Bcrypt.verify_pass(api.password, hash) do + case Device.login(api, account) do + {:ok, device} -> + device - case Device.login(account, device_id, access_token, params) do - {:ok, device} -> device - {:error, _cs} -> repo.rollback(:forbidden) + {:error, _cs} -> + IO.inspect(_cs) + repo.rollback(:forbidden) end else repo.rollback(:forbidden) @@ -97,4 +100,13 @@ defmodule MatrixServer.Account do # Subtract the "@" and ":" in the MXID. @max_mxid_length - 2 - String.length(MatrixServer.server_name()) end + + defp try_get_localpart("@" <> rest = user_id) do + case String.split(rest, ":") do + [localpart, _] -> localpart + _ -> user_id + end + end + + defp try_get_localpart(localpart), do: localpart end diff --git a/lib/matrix_server/device.ex b/lib/matrix_server/device.ex index aa7a079..77850bc 100644 --- a/lib/matrix_server/device.ex +++ b/lib/matrix_server/device.ex @@ -4,6 +4,7 @@ defmodule MatrixServer.Device do import Ecto.{Changeset, Query} alias MatrixServer.{Account, Device, Repo} + alias MatrixServerWeb.API.Login @primary_key false schema "devices" do @@ -49,22 +50,29 @@ defmodule MatrixServer.Device do "#{localpart}_#{time_string}" end - def login(account, device_id, access_token, params) do + def login(%Login{} = api, account) do + device_id = api.device_id || generate_device_id(account.localpart) + access_token = generate_access_token(account.localpart, device_id) + update_query = from(d in Device) |> update(set: [access_token: ^access_token, device_id: ^device_id]) + |> then(fn q -> + if api.initial_device_display_name do + update(q, set: [display_name: ^api.initial_device_display_name]) + else + q + end + end) - update_query = - if params[:display_name] != nil do - update(update_query, set: [display_name: ^params.display_name]) - else - update_query - end + device_params = %{ + device_id: device_id, + display_name: api.initial_device_display_name + } Ecto.build_assoc(account, :devices) - |> Map.put(:device_id, device_id) - |> Map.put(:access_token, access_token) - |> Device.changeset(params) + |> Device.changeset(device_params) + |> put_change(:access_token, access_token) |> Repo.insert(on_conflict: update_query, conflict_target: [:localpart, :device_id]) end end diff --git a/lib/matrix_server_web/controllers/auth_controller.ex b/lib/matrix_server_web/controllers/auth_controller.ex index 077a8bc..76631cf 100644 --- a/lib/matrix_server_web/controllers/auth_controller.ex +++ b/lib/matrix_server_web/controllers/auth_controller.ex @@ -73,23 +73,13 @@ defmodule MatrixServerWeb.AuthController do ) do case Login.changeset(params) do %Changeset{valid?: true} = cs -> - input = - apply_changes(cs) - |> Map.from_struct() - |> MatrixServer.maybe_update_map(:initial_device_display_name, :display_name) - |> MatrixServer.maybe_update_map(:identifier, :localpart, fn - %{user: "@" <> rest} -> - case String.split(rest) do - [localpart, _] -> localpart - # Empty string will never match in the database. - _ -> "" - end + api = apply_changes(cs) + # input = + # apply_changes(cs) + # |> Map.from_struct() + # |> MatrixServer.maybe_update_map(:initial_device_display_name, :display_name) - %{user: user} -> - user - end) - - case Account.login(input) |> Repo.transaction() do + case Account.login(api) |> Repo.transaction() do {:ok, device} -> data = %{ user_id: MatrixServer.get_mxid(device.localpart), diff --git a/test/controllers/auth_controller_test.exs b/test/controllers/auth_controller_test.exs index 83f8b81..58777a3 100644 --- a/test/controllers/auth_controller_test.exs +++ b/test/controllers/auth_controller_test.exs @@ -81,4 +81,54 @@ defmodule MatrixServerWeb.AuthControllerTest do assert %{"errcode" => "M_INVALID_USERNAME"} = json_response(conn, 400) end end + + @basic_params %{ + "type" => "m.login.password", + "identifier" => %{ + "type" => "m.id.user", + "user" => "sneed" + }, + "password" => "lemmein" + } + + describe "login endpoint" do + test "renders the list of login types", %{conn: conn} do + conn = get(conn, Routes.auth_path(Endpoint, :login)) + + assert %{"flows" => flows} = json_response(conn, 200) + assert is_list(flows) + end + + test "logs a user in with password and matrix user id", %{conn: conn} do + Factory.insert(:account, localpart: "sneed", password_hash: Bcrypt.hash_pwd_salt("lemmein")) + conn = post_json(conn, Routes.auth_path(Endpoint, :login), @basic_params) + + assert %{"user_id" => _, "access_token" => _, "device_id" => _} = json_response(conn, 200) + + conn = + recycle(conn) + |> post_json(Routes.auth_path(Endpoint, :login), %{ + @basic_params + | "identifier" => %{"type" => "m.id.user", "user" => MatrixServer.get_mxid("sneed")} + }) + + assert %{"user_id" => _, "access_token" => _, "device_id" => _} = json_response(conn, 200) + end + + test "handles unknown matrix user id", %{conn: conn} do + conn = post_json(conn, Routes.auth_path(Endpoint, :login), @basic_params) + + assert %{"errcode" => "M_FORBIDDEN"} = json_response(conn, 400) + end + + test "handles wrong password", %{conn: conn} do + Factory.insert(:account, localpart: "sneed", password_hash: Bcrypt.hash_pwd_salt("surprise")) + conn = post_json(conn, Routes.auth_path(Endpoint, :login), @basic_params) + + assert %{"errcode" => "M_FORBIDDEN"} = json_response(conn, 400) + end + + # TODO: Test display name + # TODO: Test device recycling + end end