feat: adds descriptions to feature checks and add them to logs (#4504)

mealie/app.py

@@ -64,20 +64,23 @@ async def lifespan_fn(_: FastAPI) -> AsyncGenerator[None, None]:
         settings.model_dump_json(
             indent=4,
             exclude={
-                "LDAP_QUERY_PASSWORD",
-                "OPENAI_API_KEY",
                 "SECRET",
                 "SESSION_SECRET",
-                "SFTP_PASSWORD",
-                "SFTP_USERNAME",
                 "DB_URL",  # replace by DB_URL_PUBLIC for logs
                 "DB_PROVIDER",
-                "SMTP_USER",
-                "SMTP_PASSWORD",
-                "OIDC_CLIENT_SECRET",
             },
         )
     )
+    logger.info("------APP FEATURES------")
+    logger.info("--------==SMTP==--------")
+    logger.info(settings.SMTP_FEATURE)
+    logger.info("--------==LDAP==--------")
+    logger.info(settings.LDAP_FEATURE)
+    logger.info("--------==OIDC==--------")
+    logger.info(settings.OIDC_FEATURE)
+    logger.info("-------==OPENAI==-------")
+    logger.info(settings.OPENAI_FEATURE)
+    logger.info("------------------------")
 
     yield
 

mealie/core/settings/settings.py

@@ -3,10 +3,10 @@ import os
 import secrets
 from datetime import datetime, timezone
 from pathlib import Path
-from typing import Any, NamedTuple
+from typing import Annotated, Any, NamedTuple
 
 from dateutil.tz import tzlocal
-from pydantic import field_validator
+from pydantic import PlainSerializer, field_validator
 from pydantic_settings import BaseSettings, SettingsConfigDict
 
 from mealie.core.settings.themes import Theme
@@ -19,6 +19,29 @@ class ScheduleTime(NamedTuple):
     minute: int
 
 
+class FeatureDetails(NamedTuple):
+    enabled: bool
+    """Indicates if the feature is enabled or not"""
+    description: str | None
+    """Short description describing why the feature is not ready"""
+
+    def __str__(self):
+        s = f"Enabled: {self.enabled}"
+        if not self.enabled and self.description:
+            s += f"\nReason: {self.description}"
+        return s
+
+
+MaskedNoneString = Annotated[
+    str | None,
+    PlainSerializer(lambda x: None if x is None else "*****", return_type=str | None),
+]
+"""
+Custom serializer for sensitive settings. If the setting is None, then will serialize as null, otherwise,
+the secret will be serialized as '*****'
+"""
+
+
 def determine_secrets(data_dir: Path, secret: str, production: bool) -> str:
     if not production:
         return "shh-secret-test-key"
@@ -200,12 +223,16 @@ class AppSettings(AppLoggingSettings):
     SMTP_PORT: str | None = "587"
     SMTP_FROM_NAME: str | None = "Mealie"
     SMTP_FROM_EMAIL: str | None = None
-    SMTP_USER: str | None = None
+    SMTP_USER: MaskedNoneString = None
-    SMTP_PASSWORD: str | None = None
+    SMTP_PASSWORD: MaskedNoneString = None
     SMTP_AUTH_STRATEGY: str | None = "TLS"  # Options: 'TLS', 'SSL', 'NONE'
 
     @property
     def SMTP_ENABLE(self) -> bool:
+        return self.SMTP_FEATURE.enabled
+
+    @property
+    def SMTP_FEATURE(self) -> FeatureDetails:
         return AppSettings.validate_smtp(
             self.SMTP_HOST,
             self.SMTP_PORT,
@@ -225,15 +252,30 @@ class AppSettings(AppLoggingSettings):
         strategy: str | None = None,
         user: str | None = None,
         password: str | None = None,
-    ) -> bool:
+    ) -> FeatureDetails:
         """Validates all SMTP variables are set"""
-        required = {host, port, from_name, from_email, strategy}
+        description = None
+        required = {
+            "SMTP_HOST": host,
+            "SMTP_PORT": port,
+            "SMTP_FROM_NAME": from_name,
+            "SMTP_FROM_EMAIL": from_email,
+            "SMTP_AUTH_STRATEGY": strategy,
+        }
+        missing_values = [key for (key, value) in required.items() if value is None]
+        if missing_values:
+            description = f"Missing required values for {missing_values}"
 
         if strategy and strategy.upper() in {"TLS", "SSL"}:
-            required.add(user)
+            required["SMTP_USER"] = user
-            required.add(password)
+            required["SMTP_PASSWORD"] = password
+            if not description:
+                missing_values = [key for (key, value) in required.items() if value is None]
+                description = f"Missing required values for {missing_values} because SMTP_AUTH_STRATEGY is not None"
 
-        return "" not in required and None not in required
+        not_none = "" not in required.values() and None not in required.values()
+
+        return FeatureDetails(enabled=not_none, description=description)
 
     # ===============================================
     # LDAP Configuration
@@ -245,31 +287,43 @@ class AppSettings(AppLoggingSettings):
     LDAP_ENABLE_STARTTLS: bool = False
     LDAP_BASE_DN: str | None = None
     LDAP_QUERY_BIND: str | None = None
-    LDAP_QUERY_PASSWORD: str | None = None
+    LDAP_QUERY_PASSWORD: MaskedNoneString = None
     LDAP_USER_FILTER: str | None = None
     LDAP_ADMIN_FILTER: str | None = None
     LDAP_ID_ATTRIBUTE: str = "uid"
     LDAP_MAIL_ATTRIBUTE: str = "mail"
     LDAP_NAME_ATTRIBUTE: str = "name"
 
+    @property
+    def LDAP_FEATURE(self) -> FeatureDetails:
+        description = None if self.LDAP_AUTH_ENABLED else "LDAP_AUTH_ENABLED is false"
+        required = {
+            "LDAP_SERVER_URL": self.LDAP_SERVER_URL,
+            "LDAP_BASE_DN": self.LDAP_BASE_DN,
+            "LDAP_ID_ATTRIBUTE": self.LDAP_ID_ATTRIBUTE,
+            "LDAP_MAIL_ATTRIBUTE": self.LDAP_MAIL_ATTRIBUTE,
+            "LDAP_NAME_ATTRIBUTE": self.LDAP_NAME_ATTRIBUTE,
+        }
+        not_none = None not in required.values()
+        if not not_none and not description:
+            missing_values = [key for (key, value) in required.items() if value is None]
+            description = f"Missing required values for {missing_values}"
+
+        return FeatureDetails(
+            enabled=self.LDAP_AUTH_ENABLED and not_none,
+            description=description,
+        )
+
     @property
     def LDAP_ENABLED(self) -> bool:
         """Validates LDAP settings are all set"""
-        required = {
+        return self.LDAP_FEATURE.enabled
-            self.LDAP_SERVER_URL,
-            self.LDAP_BASE_DN,
-            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
 
     # ===============================================
     # OIDC Configuration
     OIDC_AUTH_ENABLED: bool = False
     OIDC_CLIENT_ID: str | None = None
-    OIDC_CLIENT_SECRET: str | None = None
+    OIDC_CLIENT_SECRET: MaskedNoneString = None
     OIDC_CONFIGURATION_URL: str | None = None
     OIDC_SIGNUP_ENABLED: bool = True
     OIDC_USER_GROUP: str | None = None
@@ -286,29 +340,41 @@ class AppSettings(AppLoggingSettings):
         return self.OIDC_USER_GROUP is not None or self.OIDC_ADMIN_GROUP is not None
 
     @property
-    def OIDC_READY(self) -> bool:
+    def OIDC_FEATURE(self) -> FeatureDetails:
-        """Validates OIDC settings are all set"""
+        description = None if self.OIDC_AUTH_ENABLED else "OIDC_AUTH_ENABLED is false"
-
         required = {
-            self.OIDC_CLIENT_ID,
+            "OIDC_CLIENT_ID": self.OIDC_CLIENT_ID,
-            self.OIDC_CLIENT_SECRET,
+            "OIDC_CLIENT_SECRET": self.OIDC_CLIENT_SECRET,
-            self.OIDC_CONFIGURATION_URL,
+            "OIDC_CONFIGURATION_URL": self.OIDC_CONFIGURATION_URL,
-            self.OIDC_USER_CLAIM,
+            "OIDC_USER_CLAIM": self.OIDC_USER_CLAIM,
         }
-        not_none = None not in required
+        not_none = None not in required.values()
-        valid_group_claim = True
+        if not not_none and not description:
+            missing_values = [key for (key, value) in required.items() if value is None]
+            description = f"Missing required values for {missing_values}"
 
+        valid_group_claim = True
         if self.OIDC_REQUIRES_GROUP_CLAIM and self.OIDC_GROUPS_CLAIM is None:
+            if not description:
+                description = "OIDC_GROUPS_CLAIM is required when OIDC_USER_GROUP or OIDC_ADMIN_GROUP are provided"
             valid_group_claim = False
 
-        return self.OIDC_AUTH_ENABLED and not_none and valid_group_claim
+        return FeatureDetails(
+            enabled=self.OIDC_AUTH_ENABLED and not_none and valid_group_claim,
+            description=description,
+        )
+
+    @property
+    def OIDC_READY(self) -> bool:
+        """Validates OIDC settings are all set"""
+        return self.OIDC_FEATURE.enabled
 
     # ===============================================
     # OpenAI Configuration
 
     OPENAI_BASE_URL: str | None = None
     """The base URL for the OpenAI API. Leave this unset for most usecases"""
-    OPENAI_API_KEY: str | None = None
+    OPENAI_API_KEY: MaskedNoneString = None
     """Your OpenAI API key. Required to enable OpenAI features"""
     OPENAI_MODEL: str = "gpt-4o"
     """Which OpenAI model to send requests to. Leave this unset for most usecases"""
@@ -333,6 +399,24 @@ class AppSettings(AppLoggingSettings):
     The number of seconds to wait for an OpenAI request to complete before cancelling the request
     """
 
+    @property
+    def OPENAI_FEATURE(self) -> FeatureDetails:
+        description = None
+        if not self.OPENAI_API_KEY:
+            description = "OPENAI_API_KEY is not set"
+        elif self.OPENAI_MODEL:
+            description = "OPENAI_MODEL is not set"
+
+        return FeatureDetails(
+            enabled=bool(self.OPENAI_API_KEY and self.OPENAI_MODEL),
+            description=description,
+        )
+
+    @property
+    def OPENAI_ENABLED(self) -> bool:
+        """Validates OpenAI settings are all set"""
+        return self.OPENAI_FEATURE.enabled
+
     # ===============================================
     # Web Concurrency
 
@@ -346,11 +430,6 @@ class AppSettings(AppLoggingSettings):
     def WORKERS(self) -> int:
         return max(1, self.WORKER_PER_CORE * self.UVICORN_WORKERS)
 
-    @property
-    def OPENAI_ENABLED(self) -> bool:
-        """Validates OpenAI settings are all set"""
-        return bool(self.OPENAI_API_KEY and self.OPENAI_MODEL)
-
     model_config = SettingsConfigDict(arbitrary_types_allowed=True, extra="allow")
 
     # ===============================================

tests/unit_tests/test_config.py

@@ -1,3 +1,4 @@
+import json
 import re
 from dataclasses import dataclass
 
@@ -126,13 +127,27 @@ smtp_validation_cases = [
     (
         "good_data_tls",
         SMTPValidationCase(
-            "email.mealie.io", "587", "tls", "Mealie", "mealie@mealie.io", "mealie@mealie.io", "mealie-password", True
+            "email.mealie.io",
+            "587",
+            "tls",
+            "Mealie",
+            "mealie@mealie.io",
+            "mealie@mealie.io",
+            "mealie-password",
+            True,
         ),
     ),
     (
         "good_data_ssl",
         SMTPValidationCase(
-            "email.mealie.io", "465", "tls", "Mealie", "mealie@mealie.io", "mealie@mealie.io", "mealie-password", True
+            "email.mealie.io",
+            "465",
+            "tls",
+            "Mealie",
+            "mealie@mealie.io",
+            "mealie@mealie.io",
+            "mealie-password",
+            True,
         ),
     ),
 ]
@@ -151,6 +166,149 @@ def test_smtp_enable_with_bad_data_tls(data: SMTPValidationCase):
         data.auth_strategy,
         data.user,
         data.password,
-    )
+    ).enabled
 
     assert is_valid is data.is_valid
+
+
+@dataclass(slots=True)
+class EnvVar:
+    name: str
+    value: any
+
+
+class LDAPValidationCase:
+    settings = list[EnvVar]
+    is_valid: bool
+
+    def __init__(
+        self,
+        enabled: bool,
+        server_url: str | None,
+        base_dn: str | None,
+        is_valid: bool,
+    ):
+        self.settings = [
+            EnvVar("LDAP_AUTH_ENABLED", enabled),
+            EnvVar("LDAP_SERVER_URL", server_url),
+            EnvVar("LDAP_BASE_DN", base_dn),
+        ]
+        self.is_valid = is_valid
+
+
+ldap_validation_cases = [
+    ("not enabled", LDAPValidationCase(False, None, None, False)),
+    ("missing url", LDAPValidationCase(True, None, "dn", False)),
+    ("missing base dn", LDAPValidationCase(True, "url", None, False)),
+    ("all good", LDAPValidationCase(True, "url", "dn", True)),
+]
+
+ldap_cases = [x[1] for x in ldap_validation_cases]
+ldap_cases_ids = [x[0] for x in ldap_validation_cases]
+
+
+@pytest.mark.parametrize("data", ldap_cases, ids=ldap_cases_ids)
+def test_ldap_settings_validation(data: LDAPValidationCase, monkeypatch: pytest.MonkeyPatch):
+    for setting in data.settings:
+        if setting.value is not None:
+            monkeypatch.setenv(setting.name, setting.value)
+        else:
+            monkeypatch.delenv(setting.name, raising=False)
+
+    get_app_settings.cache_clear()
+    app_settings = get_app_settings()
+
+    assert app_settings.LDAP_ENABLED is data.is_valid
+
+
+class OIDCValidationCase:
+    settings = list[EnvVar]
+    is_valid: bool
+
+    def __init__(
+        self,
+        enabled: bool,
+        client_id: str | None,
+        client_secret: str | None,
+        configuration_url: str | None,
+        groups_claim: str | None,
+        user_group: str | None,
+        admin_group: str | None,
+        is_valid: bool,
+    ):
+        self.settings = [
+            EnvVar("OIDC_AUTH_ENABLED", enabled),
+            EnvVar("OIDC_CLIENT_ID", client_id),
+            EnvVar("OIDC_CLIENT_SECRET", client_secret),
+            EnvVar("OIDC_CONFIGURATION_URL", configuration_url),
+            EnvVar("OIDC_GROUPS_CLAIM", groups_claim),
+            EnvVar("OIDC_USER_GROUP", user_group),
+            EnvVar("OIDC_ADMIN_GROUP", admin_group),
+        ]
+        self.is_valid = is_valid
+
+
+oidc_validation_cases = [
+    (
+        "not enabled",
+        OIDCValidationCase(False, None, None, None, None, None, None, False),
+    ),
+    (
+        "missing client id",
+        OIDCValidationCase(True, None, "secret", "url", "groups", "user", "admin", False),
+    ),
+    (
+        "missing client secret",
+        OIDCValidationCase(True, "id", None, "url", "groups", "user", "admin", False),
+    ),
+    (
+        "missing url",
+        OIDCValidationCase(True, "id", "secret", None, "groups", "user", "admin", False),
+    ),
+    (
+        "all good no groups",
+        OIDCValidationCase(True, "id", "secret", "url", None, None, None, True),
+    ),
+    (
+        "all good with groups",
+        OIDCValidationCase(True, "id", "secret", "url", "groups", "user", "admin", True),
+    ),
+]
+
+oidc_cases = [x[1] for x in oidc_validation_cases]
+oidc_cases_ids = [x[0] for x in oidc_validation_cases]
+
+
+@pytest.mark.parametrize("data", oidc_cases, ids=oidc_cases_ids)
+def test_oidc_settings_validation(data: OIDCValidationCase, monkeypatch: pytest.MonkeyPatch):
+    for setting in data.settings:
+        if setting.value is not None:
+            monkeypatch.setenv(setting.name, setting.value)
+        else:
+            monkeypatch.delenv(setting.name, raising=False)
+
+    get_app_settings.cache_clear()
+    app_settings = get_app_settings()
+
+    assert app_settings.OIDC_READY is data.is_valid
+
+
+def test_sensitive_settings_mask(monkeypatch: pytest.MonkeyPatch):
+    sensitive_settings = [
+        "LDAP_QUERY_PASSWORD",
+        "OPENAI_API_KEY",
+        "SMTP_USER",
+        "SMTP_PASSWORD",
+        "OIDC_CLIENT_SECRET",
+    ]
+    for setting in sensitive_settings:
+        monkeypatch.setenv(setting, "super_secret")
+
+    get_app_settings.cache_clear()
+    app_settings = get_app_settings()
+    settings = app_settings.model_dump()
+    settings_json = json.loads(app_settings.model_dump_json())
+
+    for setting in sensitive_settings:
+        assert settings[setting] == "*****"
+        assert settings_json[setting] == "*****"