diff --git a/config/test.exs b/config/test.exs index 9d22f5f..f269eff 100644 --- a/config/test.exs +++ b/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" diff --git a/lib/recycledcloud/LDAP.ex b/lib/recycledcloud/LDAP.ex index de0a165..955cbfe 100644 --- a/lib/recycledcloud/LDAP.ex +++ b/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: #{reason}." + Logger.info "Terminating LDAP backend: #{inspect(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 diff --git a/lib/recycledcloud/accounts.ex b/lib/recycledcloud/accounts.ex index 3cd13d6..0464271 100644 --- a/lib/recycledcloud/accounts.ex +++ b/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, _} -> - 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 + + set_user_password(user, changeset, attrs[:password]) end ## Session @@ -347,14 +374,10 @@ defmodule RecycledCloud.Accounts do """ def reset_user_password(user, attrs) do - Ecto.Multi.new() - |> Ecto.Multi.update(:user, User.password_changeset(user, attrs)) - |> Ecto.Multi.delete_all(:tokens, UserToken.user_and_contexts_query(user, :all)) - |> Repo.transaction() - |> case do - {:ok, %{user: user}} -> {:ok, user} - {:error, :user, changeset, _} -> {:error, changeset} - end + changeset = user + |> User.password_changeset(attrs) + + set_user_password(user, changeset, attrs[:password]) end ## Keys diff --git a/lib/recycledcloud/accounts/user.ex b/lib/recycledcloud/accounts/user.ex index 139508a..0558be1 100644 --- a/lib/recycledcloud/accounts/user.ex +++ b/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]} -> - attrs = entry - |> Map.get(:attributes) - |> Enum.into(%{}) - |> Map.put('dn', entry.object_name) - {:ok, attrs} + {:ok, [entry]} -> {:ok, entry} {: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 - # Exldap does not properly implement `change_password`, hence we have to - # fallback on erlang's `:eldap`. - # See http://erlang.org/doc/man/eldap.html#modify-4 - + def set_password(%User{} = user, new_password) + when byte_size(new_password) > 0 do query = fn ldap_conn -> :eldap.modify_password( ldap_conn, - String.to_charlist(user.dn), - String.to_charlist(new_password) + to_charlist(user.dn), + 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 diff --git a/lib/recycledcloud/accounts/user_token.ex b/lib/recycledcloud/accounts/user_token.ex index 4aae369..c3497ba 100644 --- a/lib/recycledcloud/accounts/user_token.ex +++ b/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} diff --git a/mix.exs b/mix.exs index edfdacc..c7a162a 100644 --- a/mix.exs +++ b/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}, - {:exldap, git: "https://github.com/Fnux/exldap.git"} + {:phx_gen_auth, "~> 0.6", only: [:dev], runtime: false} ] end diff --git a/test/recycledcloud/accounts_test.exs b/test/recycledcloud/accounts_test.exs index 6983acd..ff84905 100644 --- a/test/recycledcloud/accounts_test.exs +++ b/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 - test "does not return the user if the email does not exist" do - refute Accounts.get_user_by_email_and_password("unknown@example.com", "hello world!") + describe "get_user_by_username_and_password/2" do + test "does not return the user if the username does not exist" do + 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 - %{email: email} = user_fixture() - {:error, changeset} = Accounts.register_user(%{email: email}) - assert "has already been taken" in errors_on(changeset).email + test "validates username uniqueness" do + %{username: username} = user_fixture() + {:error, changeset} = Accounts.register_user(%{ + 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. - {:error, changeset} = Accounts.register_user(%{email: String.upcase(email)}) - assert "has already been taken" in errors_on(changeset).email - end - - test "registers users with a hashed password" do - email = unique_user_email() - {: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) + # Now try with the upper cased username too, to check that email case is ignored. + {:error, changeset} = Accounts.register_user(%{ + username: String.upcase(username), + email: unique_user_email(), + password: valid_user_password() + }) + assert "has already been taken" in errors_on(changeset).username 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