[FEAT] improve update_admin_settings (#1903)

* [FEAT] improve update_admin_settings

while working on the push notification pr i found it very hard just to update
2 settings inside the db, so i improved upon update_admin_settings.
now you just need to provide a dict with key/values you want to update inside db.

also debugging the endpoints for update_settings i found despite the type of `EditableSettings`
fastapi did in fact pass a dict.

* t

* use `EditableSettings` as param in update_settings

* fix settings model validation

we previously overrode the pydantic validation with our own method

* make `LnbitsSettings` a `BaseModel` and only add `BaseSettings` later

this allows us to instantiate `EditableSettings` without the environment values being loaded in

* add test

* forbid extra fields in update api

* fixup

* add test

* test datadir

* move UpdateSettings

* fix compat

* fixup webpush

---------

Co-authored-by: jacksn <jkranawetter05@gmail.com>

lnbits/core/crud.py

@@ -12,6 +12,7 @@ from lnbits.db import Connection, Database, Filters, Page
 from lnbits.extension_manager import InstallableExtension
 from lnbits.settings import (
     AdminSettings,
+    EditableSettings,
     SuperSettings,
     WebPushSettings,
     settings,
@@ -797,17 +798,14 @@ async def get_admin_settings(is_super_user: bool = False) -> Optional[AdminSetti
     return admin_settings
 
 
-async def delete_admin_settings():
+async def delete_admin_settings() -> None:
     await db.execute("DELETE FROM settings")
 
 
-async def update_admin_settings(data: dict):
+async def update_admin_settings(data: EditableSettings) -> None:
     row = await db.fetchone("SELECT editable_settings FROM settings")
-    if not row:
-        return None
-    editable_settings = json.loads(row["editable_settings"])
-    for key, value in data.items():
-        editable_settings[key] = value
+    editable_settings = json.loads(row["editable_settings"]) if row else {}
+    editable_settings.update(data.dict(exclude_unset=True))
     await db.execute(
         "UPDATE settings SET editable_settings = ?", (json.dumps(editable_settings),)
     )

lnbits/core/services.py

@@ -573,7 +573,7 @@ async def check_webpush_settings():
             "lnbits_webpush_pubkey": pubkey,
         }
         update_cached_settings(push_settings)
-        await update_admin_settings(push_settings)
+        await update_admin_settings(EditableSettings(**push_settings))
 
     logger.info("Initialized webpush settings with generated VAPID key pair.")
     logger.info(f"Pubkey: {settings.lnbits_webpush_pubkey}")

lnbits/core/views/admin_api.py

@@ -19,7 +19,7 @@ from lnbits.core.services import (
 )
 from lnbits.decorators import check_admin, check_super_user
 from lnbits.server import server_restart
-from lnbits.settings import AdminSettings, settings
+from lnbits.settings import AdminSettings, UpdateSettings, settings
 
 from .. import core_app, core_app_extra
 from ..crud import delete_admin_settings, get_admin_settings, update_admin_settings
@@ -58,7 +58,7 @@ async def api_get_settings(
     "/admin/api/v1/settings/",
     status_code=HTTPStatus.OK,
 )
-async def api_update_settings(data: dict, user: User = Depends(check_admin)):
+async def api_update_settings(data: UpdateSettings, user: User = Depends(check_admin)):
     await update_admin_settings(data)
     admin_settings = await get_admin_settings(user.super_user)
     assert admin_settings, "Updated admin settings not found."

lnbits/settings.py

@@ -9,7 +9,7 @@ from typing import Any, List, Optional
 
 import httpx
 from loguru import logger
-from pydantic import BaseSettings, Extra, Field, validator
+from pydantic import BaseModel, BaseSettings, Extra, Field, validator
 
 
 def list_parse_fallback(v: str):
@@ -23,20 +23,13 @@ def list_parse_fallback(v: str):
         return []
 
 
-class LNbitsSettings(BaseSettings):
+class LNbitsSettings(BaseModel):
     @classmethod
-    def validate(cls, val):
+    def validate_list(cls, val):
         if isinstance(val, str):
             val = val.split(",") if val else []
         return val
 
-    class Config:
-        env_file = ".env"
-        env_file_encoding = "utf-8"
-        case_sensitive = False
-        json_loads = list_parse_fallback
-        extra = Extra.ignore
-
 
 class UsersSettings(LNbitsSettings):
     lnbits_admin_users: List[str] = Field(default=[])
@@ -253,7 +246,7 @@ class EditableSettings(
     )
     @classmethod
     def validate_editable_settings(cls, val):
-        return super().validate(val)
+        return super().validate_list(val)
 
     @classmethod
     def from_dict(cls, d: dict):
@@ -269,6 +262,11 @@ class EditableSettings(
                 prop.pop("env_names", None)
 
 
+class UpdateSettings(EditableSettings):
+    class Config:
+        extra = Extra.forbid
+
+
 class EnvSettings(LNbitsSettings):
     debug: bool = Field(default=False)
     bundle_assets: bool = Field(default=True)
@@ -338,19 +336,25 @@ class ReadOnlySettings(
     )
     @classmethod
     def validate_readonly_settings(cls, val):
-        return super().validate(val)
+        return super().validate_list(val)
 
     @classmethod
     def readonly_fields(cls):
         return [f for f in inspect.signature(cls).parameters if not f.startswith("_")]
 
 
-class Settings(EditableSettings, ReadOnlySettings, TransientSettings):
+class Settings(EditableSettings, ReadOnlySettings, TransientSettings, BaseSettings):
     @classmethod
     def from_row(cls, row: Row) -> "Settings":
         data = dict(row)
         return cls(**data)
 
+    class Config:
+        env_file = ".env"
+        env_file_encoding = "utf-8"
+        case_sensitive = False
+        json_loads = list_parse_fallback
+
 
 class SuperSettings(EditableSettings):
     super_user: str

tests/conftest.py

@@ -10,7 +10,7 @@ from fastapi.testclient import TestClient
 from httpx import AsyncClient
 
 from lnbits.app import create_app
-from lnbits.core.crud import create_account, create_wallet
+from lnbits.core.crud import create_account, create_wallet, get_user
 from lnbits.core.models import CreateInvoice
 from lnbits.core.services import update_wallet_balance
 from lnbits.core.views.api import api_payments_create_invoice
@@ -18,7 +18,10 @@ from lnbits.db import Database
 from lnbits.settings import settings
 from tests.helpers import get_hold_invoice, get_random_invoice_data, get_real_invoice
 
-# dont install extensions for tests
+# override settings for tests
+settings.lnbits_admin_extensions = []
+settings.lnbits_data_folder = "./tests/data"
+settings.lnbits_admin_ui = True
 settings.lnbits_extensions_default_install = []
 
 
@@ -86,6 +89,12 @@ async def to_user():
     yield user
 
 
+@pytest_asyncio.fixture(scope="session")
+async def superuser():
+    user = await get_user(settings.super_user)
+    yield user
+
+
 @pytest_asyncio.fixture(scope="session")
 async def to_wallet(to_user):
     user = to_user

tests/core/views/test_admin_api.py

@@ -0,0 +1,40 @@
+import pytest
+
+from lnbits.settings import settings
+
+
+@pytest.mark.asyncio
+async def test_admin_get_settings_permission_denied(client, from_user):
+    response = await client.get(f"/admin/api/v1/settings/?usr={from_user.id}")
+    assert response.status_code == 401
+
+
+@pytest.mark.asyncio
+async def test_admin_get_settings(client, superuser):
+    response = await client.get(f"/admin/api/v1/settings/?usr={superuser.id}")
+    assert response.status_code == 200
+    result = response.json()
+    assert "super_user" not in result
+
+
+@pytest.mark.asyncio
+async def test_admin_update_settings(client, superuser):
+    new_site_title = "UPDATED SITETITLE"
+    response = await client.put(
+        f"/admin/api/v1/settings/?usr={superuser.id}",
+        json={"lnbits_site_title": new_site_title},
+    )
+    assert response.status_code == 200
+    result = response.json()
+    assert "status" in result
+    assert result.get("status") == "Success"
+    assert settings.lnbits_site_title == new_site_title
+
+
+@pytest.mark.asyncio
+async def test_admin_update_noneditable_settings(client, superuser):
+    response = await client.put(
+        f"/admin/api/v1/settings/?usr={superuser.id}",
+        json={"super_user": "UPDATED"},
+    )
+    assert response.status_code == 400