Merge branch 'fix/ldap-auth-issues' into 'develop'

Fix LDAP auth issues

Closes #1646

See merge request pleroma/pleroma!2852

CHANGELOG.md

@@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Configuration: `:instance, rewrite_policy` moved to `:mrf, policies`, `:instance, :mrf_transparency` moved to `:mrf, :transparency`, `:instance, :mrf_transparency_exclusions` moved to `:mrf, :transparency_exclusions`. Old config namespace is deprecated.
 - Configuration: `:media_proxy, whitelist` format changed to host with scheme (e.g. `http://example.com` instead of `example.com`). Domain format is deprecated.
 - **Breaking:** Configuration: `:instance, welcome_user_nickname` moved to `:welcome, :direct_message, :sender_nickname`, `:instance, :welcome_message` moved to `:welcome, :direct_message, :message`. Old config namespace is deprecated.
+- **Breaking:** LDAP: Fallback to local database authentication has been removed for security reasons and lack of a mechanism to ensure the passwords are synchronized when LDAP passwords are updated.
 
 <details>
   <summary>API Changes</summary>

config/config.exs

@@ -743,6 +743,8 @@
 
 config :pleroma, :instances_favicons, enabled: false
 
+config :pleroma, Pleroma.Web.Auth.Authenticator, Pleroma.Web.Auth.PleromaAuthenticator
+
 # Import environment specific config. This must remain at the bottom
 # of this file so it overrides the configuration defined above.
 import_config "#{Mix.env()}.exs"

docs/configuration/cheatsheet.md

@@ -887,6 +887,9 @@ Pleroma account will be created with the same name as the LDAP user name.
 * `base`: LDAP base, e.g. "dc=example,dc=com"
 * `uid`: LDAP attribute name to authenticate the user, e.g. when "cn", the filter will be "cn=username,base"
 
+Note, if your LDAP server is an Active Directory server the correct value is commonly `uid: "cn"`, but if you use an
+OpenLDAP server the value may be `uid: "uid"`.
+
 ### OAuth consumer mode
 
 OAuth consumer mode allows sign in / sign up via external OAuth providers (e.g. Twitter, Facebook, Google, Microsoft, etc.).

lib/pleroma/user.ex

@@ -638,6 +638,34 @@ def force_password_reset_async(user) do
   @spec force_password_reset(User.t()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()}
   def force_password_reset(user), do: update_password_reset_pending(user, true)
 
+  # Used to auto-register LDAP accounts which won't have a password hash stored locally
+  def register_changeset_ldap(struct, params = %{password: password})
+      when is_nil(password) do
+    params = Map.put_new(params, :accepts_chat_messages, true)
+
+    params =
+      if Map.has_key?(params, :email) do
+        Map.put_new(params, :email, params[:email])
+      else
+        params
+      end
+
+    struct
+    |> cast(params, [
+      :name,
+      :nickname,
+      :email,
+      :accepts_chat_messages
+    ])
+    |> validate_required([:name, :nickname])
+    |> unique_constraint(:nickname)
+    |> validate_exclusion(:nickname, Config.get([User, :restricted_nicknames]))
+    |> validate_format(:nickname, local_nickname_regex())
+    |> put_ap_id()
+    |> unique_constraint(:ap_id)
+    |> put_following_and_follower_address()
+  end
+
   def register_changeset(struct, params \\ %{}, opts \\ []) do
     bio_limit = Config.get([:instance, :user_bio_length], 5000)
     name_limit = Config.get([:instance, :user_name_length], 100)

lib/pleroma/web/auth/ldap_authenticator.ex

@@ -28,10 +28,6 @@ def get_user(%Plug.Conn{} = conn) do
          %User{} = user <- ldap_user(name, password) do
       {:ok, user}
     else
-      {:error, {:ldap_connection_error, _}} ->
-        # When LDAP is unavailable, try default authenticator
-        @base.get_user(conn)
-
       {:ldap, _} ->
         @base.get_user(conn)
 
@@ -92,7 +88,7 @@ defp bind_user(connection, ldap, name, password) do
             user
 
           _ ->
-            register_user(connection, base, uid, name, password)
+            register_user(connection, base, uid, name)
         end
 
       error ->
@@ -100,34 +96,31 @@ defp bind_user(connection, ldap, name, password) do
     end
   end
 
-  defp register_user(connection, base, uid, name, password) do
+  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()},
-           {:attributes, ['mail', 'email']},
            {:timeout, @search_timeout}
          ]) do
       {:ok, {:eldap_search_result, [{:eldap_entry, _, attributes}], _}} ->
-        with {_, [mail]} <- List.keyfind(attributes, 'mail', 0) do
-          params = %{
-            email: :erlang.list_to_binary(mail),
-            name: name,
-            nickname: name,
-            password: password,
-            password_confirmation: password
-          }
+        params = %{
+          name: name,
+          nickname: name,
+          password: nil
+        }
 
-          changeset = User.register_changeset(%User{}, params)
-
-          case User.register(changeset) do
-            {:ok, user} -> user
-            error -> error
+        params =
+          case List.keyfind(attributes, 'mail', 0) do
+            {_, [mail]} -> Map.put_new(params, :email, :erlang.list_to_binary(mail))
+            _ -> params
           end
-        else
-          _ ->
-            Logger.error("Could not find LDAP attribute mail: #{inspect(attributes)}")
-            {:error, :ldap_registration_missing_attributes}
+
+        changeset = User.register_changeset_ldap(%User{}, params)
+
+        case User.register(changeset) do
+          {:ok, user} -> user
+          error -> error
         end
 
       error ->

test/web/oauth/ldap_authorization_test.exs

@@ -7,7 +7,6 @@ defmodule Pleroma.Web.OAuth.LDAPAuthorizationTest do
   alias Pleroma.Repo
   alias Pleroma.Web.OAuth.Token
   import Pleroma.Factory
-  import ExUnit.CaptureLog
   import Mock
 
   @skip if !Code.ensure_loaded?(:eldap), do: :skip
@@ -72,9 +71,7 @@ test "creates a new user after successful LDAP authorization" do
          equalityMatch: fn _type, _value -> :ok end,
          wholeSubtree: fn -> :ok end,
          search: fn _connection, _options ->
-           {:ok,
-            {:eldap_search_result, [{:eldap_entry, '', [{'mail', [to_charlist(user.email)]}]}],
-             []}}
+           {:ok, {:eldap_search_result, [{:eldap_entry, '', []}], []}}
          end,
          close: fn _connection ->
            send(self(), :close_connection)
@@ -101,50 +98,6 @@ test "creates a new user after successful LDAP authorization" do
     end
   end
 
-  @tag @skip
-  test "falls back to the default authorization when LDAP is unavailable" do
-    password = "testpassword"
-    user = insert(:user, password_hash: Pbkdf2.hash_pwd_salt(password))
-    app = insert(:oauth_app, scopes: ["read", "write"])
-
-    host = Pleroma.Config.get([:ldap, :host]) |> to_charlist
-    port = Pleroma.Config.get([:ldap, :port])
-
-    with_mocks [
-      {:eldap, [],
-       [
-         open: fn [^host], [{:port, ^port}, {:ssl, false} | _] -> {:error, 'connect failed'} end,
-         simple_bind: fn _connection, _dn, ^password -> :ok end,
-         close: fn _connection ->
-           send(self(), :close_connection)
-           :ok
-         end
-       ]}
-    ] do
-      log =
-        capture_log(fn ->
-          conn =
-            build_conn()
-            |> post("/oauth/token", %{
-              "grant_type" => "password",
-              "username" => user.nickname,
-              "password" => password,
-              "client_id" => app.client_id,
-              "client_secret" => app.client_secret
-            })
-
-          assert %{"access_token" => token} = json_response(conn, 200)
-
-          token = Repo.get_by(Token, token: token)
-
-          assert token.user_id == user.id
-        end)
-
-      assert log =~ "Could not open LDAP connection: 'connect failed'"
-      refute_received :close_connection
-    end
-  end
-
   @tag @skip
   test "disallow authorization for wrong LDAP credentials" do
     password = "testpassword"