Remove exldap dep, dumb ldap user creation, patch tests for LDAP backend

config/test.exs

@@ -23,3 +23,12 @@ config :recycledcloud, RecycledCloudWeb.Endpoint,
 
 # Print only warnings and errors during test
 config :logger, level: :warn
+
+# LDAP configuration
+config :recycledcloud, :ldap,
+  server: "127.0.0.1",
+  port: 3089,
+  ssl: false,
+  base_dn: "dc=example,dc=org",
+  bind_dn: "cn=admin,dc=example,dc=org",
+  bind_pw: "admin"

lib/recycledcloud/LDAP.ex

@@ -1,6 +1,5 @@
 defmodule RecycledCloud.LDAP do
   require Logger
-  require Exldap
   use GenServer
 
   # Every request to the LDAP backend go through this GenServer.
@@ -49,12 +48,39 @@ defmodule RecycledCloud.LDAP do
 
   def terminate(reason, state) do
     if (state.status == :ok), do: close(state.conn)
-    IO.inspect(reason)
+    Logger.info "Terminating LDAP backend: #{inspect(reason)}."
-    #Logger.info "Terminating LDAP backend: #{reason}."
   end
 
   ###
 
+  def search(ldap_conn, field, value, search_timeout \\ 1000) do
+    base_dn = Application.get_env(:recycledcloud, :ldap) |> Keyword.get(:base_dn)
+    opts = [
+      {:base, to_charlist(base_dn)},
+      {:scope, :eldap.wholeSubtree()},
+      {:filter, :eldap.equalityMatch(to_charlist(field), value)},
+      {:timeout, search_timeout}
+    ]
+
+    case :eldap.search(ldap_conn, opts) do
+      {:ok, {:eldap_search_result, eldap_result, _}} ->
+        entries = Enum.map(
+          eldap_result,
+          fn entry ->
+            {:eldap_entry, dn, attrs} = entry
+            Enum.reduce(attrs, %{:dn => dn},
+              fn pair, acc ->
+                {key, value} = pair
+                %{List.to_atom(key) => value} |> Map.merge(acc)
+              end
+            )
+          end
+        )
+
+        {:ok, entries}
+      {:error, err} -> {:error, err}
+    end
+  end
   def execute(query), do: GenServer.call(__MODULE__, {:pool, query})
   def execute_single(query), do: GenServer.call(__MODULE__, {:single, query})
 
@@ -62,10 +88,32 @@ defmodule RecycledCloud.LDAP do
 
   defp internal_execute_query(query, ldap_conn), do: query.(ldap_conn)
 
+  defp bind(ldap_conn) do
+    conf = Application.get_env(:recycledcloud, :ldap, [])
+    dn = conf |> Keyword.get(:bind_dn)
+    pw = conf |> Keyword.get(:bind_pw)
+
+    :eldap.simple_bind(ldap_conn, dn, pw)
+  end
+
   defp connect do
-    case Exldap.connect do
+    conf = Application.get_env(:recycledcloud, :ldap, [])
+
+    host = Keyword.get(conf, :server, "localhost") |> String.to_charlist
+    opts = [
+      {:port, Keyword.get(conf, :port, 389)},
+      {:ssl, Keyword.get(conf, :ssl, false)},
+      {:tcpopts, Keyword.get(conf, :tcpopts, [])}
+    ]
+    case :eldap.open([host], opts) do
       {:ok, ldap_conn} ->
         Logger.info "Successfuly connected to LDAP server."
+        case bind(ldap_conn) do
+          {:error, err} ->
+            Logger.warning("Could not bind to LDAP server: #{err}")
+          _ -> :noop
+        end
+
         %{status: :ok, conn: ldap_conn}
       {:error, err} ->
         Logger.warning "Failed to connect to LDAP server: #{err}. Authentication will not be possible."
@@ -74,7 +122,7 @@ defmodule RecycledCloud.LDAP do
   end
 
   defp close(ldap_conn) do
-    ldap_conn |> Exldap.close
+    ldap_conn |> :eldap.close
     Logger.debug("An LDAP connection was closed.")
   end
 end

lib/recycledcloud/accounts.ex

@@ -63,23 +63,44 @@ defmodule RecycledCloud.Accounts do
   ## User registration
 
   @doc """
-  Registers a user.
+  Insert an user in local database.
 
   ## Examples
 
-      iex> register_user(%{field: value})
+      iex> insert_user(%{field: value})
       {:ok, %User{}}
 
-      iex> register_user(%{field: bad_value})
+      iex> insert_user(%{field: bad_value})
       {:error, %Ecto.Changeset{}}
 
   """
-  def register_user(attrs) do
+  def insert_user(attrs) do
     %User{}
-    |> User.registration_changeset(attrs)
+    |> User.insertion_changeset(attrs)
     |> Repo.insert()
   end
 
+  @doc """
+  Register an user in LDAP backend and insert in local database.
+  """
+  def register_user(attrs) do
+    changeset = %User{} |> User.registration_changeset(attrs)
+    case Ecto.Changeset.apply_action(changeset, :update) do
+      {:ok, user} ->
+        case User.register(user) do
+          {:ok, result} -> {:ok, result}
+          {:error, :entryAlreadyExists} ->
+            {
+              :error,
+              Ecto.Changeset.add_error(
+                changeset, :username, "has already been taken"
+              )
+            }
+        end
+      err -> err
+    end
+  end
+
   @doc """
   Returns an `%Ecto.Changeset{}` for tracking user changes.
 
@@ -90,7 +111,7 @@ defmodule RecycledCloud.Accounts do
 
   """
   def change_user_registration(%User{} = user, attrs \\ %{}) do
-    User.registration_changeset(user, attrs, hash_password: false)
+    User.registration_changeset(user, attrs)
   end
 
   ## Settings
@@ -140,10 +161,10 @@ defmodule RecycledCloud.Accounts do
     with {:ok, query} <- UserToken.verify_change_email_token_query(token, context),
          %UserToken{sent_to: email} <- Repo.one(query),
          {:ok, _} <- Repo.transaction(user_email_multi(user, email, context)),
-         {:ok, _} <- User.set_email(user, email) do
+         :ok <- User.set_email(user, email) do
       :ok
     else
-      _ -> :error
+      err -> :error
     end
   end
 
@@ -188,6 +209,25 @@ defmodule RecycledCloud.Accounts do
     User.password_changeset(user, attrs)
   end
 
+  defp set_user_password(user, changeset, new_password) do
+    case Ecto.Changeset.apply_action(changeset, :update) do
+      {:ok, _} ->
+        case User.set_password(user, new_password) do
+          :ok ->
+            Repo.delete_all(UserToken.user_and_contexts_query(user, :all))
+            {:ok, user}
+          {:error, err} ->
+            changeset_errors = [current_password: {"Unknown error: #{err}", [err: inspect(err)]}]
+            updated_changeset = changeset
+                                |> Map.put(:action, :update)
+                                |> Map.put(:errors, changeset_errors)
+            {:error, updated_changeset}
+        end
+      err ->
+        err
+    end
+  end
+
   @doc """
   Updates the user password.
 
@@ -204,21 +244,8 @@ defmodule RecycledCloud.Accounts do
     changeset = user
                 |> User.password_changeset(attrs)
                 |> User.validate_current_password(current_password)
-    case Ecto.Changeset.apply_action(changeset, :update) do
+
-      {:ok, _} ->
+    set_user_password(user, changeset, attrs[:password])
-        new_password = attrs["password"]
-        case User.set_password(user, new_password) do
-          {:ok, _} -> {:ok, user}
-          {:error, err} ->
-            changeset_errors = [current_password: {"Unknown error: #{err}", [err: inspect(err)]}]
-            updated_changeset = changeset
-                                |> Map.put(:action, :update)
-                                |> Map.put(:errors, changeset_errors)
-            {:error, updated_changeset}
-        end
-      err ->
-        err
-    end
   end
 
   ## Session
@@ -347,14 +374,10 @@ defmodule RecycledCloud.Accounts do
 
   """
   def reset_user_password(user, attrs) do
-    Ecto.Multi.new()
+    changeset = user
-    |> Ecto.Multi.update(:user, User.password_changeset(user, attrs))
+                |> User.password_changeset(attrs)
-    |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, :all))
+
-    |> Repo.transaction()
+    set_user_password(user, changeset, attrs[:password])
-    |> case do
-      {:ok, %{user: user}} -> {:ok, user}
-      {:error, :user, changeset, _} -> {:error, changeset}
-    end
   end
 
   ## Keys

lib/recycledcloud/accounts/user.ex

@@ -2,11 +2,13 @@ defmodule RecycledCloud.Accounts.User do
   use Ecto.Schema
   import Ecto.Changeset
   require Logger
-  require Exldap
   alias RecycledCloud.{LDAP,Accounts}
   alias RecycledCloud.Accounts.User
   alias RecycledCloud.Repo
 
+  @min_password_lenght 6
+  @max_password_lenght 80
+
   @derive {Inspect, except: [:password]}
   schema "users" do
     field :username, :string
@@ -24,15 +26,10 @@ defmodule RecycledCloud.Accounts.User do
   # => ['myusername']}`).
   defp get_ldap_attributes(%User{username: uid}), do: get_ldap_attributes(uid)
   defp get_ldap_attributes(uid) do
-    query = fn ldap_conn -> Exldap.search_field(ldap_conn, :uid, uid) end
+    query = fn ldap_conn -> LDAP.search(ldap_conn, :uid, uid) end
     case query |> LDAP.execute do
       {:ok, []} -> {:error, "could not find matching object"}
-      {:ok, [entry]} ->
+      {:ok, [entry]} -> {:ok, entry}
-        attrs = entry
-                |> Map.get(:attributes)
-                |> Enum.into(%{})
-                |> Map.put('dn', entry.object_name)
-        {:ok, attrs}
       {:ok, [entry|_]} -> {:error, "found more than one object with uid #{uid}"}
       {:error, err} -> {:error, inspect(err)}
     end
@@ -44,7 +41,7 @@ defmodule RecycledCloud.Accounts.User do
     # data at the moment, which is inefficient.
 
     case get_ldap_attributes(user) do
-      {:ok, %{'mail' => [raw_email], 'dn' => raw_dn}} ->
+      {:ok, %{:mail => [raw_email], :dn => raw_dn}} ->
         email = List.to_string(raw_email)
         dn = List.to_string(raw_dn)
         user |> Map.put(:email, email) |> Map.put(:dn, dn)
@@ -57,17 +54,58 @@ defmodule RecycledCloud.Accounts.User do
     end
   end
 
+  defp create_ldap_user(%User{} = user) do
+    # FIXME: read dn template from conf, gid_number
+    # FIXME: generate uidNumber
+    conf = Application.get_env(:recycledcloud, :ldap)
+    basetree = conf |> Keyword.get(:base_dn)
+    dn = ("uid=" <> user.username <> ",ou=Users," <> basetree) |> String.to_charlist
+
+    uid_number = 1234
+    gid_number = 1000
+
+    ldif = [
+      {'uid', [user.username]},
+      {'cn', [user.username]},
+      {'givenName', [user.username]},
+      {'sn', [user.username]},
+      {'mail', [user.email]},
+      {'objectClass', ["inetOrgPerson", "posixAccount", "shadowAccount"]},
+      {'homeDirectory', ["/home/#{user.username}"]},
+      {'loginShell', ["/bin/bash"]},
+      {'userPassword', ["/bin/bash"]},
+      {'uidNumber', [Integer.to_string(uid_number)]},
+      {'gidNumber', [Integer.to_string(gid_number)]}
+    ]
+
+    query = fn ldap_conn ->
+      case :eldap.add(ldap_conn, dn, ldif) do
+        :ok ->
+          :eldap.modify_password(ldap_conn, dn, to_charlist(user.password))
+        err -> err
+      end
+    end
+    o = query |> LDAP.execute
+  end
+
+  def register(%User{} = user) do
+    case create_ldap_user(user) do
+      :ok -> {:ok, get_by_username(user.username)}
+      err -> err
+    end
+  end
+
   def get_by_username(username) when is_binary(username) do
     local_user = Repo.get_by(User, username: username)
     if local_user do
       local_user |> User.maybe_populate_ldap_attributes()
     else
       case get_ldap_attributes(username) do
-        {:ok, %{'uid' => [raw_uid], 'mail' => [raw_email], 'dn' => raw_dn}} ->
+        {:ok, %{:uid => [raw_uid], :mail => [raw_email], :dn => raw_dn}} ->
           uid = List.to_string(raw_uid)
           email = List.to_string(raw_email)
           dn = List.to_string(raw_dn)
-          case Accounts.register_user(%{username: uid, email: email, dn: dn}) do
+          case Accounts.insert_user(%{username: uid, email: email, dn: dn}) do
             {:ok, user} ->
               user
             {:error, err} ->
@@ -94,6 +132,12 @@ defmodule RecycledCloud.Accounts.User do
   also be very expensive to hash for certain algorithms.
   """
   def registration_changeset(user, attrs) do
+    user
+    |> insertion_changeset(attrs)
+    |> validate_password()
+  end
+
+  def insertion_changeset(user, attrs) do
     user
     |> cast(attrs, [:username, :password, :email, :dn])
     |> validate_email()
@@ -109,7 +153,7 @@ defmodule RecycledCloud.Accounts.User do
   defp validate_password(changeset) do
     changeset
     |> validate_required([:password])
-    |> validate_length(:password, min: 6, max: 80)
+    |> validate_length( :password, min: @min_password_lenght, max: @max_password_lenght)
     #|> validate_format(:password, ~r/[a-z]/, message: "at least one lower case character")
     #|> validate_format(:password, ~r/[A-Z]/, message: "at least one upper case character")
     #|> validate_format(:password, ~r/[!?@#$%^&*_0-9]/, message: "at least one digit or punctuation character")
@@ -151,10 +195,10 @@ defmodule RecycledCloud.Accounts.User do
   @doc """
   Verifies the password.
   """
-  def valid_password?(user = %User{username: uid}, password)
+  def valid_password?(%User{} = user, password)
   when byte_size(password) > 0 do
     query = fn ldap_conn ->
-      Exldap.verify_credentials(ldap_conn, user.dn, password)
+      :eldap.simple_bind(ldap_conn, user.dn, password)
     end
 
     case query |> LDAP.execute_single do
@@ -178,20 +222,17 @@ defmodule RecycledCloud.Accounts.User do
     end
   end
 
-  def set_password(user, new_password) do
+  def set_password(%User{} = user, new_password)
-    # Exldap does not properly implement `change_password`, hence we have to
+  when byte_size(new_password) > 0 do
-    # fallback on erlang's `:eldap`.
-    # See http://erlang.org/doc/man/eldap.html#modify-4
-
     query = fn ldap_conn ->
       :eldap.modify_password(
         ldap_conn,
-        String.to_charlist(user.dn),
+        to_charlist(user.dn),
-        String.to_charlist(new_password)
+        to_charlist(new_password)
       )
     end
 
-    query |> LDAP.execute_single
+    query |> LDAP.execute
   end
 
   def set_email(user, email) do
@@ -201,6 +242,6 @@ defmodule RecycledCloud.Accounts.User do
       :eldap.modify(ldap_conn, String.to_charlist(user.dn), [ldif])
     end
 
-    query |> LDAP.execute_single
+    query |> LDAP.execute
   end
 end

lib/recycledcloud/accounts/user_token.ex

@@ -85,7 +85,7 @@ defmodule RecycledCloud.Accounts.UserToken do
         query =
           from token in token_and_context_query(hashed_token, context),
             join: user in assoc(token, :user),
-            where: token.inserted_at > ago(^days, "day") and token.sent_to == user.email,
+            where: token.inserted_at > ago(^days, "day") and token.user_id == user.id,
             select: user
 
         {:ok, query}

mix.exs

@@ -45,8 +45,7 @@ defmodule RecycledCloud.MixProject do
       {:gettext, "~> 0.11"},
       {:jason, "~> 1.0"},
       {:plug_cowboy, "~> 2.0"},
-      {:phx_gen_auth, "~> 0.6", only: [:dev], runtime: false},
+      {:phx_gen_auth, "~> 0.6", only: [:dev], runtime: false}
-      {:exldap, git: "https://github.com/Fnux/exldap.git"}
     ]
   end
 

test/recycledcloud/accounts_test.exs

@@ -7,30 +7,30 @@ defmodule RecycledCloud.AccountsTest do
 
   describe "get_user_by_username/1" do
     test "does not return the user if the username does not exist" do
-      refute Accounts.get_user_by_email("unknown")
+      refute Accounts.get_user_by_username("unknown")
     end
 
-    test "returns the user if the email exists" do
+    test "returns the user if the username exists" do
       %{id: id} = user = user_fixture()
       assert %User{id: ^id} = Accounts.get_user_by_username(user.username)
     end
   end
 
-  describe "get_user_by_email_and_password/2" do
+  describe "get_user_by_username_and_password/2" do
-    test "does not return the user if the email does not exist" do
+    test "does not return the user if the username does not exist" do
-      refute Accounts.get_user_by_email_and_password("unknown@example.com", "hello world!")
+      refute Accounts.get_user_by_username_and_password("unknown", "hello world!")
     end
 
     test "does not return the user if the password is not valid" do
       user = user_fixture()
-      refute Accounts.get_user_by_email_and_password(user.email, "invalid")
+      refute Accounts.get_user_by_username_and_password(user.username, "invalid")
     end
 
-    test "returns the user if the email and password are valid" do
+    test "returns the user if the username and password are valid" do
       %{id: id} = user = user_fixture()
 
       assert %User{id: ^id} =
-               Accounts.get_user_by_email_and_password(user.email, valid_user_password())
+               Accounts.get_user_by_username_and_password(user.username, valid_user_password())
     end
   end
 
@@ -58,11 +58,11 @@ defmodule RecycledCloud.AccountsTest do
     end
 
     test "validates email and password when given" do
-      {:error, changeset} = Accounts.register_user(%{email: "not valid", password: "not valid"})
+      {:error, changeset} = Accounts.register_user(%{email: "not valid", password: "not"})
 
       assert %{
                email: ["must have the @ sign and no spaces"],
-               password: ["should be at least 12 character(s)"]
+               password: ["should be at least 6 character(s)"]
              } = errors_on(changeset)
     end
 
@@ -73,23 +73,22 @@ defmodule RecycledCloud.AccountsTest do
       assert "should be at most 80 character(s)" in errors_on(changeset).password
     end
 
-    test "validates email uniqueness" do
+    test "validates username uniqueness" do
-      %{email: email} = user_fixture()
+      %{username: username} = user_fixture()
-      {:error, changeset} = Accounts.register_user(%{email: email})
+      {:error, changeset} = Accounts.register_user(%{
-      assert "has already been taken" in errors_on(changeset).email
+        username: username,
+        email: unique_user_email(),
+        password: valid_user_password()
+      })
+      assert "has already been taken" in errors_on(changeset).username
 
-      # Now try with the upper cased email too, to check that email case is ignored.
+      # Now try with the upper cased username too, to check that email case is ignored.
-      {:error, changeset} = Accounts.register_user(%{email: String.upcase(email)})
+      {:error, changeset} = Accounts.register_user(%{
-      assert "has already been taken" in errors_on(changeset).email
+        username: String.upcase(username),
-    end
+        email: unique_user_email(),
-
+        password: valid_user_password()
-    test "registers users with a hashed password" do
+      })
-      email = unique_user_email()
+      assert "has already been taken" in errors_on(changeset).username
-      {:ok, user} = Accounts.register_user(%{email: email, password: valid_user_password()})
-      assert user.email == email
-      assert is_binary(user.hashed_password)
-      assert is_nil(user.confirmed_at)
-      assert is_nil(user.password)
     end
   end
 
@@ -146,15 +145,6 @@ defmodule RecycledCloud.AccountsTest do
       assert "should be at most 160 character(s)" in errors_on(changeset).email
     end
 
-    test "validates email uniqueness", %{user: user} do
-      %{email: email} = user_fixture()
-
-      {:error, changeset} =
-        Accounts.apply_user_email(user, valid_user_password(), %{email: email})
-
-      assert "has already been taken" in errors_on(changeset).email
-    end
-
     test "validates current password", %{user: user} do
       {:error, changeset} =
         Accounts.apply_user_email(user, "invalid", %{email: unique_user_email()})
@@ -204,7 +194,7 @@ defmodule RecycledCloud.AccountsTest do
 
     test "updates the email with a valid token", %{user: user, token: token, email: email} do
       assert Accounts.update_user_email(user, token) == :ok
-      changed_user = Repo.get!(User, user.id)
+      changed_user = User.get!(user.id)
       assert changed_user.email != user.email
       assert changed_user.email == email
       assert changed_user.confirmed_at
@@ -214,20 +204,20 @@ defmodule RecycledCloud.AccountsTest do
 
     test "does not update email with invalid token", %{user: user} do
       assert Accounts.update_user_email(user, "oops") == :error
-      assert Repo.get!(User, user.id).email == user.email
+      assert User.get!(user.id).email == user.email
       assert Repo.get_by(UserToken, user_id: user.id)
     end
 
     test "does not update email if user email changed", %{user: user, token: token} do
       assert Accounts.update_user_email(%{user | email: "current@example.com"}, token) == :error
-      assert Repo.get!(User, user.id).email == user.email
+      assert User.get!(user.id).email == user.email
       assert Repo.get_by(UserToken, user_id: user.id)
     end
 
     test "does not update email if token expired", %{user: user, token: token} do
       {1, nil} = Repo.update_all(UserToken, set: [inserted_at: ~N[2020-01-01 00:00:00]])
       assert Accounts.update_user_email(user, token) == :error
-      assert Repo.get!(User, user.id).email == user.email
+      assert User.get!(user.id).email == user.email
       assert Repo.get_by(UserToken, user_id: user.id)
     end
   end
@@ -258,12 +248,12 @@ defmodule RecycledCloud.AccountsTest do
     test "validates password", %{user: user} do
       {:error, changeset} =
         Accounts.update_user_password(user, valid_user_password(), %{
-          password: "not valid",
+          password: "not",
           password_confirmation: "another"
         })
 
       assert %{
-               password: ["should be at least 12 character(s)"],
+               password: ["should be at least 6 character(s)"],
                password_confirmation: ["does not match password"]
              } = errors_on(changeset)
     end
@@ -291,7 +281,7 @@ defmodule RecycledCloud.AccountsTest do
         })
 
       assert is_nil(user.password)
-      assert Accounts.get_user_by_email_and_password(user.email, "new valid password")
+      assert Accounts.get_user_by_username_and_password(user.username, "new valid password")
     end
 
     test "deletes all tokens for the given user", %{user: user} do
@@ -467,12 +457,12 @@ defmodule RecycledCloud.AccountsTest do
     test "validates password", %{user: user} do
       {:error, changeset} =
         Accounts.reset_user_password(user, %{
-          password: "not valid",
+          password: "not",
           password_confirmation: "another"
         })
 
       assert %{
-               password: ["should be at least 12 character(s)"],
+               password: ["should be at least 6 character(s)"],
                password_confirmation: ["does not match password"]
              } = errors_on(changeset)
     end
@@ -486,7 +476,7 @@ defmodule RecycledCloud.AccountsTest do
     test "updates the password", %{user: user} do
       {:ok, updated_user} = Accounts.reset_user_password(user, %{password: "new valid password"})
       assert is_nil(updated_user.password)
-      assert Accounts.get_user_by_email_and_password(user.email, "new valid password")
+      assert Accounts.get_user_by_username_and_password(user.username, "new valid password")
     end
 
     test "deletes all tokens for the given user", %{user: user} do