Add tests for login

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

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])
-
-    update_query =
-      if params[:display_name] != nil do
-        update(update_query, set: [display_name: ^params.display_name])
+      |> then(fn q ->
+        if api.initial_device_display_name do
+          update(q, set: [display_name: ^api.initial_device_display_name])
         else
-        update_query
+          q
         end
+      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

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),

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