fix: one session per host and credential, named by host alone

- a credential registered as host/path no longer becomes the WtSite iri,
  which built an API endpoint that does not exist
- the cache is keyed by (host, credential), so one credential matching
  several subdomains does not serve them all the same session
- pin oold to the branch carrying registered_credentials()
- tests inspect the config the session is built from

Author: simontaurus

pyproject.toml

@@ -21,8 +21,10 @@ classifiers = [
     "Topic :: Software Development :: Libraries :: Python Modules",
 ]
 dependencies = [
-    # registered_credentials(), used to find the wiki of a file without a URL
-    "oold>=1.1.0",
+    # TEMPORARY: registered_credentials(), used to find the wiki of a file
+    # without a URL, is on the oold branch below and in no release (main is
+    # 2.2.0). Replace with oold>=2.3.0 once released.
+    "oold @ git+https://github.com/OO-LD/oold-python@feat/controller-type-lookup",
     "opensemantic",
     # TEMPORARY: the file controllers this module wraps are on the core branch
     # below. Replace with opensemantic.core>=0.59.0 once released.

src/osw/controller/file/wiki.py

@@ -83,13 +83,15 @@ def store_params_for_upload(
     return {**se_params, "overwrite": "replace remote"}
 
 
-_sessions: Dict[str, Tuple[Any, OSW]] = {}
-"""Sessions by credential IRI, with the credential they were built from.
-
-Keyed by the credential rather than by the host, because two credentials
-scoped to different paths of one wiki are two different logins. The credential
-is kept so a later ``set_credential`` replaces the session instead of being
-ignored for the lifetime of the process.
+_sessions: Dict[Tuple[str, str], Tuple[Any, OSW]] = {}
+"""Sessions by (host, credential IRI), with the credential they were built from.
+
+The credential is part of the key because two credentials scoped to different
+paths of one wiki are two different logins; the host is, because one credential
+can match several hosts - oold's lookup treats a stored IRI contained in the
+searched one as a match, so a credential for ``example.org`` answers for every
+subdomain. The credential itself is kept so a later ``set_credential`` replaces
+the session instead of being ignored for the lifetime of the process.
 """
 
 
@@ -98,34 +100,44 @@ def clear_sessions() -> None:
     _sessions.clear()
 
 
+def host_of(url: str) -> str:
+    """The wiki host a URL or a bare credential IRI names.
+
+    A credential is registered under whatever its owner chose - a bare host, a
+    host with a path, a full URL. ``WtSite`` wants the host alone: given a path
+    it builds an API endpoint that does not exist.
+    """
+    parsed = urlparse(url if "//" in url else f"//{url}")
+    return parsed.netloc or url.split("/")[0]
+
+
 def credential_for(url: str) -> Any:
     """The credential registered for ``url``, or a readable error."""
-    host = urlparse(url).netloc or url
     try:
         return _get_registered_credential(url)
     except ValueError as error:
         raise ValueError(
-            f"No credential registered for {host}. Pass osw= explicitly or "
-            "register one with oold.backend.auth.set_credential."
+            f"No credential registered for {host_of(url)}. Pass osw= explicitly "
+            "or register one with oold.backend.auth.set_credential."
         ) from error
 
 
-def session_for(url: str, credential: Any = None) -> OSW:
+def session_for(url: str) -> OSW:
     """An OSW session for the host of ``url``, from the registered credential.
 
     The file entity may be stored somewhere other than its bytes, so the
     lookup key is the URL the content lives at, not the entity's own IRI.
     """
-    host = urlparse(url).netloc or url
-    if credential is None:
-        credential = credential_for(url)
-    cached = _sessions.get(credential.iri)
+    host = host_of(url)
+    credential = credential_for(url)
+    key = (host, credential.iri)
+    cached = _sessions.get(key)
     if cached is not None and cached[0] == credential:
         return cached[1]
     cred_mngr = CredentialManager()
     cred_mngr.add_credential(credential)
     session = OSW(site=WtSite(WtSite.WtSiteConfig(iri=host, cred_mngr=cred_mngr)))
-    _sessions[credential.iri] = (credential, session)
+    _sessions[key] = (credential, session)
     return session
 
 

tests/test_wiki_file_session.py

@@ -84,14 +84,25 @@ def test_a_session_that_was_passed_in_is_kept():
 # -- the session cache --------------------------------------------------------
 
 
-def test_sessions_are_cached_per_credential(monkeypatch):
-    """Two path-scoped credentials of one host are two logins."""
-    built = []
+@pytest.fixture
+def built_sites(monkeypatch):
+    """Record the WtSite configs built, instead of connecting to a wiki."""
+    configs = []
+
+    def _config(**kwargs):
+        configs.append(kwargs)
+        return kwargs
+
     monkeypatch.setattr(
-        wiki_module, "OSW", lambda site: built.append(site) or f"session{len(built)}"
+        wiki_module, "OSW", lambda site: f"session{len(configs)}:{site['iri']}"
     )
     monkeypatch.setattr(wiki_module, "WtSite", lambda config: config)
-    monkeypatch.setattr(wiki_module.WtSite, "WtSiteConfig", dict, raising=False)
+    monkeypatch.setattr(wiki_module.WtSite, "WtSiteConfig", _config, raising=False)
+    return configs
+
+
+def test_sessions_are_cached_per_credential(built_sites):
+    """Two path-scoped credentials of one host are two logins."""
     set_credential(_credential("wiki.example.org/a", username="a"))
     set_credential(_credential("wiki.example.org/b", username="b"))
 
@@ -101,23 +112,44 @@ def test_sessions_are_cached_per_credential(monkeypatch):
 
     assert first != second
     assert again == first
+    # and every one of them talks to the host, not to host/path
+    assert {config["iri"] for config in built_sites} == {"wiki.example.org"}
+
+
+def test_one_credential_does_not_collapse_two_hosts(built_sites):
+    """oold matches a stored IRI inside the searched one, so one credential
+    answers for every subdomain - that must not share a session."""
+    set_credential(_credential("example.org"))
+
+    first = wiki_module.session_for("https://wiki-a.example.org/wiki/File:X")
+    second = wiki_module.session_for("https://wiki-b.example.org/wiki/File:Y")
 
+    assert first != second
+    assert [config["iri"] for config in built_sites] == [
+        "wiki-a.example.org",
+        "wiki-b.example.org",
+    ]
+
+
+def test_a_bare_credential_iri_still_names_the_host(built_sites):
+    """The no-URL path hands the credential key straight to session_for."""
+    set_credential(_credential("wiki.example.org/w"))
+
+    wiki_module.session_for(_controller()._session_url())
+
+    assert built_sites[0]["iri"] == "wiki.example.org"
 
-def test_a_replaced_credential_replaces_the_session(monkeypatch):
+
+def test_a_replaced_credential_replaces_the_session(built_sites):
     """A later set_credential must not be ignored for the process lifetime."""
-    built = []
-    monkeypatch.setattr(
-        wiki_module, "OSW", lambda site: built.append(site) or f"session{len(built)}"
-    )
-    monkeypatch.setattr(wiki_module, "WtSite", lambda config: config)
-    monkeypatch.setattr(wiki_module.WtSite, "WtSiteConfig", dict, raising=False)
     set_credential(_credential("wiki.example.org", password="old"))
     first = wiki_module.session_for("https://wiki.example.org/File:X")
 
     set_credential(_credential("wiki.example.org", password="new"))
     second = wiki_module.session_for("https://wiki.example.org/File:X")
 
     assert second != first
+    assert len(built_sites) == 2
 
 
 def test_a_missing_credential_is_reported_with_the_host():