Merge pull request #97 from TaskarCenterAtUW/jeff-josm

capabilities call fix for JOSM

Author: jeffmaki

api/core/security.py

@@ -167,6 +167,9 @@ def evict_user_from_cache(auth_uid: UUID) -> None:
 # Every Basic-auth rejection repeats this, because the field placement is the
 # thing callers get wrong and a bare "Not authenticated" gives them nothing to
 # act on.
+# Advertised in the Basic challenge on the proxied OSM surface.
+OSM_BASIC_REALM = "TDEI Workspaces"
+
 _BASIC_USAGE_HINT = (
     "Supply the TDEI token as the HTTP Basic *username*; the password is "
     'ignored. For example: `curl -u "$TDEI_TOKEN:" ...`, or a URL of the form '
@@ -178,7 +181,9 @@ def _basic_auth_error(reason: str) -> HTTPException:
     return HTTPException(
         status_code=status.HTTP_401_UNAUTHORIZED,
         detail=f"{reason} {_BASIC_USAGE_HINT}",
-        headers={"WWW-Authenticate": "Bearer"},
+        # Challenge with the scheme the caller was using, so a client that only speaks Basic can
+        # correct itself and retry.
+        headers={"WWW-Authenticate": f'Basic realm="{OSM_BASIC_REALM}"'},
     )
 
 
@@ -275,11 +280,25 @@ async def __call__(  # type: ignore[override]
         scheme, param = get_authorization_scheme_param(
             request.headers.get("Authorization")
         )
+        basic_allowed = not request.url.path.startswith(BEARER_ONLY_PATH_PREFIXES)
+
         if scheme.lower() != "basic":
+            # No credentials at all, on a path where Basic is the accepted scheme: answer with a
+            # Basic challenge. HTTPBearer answers `WWW-Authenticate: Bearer`, which a client that
+            # only speaks Basic -- JOSM and other OSM editors -- cannot act on: it never sends its
+            # credentials and reports that it could not reach the server. The /api/v1 surface is
+            # unaffected, since Basic is not accepted there.
+            if not scheme and basic_allowed:
+                raise HTTPException(
+                    status_code=status.HTTP_401_UNAUTHORIZED,
+                    detail="Not authenticated",
+                    headers={"WWW-Authenticate": f'Basic realm="{OSM_BASIC_REALM}"'},
+                )
+
             # Bearer (and every rejection path) keeps FastAPI's own behavior.
             return await super().__call__(request)
 
-        if request.url.path.startswith(BEARER_ONLY_PATH_PREFIXES):
+        if not basic_allowed:
             raise HTTPException(
                 status_code=status.HTTP_401_UNAUTHORIZED,
                 detail=(

api/main.py

@@ -209,9 +209,23 @@ def get_workspace_repository(
 _WORKSPACE_PREFIX_RE = re.compile(r"^/workspace/(\d+)(/.*)$")
 
 
+# OSM clients ask for capabilities before anything else, and do so anonymously -- JOSM will not send
+# credentials until it has been challenged, and it cannot act on the Bearer challenge the rest of this
+# surface answers with. Serve every spelling they use, with or without the /workspace/{id}/ prefix.
+# These are declared above the catch-all so they match first and never reach validate_token.
+@app.get("/api/capabilities")
 @app.get("/api/capabilities.json")
-async def capabilities(request: Request):
-    """Proxy OSM capabilities manifest without requiring authentication."""
+@app.get("/api/0.6/capabilities")
+@app.get("/workspace/{workspace_id}/api/capabilities")
+@app.get("/workspace/{workspace_id}/api/capabilities.json")
+@app.get("/workspace/{workspace_id}/api/0.6/capabilities")
+async def capabilities(request: Request, workspace_id: int | None = None):
+    """Proxy the OSM capabilities manifest without requiring authentication.
+
+    The manifest is public metadata and carries nothing workspace-specific, so the
+    `/workspace/{id}/` prefix is accepted only because clients are configured with it as their
+    server URL; it is stripped and otherwise ignored.
+    """
 
     client = _require_osm_client()
     client_host = request.client.host if request.client else "unknown"
@@ -227,7 +241,13 @@ async def capabilities(request: Request):
         (b"X-Forwarded-Proto", request.url.scheme.encode()),
     ]
 
-    url = httpx.URL(path="/api/capabilities.json")
+    # Forward whichever spelling was asked for, minus any workspace prefix, rather than a fixed path.
+    proxied_path = request.url.path
+    prefix_match = _WORKSPACE_PREFIX_RE.match(proxied_path)
+    if prefix_match is not None:
+        proxied_path = prefix_match.group(2)
+
+    url = httpx.URL(path=proxied_path)
     rp_req = client.build_request("GET", url, headers=req_headers)
 
     try:

tests/integration/test_proxy.py

@@ -55,6 +55,29 @@ async def test_capabilities_proxies_without_auth(client, mock_osm):
     assert mock_osm.last_request.url.path == "/api/capabilities.json"
 
 
+# OSM editors fetch capabilities before anything else and do so anonymously, under whatever server
+# URL they are configured with -- which for this proxy includes the /workspace/{id}/ prefix.
+
+
+@pytest.mark.parametrize(
+    "path",
+    [
+        "/api/capabilities",
+        "/api/capabilities.json",
+        "/api/0.6/capabilities",
+        "/workspace/7/api/capabilities",
+        "/workspace/7/api/capabilities.json",
+        "/workspace/7/api/0.6/capabilities",
+    ],
+)
+async def test_capabilities_needs_no_auth_under_every_spelling(client, mock_osm, path):
+    response = await client.get(path)
+
+    assert response.status_code == 200
+    # The workspace prefix is stripped before proxying; upstream only ever sees the OSM path.
+    assert mock_osm.last_request.url.path == path.replace("/workspace/7", "")
+
+
 # --- auth / tenant gating --------------------------------------------------
 
 

tests/unit/test_security.py

@@ -538,6 +538,46 @@ async def test_basic_is_accepted_on_proxied_osm_paths(path):
     assert creds.credentials == "a.valid.jwt"
 
 
+# A client that only speaks Basic never sends credentials until it has been challenged, so the
+# challenge has to name a scheme it can act on. JOSM reports "failed to initialize communication"
+# against a Bearer challenge and never gets as far as sending its token.
+
+
+async def test_missing_credentials_on_an_osm_path_challenge_basic():
+    with pytest.raises(HTTPException) as excinfo:
+        await sec.security(_request_with_auth(None, path="/api/0.6/map"))
+
+    assert excinfo.value.status_code == 401
+    assert excinfo.value.headers is not None
+    assert excinfo.value.headers["WWW-Authenticate"].startswith("Basic")
+
+
+async def test_missing_credentials_on_a_prefixed_osm_path_challenge_basic():
+    with pytest.raises(HTTPException) as excinfo:
+        await sec.security(_request_with_auth(None, path="/workspace/7/api/0.6/map"))
+
+    assert excinfo.value.status_code == 401
+    assert excinfo.value.headers["WWW-Authenticate"].startswith("Basic")
+
+
+async def test_missing_credentials_on_native_api_paths_still_challenge_bearer():
+    # The /api/v1 surface does not accept Basic, so it must not invite it either.
+    with pytest.raises(HTTPException) as excinfo:
+        await sec.security(_request_with_auth(None, path="/api/v1/workspaces"))
+
+    assert excinfo.value.status_code in (401, 403)
+    headers = excinfo.value.headers or {}
+    assert not str(headers.get("WWW-Authenticate", "")).startswith("Basic")
+
+
+async def test_unusable_basic_credentials_are_rechallenged_with_basic():
+    with pytest.raises(HTTPException) as excinfo:
+        await sec.security(_request_with_auth("Basic !!!not-base64!!!", path="/api/0.6/map"))
+
+    assert excinfo.value.status_code == 401
+    assert excinfo.value.headers["WWW-Authenticate"].startswith("Basic")
+
+
 async def test_bearer_is_accepted_on_native_api_paths():
     # The scoping restricts Basic only; Bearer works everywhere as before.
     creds = await sec.security(