From 9264b21907f5c6890694d6d611ade9b13433463a Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 16 Sep 2024 00:26:57 -0400 Subject: [PATCH 01/17] Pleroma.LDAP This adds a GenServer which will keep an LDAP connection open and auto reconnect on failure with a 5 second wait between retries. Another benefit is this prevents parsing the Root CAs for every login attempt as we only need to do it once per connection. --- lib/pleroma/application.ex | 1 + lib/pleroma/ldap.ex | 233 +++++++++++++++++++++ lib/pleroma/web/auth/ldap_authenticator.ex | 147 +------------ 3 files changed, 236 insertions(+), 145 deletions(-) create mode 100644 lib/pleroma/ldap.ex diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index cb15dc1e9a..3f199c002d 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -94,6 +94,7 @@ def start(_type, _args) do children = [ Pleroma.PromEx, + Pleroma.LDAP, Pleroma.Repo, Config.TransferTask, Pleroma.Emoji, diff --git a/lib/pleroma/ldap.ex b/lib/pleroma/ldap.ex new file mode 100644 index 0000000000..8689325012 --- /dev/null +++ b/lib/pleroma/ldap.ex @@ -0,0 +1,233 @@ +defmodule Pleroma.LDAP do + use GenServer + + require Logger + + alias Pleroma.Config + alias Pleroma.User + + import Pleroma.Web.Auth.Helpers, only: [fetch_user: 1] + + @connection_timeout 10_000 + @search_timeout 10_000 + + def start_link(_) do + GenServer.start_link(__MODULE__, [], name: __MODULE__) + end + + @impl true + def init(state) do + case {Config.get(Pleroma.Web.Auth.Authenticator), Config.get([:ldap, :enabled])} do + {Pleroma.Web.Auth.LDAPAuthenticator, true} -> + {:ok, state, {:continue, :connect}} + + {Pleroma.Web.Auth.LDAPAuthenticator, false} -> + Logger.error( + "LDAP Authenticator enabled but :pleroma, :ldap is not enabled. Auth will not work." + ) + + {:ok, state} + + {_, true} -> + Logger.warning( + ":pleroma, :ldap is enabled but Pleroma.Web.Authenticator is not set to the LDAPAuthenticator. LDAP will not be used." + ) + + {:ok, state} + end + end + + @impl true + def handle_continue(:connect, _state), do: do_handle_connect() + + @impl true + def handle_info(:connect, _state), do: do_handle_connect() + + def handle_info({:bind_after_reconnect, name, password, from}, state) do + result = bind_user(state[:connection], name, password) + + GenServer.reply(from, result) + + {:noreply, state} + end + + defp do_handle_connect() do + state = + case connect() do + {:ok, connection} -> + :eldap.controlling_process(connection, self()) + [connection: connection] + + _ -> + Logger.error("Failed to connect to LDAP. Retrying in 5000ms") + Process.send_after(self(), :connect, 5_000) + [] + end + + {:noreply, state} + end + + @impl true + def handle_call({:bind_user, name, password}, from, state) do + case bind_user(state[:connection], name, password) do + :needs_reconnect -> + Process.send(self(), {:bind_after_reconnect, name, password, from}, []) + {:noreply, state, {:continue, :connect}} + + result -> + {:reply, result, state, :hibernate} + end + end + + @impl true + def terminate(_, state) do + :eldap.close(state[:connection]) + + :ok + end + + defp connect() do + ldap = Config.get(:ldap, []) + host = Keyword.get(ldap, :host, "localhost") + port = Keyword.get(ldap, :port, 389) + ssl = Keyword.get(ldap, :ssl, false) + tls = Keyword.get(ldap, :tls, false) + cacertfile = Keyword.get(ldap, :cacertfile) || CAStore.file_path() + + default_secure_opts = [ + verify: :verify_peer, + cacerts: decode_certfile(cacertfile), + customize_hostname_check: [ + fqdn_fun: fn _ -> to_charlist(host) end + ] + ] + + sslopts = Keyword.merge(default_secure_opts, Keyword.get(ldap, :sslopts, [])) + tlsopts = Keyword.merge(default_secure_opts, Keyword.get(ldap, :tlsopts, [])) + + default_options = [{:port, port}, {:ssl, ssl}, {:timeout, @connection_timeout}] + + # :sslopts can only be included in :eldap.open/2 when {ssl: true} + # or the connection will fail + options = + if ssl do + default_options ++ [{:sslopts, sslopts}] + else + default_options + end + + case :eldap.open([to_charlist(host)], options) do + {:ok, connection} -> + try do + cond do + ssl -> + :application.ensure_all_started(:ssl) + {:ok, connection} + + tls -> + case :eldap.start_tls( + connection, + tlsopts, + @connection_timeout + ) do + :ok -> + {:ok, connection} + + error -> + Logger.error("Could not start TLS: #{inspect(error)}") + :eldap.close(connection) + end + + true -> + {:ok, :connection} + end + after + :ok + end + + {:error, error} -> + Logger.error("Could not open LDAP connection: #{inspect(error)}") + {:error, {:ldap_connection_error, error}} + end + end + + defp bind_user(connection, name, password) do + uid = Config.get([:ldap, :uid], "cn") + base = Config.get([:ldap, :base]) + + case :eldap.simple_bind(connection, "#{uid}=#{name},#{base}", password) do + :ok -> + case fetch_user(name) do + %User{} = user -> + user + + _ -> + register_user(connection, base, uid, name) + end + + # eldap does not inform us of socket closure + # until it is used + {:error, {:gen_tcp_error, :closed}} -> + :eldap.close(connection) + :needs_reconnect + + error -> + Logger.error("Could not bind LDAP user #{name}: #{inspect(error)}") + {:error, {:ldap_bind_error, error}} + end + end + + defp register_user(connection, base, uid, name) do + case :eldap.search(connection, [ + {:base, to_charlist(base)}, + {:filter, :eldap.equalityMatch(to_charlist(uid), to_charlist(name))}, + {:scope, :eldap.wholeSubtree()}, + {:timeout, @search_timeout} + ]) do + # The :eldap_search_result record structure changed in OTP 24.3 and added a controls field + # https://github.com/erlang/otp/pull/5538 + {:ok, {:eldap_search_result, [{:eldap_entry, _object, attributes}], _referrals}} -> + try_register(name, attributes) + + {:ok, {:eldap_search_result, [{:eldap_entry, _object, attributes}], _referrals, _controls}} -> + try_register(name, attributes) + + error -> + Logger.error("Couldn't register user because LDAP search failed: #{inspect(error)}") + {:error, {:ldap_search_error, error}} + end + end + + defp try_register(name, attributes) do + params = %{ + name: name, + nickname: name, + password: nil + } + + params = + case List.keyfind(attributes, ~c"mail", 0) do + {_, [mail]} -> Map.put_new(params, :email, :erlang.list_to_binary(mail)) + _ -> params + end + + changeset = User.register_changeset_ldap(%User{}, params) + + case User.register(changeset) do + {:ok, user} -> user + error -> error + end + end + + defp decode_certfile(file) do + with {:ok, data} <- File.read(file) do + data + |> :public_key.pem_decode() + |> Enum.map(fn {_, b, _} -> b end) + else + _ -> + Logger.error("Unable to read certfile: #{file}") + [] + end + end +end diff --git a/lib/pleroma/web/auth/ldap_authenticator.ex b/lib/pleroma/web/auth/ldap_authenticator.ex index ad5bc9863f..c420c8bc30 100644 --- a/lib/pleroma/web/auth/ldap_authenticator.ex +++ b/lib/pleroma/web/auth/ldap_authenticator.ex @@ -5,16 +5,11 @@ defmodule Pleroma.Web.Auth.LDAPAuthenticator do alias Pleroma.User - require Logger - - import Pleroma.Web.Auth.Helpers, only: [fetch_credentials: 1, fetch_user: 1] + import Pleroma.Web.Auth.Helpers, only: [fetch_credentials: 1] @behaviour Pleroma.Web.Auth.Authenticator @base Pleroma.Web.Auth.PleromaAuthenticator - @connection_timeout 10_000 - @search_timeout 10_000 - defdelegate get_registration(conn), to: @base defdelegate create_from_registration(conn, registration), to: @base defdelegate handle_error(conn, error), to: @base @@ -24,7 +19,7 @@ defmodule Pleroma.Web.Auth.LDAPAuthenticator do def get_user(%Plug.Conn{} = conn) do with {:ldap, true} <- {:ldap, Pleroma.Config.get([:ldap, :enabled])}, {:ok, {name, password}} <- fetch_credentials(conn), - %User{} = user <- ldap_user(name, password) do + %User{} = user <- GenServer.call(Pleroma.LDAP, {:bind_user, name, password}) do {:ok, user} else {:ldap, _} -> @@ -34,142 +29,4 @@ def get_user(%Plug.Conn{} = conn) do error end end - - defp ldap_user(name, password) do - ldap = Pleroma.Config.get(:ldap, []) - host = Keyword.get(ldap, :host, "localhost") - port = Keyword.get(ldap, :port, 389) - ssl = Keyword.get(ldap, :ssl, false) - tls = Keyword.get(ldap, :tls, false) - cacertfile = Keyword.get(ldap, :cacertfile) || CAStore.file_path() - - default_secure_opts = [ - verify: :verify_peer, - cacerts: decode_certfile(cacertfile), - customize_hostname_check: [ - fqdn_fun: fn _ -> to_charlist(host) end - ] - ] - - sslopts = Keyword.merge(default_secure_opts, Keyword.get(ldap, :sslopts, [])) - tlsopts = Keyword.merge(default_secure_opts, Keyword.get(ldap, :tlsopts, [])) - - # :sslopts can only be included in :eldap.open/2 when {ssl: true} - # or the connection will fail - options = - if ssl do - [{:port, port}, {:ssl, ssl}, {:sslopts, sslopts}, {:timeout, @connection_timeout}] - else - [{:port, port}, {:ssl, ssl}, {:timeout, @connection_timeout}] - end - - case :eldap.open([to_charlist(host)], options) do - {:ok, connection} -> - try do - cond do - ssl -> - :application.ensure_all_started(:ssl) - - tls -> - case :eldap.start_tls( - connection, - tlsopts, - @connection_timeout - ) do - :ok -> - :ok - - error -> - Logger.error("Could not start TLS: #{inspect(error)}") - :eldap.close(connection) - end - - true -> - :ok - end - - bind_user(connection, ldap, name, password) - after - :eldap.close(connection) - end - - {:error, error} -> - Logger.error("Could not open LDAP connection: #{inspect(error)}") - {:error, {:ldap_connection_error, error}} - end - end - - defp bind_user(connection, ldap, name, password) do - uid = Keyword.get(ldap, :uid, "cn") - base = Keyword.get(ldap, :base) - - case :eldap.simple_bind(connection, "#{uid}=#{name},#{base}", password) do - :ok -> - case fetch_user(name) do - %User{} = user -> - user - - _ -> - register_user(connection, base, uid, name) - end - - error -> - Logger.error("Could not bind LDAP user #{name}: #{inspect(error)}") - {:error, {:ldap_bind_error, error}} - end - end - - defp register_user(connection, base, uid, name) do - case :eldap.search(connection, [ - {:base, to_charlist(base)}, - {:filter, :eldap.equalityMatch(to_charlist(uid), to_charlist(name))}, - {:scope, :eldap.wholeSubtree()}, - {:timeout, @search_timeout} - ]) do - # The :eldap_search_result record structure changed in OTP 24.3 and added a controls field - # https://github.com/erlang/otp/pull/5538 - {:ok, {:eldap_search_result, [{:eldap_entry, _object, attributes}], _referrals}} -> - try_register(name, attributes) - - {:ok, {:eldap_search_result, [{:eldap_entry, _object, attributes}], _referrals, _controls}} -> - try_register(name, attributes) - - error -> - Logger.error("Couldn't register user because LDAP search failed: #{inspect(error)}") - {:error, {:ldap_search_error, error}} - end - end - - defp try_register(name, attributes) do - params = %{ - name: name, - nickname: name, - password: nil - } - - params = - case List.keyfind(attributes, ~c"mail", 0) do - {_, [mail]} -> Map.put_new(params, :email, :erlang.list_to_binary(mail)) - _ -> params - end - - changeset = User.register_changeset_ldap(%User{}, params) - - case User.register(changeset) do - {:ok, user} -> user - error -> error - end - end - - defp decode_certfile(file) do - with {:ok, data} <- File.read(file) do - data - |> :public_key.pem_decode() - |> Enum.map(fn {_, b, _} -> b end) - else - _ -> - Logger.error("Unable to read certfile: #{file}") - [] - end - end end From ead287d623e83b8d9ffaa327b9edf96e046bfacd Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 16 Sep 2024 13:14:19 -0400 Subject: [PATCH 02/17] Credo --- lib/pleroma/ldap.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/ldap.ex b/lib/pleroma/ldap.ex index 8689325012..11544f0d9e 100644 --- a/lib/pleroma/ldap.ex +++ b/lib/pleroma/ldap.ex @@ -51,7 +51,7 @@ def handle_info({:bind_after_reconnect, name, password, from}, state) do {:noreply, state} end - defp do_handle_connect() do + defp do_handle_connect do state = case connect() do {:ok, connection} -> @@ -86,7 +86,7 @@ def terminate(_, state) do :ok end - defp connect() do + defp connect do ldap = Config.get(:ldap, []) host = Keyword.get(ldap, :host, "localhost") port = Keyword.get(ldap, :port, 389) From 7c04098dde0681f7ad299782bc09eaa9bc3a6bad Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 16 Sep 2024 16:15:53 -0400 Subject: [PATCH 03/17] Catchall for when LDAP is not enabled --- lib/pleroma/ldap.ex | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/pleroma/ldap.ex b/lib/pleroma/ldap.ex index 11544f0d9e..ac819613e0 100644 --- a/lib/pleroma/ldap.ex +++ b/lib/pleroma/ldap.ex @@ -34,6 +34,9 @@ def init(state) do ) {:ok, state} + + _ -> + {:ok, state} end end From 44b836c94c1059551fbc7564770001311d2d1e6a Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 16 Sep 2024 16:24:27 -0400 Subject: [PATCH 04/17] Fix tests We do not need to mock and verify connections are closed as the new Pleroma.LDAP GenServer will handle managing the connection lifetime --- .../web/o_auth/ldap_authorization_test.exs | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/test/pleroma/web/o_auth/ldap_authorization_test.exs b/test/pleroma/web/o_auth/ldap_authorization_test.exs index 07ce2eed84..35b947fd04 100644 --- a/test/pleroma/web/o_auth/ldap_authorization_test.exs +++ b/test/pleroma/web/o_auth/ldap_authorization_test.exs @@ -28,11 +28,7 @@ test "authorizes the existing user using LDAP credentials" do {:eldap, [], [ open: fn [^host], [{:port, ^port}, {:ssl, false} | _] -> {:ok, self()} end, - simple_bind: fn _connection, _dn, ^password -> :ok end, - close: fn _connection -> - send(self(), :close_connection) - :ok - end + simple_bind: fn _connection, _dn, ^password -> :ok end ]} ] do conn = @@ -50,7 +46,6 @@ test "authorizes the existing user using LDAP credentials" do token = Repo.get_by(Token, token: token) assert token.user_id == user.id - assert_received :close_connection end end @@ -72,10 +67,6 @@ test "creates a new user after successful LDAP authorization" do wholeSubtree: fn -> :ok end, search: fn _connection, _options -> {:ok, {:eldap_search_result, [{:eldap_entry, ~c"", []}], []}} - end, - close: fn _connection -> - send(self(), :close_connection) - :ok end ]} ] do @@ -94,7 +85,6 @@ test "creates a new user after successful LDAP authorization" do token = Repo.get_by(Token, token: token) |> Repo.preload(:user) assert token.user.nickname == user.nickname - assert_received :close_connection end end @@ -111,11 +101,7 @@ test "disallow authorization for wrong LDAP credentials" do {:eldap, [], [ open: fn [^host], [{:port, ^port}, {:ssl, false} | _] -> {:ok, self()} end, - simple_bind: fn _connection, _dn, ^password -> {:error, :invalidCredentials} end, - close: fn _connection -> - send(self(), :close_connection) - :ok - end + simple_bind: fn _connection, _dn, ^password -> {:error, :invalidCredentials} end ]} ] do conn = @@ -129,7 +115,6 @@ test "disallow authorization for wrong LDAP credentials" do }) assert %{"error" => "Invalid credentials"} = json_response(conn, 400) - assert_received :close_connection end end end From d82abf925ddbe8b98ba8191713115db50c38a0c0 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 16 Sep 2024 16:25:44 -0400 Subject: [PATCH 05/17] Ensure :cacertfile is configurable in ConfigDB --- config/description.exs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/description.exs b/config/description.exs index 15faecb38c..ade47b7e02 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2297,6 +2297,12 @@ description: "LDAP attribute name to authenticate the user, e.g. when \"cn\", the filter will be \"cn=username,base\"", suggestions: ["cn"] + }, + %{ + key: :cacertfile, + label: "CACertfile", + type: :string, + description: "Path to CA certificate file" } ] }, From 65a7b387c35b4913b6109692a84bae80af8b9a96 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Mon, 16 Sep 2024 16:28:37 -0400 Subject: [PATCH 06/17] Require a reboot if LDAP configuration changes --- lib/pleroma/config/transfer_task.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/config/transfer_task.ex b/lib/pleroma/config/transfer_task.ex index ffc95f1447..140dd77111 100644 --- a/lib/pleroma/config/transfer_task.ex +++ b/lib/pleroma/config/transfer_task.ex @@ -22,7 +22,8 @@ defp reboot_time_keys, {:pleroma, :markup}, {:pleroma, :streamer}, {:pleroma, :pools}, - {:pleroma, :connections_pool} + {:pleroma, :connections_pool}, + {:pleroma, :ldap} ] defp reboot_time_subkeys, From 123093a1868b25f101dfc4b02895c22a0daf5733 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 17 Sep 2024 13:07:26 -0400 Subject: [PATCH 07/17] Ensure :ssl is started before we attempt to make the LDAP connection --- lib/pleroma/ldap.ex | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/ldap.ex b/lib/pleroma/ldap.ex index ac819613e0..042a4daa21 100644 --- a/lib/pleroma/ldap.ex +++ b/lib/pleroma/ldap.ex @@ -97,6 +97,8 @@ defp connect do tls = Keyword.get(ldap, :tls, false) cacertfile = Keyword.get(ldap, :cacertfile) || CAStore.file_path() + if ssl, do: Application.ensure_all_started(:ssl) + default_secure_opts = [ verify: :verify_peer, cacerts: decode_certfile(cacertfile), @@ -123,10 +125,6 @@ defp connect do {:ok, connection} -> try do cond do - ssl -> - :application.ensure_all_started(:ssl) - {:ok, connection} - tls -> case :eldap.start_tls( connection, From d0ee899ab94788e37e6ac3c43342017d1b27903a Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 17 Sep 2024 13:14:09 -0400 Subject: [PATCH 08/17] Only close connection if it is not nil --- lib/pleroma/ldap.ex | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/ldap.ex b/lib/pleroma/ldap.ex index 042a4daa21..3df20fe094 100644 --- a/lib/pleroma/ldap.ex +++ b/lib/pleroma/ldap.ex @@ -84,7 +84,11 @@ def handle_call({:bind_user, name, password}, from, state) do @impl true def terminate(_, state) do - :eldap.close(state[:connection]) + connection = Keyword.get(state, :connection) + + if not is_nil(connection) do + :eldap.close(connection) + end :ok end From 164ffbcab822eda4c28f912082b6a7a3ec64a7e5 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 17 Sep 2024 13:17:40 -0400 Subject: [PATCH 09/17] Fix return value when not doing STARTTLS --- lib/pleroma/ldap.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/ldap.ex b/lib/pleroma/ldap.ex index 3df20fe094..6be2188f43 100644 --- a/lib/pleroma/ldap.ex +++ b/lib/pleroma/ldap.ex @@ -144,7 +144,7 @@ defp connect do end true -> - {:ok, :connection} + {:ok, connection} end after :ok From a1972d57e30f41e5173d61c0d0936685738c560d Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 17 Sep 2024 13:19:54 -0400 Subject: [PATCH 10/17] Link the eldap connection process Ensure if LDAP GenServer crashes it gets cleaned up, and we should crash and restart if somehow the eldap connection process crashes unexpectedly as we can't seem to receive any DOWN messages from it, etc. --- lib/pleroma/ldap.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/pleroma/ldap.ex b/lib/pleroma/ldap.ex index 6be2188f43..0723cd0941 100644 --- a/lib/pleroma/ldap.ex +++ b/lib/pleroma/ldap.ex @@ -59,6 +59,7 @@ defp do_handle_connect do case connect() do {:ok, connection} -> :eldap.controlling_process(connection, self()) + Process.link(connection) [connection: connection] _ -> From 14a9663f1abe49b8f4f4f719fa2f4db3a5dd81b7 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 17 Sep 2024 13:28:42 -0400 Subject: [PATCH 11/17] Remove cacertfile as child of SSL and TLS options We need to pass the cacerts (list of charlist encoded certs) not cacertfile, so our new cacertfile setting handles this for us. --- config/description.exs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/config/description.exs b/config/description.exs index ade47b7e02..5062842f04 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2241,14 +2241,8 @@ label: "SSL options", type: :keyword, description: "Additional SSL options", - suggestions: [cacertfile: "path/to/file/with/PEM/cacerts", verify: :verify_peer], + suggestions: [verify: :verify_peer], children: [ - %{ - key: :cacertfile, - type: :string, - description: "Path to file with PEM encoded cacerts", - suggestions: ["path/to/file/with/PEM/cacerts"] - }, %{ key: :verify, type: :atom, @@ -2268,14 +2262,8 @@ label: "TLS options", type: :keyword, description: "Additional TLS options", - suggestions: [cacertfile: "path/to/file/with/PEM/cacerts", verify: :verify_peer], + suggestions: [verify: :verify_peer], children: [ - %{ - key: :cacertfile, - type: :string, - description: "Path to file with PEM encoded cacerts", - suggestions: ["path/to/file/with/PEM/cacerts"] - }, %{ key: :verify, type: :atom, From 363b462c54c454e847072869db09f8f4d5da4426 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 17 Sep 2024 13:36:46 -0400 Subject: [PATCH 12/17] Make the email attribute configurable While here, fix the System.get_env usage to use the normal fallback value method and improve the UID label description --- config/config.exs | 11 ++++++----- config/description.exs | 9 ++++++++- lib/pleroma/ldap.ex | 4 +++- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/config/config.exs b/config/config.exs index f53a083d04..47ddfac5ac 100644 --- a/config/config.exs +++ b/config/config.exs @@ -612,16 +612,17 @@ config :pleroma, :ldap, enabled: System.get_env("LDAP_ENABLED") == "true", - host: System.get_env("LDAP_HOST") || "localhost", - port: String.to_integer(System.get_env("LDAP_PORT") || "389"), + host: System.get_env("LDAP_HOST", "localhost"), + port: String.to_integer(System.get_env("LDAP_PORT", "389")), ssl: System.get_env("LDAP_SSL") == "true", sslopts: [], tls: System.get_env("LDAP_TLS") == "true", tlsopts: [], - base: System.get_env("LDAP_BASE") || "dc=example,dc=com", - uid: System.get_env("LDAP_UID") || "cn", + base: System.get_env("LDAP_BASE", "dc=example,dc=com"), + uid: System.get_env("LDAP_UID", "cn"), # defaults to CAStore's Mozilla roots - cacertfile: nil + cacertfile: System.get_env("LDAP_CACERTFILE", nil), + mail: System.get_env("LDAP_MAIL", "mail") oauth_consumer_strategies = System.get_env("OAUTH_CONSUMER_STRATEGIES") diff --git a/config/description.exs b/config/description.exs index 5062842f04..e85ec0ff80 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2280,7 +2280,7 @@ }, %{ key: :uid, - label: "UID", + label: "UID Attribute", type: :string, description: "LDAP attribute name to authenticate the user, e.g. when \"cn\", the filter will be \"cn=username,base\"", @@ -2291,6 +2291,13 @@ label: "CACertfile", type: :string, description: "Path to CA certificate file" + }, + %{ + key: :mail, + label: "Mail Attribute", + type: :string, + description: "LDAP attribute name to use as the email address when automatically registering the user on first login", + suggestions: ["mail"] } ] }, diff --git a/lib/pleroma/ldap.ex b/lib/pleroma/ldap.ex index 0723cd0941..8e9c591b29 100644 --- a/lib/pleroma/ldap.ex +++ b/lib/pleroma/ldap.ex @@ -205,6 +205,8 @@ defp register_user(connection, base, uid, name) do end defp try_register(name, attributes) do + mail_attribute = Config.get([:ldap, :mail]) + params = %{ name: name, nickname: name, @@ -212,7 +214,7 @@ defp try_register(name, attributes) do } params = - case List.keyfind(attributes, ~c"mail", 0) do + case List.keyfind(attributes, to_charlist(mail_attribute), 0) do {_, [mail]} -> Map.put_new(params, :email, :erlang.list_to_binary(mail)) _ -> params end From 21bf229731f27426564140650397e51ba4bb4b93 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 17 Sep 2024 13:43:21 -0400 Subject: [PATCH 13/17] Reduce LDAP timeouts 10 seconds is way too long for any login attempt or search result. LDAP should always be fast. --- lib/pleroma/ldap.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/ldap.ex b/lib/pleroma/ldap.ex index 8e9c591b29..33c9cff295 100644 --- a/lib/pleroma/ldap.ex +++ b/lib/pleroma/ldap.ex @@ -8,8 +8,8 @@ defmodule Pleroma.LDAP do import Pleroma.Web.Auth.Helpers, only: [fetch_user: 1] - @connection_timeout 10_000 - @search_timeout 10_000 + @connection_timeout 2_000 + @search_timeout 2_000 def start_link(_) do GenServer.start_link(__MODULE__, [], name: __MODULE__) From 1d123832da6a2b8c67f34006b4ea05e0be86e366 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 17 Sep 2024 13:46:49 -0400 Subject: [PATCH 14/17] Formatting --- config/description.exs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/description.exs b/config/description.exs index e85ec0ff80..47f4771eb8 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2296,7 +2296,8 @@ key: :mail, label: "Mail Attribute", type: :string, - description: "LDAP attribute name to use as the email address when automatically registering the user on first login", + description: + "LDAP attribute name to use as the email address when automatically registering the user on first login", suggestions: ["mail"] } ] From ea63533cf28be8218a27806b1f6430f0c1ca4a01 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 17 Sep 2024 13:46:56 -0400 Subject: [PATCH 15/17] Change :connection to :handle to match upstream nomenclature --- lib/pleroma/ldap.ex | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/pleroma/ldap.ex b/lib/pleroma/ldap.ex index 33c9cff295..b357b371ad 100644 --- a/lib/pleroma/ldap.ex +++ b/lib/pleroma/ldap.ex @@ -47,7 +47,7 @@ def handle_continue(:connect, _state), do: do_handle_connect() def handle_info(:connect, _state), do: do_handle_connect() def handle_info({:bind_after_reconnect, name, password, from}, state) do - result = bind_user(state[:connection], name, password) + result = bind_user(state[:handle], name, password) GenServer.reply(from, result) @@ -57,10 +57,10 @@ def handle_info({:bind_after_reconnect, name, password, from}, state) do defp do_handle_connect do state = case connect() do - {:ok, connection} -> - :eldap.controlling_process(connection, self()) - Process.link(connection) - [connection: connection] + {:ok, handle} -> + :eldap.controlling_process(handle, self()) + Process.link(handle) + [handle: handle] _ -> Logger.error("Failed to connect to LDAP. Retrying in 5000ms") @@ -73,7 +73,7 @@ defp do_handle_connect do @impl true def handle_call({:bind_user, name, password}, from, state) do - case bind_user(state[:connection], name, password) do + case bind_user(state[:handle], name, password) do :needs_reconnect -> Process.send(self(), {:bind_after_reconnect, name, password, from}, []) {:noreply, state, {:continue, :connect}} @@ -85,10 +85,10 @@ def handle_call({:bind_user, name, password}, from, state) do @impl true def terminate(_, state) do - connection = Keyword.get(state, :connection) + handle = Keyword.get(state, :handle) - if not is_nil(connection) do - :eldap.close(connection) + if not is_nil(handle) do + :eldap.close(handle) end :ok @@ -127,25 +127,25 @@ defp connect do end case :eldap.open([to_charlist(host)], options) do - {:ok, connection} -> + {:ok, handle} -> try do cond do tls -> case :eldap.start_tls( - connection, + handle, tlsopts, @connection_timeout ) do :ok -> - {:ok, connection} + {:ok, handle} error -> Logger.error("Could not start TLS: #{inspect(error)}") - :eldap.close(connection) + :eldap.close(handle) end true -> - {:ok, connection} + {:ok, handle} end after :ok @@ -157,24 +157,24 @@ defp connect do end end - defp bind_user(connection, name, password) do + defp bind_user(handle, name, password) do uid = Config.get([:ldap, :uid], "cn") base = Config.get([:ldap, :base]) - case :eldap.simple_bind(connection, "#{uid}=#{name},#{base}", password) do + case :eldap.simple_bind(handle, "#{uid}=#{name},#{base}", password) do :ok -> case fetch_user(name) do %User{} = user -> user _ -> - register_user(connection, base, uid, name) + register_user(handle, base, uid, name) end # eldap does not inform us of socket closure # until it is used {:error, {:gen_tcp_error, :closed}} -> - :eldap.close(connection) + :eldap.close(handle) :needs_reconnect error -> @@ -183,8 +183,8 @@ defp bind_user(connection, name, password) do end end - defp register_user(connection, base, uid, name) do - case :eldap.search(connection, [ + defp register_user(handle, base, uid, name) do + case :eldap.search(handle, [ {:base, to_charlist(base)}, {:filter, :eldap.equalityMatch(to_charlist(uid), to_charlist(name))}, {:scope, :eldap.wholeSubtree()}, From 2b482e34ebf5aee49350d8198e6fade8820b3834 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 17 Sep 2024 13:54:57 -0400 Subject: [PATCH 16/17] Improve matching on bind errors --- lib/pleroma/ldap.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/ldap.ex b/lib/pleroma/ldap.ex index b357b371ad..cd84dee023 100644 --- a/lib/pleroma/ldap.ex +++ b/lib/pleroma/ldap.ex @@ -177,9 +177,9 @@ defp bind_user(handle, name, password) do :eldap.close(handle) :needs_reconnect - error -> + {:error, error} = e -> Logger.error("Could not bind LDAP user #{name}: #{inspect(error)}") - {:error, {:ldap_bind_error, error}} + e end end From 35ddb1d2c8a53dcb54178522811242ef40a63211 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 17 Sep 2024 13:57:10 -0400 Subject: [PATCH 17/17] LDAP genserver changelog --- changelog.d/ldap-refactor.change | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/ldap-refactor.change diff --git a/changelog.d/ldap-refactor.change b/changelog.d/ldap-refactor.change new file mode 100644 index 0000000000..1510eea6aa --- /dev/null +++ b/changelog.d/ldap-refactor.change @@ -0,0 +1 @@ +LDAP authentication has been refactored to operate as a GenServer process which will maintain an active connection to the LDAP server.