diff --git a/docker/api.Dockerfile b/docker/api.Dockerfile
index 956b5317c939..098eeff29d47 100644
--- a/docker/api.Dockerfile
+++ b/docker/api.Dockerfile
@@ -72,6 +72,7 @@ RUN apt-get update \
&& apt-get install --no-install-recommends -y \
gosu \
tesseract-ocr-all \
+ libldap-2.4-2 \
&& apt-get autoremove \
&& rm -rf /var/lib/apt/lists/*
diff --git a/docker/omni.Dockerfile b/docker/omni.Dockerfile
index 1c80b657ed1d..1997bb88ab21 100644
--- a/docker/omni.Dockerfile
+++ b/docker/omni.Dockerfile
@@ -97,6 +97,7 @@ RUN apt-get update \
tesseract-ocr-all \
curl \
gnupg \
+ libldap-2.4-2 \
&& apt-get autoremove \
&& rm -rf /var/lib/apt/lists/*
diff --git a/docs/docs/documentation/getting-started/installation/backend-config.md b/docs/docs/documentation/getting-started/installation/backend-config.md
index 5f67c86d1a42..36738700e17e 100644
--- a/docs/docs/documentation/getting-started/installation/backend-config.md
+++ b/docs/docs/documentation/getting-started/installation/backend-config.md
@@ -68,6 +68,11 @@ Changing the webworker settings may cause unforeseen memory leak issues with Mea
| LDAP_SERVER_URL | None | LDAP server URL (e.g. ldap://ldap.example.com) |
| LDAP_TLS_INSECURE | False | Do not verify server certificate when using secure LDAP |
| LDAP_TLS_CACERTFILE | None | File path to Certificate Authority used to verify server certificate (e.g. `/path/to/ca.crt`) |
-| LDAP_BIND_TEMPLATE | None | Templated DN for users, `{}` will be replaced with the username (e.g. `cn={},dc=example,dc=com`, `{}@example.com`) |
| LDAP_BASE_DN | None | Starting point when searching for users authentication (e.g. `CN=Users,DC=xx,DC=yy,DC=de`) |
+| LDAP_QUERY_BIND | None | A bind user for LDAP search queries (e.g. `cn=admin,cn=users,dc=example,dc=com`) |
+| LDAP_QUERY_PASSWORD | None | The password for the bind user used in LDAP_QUERY_BIND |
+| LDAP_USER_FILTER | None | The LDAP search filter to find users (e.g. `(&( | ({id_attribute}={input})({mail_attribute}={input}))(objectClass=person))`).
**Note** `id_attribute` and `mail_attribute` will be replaced with `LDAP_ID_ATTRIBUTE` and `LDAP_MAIL_ATTRIBUTE`, respectively. `input` will be replaced with either the username or email the user logs in with. |
| LDAP_ADMIN_FILTER | None | Optional LDAP filter, which tells Mealie the LDAP user is an admin (e.g. `(memberOf=cn=admins,dc=example,dc=com)`) |
+| LDAP_ID_ATTRIBUTE | uid | The LDAP attribute that maps to the user's id |
+| LDAP_NAME_ATTRIBUTE | name | The LDAP attribute that maps to the user's name |
+| LDAP_MAIL_ATTRIBUTE | mail | The LDAP attribute that maps to the user's email |
diff --git a/docs/docs/overrides/api.html b/docs/docs/overrides/api.html
index 2fd24048046e..2d7e85f36633 100644
--- a/docs/docs/overrides/api.html
+++ b/docs/docs/overrides/api.html
@@ -14,7 +14,7 @@
diff --git a/mealie/core/security/security.py b/mealie/core/security/security.py
index e75c09abd005..f5219970a465 100644
--- a/mealie/core/security/security.py
+++ b/mealie/core/security/security.py
@@ -54,6 +54,7 @@ def user_from_ldap(db: AllRepositories, username: str, password: str) -> Private
if settings.LDAP_TLS_INSECURE:
ldap.set_option(ldap.OPT_X_TLS_REQUIRE_CERT, ldap.OPT_X_TLS_NEVER)
+
ldap.set_option(ldap.OPT_REFERRALS, 0)
ldap.set_option(ldap.OPT_PROTOCOL_VERSION, 3)
conn = ldap.initialize(settings.LDAP_SERVER_URL)
@@ -61,19 +62,11 @@ def user_from_ldap(db: AllRepositories, username: str, password: str) -> Private
if settings.LDAP_TLS_CACERTFILE:
conn.set_option(ldap.OPT_X_TLS_CACERTFILE, settings.LDAP_TLS_CACERTFILE)
conn.set_option(ldap.OPT_X_TLS_NEWCTX, 0)
- user = db.users.get_one(username, "email", any_case=True)
-
- if not settings.LDAP_BIND_TEMPLATE:
- return False
-
- if not user:
- user_bind = settings.LDAP_BIND_TEMPLATE.format(username)
- user = db.users.get_one(username, "username", any_case=True)
- else:
- user_bind = settings.LDAP_BIND_TEMPLATE.format(user.username)
+ # Use query user for the search instead of the logged in user
+ # This prevents the need for every user to have query permissions in LDAP
try:
- conn.simple_bind_s(user_bind, password)
+ conn.simple_bind_s(settings.LDAP_QUERY_BIND, settings.LDAP_QUERY_PASSWORD)
except (ldap.INVALID_CREDENTIALS, ldap.NO_SUCH_OBJECT):
return False
@@ -83,21 +76,44 @@ def user_from_ldap(db: AllRepositories, username: str, password: str) -> Private
user_entry = conn.search_s(
settings.LDAP_BASE_DN,
ldap.SCOPE_SUBTREE,
- f"(&(objectClass=user)(|(cn={username})(sAMAccountName={username})(mail={username})))",
- ["name", "mail"],
+ settings.LDAP_USER_FILTER.format(
+ id_attribute=settings.LDAP_ID_ATTRIBUTE, mail_attribute=settings.LDAP_MAIL_ATTRIBUTE, input=username
+ ),
+ [settings.LDAP_ID_ATTRIBUTE, settings.LDAP_NAME_ATTRIBUTE, settings.LDAP_MAIL_ATTRIBUTE],
)
- if user_entry is not None and len(user_entry[0]) != 0 and user_entry[0][0] is not None:
- user_dn, user_attr = user_entry[0]
- else:
+
+ if not user_entry:
+ conn.unbind_s()
return False
+ user_dn, user_attr = user_entry[0]
+
+ # Check the credentials of the user
+ try:
+ conn.simple_bind_s(user_dn, password)
+ except (ldap.INVALID_CREDENTIALS, ldap.NO_SUCH_OBJECT):
+ return False
+
+ # Check for existing user
+ user = db.users.get_one(username, "email", any_case=True)
+ if not user:
+ user = db.users.get_one(username, "username", any_case=True)
+
if user is None:
+ try:
+ user_id = user_attr[settings.LDAP_ID_ATTRIBUTE][0].decode("utf-8")
+ full_name = user_attr[settings.LDAP_NAME_ATTRIBUTE][0].decode("utf-8")
+ email = user_attr[settings.LDAP_MAIL_ATTRIBUTE][0].decode("utf-8")
+ except KeyError:
+ conn.unbind_s()
+ return False
+
user = db.users.create(
{
- "username": username,
+ "username": user_id,
"password": "LDAP",
- "full_name": user_attr["name"][0],
- "email": user_attr["mail"][0],
+ "full_name": full_name,
+ "email": email,
"admin": False,
},
)
@@ -105,6 +121,8 @@ def user_from_ldap(db: AllRepositories, username: str, password: str) -> Private
if settings.LDAP_ADMIN_FILTER:
user.admin = len(conn.search_s(user_dn, ldap.SCOPE_BASE, settings.LDAP_ADMIN_FILTER, [])) > 0
db.users.update(user.id, user)
+
+ conn.unbind_s()
return user
diff --git a/mealie/core/settings/settings.py b/mealie/core/settings/settings.py
index fb60491f9e15..f80ed3b8192a 100644
--- a/mealie/core/settings/settings.py
+++ b/mealie/core/settings/settings.py
@@ -117,18 +117,27 @@ class AppSettings(BaseSettings):
LDAP_SERVER_URL: NoneStr = None
LDAP_TLS_INSECURE: bool = False
LDAP_TLS_CACERTFILE: NoneStr = None
- LDAP_BIND_TEMPLATE: NoneStr = None
LDAP_BASE_DN: NoneStr = None
+ LDAP_QUERY_BIND: NoneStr = None
+ LDAP_QUERY_PASSWORD: NoneStr = None
+ LDAP_USER_FILTER: NoneStr = None
LDAP_ADMIN_FILTER: NoneStr = None
+ LDAP_ID_ATTRIBUTE: str = "uid"
+ LDAP_MAIL_ATTRIBUTE: str = "mail"
+ LDAP_NAME_ATTRIBUTE: str = "name"
@property
def LDAP_ENABLED(self) -> bool:
"""Validates LDAP settings are all set"""
required = {
self.LDAP_SERVER_URL,
- self.LDAP_BIND_TEMPLATE,
self.LDAP_BASE_DN,
- self.LDAP_ADMIN_FILTER,
+ self.LDAP_USER_FILTER,
+ self.LDAP_QUERY_BIND,
+ self.LDAP_QUERY_PASSWORD,
+ self.LDAP_ID_ATTRIBUTE,
+ self.LDAP_MAIL_ATTRIBUTE,
+ self.LDAP_NAME_ATTRIBUTE,
}
not_none = None not in required
return self.LDAP_AUTH_ENABLED and not_none
diff --git a/template.env b/template.env
index 11a06cb3efe9..921efebf0f45 100644
--- a/template.env
+++ b/template.env
@@ -36,9 +36,14 @@ LANG=en-US
# Configuration for authentication via an external LDAP server
LDAP_AUTH_ENABLED=False
-LDAP_SERVER_URL=None
-LDAP_TLS_INSECURE=False
-LDAP_TLS_CACERTFILE=None
-LDAP_BIND_TEMPLATE=None
-LDAP_BASE_DN=None
-LDAP_ADMIN_FILTER=None
+# LDAP_SERVER_URL=""
+# LDAP_TLS_INSECURE=False
+# LDAP_TLS_CACERTFILE=
+# LDAP_BASE_DN=""
+# LDAP_QUERY_BIND=""
+# LDAP_QUERY_PASSWORD=""
+# LDAP_USER_FILTER=""
+# LDAP_ADMIN_FILTER=""
+# LDAP_ID_ATTRIBUTE=uid
+# LDAP_NAME_ATTRIBUTE=name
+# LDAP_MAIL_ATTRIBUTE=mail
diff --git a/tests/unit_tests/test_security.py b/tests/unit_tests/test_security.py
index 4bf6757dbdab..c1bbcf451cb8 100644
--- a/tests/unit_tests/test_security.py
+++ b/tests/unit_tests/test_security.py
@@ -1,5 +1,6 @@
from pathlib import Path
+import ldap
from pytest import MonkeyPatch
from mealie.core import security
@@ -9,6 +10,90 @@ from mealie.db.db_setup import session_context
from tests.utils.factories import random_string
+class LdapConnMock:
+ def __init__(self, user, password, admin, query_bind, query_password, mail, name) -> None:
+ self.app_settings = get_app_settings()
+ self.user = user
+ self.password = password
+ self.query_bind = query_bind
+ self.query_password = query_password
+ self.admin = admin
+ self.mail = mail
+ self.name = name
+
+ def simple_bind_s(self, dn, bind_pw):
+ if dn == "cn={}, {}".format(self.user, self.app_settings.LDAP_BASE_DN):
+ valid_password = self.password
+ elif "cn={}, {}".format(self.query_bind, self.app_settings.LDAP_BASE_DN):
+ valid_password = self.query_password
+
+ if bind_pw == valid_password:
+ return
+
+ raise ldap.INVALID_CREDENTIALS
+
+ # Default search mock implementation
+ def search_s(self, dn, scope, filter, attrlist):
+ if filter == self.app_settings.LDAP_ADMIN_FILTER:
+ assert attrlist == []
+ assert filter == self.app_settings.LDAP_ADMIN_FILTER
+ assert dn == "cn={}, {}".format(self.user, self.app_settings.LDAP_BASE_DN)
+ assert scope == ldap.SCOPE_BASE
+
+ if not self.admin:
+ return []
+
+ return [(dn, {})]
+
+ assert attrlist == [
+ self.app_settings.LDAP_ID_ATTRIBUTE,
+ self.app_settings.LDAP_NAME_ATTRIBUTE,
+ self.app_settings.LDAP_MAIL_ATTRIBUTE,
+ ]
+ assert filter == self.app_settings.LDAP_USER_FILTER.format(
+ id_attribute=self.app_settings.LDAP_ID_ATTRIBUTE,
+ mail_attribute=self.app_settings.LDAP_MAIL_ATTRIBUTE,
+ input=self.user,
+ )
+ assert dn == self.app_settings.LDAP_BASE_DN
+ assert scope == ldap.SCOPE_SUBTREE
+
+ return [
+ (
+ "cn={}, {}".format(self.user, self.app_settings.LDAP_BASE_DN),
+ {
+ self.app_settings.LDAP_ID_ATTRIBUTE: [self.user.encode()],
+ self.app_settings.LDAP_NAME_ATTRIBUTE: [self.name.encode()],
+ self.app_settings.LDAP_MAIL_ATTRIBUTE: [self.mail.encode()],
+ },
+ )
+ ]
+
+ def set_option(self, option, invalue):
+ pass
+
+ def unbind_s(self):
+ pass
+
+
+def setup_env(monkeypatch: MonkeyPatch):
+ user = random_string(10)
+ mail = random_string(10)
+ name = random_string(10)
+ password = random_string(10)
+ query_bind = random_string(10)
+ query_password = random_string(10)
+ base_dn = "(dc=example,dc=com)"
+ monkeypatch.setenv("LDAP_AUTH_ENABLED", "true")
+ monkeypatch.setenv("LDAP_SERVER_URL", "") # Not needed due to mocking
+ monkeypatch.setenv("LDAP_BASE_DN", base_dn)
+ monkeypatch.setenv("LDAP_QUERY_BIND", query_bind)
+ monkeypatch.setenv("LDAP_QUERY_PASSWORD", query_password)
+ monkeypatch.setenv("LDAP_USER_FILTER", "(&(objectClass=user)(|({id_attribute}={input})({mail_attribute}={input})))")
+
+ return user, mail, name, password, query_bind, query_password
+
+
def test_create_file_token():
file_path = Path(__file__).parent
file_token = security.create_file_token(file_path)
@@ -17,30 +102,104 @@ def test_create_file_token():
def test_ldap_authentication_mocked(monkeypatch: MonkeyPatch):
- import ldap
+ user, mail, name, password, query_bind, query_password = setup_env(monkeypatch)
+
+ def ldap_initialize_mock(url):
+ assert url == ""
+ return LdapConnMock(user, password, False, query_bind, query_password, mail, name)
+
+ monkeypatch.setattr(ldap, "initialize", ldap_initialize_mock)
+
+ get_app_settings.cache_clear()
+
+ with session_context() as session:
+ result = security.authenticate_user(session, user, password)
+
+ assert result
+ assert result.username == user
+ assert result.email == mail
+ assert result.full_name == name
+ assert result.admin is False
+
+
+def test_ldap_authentication_failed_mocked(monkeypatch: MonkeyPatch):
+ user, mail, name, password, query_bind, query_password = setup_env(monkeypatch)
+
+ def ldap_initialize_mock(url):
+ assert url == ""
+ return LdapConnMock(user, password, False, query_bind, query_password, mail, name)
+
+ monkeypatch.setattr(ldap, "initialize", ldap_initialize_mock)
+
+ get_app_settings.cache_clear()
+
+ with session_context() as session:
+ result = security.authenticate_user(session, user, password + "a")
+
+ assert result is False
+
+
+def test_ldap_authentication_non_admin_mocked(monkeypatch: MonkeyPatch):
+ user, mail, name, password, query_bind, query_password = setup_env(monkeypatch)
+ monkeypatch.setenv("LDAP_ADMIN_FILTER", "(memberOf=cn=admins,dc=example,dc=com)")
+
+ def ldap_initialize_mock(url):
+ assert url == ""
+ return LdapConnMock(user, password, False, query_bind, query_password, mail, name)
+
+ monkeypatch.setattr(ldap, "initialize", ldap_initialize_mock)
+
+ get_app_settings.cache_clear()
+
+ with session_context() as session:
+ result = security.authenticate_user(session, user, password)
+
+ assert result
+ assert result.username == user
+ assert result.email == mail
+ assert result.full_name == name
+ assert result.admin is False
+
+
+def test_ldap_authentication_admin_mocked(monkeypatch: MonkeyPatch):
+ user, mail, name, password, query_bind, query_password = setup_env(monkeypatch)
+ monkeypatch.setenv("LDAP_ADMIN_FILTER", "(memberOf=cn=admins,dc=example,dc=com)")
+
+ def ldap_initialize_mock(url):
+ assert url == ""
+ return LdapConnMock(user, password, True, query_bind, query_password, mail, name)
+
+ monkeypatch.setattr(ldap, "initialize", ldap_initialize_mock)
+
+ get_app_settings.cache_clear()
+
+ with session_context() as session:
+ result = security.authenticate_user(session, user, password)
+
+ assert result
+ assert result.username == user
+ assert result.email == mail
+ assert result.full_name == name
+ assert result.admin
+
+
+def test_ldap_authentication_disabled_mocked(monkeypatch: MonkeyPatch):
+ monkeypatch.setenv("LDAP_AUTH_ENABLED", "False")
user = random_string(10)
password = random_string(10)
- bind_template = "cn={},dc=example,dc=com"
- base_dn = "(dc=example,dc=com)"
- monkeypatch.setenv("LDAP_AUTH_ENABLED", "true")
- monkeypatch.setenv("LDAP_SERVER_URL", "") # Not needed due to mocking
- monkeypatch.setenv("LDAP_BIND_TEMPLATE", bind_template)
- monkeypatch.setenv("LDAP_BASE_DN", base_dn)
class LdapConnMock:
def simple_bind_s(self, dn, bind_pw):
- assert dn == bind_template.format(user)
- return bind_pw == password
+ assert False # When LDAP is disabled, this method should not be called
def search_s(self, dn, scope, filter, attrlist):
- assert attrlist == ["name", "mail"]
- assert filter == f"(&(objectClass=user)(|(cn={user})(sAMAccountName={user})(mail={user})))"
- assert dn == base_dn
- assert scope == ldap.SCOPE_SUBTREE
- return [()]
+ pass
- def set_option(*args, **kwargs):
+ def set_option(self, option, invalue):
+ pass
+
+ def unbind_s(self):
pass
def ldap_initialize_mock(url):
@@ -52,6 +211,4 @@ def test_ldap_authentication_mocked(monkeypatch: MonkeyPatch):
get_app_settings.cache_clear()
with session_context() as session:
- result = security.authenticate_user(session, user, password)
-
- assert result is False
+ security.authenticate_user(session, user, password)