From bb6c87cdd573e1c6607113f0a7467b116215507c Mon Sep 17 00:00:00 2001 From: Timothy Allen Date: Fri, 2 Oct 2026 21:56:55 +0200 Subject: [PATCH] Let only admins change the global settings BINDERY_ADMIN_USERS lists the admins of the cwa and proxy modes. Only an admin can change the global settings and the global rules, and read or change personal.ini. Without a list, each user is an admin. --- README.md | 18 +++++++++++++- bindery/auth.py | 11 +++++++++ bindery/config.py | 2 ++ bindery/static/js/main.js | 20 ++++++++++++---- bindery/static/js/settings.js | 3 ++- bindery/web.py | 16 +++++++++++-- docker-compose.yml | 2 ++ tests/js/admin.test.mjs | 26 ++++++++++++++++++++ tests/test_api.py | 45 +++++++++++++++++++++++++++++++++++ tests/test_config.py | 4 ++++ tests/test_local_auth.py | 1 + 11 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 tests/js/admin.test.mjs diff --git a/README.md b/README.md index 452f35e..c13e732 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,7 @@ Environment variables set the configuration. **Restart the container after a cha | `BINDERY_USERNAME` | The user name for `AUTH_METHOD=local`. Case-sensitive. | None | | `BINDERY_PASSWORD_HASH` | The password hash for `AUTH_METHOD=local`. See "Local login". | None | | `CWA_ALLOWED_USERS` | The CWA users that can log in. Empty allows every CWA user. | Empty | +| `BINDERY_ADMIN_USERS` | The users that can change the global settings. See "Admins". | Empty | | `PROXY_AUTH_USER_HEADER` | The header in which a reverse proxy sends the user name | `X-Remote-User` | | `PROXY_AUTH_TRUSTED_IPS` | The addresses and networks that can send the user header. See "Logins". | Loopback and private addresses | | `PROXY_AUTH_LOGOUT_URL` | Where the Log out button goes in proxy mode | None | @@ -210,6 +211,21 @@ open session until that session ends. To stop a CWA user at once, remove the use `CWA_ALLOWED_USERS` and restart Bindery. To end all sessions, delete `/config/.flask_secret` and restart Bindery. +#### Admins + +In the `cwa` and `proxy` modes, `BINDERY_ADMIN_USERS` lists the users that are admins, for example +`BINDERY_ADMIN_USERS=alice, bob`. Only an admin can do these things: + +- Open the Settings page, and change the settings on it. +- Change the global replacement rules. +- Read and change `personal.ini`, which can hold passwords for story sites. + +Each other user can still add, update and delete stories, and change the settings and the rules of a +story. Bindery ignores the case of the names. + +If the list is empty, each user is an admin. The `none` and `local` modes have only one user, who is +always an admin. The API key is always an admin too. + ### Host names Without a login, a web page that you visit can try to reach Bindery from your browser, under a host @@ -277,7 +293,7 @@ setting. Bindery reads the file for each job, so a change applies to the next jo You can edit the file on the Settings page. Bindery refuses a file that FanFicFare cannot read, and shows FanFicFare's warnings for other problems. **The file can hold passwords for story sites, and -every user who can log in to Bindery can read them.** +each admin can read them.** If `BINDERY_ADMIN_USERS` is not set, each user is an admin. See "Admins". Bindery always sets `always_reload_first_chapter`, `mark_new_chapters`, `continue_on_chapter_error`, `make_directories`, `zip_output` and `calibre_series_meta`, because it needs fixed values for them. diff --git a/bindery/auth.py b/bindery/auth.py index 626cafb..ba8d822 100644 --- a/bindery/auth.py +++ b/bindery/auth.py @@ -141,6 +141,17 @@ def _allowed(name: str) -> bool: return not names or name.casefold() in names +def is_admin(identity: Identity) -> bool: + """Whether this user can change the global settings, the global rules and personal.ini. + + Only the cwa and proxy modes have more than one user. The API key is always an admin. + """ + names = {n.casefold() for n in config.ADMIN_USERS} + if not names or identity.via in ("api_key", "none") or active_mode() == AUTH_LOCAL: + return True + return identity.name.casefold() in names + + def _api_key_identity(headers: Mapping[str, str]) -> Identity | None: if not config.API_KEY: return None diff --git a/bindery/config.py b/bindery/config.py index 8cf2be3..649e309 100644 --- a/bindery/config.py +++ b/bindery/config.py @@ -73,6 +73,8 @@ AUTH_METHOD = os.getenv("AUTH_METHOD", "none").strip().lower() LOCAL_USERNAME = os.getenv("BINDERY_USERNAME", "").strip() LOCAL_PASSWORD_HASH = os.getenv("BINDERY_PASSWORD_HASH", "").strip() CWA_ALLOWED_USERS = _list("CWA_ALLOWED_USERS") +# The users of the cwa and proxy modes that can change the global settings. Without a list, each user can. +ADMIN_USERS = _list("BINDERY_ADMIN_USERS") PROXY_AUTH_USER_HEADER = os.getenv("PROXY_AUTH_USER_HEADER", "X-Remote-User").strip() PROXY_AUTH_LOGOUT_URL = os.getenv("PROXY_AUTH_LOGOUT_URL", "").strip() # The addresses and networks that can send the user header. The default is each loopback and private address, so diff --git a/bindery/static/js/main.js b/bindery/static/js/main.js index 7a7a148..4c37181 100644 --- a/bindery/static/js/main.js +++ b/bindery/static/js/main.js @@ -11,7 +11,7 @@ const ROUTES = [ [/^\/$/, storiesPage, []], [/^\/story\/(\d+)$/, storyPage, ['storyId']], [/^\/story\/(\d+)\/chapter\/(\d+)$/, chapterPage, ['storyId', 'chapterId']], - [/^\/settings$/, settingsPage, []], + [/^\/settings$/, settingsPage, [], true], ]; const app = document.getElementById('bindery-app'); @@ -23,6 +23,9 @@ let nav = null; let storyTab = null; let storyTabShown = false; +// The server refuses the changes of a user who is not an admin. The interface only hides what that user cannot use. +const isAdmin = () => auth.admin !== false; + const currentPath = () => window.location.hash.replace(/^#/, '') || '/'; // The story of #/story/3 and of its chapter pages, such as #/story/3/chapter/5. const currentStoryId = () => currentPath().match(/^\/story\/(\d+)(?:\/|$)/)?.[1]; @@ -44,6 +47,14 @@ function notFound(main) { fill(main, h('section', { class: 'card' }, h('h1', {}, 'Page not found'), h('p', {}, h('a', { href: '#/' }, 'Go to the story list')))); } +function adminOnly(main) { + document.title = 'Settings ยท Bindery'; + fill( + main, + h('section', { class: 'card' }, h('h1', {}, 'Settings'), h('p', {}, 'Only an admin can open the settings. BINDERY_ADMIN_USERS lists the admins.')), + ); +} + function route() { const main = document.getElementById('main'); if (!main) return; @@ -54,9 +65,10 @@ function route() { if (storyTab?.getAttribute('href') !== `#/story/${currentStoryId()}`) hideStoryTab(); const found = ROUTES.find(([pattern]) => pattern.test(path)); if (found) { - const [pattern, page, names] = found; + const [pattern, page, names, admin] = found; const match = path.match(pattern); - cleanup = page(main, Object.fromEntries(names.map((name, i) => [name, match[i + 1]])), config) || null; + if (admin && !isAdmin()) adminOnly(main); + else cleanup = page(main, Object.fromEntries(names.map((name, i) => [name, match[i + 1]])), config) || null; } else { notFound(main); } @@ -81,7 +93,7 @@ function shell() { (auth.mode === 'cwa' || auth.mode === 'local' || auth.logout_url) && h('button', { type: 'button', onclick: () => logout().catch(showError) }, 'Log out'), ]; - nav = h('nav', { 'aria-label': 'Main navigation' }, h('a', { href: '#/' }, 'Stories'), h('a', { href: '#/settings' }, 'Settings')); + nav = h('nav', { 'aria-label': 'Main navigation' }, h('a', { href: '#/' }, 'Stories'), isAdmin() && h('a', { href: '#/settings' }, 'Settings')); storyTab = h('a', { class: 'story-tab' }); storyTabShown = false; fill( diff --git a/bindery/static/js/settings.js b/bindery/static/js/settings.js index 5d9cc1f..e4c187c 100644 --- a/bindery/static/js/settings.js +++ b/bindery/static/js/settings.js @@ -197,7 +197,8 @@ export function settingsPage(main, _params, config) { externalLink(INI_DOCS, 'personal.ini documentation'), ' and the ', externalLink(DEFAULTS_INI, 'list of all settings with their defaults'), - '. The file can hold passwords for story sites. Each user who can log in to Bindery can read them here.', + '. The file can hold passwords for story sites. Each admin can read them here. If BINDERY_ADMIN_USERS is not ' + + 'set, each user is an admin.', ), ini, warnings, diff --git a/bindery/web.py b/bindery/web.py index d931ecc..50be79c 100644 --- a/bindery/web.py +++ b/bindery/web.py @@ -14,7 +14,7 @@ from pathlib import Path from typing import Any from flask import Flask, Response, g, jsonify, request, send_file, session -from werkzeug.exceptions import HTTPException +from werkzeug.exceptions import Forbidden, HTTPException from bindery import auth, config, orchestrator from bindery.cwa import CwaError @@ -139,6 +139,11 @@ def _identity() -> auth.Identity | None: return auth.current_identity(request.headers, session, request.remote_addr) +def _admin_only() -> None: + if not auth.is_admin(g.identity): + raise Forbidden("Only an admin can do this. BINDERY_ADMIN_USERS lists the admins.") + + @app.before_request def check_host() -> Any: if request.path == "/api/health" or auth.host_allowed(request.host): @@ -222,6 +227,7 @@ def auth_check() -> Any: "authenticated": identity is not None, "mode": mode, "user": identity.name if identity else None, + "admin": identity is not None and auth.is_admin(identity), "logout_url": config.PROXY_AUTH_LOGOUT_URL or None if mode == auth.AUTH_PROXY else None, } @@ -271,6 +277,7 @@ def app_config() -> Any: @app.route("/api/settings", methods=["GET", "PUT"]) def settings() -> Any: if request.method == "PUT": + _admin_only() data = _json() return orchestrator.save_settings( data.get("recent_chapters"), @@ -283,7 +290,10 @@ def settings() -> Any: data.get("main_jobs_count"), link_books=data.get("link_books"), ) - return orchestrator.get_settings() + settings = orchestrator.get_settings() + if not auth.is_admin(g.identity): + del settings["personal_ini"] # it can hold passwords for story sites + return settings # Jobs @@ -427,6 +437,8 @@ def story_cover(story_id: int) -> Any: @app.route("/api/rules", methods=["GET", "PUT"]) def rules(story_id: int | None = None) -> Any: if request.method == "PUT": + if story_id is None: + _admin_only() data = _json() return orchestrator.save_rules(story_id, _rules(data), data.get("paragraphs", ...)) return orchestrator.get_rules(story_id) diff --git a/docker-compose.yml b/docker-compose.yml index 1c73533..ba1e9f1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -27,6 +27,8 @@ services: # TRUST_PROXY: "true" # For AUTH_METHOD=proxy: the address of the proxy. The default trusts each private address. # PROXY_AUTH_TRUSTED_IPS: 172.18.0.3 + # For the cwa and proxy modes: the users that can change the global settings. Without a list, each user can. + # BINDERY_ADMIN_USERS: alice, bob # BINDERY_API_KEY: a-long-random-string ports: # If a reverse proxy on this host is the only way in, publish the port only on the host: 127.0.0.1:8085:8085 diff --git a/tests/js/admin.test.mjs b/tests/js/admin.test.mjs new file mode 100644 index 0000000..d801182 --- /dev/null +++ b/tests/js/admin.test.mjs @@ -0,0 +1,26 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { find, install, server, settle } from './flow-helpers.mjs'; + +const ui = install(); +const { stopPolling } = await import('../../bindery/static/js/jobs.js'); + +test('a user who is not an admin gets no settings link and no settings page', async (t) => { + t.after(stopPolling); + const backend = server(t); + backend.on('GET', 'auth/check', { authenticated: true, mode: 'proxy', user: 'bob', admin: false }); + backend.on('GET', 'config', { metadata_fields: {}, mode_labels: {}, library_available: false }); + backend.on('GET', 'jobs', []); + backend.on('GET', 'stories', []); + await import('../../bindery/static/js/main.js'); + await settle(); + assert.ok(find(ui.app, 'a', 'Stories')); + assert.equal(find(ui.app, 'a', 'Settings'), undefined); + window.location.hash = '#/settings'; + ui.windowEvents.hashchange(); + await settle(); + assert.match(ui.app.textContent, /Only an admin can open the settings/); + // The page asks the server for no settings and no rules. + assert.deepEqual(backend.sent('GET', 'settings'), []); + assert.deepEqual(backend.sent('GET', 'rules'), []); +}); diff --git a/tests/test_api.py b/tests/test_api.py index acea35e..01d470c 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -71,6 +71,7 @@ def test_cwa_login(client, cwa): "authenticated": False, "mode": "cwa", "user": None, + "admin": False, "logout_url": None, } assert login(client, password="wrong").status_code == 401 @@ -170,10 +171,54 @@ def test_proxy_header(client, monkeypatch): "authenticated": True, "mode": "proxy", "user": "alice", + "admin": True, "logout_url": "https://auth.example.com/logout", } +@pytest.mark.parametrize(("user", "admin"), [("alice", True), ("Alice", True), ("bob", False)]) +def test_only_an_admin_changes_the_global_settings(client, monkeypatch, tmp_path, user, admin): + monkeypatch.setattr(config, "PERSONAL_INI", tmp_path / "personal.ini") + monkeypatch.setattr(config, "AUTH_METHOD", "proxy") + monkeypatch.setattr(config, "ADMIN_USERS", ["alice"]) + sid = orchestrator.store.create_story("https://www.royalroad.com/fiction/1", "www.royalroad.com", "S", "W", []) + headers = {**H, "X-Remote-User": user} + status = 200 if admin else 403 + assert client.get("/api/auth/check", headers=headers).json["admin"] is admin + assert ("personal_ini" in client.get("/api/settings", headers=headers).json) is admin + for data in ({"recent_chapters": 5}, {"personal_ini": "[defaults]\n"}): + assert client.put("/api/settings", json=data, headers=headers).status_code == status + assert (orchestrator.get_settings()["recent_chapters"] == 5) is admin + assert config.PERSONAL_INI.exists() is admin + rules = {"rules": [{"pattern": "a", "replacement": "b"}]} + response = client.put("/api/rules", json=rules, headers=headers) + assert response.status_code == status + assert bool(orchestrator.store.rules(None)) is admin + if not admin: + assert response.json == {"error": "Only an admin can do this. BINDERY_ADMIN_USERS lists the admins."} + # Each user can read the global rules, and change the rules and the settings of a story. + assert client.get("/api/rules", headers=headers).status_code == 200 + assert client.put(f"/api/stories/{sid}/rules", json=rules, headers=headers).status_code == 200 + assert client.put(f"/api/stories/{sid}/settings", json={"recheck_days": 3}, headers=headers).status_code == 200 + + +def test_the_api_key_and_single_user_modes_are_admins(client, monkeypatch): + monkeypatch.setattr(config, "ADMIN_USERS", ["alice"]) + assert client.get("/api/auth/check").json["admin"] # the none mode has no users + monkeypatch.setattr(config, "AUTH_METHOD", "proxy") + monkeypatch.setattr(config, "API_KEY", "secret") + key = {"X-Api-Key": "secret"} + assert client.get("/api/auth/check", headers=key).json["admin"] + assert client.put("/api/settings", json={"recent_chapters": 2}, headers=key).status_code == 200 + assert auth.is_admin(auth.Identity("bob", "proxy")) is False + monkeypatch.setattr(config, "AUTH_METHOD", "local") + assert auth.is_admin(auth.Identity("bob", "session")) + # Without a list, each user is an admin. + monkeypatch.setattr(config, "AUTH_METHOD", "proxy") + monkeypatch.setattr(config, "ADMIN_USERS", []) + assert auth.is_admin(auth.Identity("bob", "proxy")) + + @pytest.mark.parametrize( ("trusted", "address", "accepted"), [ diff --git a/tests/test_config.py b/tests/test_config.py index 071a40f..4ae1652 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -69,6 +69,7 @@ def test_values_from_the_environment(monkeypatch, reload_config): monkeypatch.setenv("UPDATE_INTERVAL_HOURS", "1.5") monkeypatch.setenv("DEFAULT_LOCKED_FIELDS", "title, authors") monkeypatch.setenv("CWA_ALLOWED_USERS", "alice,bob") + monkeypatch.setenv("BINDERY_ADMIN_USERS", "alice") monkeypatch.setenv("CONFIG_DIR", "/data/bindery") monkeypatch.setenv("TRUST_PROXY", "yes") monkeypatch.setenv("CWA_DB_PATH", " /cwa/app.db ") @@ -83,6 +84,7 @@ def test_values_from_the_environment(monkeypatch, reload_config): assert config.UPDATE_INTERVAL_HOURS == 1.5 assert config.DEFAULT_LOCKED_FIELDS == ["title", "authors"] assert config.CWA_ALLOWED_USERS == ["alice", "bob"] + assert config.ADMIN_USERS == ["alice"] assert str(config.DB_PATH) == "/data/bindery/bindery.db" assert config.TRUST_PROXY is True assert str(config.CWA_DB_PATH) == "/cwa/app.db" @@ -101,6 +103,7 @@ def test_defaults(monkeypatch, reload_config): "PROXY_AUTH_USER_HEADER", "PROXY_AUTH_TRUSTED_IPS", "ALLOWED_HOSTS", + "BINDERY_ADMIN_USERS", "SESSION_COOKIE_SECURE", "TRUST_PROXY", "CWA_DB_PATH", @@ -127,6 +130,7 @@ def test_defaults(monkeypatch, reload_config): "fe80::/10", ] assert config.ALLOWED_HOSTS == [] + assert config.ADMIN_USERS == [] assert config.SESSION_COOKIE_SECURE is False assert config.TRUST_PROXY is False assert config.CWA_DB_PATH is None diff --git a/tests/test_local_auth.py b/tests/test_local_auth.py index 8a84105..68e8005 100644 --- a/tests/test_local_auth.py +++ b/tests/test_local_auth.py @@ -37,6 +37,7 @@ def test_local_login_logout_and_password_hashes(client, local, monkeypatch, meth "authenticated": False, "mode": "local", "user": None, + "admin": False, "logout_url": None, } assert client.get("/api/stories").status_code == 401