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

@@ -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

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)
 
 

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] = []

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

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)

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

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
 

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") == {