Mirrors/mealie
1
0
Fork 0
mirror of https://github.com/mealie-recipes/mealie.git synced 2026-07-19 14:10:15 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix: harden LDAP and OIDC authentication providers (#7902)

Browse Source
This commit is contained in:
Hayden
2026-07-18 12:27:10 -05:00
committed by GitHub
parent f287322707
commit 280cd58b9d
7 changed files with 193 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
docs/docs/documentation/getting-started/installation/backend-config.md
View File

@@ -101,6 +101,7 @@ For usage, see [Usage - OpenID Connect](../authentication/oidc-v2.md)
| ----------------------------------------------------------------------------------- | :-----: | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| OIDC_AUTH_ENABLED | False | Enables authentication via OpenID Connect |
| OIDC_SIGNUP_ENABLED | True | Enables new users to be created when signing in for the first time with OIDC |
| OIDC_REQUIRES_EMAIL_VERIFICATION | True | Requires the `email_verified` claim to be true before a user can sign in. This prevents an unverified email from being used to match an existing account. Only disable this if your identity provider does not emit the `email_verified` claim. |
| OIDC_CONFIGURATION_URL<super>[&dagger;][secrets]</super> | None | The URL to the OIDC configuration of your provider. This is usually something like https://auth.example.com/.well-known/openid-configuration |
| OIDC_CLIENT_ID<super>[&dagger;][secrets]</super> | None | The client id of your configured client in your provider |
| OIDC_CLIENT_SECRET<super>[&dagger;][secrets]</super> <br/> :octicons-tag-24: v2.0.0 | None | The client secret of your configured client in your provider |

16
mealie/core/security/providers/ldap_provider.py
View File

@@ -1,6 +1,7 @@
from datetime import timedelta
import ldap
import ldap.filter
from ldap.ldapobject import LDAPObject
from sqlalchemy.orm.session import Session
@@ -45,19 +46,23 @@ class LDAPProvider(CredentialsProvider):
return None
settings = get_app_settings()
# Escape the user-supplied username so filter metacharacters (*, (, ), \, NUL)
# are treated as literal data rather than LDAP filter syntax.
escaped_username = ldap.filter.escape_filter_chars(self.data.username)
user_filter = ""
if settings.LDAP_USER_FILTER:
# fill in the template provided by the user to maintain backwards compatibility
user_filter = settings.LDAP_USER_FILTER.format(
id_attribute=settings.LDAP_ID_ATTRIBUTE,
mail_attribute=settings.LDAP_MAIL_ATTRIBUTE,
input=self.data.username,
input=escaped_username,
)
# Don't assume the provided search filter has (|({id_attribute}={input})({mail_attribute}={input}))
search_filter = "(&(|({id_attribute}={input})({mail_attribute}={input})){filter})".format(
id_attribute=settings.LDAP_ID_ATTRIBUTE,
mail_attribute=settings.LDAP_MAIL_ATTRIBUTE,
input=self.data.username,
input=escaped_username,
filter=user_filter,
)
@@ -108,6 +113,13 @@ class LDAPProvider(CredentialsProvider):
return None
data = self.data
# Reject empty passwords before binding. Many directories treat a bind with
# a valid DN and an empty password as an anonymous/unauthenticated bind that
# succeeds, which would otherwise let anyone log in as a known user.
if not data.password:
self._logger.error("[LDAP] Empty password is not permitted; refusing to bind")
return None
if settings.LDAP_TLS_INSECURE:
ldap.set_option(ldap.OPT_X_TLS_REQUIRE_CERT, ldap.OPT_X_TLS_NEVER)

16
mealie/core/security/providers/openid_provider.py
View File

@@ -48,6 +48,15 @@ class OpenIDProvider(AuthProvider[UserInfo]):
self._logger.debug("[OIDC] Required claim '%s' is empty", claim)
raise MissingClaimException()
# Never trust an unverified email. An IdP that lets a user self-assert an
# arbitrary, unverified address would otherwise allow that user to match
# (and log into) another account by claiming its email. When the email is
# verified, matching an existing account is legitimate account linking.
# Admins whose IdP does not emit the claim can opt out via this setting.
if settings.OIDC_REQUIRES_EMAIL_VERIFICATION and not claims.get("email_verified", False):
self._logger.warning("[OIDC] email_verified claim is missing or false; refusing to authenticate")
raise MissingClaimException()
repos = get_repositories(self.session, group_id=None, household_id=None)
is_admin = False
@@ -106,16 +115,15 @@ class OpenIDProvider(AuthProvider[UserInfo]):
return self.get_access_token(user, settings.OIDC_REMEMBER_ME) # type: ignore
if user:
# A matched account is adopted here (including local/LDAP accounts). This is
# safe because the email that produced the match was verified by the IdP
# above, unless the admin has explicitly disabled that requirement.
if settings.OIDC_ADMIN_GROUP and user.admin != is_admin:
self._logger.debug("[OIDC] %s user as admin", "Setting" if is_admin else "Removing")
user.admin = is_admin
repos.users.update(user.id, user)
return self.get_access_token(user, settings.OIDC_REMEMBER_ME)
self._logger.warning("[OIDC] Found user but their AuthMethod does not match OIDC")
return None
@property
def required_claims(self):
settings = get_app_settings()

1
mealie/core/settings/settings.py
View File

@@ -368,6 +368,7 @@ class AppSettings(AppLoggingSettings):
OIDC_CLIENT_SECRET: MaskedNoneString = None
OIDC_CONFIGURATION_URL: str | None = None
OIDC_SIGNUP_ENABLED: bool = True
OIDC_REQUIRES_EMAIL_VERIFICATION: bool = True
OIDC_USER_GROUP: str | None = None
OIDC_ADMIN_GROUP: str | None = None
OIDC_AUTO_REDIRECT: bool = False

5
tests/e2e/login.spec.ts
View File

@@ -51,6 +51,7 @@ test('oidc initial login', async ({ page }) => {
const claims = {
"sub": username,
"email": `${username}@example.com`,
"email_verified": true,
"preferred_username": username,
"name": name,
"groups": ["user"]
@@ -73,6 +74,7 @@ test('oidc login with user not in propery group', async ({ page }) => {
const claims = {
"sub": username,
"email": `${username}@example.com`,
"email_verified": true,
"preferred_username": username,
"name": name,
"groups": []
@@ -93,6 +95,7 @@ test('oidc sequential login', async ({ page }) => {
const claims = {
"sub": username,
"email": `${username}@example.com`,
"email_verified": true,
"preferred_username": username,
"name": name,
"groups": ["user"]
@@ -121,6 +124,7 @@ test('settings page verify oidc', async ({ page }) => {
const claims = {
"sub": username,
"email": `${username}@example.com`,
"email_verified": true,
"preferred_username": username,
"name": name,
"groups": ["user"]
@@ -156,6 +160,7 @@ test('oidc admin user', async ({ page }) => {
const claims = {
"sub": username,
"email": `${username}@example.com`,
"email_verified": true,
"preferred_username": username,
"name": name,
"groups": ["user", "admin"]

93
tests/unit_tests/core/security/providers/test_openid_provider.py
View File

@@ -53,6 +53,7 @@ def test_missing_groups_claim(monkeypatch: MonkeyPatch):
data = {
"preferred_username": "dude1",
"email": "email@email.com",
"email_verified": True,
"name": "Firstname Lastname",
}
auth_provider = OpenIDProvider(None, data)
@@ -68,6 +69,7 @@ def test_missing_groups_claim_admin(monkeypatch: MonkeyPatch):
data = {
"preferred_username": "dude1",
"email": "email@email.com",
"email_verified": True,
"name": "Firstname Lastname",
}
auth_provider = OpenIDProvider(None, data)
@@ -83,6 +85,7 @@ def test_missing_groups_claim_with_default(monkeypatch: MonkeyPatch):
data = {
"preferred_username": "dude1",
"email": "email@email.com",
"email_verified": True,
"name": "Firstname Lastname",
}
auth_provider = OpenIDProvider(None, data, True)
@@ -97,6 +100,7 @@ def test_missing_groups_claim_admin_group_with_default(monkeypatch: MonkeyPatch,
data = {
"preferred_username": "dude1",
"email": unique_user.email,
"email_verified": True,
"name": "Firstname Lastname",
}
auth_provider = OpenIDProvider(unique_user.repos.session, data, True)
@@ -111,6 +115,7 @@ def test_missing_user_group(monkeypatch: MonkeyPatch):
data = {
"preferred_username": "dude1",
"email": "email@email.com",
"email_verified": True,
"name": "Firstname Lastname",
"groups": ["not_mealie_user"],
}
@@ -126,6 +131,7 @@ def test_has_user_group_existing_user(monkeypatch: MonkeyPatch, unique_user: Tes
data = {
"preferred_username": "dude1",
"email": unique_user.email,
"email_verified": True,
"name": "Firstname Lastname",
"groups": ["mealie_user"],
}
@@ -142,6 +148,7 @@ def test_has_admin_group_existing_user(monkeypatch: MonkeyPatch, unique_user: Te
data = {
"preferred_username": "dude1",
"email": unique_user.email,
"email_verified": True,
"name": "Firstname Lastname",
"groups": ["mealie_admin"],
}
@@ -158,6 +165,7 @@ def test_has_user_group_new_user(monkeypatch: MonkeyPatch, session: Session):
data = {
"preferred_username": "dude1",
"email": "dude1@email.com",
"email_verified": True,
"name": "Firstname Lastname",
"groups": ["mealie_user"],
}
@@ -179,6 +187,7 @@ def test_has_admin_group_new_user(monkeypatch: MonkeyPatch, session: Session):
data = {
"preferred_username": "dude2",
"email": "dude2@email.com",
"email_verified": True,
"name": "Firstname Lastname",
"groups": ["mealie_admin"],
}
@@ -208,6 +217,7 @@ def test_ldap_user_creation_invalid_group_or_household(
data = {
"preferred_username": random_string(),
"email": random_email(),
"email_verified": True,
"name": random_string(),
"groups": ["mealie_user"],
}
@@ -227,11 +237,92 @@ def test_ldap_user_creation_invalid_group_or_household(
assert user is None
def test_claims_logging(caplog, session: Session):
def test_rejects_unverified_email(monkeypatch: MonkeyPatch, session: Session):
monkeypatch.setenv("OIDC_REQUIRES_EMAIL_VERIFICATION", "true")
get_app_settings.cache_clear()
data = {
"preferred_username": random_string(),
"email": random_email(),
"email_verified": False,
"name": random_string(),
}
auth_provider = OpenIDProvider(session, data)
with pytest.raises(MissingClaimException):
auth_provider.authenticate()
def test_rejects_missing_email_verified(monkeypatch: MonkeyPatch, session: Session):
monkeypatch.setenv("OIDC_REQUIRES_EMAIL_VERIFICATION", "true")
get_app_settings.cache_clear()
data = {
"preferred_username": random_string(),
"email": random_email(),
"name": random_string(),
}
auth_provider = OpenIDProvider(session, data)
with pytest.raises(MissingClaimException):
auth_provider.authenticate()
def test_does_not_adopt_existing_account_with_unverified_email(monkeypatch: MonkeyPatch, unique_user: TestUser):
"""An unverified email must not be able to log into (take over) an existing account."""
monkeypatch.setenv("OIDC_REQUIRES_EMAIL_VERIFICATION", "true")
get_app_settings.cache_clear()
data = {
"preferred_username": random_string(),
"email": unique_user.email,
"email_verified": False,
"name": random_string(),
}
auth_provider = OpenIDProvider(unique_user.repos.session, data)
with pytest.raises(MissingClaimException):
auth_provider.authenticate()
def test_adopts_existing_account_with_verified_email(monkeypatch: MonkeyPatch, unique_user: TestUser):
"""A verified email legitimately links to an existing (non-OIDC) account."""
monkeypatch.setenv("OIDC_REQUIRES_EMAIL_VERIFICATION", "true")
get_app_settings.cache_clear()
data = {
"preferred_username": random_string(),
"email": unique_user.email,
"email_verified": True,
"name": random_string(),
}
auth_provider = OpenIDProvider(unique_user.repos.session, data)
assert auth_provider.authenticate() is not None
def test_allows_unverified_email_when_verification_disabled(monkeypatch: MonkeyPatch, session: Session):
monkeypatch.setenv("OIDC_REQUIRES_EMAIL_VERIFICATION", "false")
get_app_settings.cache_clear()
data = {
"preferred_username": random_string(),
"email": random_email(),
"name": random_string(),
}
auth_provider = OpenIDProvider(session, data)
assert auth_provider.authenticate() is not None
def test_claims_logging(monkeypatch: MonkeyPatch, caplog, session: Session):
monkeypatch.setenv("OIDC_REQUIRES_EMAIL_VERIFICATION", "true")
get_app_settings.cache_clear()
caplog.set_level(logging.DEBUG)
data = {
"preferred_username": "testuser",
"email": "test@example.com",
"email_verified": True,
"name": "Test User",
"groups": ["mealie_user"],
}

63
tests/unit_tests/test_security.py
View File

@@ -344,3 +344,66 @@ def test_user_login_ldap_auth_method(monkeypatch: MonkeyPatch, ldap_user: Privat
assert result.full_name == ldap_user.full_name
assert result.admin == ldap_user.admin
assert result.auth_method == AuthMethod.LDAP
class PermissiveBindConnMock(LdapConnMock):
"""Simulates a directory that accepts a bind with a valid DN and an empty
password (anonymous/unauthenticated bind) as success."""
def simple_bind_s(self, dn, bind_pw):
if bind_pw == "":
return
return super().simple_bind_s(dn, bind_pw)
class CapturingConnMock(LdapConnMock):
"""Records the search filter passed to search_s and returns no matches."""
captured_filter: str | None = None
def search_s(self, dn, scope, filter, attrlist):
if filter != self.app_settings.LDAP_ADMIN_FILTER:
CapturingConnMock.captured_filter = filter
return []
def test_ldap_rejects_empty_password(monkeypatch: MonkeyPatch):
user, mail, name, _, query_bind, query_password = setup_env(monkeypatch)
def ldap_initialize_mock(url):
# Even against a directory that would accept the empty-password bind,
# the provider must refuse before binding.
return PermissiveBindConnMock(user, "", False, query_bind, query_password, mail, name)
monkeypatch.setattr(ldap, "initialize", ldap_initialize_mock)
get_app_settings.cache_clear()
with session_context() as session:
provider = get_provider(session, user, "")
result = provider.get_user()
assert result is None
def test_ldap_escapes_username_in_search_filter(monkeypatch: MonkeyPatch):
_, mail, name, password, query_bind, query_password = setup_env(monkeypatch)
injected_username = "a*)(uid=*"
CapturingConnMock.captured_filter = None
def ldap_initialize_mock(url):
return CapturingConnMock(injected_username, 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:
provider = get_provider(session, injected_username, password)
provider.get_user()
captured = CapturingConnMock.captured_filter
assert captured is not None
# Metacharacters from the username must be escaped, never present as raw syntax.
assert "\\2a" in captured # '*' -> \2a
assert "*" not in captured