From 9196dd75a89363fc67b6e25dc80d24048f9f2877 Mon Sep 17 00:00:00 2001 From: Timothy Allen Date: Fri, 2 Oct 2026 21:41:46 +0200 Subject: [PATCH] Refuse OPDS logins when CWA accepts every password A CWA with anonymous browsing can answer an OPDS request whatever the password, so each password logged a user in to Bindery. Bindery now tests CWA with a login that cannot exist, and refuses OPDS logins if CWA accepts it. A login check no longer shares the session of the account of Bindery. The CWA account stays optional for a CWA that asks for no login, and a missing account now gets its own message. --- README.md | 14 +++++++++++--- bindery/auth.py | 8 +++++++- bindery/cwa.py | 29 +++++++++++++++++++++++++++-- docker-compose.yml | 1 + tests/fake_cwa.py | 4 +++- tests/test_api.py | 14 ++++++++++++++ tests/test_calibre_db.py | 1 + tests/test_cwa.py | 33 ++++++++++++++++++++++++++++++++- 8 files changed, 96 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 3698014..69700a4 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ Environment variables set the configuration. **Restart the container after a cha | Variable | Description | Default value | | --- | --- | --- | | `CWA_URL` | The address of CWA, for example `http://calibre-web-automated:8083` | None | -| `CWA_USERNAME`, `CWA_PASSWORD` | The CWA account of Bindery | None | +| `CWA_USERNAME`, `CWA_PASSWORD` | The CWA account of Bindery. Leave them unset if CWA asks for no login. | None | | `CWA_DB_PATH` | Calibre-Web's `app.db`, for logins. See "CWA logins". | None | | `CALIBRE_DB_PATH` | The calibre `metadata.db`, for book ids. See "How books reach the library". | None | | `UPDATE_INTERVAL_HOURS` | Hours between checks for new chapters. `0` turns them off. | `24` | @@ -158,6 +158,11 @@ open sessions. `AUTH_METHOD=cwa` needs `CWA_URL`, `CWA_DB_PATH` or both. Without `CWA_DB_PATH`, Bindery sends the user name and password to `CWA_URL/opds` as an HTTP Basic login. This is the OPDS login. +**The OPDS login works only if CWA refuses a wrong password.** A CWA with anonymous browsing can +accept every OPDS login. Bindery tests this with a login that cannot exist. If CWA accepts it, +Bindery refuses each OPDS login, and the log names the reason. Set `CWA_DB_PATH` then, or use +another `AUTH_METHOD`. + With `CWA_DB_PATH`, Bindery checks the password against the account in Calibre-Web's `app.db`. A wrong password or an unknown account fails at once. If Bindery cannot read the file or its password hash, it uses the OPDS login. @@ -288,8 +293,8 @@ files beside the database. The Compose file has an example. **Bindery does not w Be sure to use the library that gets the ingest files. A different library can make Bindery update the wrong book. Database lookups also find books that the CWA account of Bindery cannot see. If Bindery -cannot read the database, it uses CWA for every lookup. Keep `CWA_URL`, `CWA_USERNAME` and -`CWA_PASSWORD` set, because library imports and CWA lookups use them. +cannot read the database, it uses CWA for every lookup. Keep `CWA_URL` and the CWA account set, +because library imports and CWA lookups use them. A book that the FanFicFare calibre plugin made holds its story URL. If you add its URL, or import it from the library, Bindery links that book and imports its chapters from the ePub. @@ -355,6 +360,9 @@ Make a CWA user for Bindery with these settings: Bindery sends the password of this account with each OPDS request. **Use a password that no other service uses.** +If CWA allows anonymous browsing, Bindery needs no account. Leave `CWA_USERNAME` and `CWA_PASSWORD` +unset, and give the guest user of CWA the settings above. + ## Import an ePub file If the site removed chapters before Bindery tracked the story, import an ePub that you made earlier diff --git a/bindery/auth.py b/bindery/auth.py index 277f586..0d98a16 100644 --- a/bindery/auth.py +++ b/bindery/auth.py @@ -1,7 +1,8 @@ """Logins: none, one local account, CWA accounts, or a proxy. API keys work in every mode. CWA accounts use an optional read-only app.db, with OPDS as the fallback when -the database cannot answer. A rejected database login never falls back. +the database cannot answer. A rejected database login never falls back. The +OPDS login works only with a CWA that refuses a wrong password. """ import hashlib @@ -174,6 +175,11 @@ def _login_ok(username: str, password: str, client: CwaClient) -> bool: return calibre_db.login_ok(config.CWA_DB_PATH, username, password) except calibre_db.DatabaseUnavailable as e: logger.warning(f"Cannot read the Calibre-Web login database. Trying OPDS: {e}") + if client.accepts_any_login(): + raise CwaError( + "CWA accepts every OPDS login, most often because anonymous browsing is on, so Bindery cannot check " + "a password through OPDS. Set CWA_DB_PATH, or use another AUTH_METHOD." + ) return client.login_ok(username, password) diff --git a/bindery/cwa.py b/bindery/cwa.py index 6ff08f7..5070863 100644 --- a/bindery/cwa.py +++ b/bindery/cwa.py @@ -7,7 +7,9 @@ by title, then reads the story URL inside each candidate epub. import logging import re +import secrets import threading +import time import uuid from collections.abc import Callable from dataclasses import dataclass, field @@ -30,6 +32,8 @@ ACQUISITION = "http://opds-spec.org/acquisition" # The most epubs that one search downloads to find a story. MAX_DOWNLOADS = 5 MAX_PAGES = 100 +# The seconds that Bindery trusts its last test of the logins of CWA. +LOGIN_TEST_SECONDS = 10 * 60 class CwaError(Exception): @@ -71,21 +75,30 @@ class CwaClient: self.auth = (username, password or "") if username else None self.timeout = timeout self.session = requests.Session() + self._login_test: tuple[float, bool] | None = None # (time, CWA accepted a login that cannot exist) def available(self) -> bool: return bool(self.base_url) def _get(self, href: str, auth: Any = None, **kwargs: Any) -> requests.Response: + """Get a page as the account of Bindery, or with `auth` as a user who logs in.""" if not self.available(): raise CwaError("CWA address is not set. Set CWA_URL.") # hrefs from the feed are absolute paths, and include any path prefix of CWA. url = urljoin(self.base_url, href) - kwargs.setdefault("timeout", self.timeout) + timeout = kwargs.pop("timeout", self.timeout) try: - response = self.session.get(url, auth=auth or self.auth, **kwargs) + if auth: + # Without the session, so a cookie of the account of Bindery cannot stand in for the password. + response = requests.get(url, auth=auth, timeout=timeout, **kwargs) + else: + response = self.session.get(url, auth=self.auth, timeout=timeout, **kwargs) except requests.RequestException as e: raise CwaError(f"Cannot reach CWA at {self.base_url}: {e}") from e if response.status_code == 401: + if not (auth or self.auth): + # A CWA with anonymous browsing needs no account. This one does. + raise CwaLoginError("CWA asks for a login. Set CWA_USERNAME and CWA_PASSWORD.") raise CwaLoginError("CWA refused the login") if response.status_code >= 400: raise CwaError(f"CWA answered {response.status_code} for {url}") @@ -104,6 +117,18 @@ class CwaClient: return False return True + def accepts_any_login(self) -> bool: + """True if CWA accepts a login that cannot exist. + + A CWA with anonymous browsing can answer every OPDS request, whatever the + password. Its OPDS feed then cannot check the password of a user. + """ + test = self._login_test + if test is None or time.monotonic() - test[0] > LOGIN_TEST_SECONDS: + accepted = self.login_ok(f"bindery-test-{secrets.token_hex(8)}", secrets.token_hex(16)) + test = self._login_test = (time.monotonic(), accepted) + return test[1] + def _feed(self, href: str, params: dict[str, str] | None = None) -> tuple[list[Book], list[tuple[str, str]]]: """Books and navigation entries (title, href) of an OPDS feed, with all its pages.""" books: list[Book] = [] diff --git a/docker-compose.yml b/docker-compose.yml index dce788d..8a189ec 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -8,6 +8,7 @@ services: # Omit this and its volume mount to keep uploads paused. INGEST_DIR: /cwa-book-ingest # The address of CWA, as the Bindery container reaches it, and the CWA account that Bindery uses. + # Leave the account out if CWA asks for no login. CWA_URL: http://calibre-web-automated:8083 CWA_USERNAME: bindery CWA_PASSWORD: change-me diff --git a/tests/fake_cwa.py b/tests/fake_cwa.py index 5f0dca2..e044cc8 100644 --- a/tests/fake_cwa.py +++ b/tests/fake_cwa.py @@ -26,6 +26,8 @@ class FakeCwa: self.requests: list[str] = [] # CWA's "Show Publisher Section" setting. Without it, the publisher feed is 404. self.publisher_view = True + # CWA's anonymous browsing. With it, the fake answers every request, with or without a login. + self.anonymous = False fake = self class Handler(BaseHTTPRequestHandler): @@ -87,7 +89,7 @@ class FakeCwa: self._send(h, 200, self._feed([self._book_entry(b) for b in page], next_href), "application/atom+xml") def handle(self, h) -> None: - if not self._authorized(h): + if not self.anonymous and not self._authorized(h): self._send(h, 401, b"Unauthorized", "text/plain") return url = urlparse(h.path) diff --git a/tests/test_api.py b/tests/test_api.py index 697c39a..9fae276 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -81,6 +81,20 @@ def test_cwa_login(client, cwa): assert client.get("/api/stories").status_code == 401 +def test_cwa_that_accepts_every_login_cannot_check_a_password(client, cwa, caplog): + cwa.anonymous = True + for password in ("pw", "wrong"): + response = login(client, password=password) + assert response.status_code == 401 + assert response.json["error"] == "Cannot check the login with CWA. The log has the details." + assert "CWA accepts every OPDS login" in caplog.text + assert client.get("/api/stories").status_code == 401 + # These are not wrong passwords, so they lock no one out. + cwa.anonymous = False + orchestrator.cwa()._login_test = None + assert login(client).status_code == 200 + + def test_a_session_ends_after_one_day(client, cwa, monkeypatch): response = client.post( "/api/auth/login", json={"username": "reader", "password": "pw", "remember": True}, headers=H diff --git a/tests/test_calibre_db.py b/tests/test_calibre_db.py index d195a54..7a8f7dc 100644 --- a/tests/test_calibre_db.py +++ b/tests/test_calibre_db.py @@ -53,6 +53,7 @@ def db_login(client, user_db, monkeypatch): monkeypatch.setattr(config, "CWA_DB_PATH", user_db) remote = Mock(spec=CwaClient) remote.login_ok.return_value = True + remote.accepts_any_login.return_value = False monkeypatch.setattr(orchestrator, "cwa", lambda: remote) return remote diff --git a/tests/test_cwa.py b/tests/test_cwa.py index c09ee05..bb299af 100644 --- a/tests/test_cwa.py +++ b/tests/test_cwa.py @@ -1,13 +1,15 @@ """The OPDS client, the book finder, the login check, the ingest folder and the epub reader.""" import json +import time +from unittest.mock import Mock import pytest from fake_cwa import FakeBook, FakeCwa from test_build import URL, chapters, metahtml from bindery import fff -from bindery.cwa import BookFinder, CwaClient, CwaError, plain_title, search_text, title_rank +from bindery.cwa import LOGIN_TEST_SECONDS, BookFinder, CwaClient, CwaError, plain_title, search_text, title_rank from bindery.epub import epub_metadata, epub_urls from bindery.ingest import MAX_NAME_LENGTH, IngestError, IngestFolder @@ -63,6 +65,35 @@ def test_wrong_password_and_missing_address(fake): CwaClient("").search("x") +def test_cwa_that_needs_no_login_works_without_an_account(fake): + fake.anonymous = True + open_client = CwaClient(fake.url) + assert [b.id for b in open_client.search("Another")] == [3] + assert open_client.get_book("uuid-2")["id"] == 2 + fake.anonymous = False + with pytest.raises(CwaError, match="CWA asks for a login. Set CWA_USERNAME and CWA_PASSWORD."): + open_client.search("Test Story") + + +def test_a_login_check_does_not_use_the_session_of_the_account(fake, monkeypatch): + c = client(fake) + monkeypatch.setattr(c.session, "get", Mock(side_effect=AssertionError("The login check used the session"))) + assert c.login_ok("bindery", "secret") + assert not c.login_ok("bindery", "wrong") + + +def test_the_test_of_the_logins_of_cwa_is_kept_for_a_time(fake, monkeypatch): + c = client(fake) + assert not c.accepts_any_login() + fake.anonymous = True + assert not c.accepts_any_login() # the last test still counts + assert len([r for r in fake.requests if r == "/opds"]) == 1 + clock = time.monotonic() + monkeypatch.setattr(time, "monotonic", lambda: clock + LOGIN_TEST_SECONDS + 1) + assert c.accepts_any_login() + assert CwaClient(fake.url).accepts_any_login() + + def test_get_book(fake): c = client(fake) assert c.get_book("uuid-2") == {