An instance that can be somebody else's
A name, a tagline, a logo, a favicon and the launcher icons derived from it; the Middle-earth strings as data; themes as token sets; and a stylesheet for what none of that reaches. All four are on one page, in one settings group. The snapshot is a Jinja global over a process-level cache, because render() has no session and four render paths never reach it at all -- the sign-in page, the error pages, the offline page and the SSE fragments. A context value would have had to be threaded through every one and would still have missed those. It being a global is also what lets mark() branch on an uploaded logo without any of its six call sites learning about branding; the macro that renders the sidebar link is called brandlink now, because a macro imported as `brand` shadows the global for the whole template and took out every page at once. Defaults in code and overrides in the database, as the prompt fragments do, with one difference stated in the module: an empty fragment means off, an empty flavour string means the shipped wording. And blanked rather than dropped -- settings_store.update merges, so an omitted key leaves what was stored last time and "I typed the default back in" would store something different from "I changed nothing". A custom theme sets a handful of tokens and inherits the rest, and the inheritance is a CSS fact: tokens.css matches [data-base="shire"] as well as [data-theme="shire"], so a custom light theme lands on parchment rather than four light colours on near-black. Values are validated on read rather than on save, because a theme written straight into the settings table still has to produce a stylesheet that parses -- a `}` in a value ends the rule and silently breaks every rule after it. The soft variants are derived from the accent, or a changed accent leaves focus rings in the old hue and reads as half-working. /branding.css is a route, not an inline block: an external stylesheet has no HTML context to escape from. The link carries a content hash, so a save is not left to the browser's cache, and it is deliberately outside the service worker's precache list, which is versioned by the release. The instance name moved off /admin/general rather than being duplicated there. An upgrade keeps it: the general row is read as a seed exactly while the branding row has never mentioned the name, which is `key in row` and not `row[key] is truthy` -- the two read alike would resurrect the old name underneath a cleared one. The theme list stops being a hard-coded pair in five places. Every failure mode in that area is silent, so it is driven under a DOM stub as well as tested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jaroslav Beneš
@@ -0,0 +1,376 @@
|
||||
"""Making an instance somebody else's.
|
||||
|
||||
Three things are worth pinning here and the rest follows from them: that a
|
||||
default is never stored, that the snapshot is dropped when it changes, and that
|
||||
a colour reaching a stylesheet is a colour and not whatever was typed.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from lembas.services import branding, settings_store
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def clean(db):
|
||||
branding.forget()
|
||||
yield
|
||||
branding.forget()
|
||||
|
||||
|
||||
def _identity(client: TestClient, *, files=None, **fields):
|
||||
return client.post(
|
||||
"/admin/customization/identity",
|
||||
data={"instance_name": "", "tagline": "", **fields},
|
||||
files=files,
|
||||
follow_redirects=False,
|
||||
)
|
||||
|
||||
|
||||
# --- Defaults in code, overrides in the database --------------------------------
|
||||
def test_the_shipped_wording_is_never_stored_as_an_override(db, client, registered):
|
||||
"""The prompt-fragment rule, and the reason the page can render every
|
||||
flavour string as a box: leaving one alone must not freeze it, or a later
|
||||
release improving the wording would reach nobody who had ever pressed Save.
|
||||
|
||||
Blanked rather than dropped, because `settings_store.update` merges — an
|
||||
omitted key leaves whatever was stored last time, so dropping would make
|
||||
"I typed the default back" and "I changed nothing" store different things.
|
||||
"""
|
||||
client.post(
|
||||
"/admin/customization/flavour",
|
||||
data={key: value for key, (_, _, value) in branding.FLAVOUR.items()}
|
||||
| {f"text_{key}": value for key, (_, _, value) in branding.FLAVOUR.items()},
|
||||
follow_redirects=False,
|
||||
)
|
||||
|
||||
stored = settings_store.get_group(db, branding.BRANDING)
|
||||
written = {key for key, value in stored.items() if key.startswith("text_") and value}
|
||||
assert written == set()
|
||||
|
||||
|
||||
def test_an_override_is_written_and_read_back(db, client, registered):
|
||||
client.post(
|
||||
"/admin/customization/flavour",
|
||||
data={"text_error_404": "There is nothing here."},
|
||||
follow_redirects=False,
|
||||
)
|
||||
|
||||
assert branding.snapshot().text["error_404"] == "There is nothing here."
|
||||
# And the others still say what they always did.
|
||||
assert branding.snapshot().text["error_403"] == branding.FLAVOUR["error_403"][2]
|
||||
|
||||
|
||||
def test_clearing_a_box_gives_the_shipped_wording_back(db, client, registered):
|
||||
client.post(
|
||||
"/admin/customization/flavour", data={"text_error_404": "Gone."}, follow_redirects=False
|
||||
)
|
||||
client.post(
|
||||
"/admin/customization/flavour", data={"text_error_404": ""}, follow_redirects=False
|
||||
)
|
||||
|
||||
assert branding.snapshot().text["error_404"] == branding.FLAVOUR["error_404"][2]
|
||||
|
||||
|
||||
def test_a_string_not_submitted_is_left_alone(db, client, registered):
|
||||
"""The page posts one card at a time, so a save of the identity must not
|
||||
wipe the wording. `save_flavour` only touches keys the form carried."""
|
||||
client.post(
|
||||
"/admin/customization/flavour", data={"text_error_404": "Gone."}, follow_redirects=False
|
||||
)
|
||||
_identity(client, instance_name="Rivendell")
|
||||
|
||||
assert branding.snapshot().text["error_404"] == "Gone."
|
||||
assert branding.snapshot().name == "Rivendell"
|
||||
|
||||
|
||||
# --- The cache ------------------------------------------------------------------
|
||||
def test_a_save_drops_the_snapshot(db, client, registered):
|
||||
"""A process-level cache read by a Jinja global. A save that did not drop it
|
||||
would take effect at the next restart, which is a control that looks like it
|
||||
worked and did nothing -- the failure this codebase keeps cataloguing."""
|
||||
assert branding.snapshot().name == "LLeMbas"
|
||||
|
||||
_identity(client, instance_name="Rivendell")
|
||||
|
||||
assert branding.snapshot().name == "Rivendell"
|
||||
assert "Rivendell" in client.get("/chat").text
|
||||
|
||||
|
||||
def test_the_name_reaches_the_title_and_the_model(db, client, registered):
|
||||
from lembas.services import harness
|
||||
|
||||
_identity(client, instance_name="Rivendell")
|
||||
|
||||
assert "<title>New chat - Rivendell</title>" in client.get("/chat").text
|
||||
assert harness.context_variables(db, None, [])["instance_name"] == "Rivendell"
|
||||
|
||||
|
||||
def test_a_name_stored_by_an_older_version_is_kept(db, registered):
|
||||
"""`instance_name` lived in the general group before there was a branding
|
||||
one. An upgrade must not quietly rename somebody's instance back."""
|
||||
settings_store.update(db, {"instance_name": "Rivendell"})
|
||||
branding.forget()
|
||||
|
||||
assert branding.snapshot().name == "Rivendell"
|
||||
|
||||
|
||||
def test_the_legacy_name_is_a_seed_and_not_a_fallback(db, client, registered):
|
||||
"""Consulted only while the branding group has nothing of its own. A
|
||||
fallback read every time would resurrect the old name underneath a cleared
|
||||
one, which is exactly what a cleared reasoning effort documents."""
|
||||
settings_store.update(db, {"instance_name": "Rivendell"})
|
||||
branding.forget()
|
||||
_identity(client, instance_name="Bree")
|
||||
assert branding.snapshot().name == "Bree"
|
||||
|
||||
_identity(client, instance_name="")
|
||||
assert branding.snapshot().name == "LLeMbas"
|
||||
|
||||
|
||||
# --- Themes ---------------------------------------------------------------------
|
||||
def _save_theme(client, **fields):
|
||||
return client.post(
|
||||
"/admin/customization/themes",
|
||||
data={"theme_0_id": "dusk", "theme_0_label": "Dusk", "theme_0_base": "moria", **fields},
|
||||
follow_redirects=False,
|
||||
)
|
||||
|
||||
|
||||
def test_a_custom_theme_becomes_a_rule(db, client, registered):
|
||||
_save_theme(client, theme_0_bg="#123456", theme_0_accent="#abcdef")
|
||||
|
||||
css = client.get("/branding.css").text
|
||||
assert ':root[data-theme="dusk"]' in css
|
||||
assert "--bg: #123456;" in css
|
||||
assert "--accent: #abcdef;" in css
|
||||
|
||||
|
||||
def test_the_soft_variants_are_derived(db, client, registered):
|
||||
"""The same colour at 14%. An administrator who set an accent without them
|
||||
would get focus rings in the old hue, which reads as the setting
|
||||
half-working rather than as a field they missed."""
|
||||
_save_theme(client, theme_0_accent="#8FB3CC")
|
||||
|
||||
assert "--accent-soft: rgba(143, 179, 204, 0.14);" in client.get("/branding.css").text
|
||||
|
||||
|
||||
def test_a_value_that_is_not_a_colour_is_dropped(db, client, registered):
|
||||
"""It reaches a stylesheet, so a `}` in one would end the rule and silently
|
||||
break every rule after it, and `url(...)` in a colour slot is a request to a
|
||||
third party from every page."""
|
||||
_save_theme(
|
||||
client,
|
||||
theme_0_bg="#123456} body { display: none } .x {",
|
||||
theme_0_accent="url(http://evil.test/x)",
|
||||
theme_0_ink="#fff",
|
||||
)
|
||||
|
||||
css = client.get("/branding.css").text
|
||||
assert "display: none" not in css
|
||||
assert "evil.test" not in css
|
||||
assert "--ink: #fff;" in css
|
||||
|
||||
|
||||
def test_a_token_nobody_offered_is_dropped(db):
|
||||
"""Validated on **read**, so a theme written straight into the settings
|
||||
table -- or stored by an earlier version -- still has to produce a
|
||||
stylesheet that parses."""
|
||||
brand = branding.build(
|
||||
{"themes": [{"id": "dusk", "tokens": {"bg": "#111", "position": "absolute"}}]}
|
||||
)
|
||||
|
||||
assert brand.theme("dusk").tokens == {"bg": "#111"}
|
||||
|
||||
|
||||
def test_a_theme_cannot_take_a_built_in_name(db):
|
||||
brand = branding.build({"themes": [{"id": "moria", "tokens": {"bg": "#111"}}]})
|
||||
assert len(brand.themes) == 2
|
||||
|
||||
|
||||
def test_a_custom_theme_is_offered_and_can_be_chosen(db, client, registered):
|
||||
_save_theme(client, theme_0_bg="#123456")
|
||||
|
||||
page = client.get("/chat").text
|
||||
assert 'data-themes="moria:moria shire:shire dusk:moria"' in page
|
||||
assert client.post("/api/preferences/theme", json={"theme": "dusk"}).json()["ok"] is True
|
||||
# And the settings screen lists it by name.
|
||||
assert "Dusk" in client.get("/settings").text
|
||||
|
||||
|
||||
def test_an_unknown_theme_is_still_refused(db, client, registered):
|
||||
assert client.post("/api/preferences/theme", json={"theme": "dusk"}).json()["ok"] is False
|
||||
|
||||
|
||||
def test_a_light_theme_carries_its_base(db, client, registered):
|
||||
"""`data-base` is what makes it inherit. Without it a custom light theme is
|
||||
four light colours on Moria's near-black surfaces."""
|
||||
_save_theme(client, theme_0_base="shire", theme_0_bg="#fff")
|
||||
|
||||
assert branding.snapshot().theme("dusk").base == "shire"
|
||||
assert 'dusk:shire' in client.get("/chat").text
|
||||
|
||||
|
||||
def test_tokens_css_matches_the_base_attribute(db):
|
||||
"""The other half of that, and it lives in the stylesheet: `shire`'s block
|
||||
has to match `[data-base="shire"]` or the inheritance is a comment."""
|
||||
from pathlib import Path
|
||||
|
||||
import lembas
|
||||
|
||||
css = (Path(lembas.__file__).parent / "web/static/css/tokens.css").read_text()
|
||||
assert ':root[data-base="shire"]' in css
|
||||
|
||||
|
||||
def test_clearing_an_id_removes_the_theme(db, client, registered):
|
||||
_save_theme(client, theme_0_bg="#123456")
|
||||
assert "dusk" in branding.snapshot().theme_ids
|
||||
|
||||
_save_theme(client, theme_0_id="")
|
||||
assert "dusk" not in branding.snapshot().theme_ids
|
||||
|
||||
|
||||
# --- The stylesheet -------------------------------------------------------------
|
||||
def test_custom_css_is_served_as_a_stylesheet(db, client, registered):
|
||||
"""A route rather than an inline `<style>`, which is a security property
|
||||
before it is a caching one: an external stylesheet has no HTML context to
|
||||
escape from."""
|
||||
client.post(
|
||||
"/admin/customization/css",
|
||||
data={"custom_css": ".sidebar { border: 0 }"},
|
||||
follow_redirects=False,
|
||||
)
|
||||
|
||||
response = client.get("/branding.css")
|
||||
assert response.headers["content-type"].startswith("text/css")
|
||||
assert ".sidebar { border: 0 }" in response.text
|
||||
|
||||
|
||||
def test_the_link_changes_when_the_stylesheet_does(db, client, registered):
|
||||
"""A fixed URL would leave the browser's cache deciding when a rebrand takes
|
||||
effect, which is a save that looks like it worked and did nothing."""
|
||||
before = client.get("/chat").text
|
||||
client.post(
|
||||
"/admin/customization/css", data={"custom_css": ".x { color: red }"},
|
||||
follow_redirects=False,
|
||||
)
|
||||
after = client.get("/chat").text
|
||||
|
||||
def revision(page: str) -> str:
|
||||
return page.split("/branding.css?v=", 1)[1].split('"', 1)[0]
|
||||
|
||||
assert revision(before) != revision(after)
|
||||
|
||||
|
||||
def test_the_stylesheet_is_not_precached_by_the_worker(db):
|
||||
"""The worker's cache is versioned by the release; branding changes between
|
||||
releases. A precached copy would outlive every rebrand until the next
|
||||
version bump."""
|
||||
from pathlib import Path
|
||||
|
||||
import lembas
|
||||
|
||||
worker = (Path(lembas.__file__).parent / "web/static/js/sw.js").read_text()
|
||||
assert "/branding.css" not in worker
|
||||
|
||||
|
||||
# --- Assets ---------------------------------------------------------------------
|
||||
PNG = (
|
||||
b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01\x08\x06"
|
||||
b"\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc\x00\x01\x00\x00\x05\x00"
|
||||
b"\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`\x82"
|
||||
)
|
||||
|
||||
|
||||
def test_a_logo_replaces_the_mark_everywhere(db, client, registered):
|
||||
_identity(client, files={"logo": ("logo.png", PNG, "image/png")})
|
||||
|
||||
stored = branding.snapshot().logo_path
|
||||
assert stored
|
||||
assert f'src="/branding/{stored}"' in client.get("/chat").text
|
||||
# Including the sign-in page, which is why the route is unauthenticated.
|
||||
client.post("/auth/logout", follow_redirects=False)
|
||||
assert f'src="/branding/{stored}"' in client.get("/auth/login").text
|
||||
|
||||
|
||||
def test_an_asset_is_served_to_somebody_who_is_not_signed_in(db, client, registered):
|
||||
_identity(client, files={"logo": ("logo.png", PNG, "image/png")})
|
||||
stored = branding.snapshot().logo_path
|
||||
client.post("/auth/logout", follow_redirects=False)
|
||||
|
||||
response = client.get(f"/branding/{stored}")
|
||||
assert response.status_code == 200
|
||||
assert response.headers["x-content-type-options"] == "nosniff"
|
||||
|
||||
|
||||
def test_an_svg_is_refused(db, client, registered):
|
||||
"""The one place somebody will most want it, and the one place it is least
|
||||
safe: these files are served to people who are not signed in, and an SVG can
|
||||
carry a script."""
|
||||
response = _identity(
|
||||
client, files={"logo": ("logo.svg", b"<svg xmlns='x'></svg>", "image/svg+xml")}
|
||||
)
|
||||
|
||||
assert response.status_code == 200 # the page again, with the reason on it
|
||||
assert "SVG" in response.text
|
||||
assert branding.snapshot().logo_path == ""
|
||||
|
||||
|
||||
def test_a_path_outside_the_directory_is_refused(db, client, registered):
|
||||
assert client.get("/branding/..%2F..%2Flembas.db").status_code == 404
|
||||
|
||||
|
||||
def test_the_launcher_icons_come_from_the_logo(db, client, registered):
|
||||
_identity(client, files={"logo": ("logo.png", PNG, "image/png")})
|
||||
|
||||
icons = branding.snapshot().icon_paths
|
||||
assert set(icons) >= {"icon-192", "icon-512", "maskable", "apple-touch", "favicon"}
|
||||
manifest = client.get("/manifest.webmanifest").json()["icons"]
|
||||
assert all(entry["src"].startswith("/branding/") for entry in manifest)
|
||||
assert any(entry["purpose"] == "maskable" for entry in manifest)
|
||||
for entry in manifest:
|
||||
assert client.get(entry["src"]).status_code == 200
|
||||
|
||||
|
||||
def test_removing_the_logo_takes_its_icons_with_it(db, client, registered):
|
||||
_identity(client, files={"logo": ("logo.png", PNG, "image/png")})
|
||||
_identity(client, remove_logo="true")
|
||||
|
||||
assert branding.snapshot().logo_path == ""
|
||||
assert branding.snapshot().icon_paths == {}
|
||||
# And the manifest is back to the shipped set rather than half of each.
|
||||
icons = client.get("/manifest.webmanifest").json()["icons"]
|
||||
assert all(entry["src"].startswith("/static/img/") for entry in icons)
|
||||
|
||||
|
||||
# --- The page -------------------------------------------------------------------
|
||||
def test_the_page_renders_and_is_in_the_nav(db, client, registered):
|
||||
page = client.get("/admin/customization")
|
||||
assert page.status_code == 200
|
||||
assert 'href="/admin/customization"' in page.text
|
||||
# Every flavour string has a box, from the table rather than from a list in
|
||||
# the template -- so one added in code appears here with no template change.
|
||||
for key in branding.FLAVOUR:
|
||||
assert f'name="text_{key}"' in page.text
|
||||
|
||||
|
||||
def test_the_name_is_no_longer_on_general(db, client, registered):
|
||||
"""Two controls writing one value is how each becomes the answer to "why did
|
||||
my change not stick?". General links to it instead."""
|
||||
page = client.get("/admin/general").text
|
||||
assert 'name="instance_name"' not in page
|
||||
assert 'href="/admin/customization"' in page
|
||||
|
||||
|
||||
def test_only_an_administrator_may_customise(db, client, registered):
|
||||
client.post("/auth/logout", follow_redirects=False)
|
||||
client.post(
|
||||
"/auth/register",
|
||||
data={"name": "Sam", "email": "sam@shire.test", "password": "potatoes-po-ta-toes"},
|
||||
follow_redirects=False,
|
||||
)
|
||||
|
||||
for path in ("identity", "flavour", "css", "themes"):
|
||||
assert client.post(f"/admin/customization/{path}", data={}).status_code == 403
|
||||
Reference in New Issue
Block a user