feat: route checks honour the route verb

.github/workflows/tests-deploy-docker.yml

@@ -549,6 +549,10 @@ jobs:
           test_req admin "-u usr:password" 403
           test_req admin "-u admin:admin" 200
 
+          # A method that no route of the path lists: HaRP answers 405 itself, with HEAD added to the GET route's verb
+          curl -sS -o /dev/null -D - -X POST http://127.0.0.1/exapps/app-skeleton-python/public | tr -d '\r' \
+            | grep -ix 'allow: GET, HEAD' >/dev/null || error
+
           # A chunked request with credentials, sent to HaRP directly because the nginx in front buffers bodies
           code=$(printf 'body' | curl -sS -m 10 -o /dev/null -w "%{http_code}" -u usr:password -X GET \
             -H "Transfer-Encoding: chunked" --data-binary @- http://127.0.0.1:8780/exapps/app-skeleton-python/user)

README.md

@@ -717,6 +717,8 @@ image has to be present on every node that can run the pod.
 
     > **Note:** On Alpine Linux, `apk add frp` also installs the FRP client, but the Alpine package can be several releases behind the version above.
 
+5. List in the `<verb>` of each route in your `info.xml` every HTTP method that route serves, separated by commas (e.g. `GET,POST`). As in AppAPI's PHP proxy, the first route whose URL and verb both match a request decides its access level. Unlike the PHP proxy, HaRP allows `HEAD` wherever `GET` is listed. It answers a method that no route of the path lists with `405 Method Not Allowed`. HaRP 0.4.7 and older, and HaRP with an AppAPI version that does not send the verbs, match routes by URL only: there the first route whose URL matches decides for every method, so use different URLs where methods need different access levels.
+
 That's it! Your ExApp is now adapted to Nextcloud 32.
 
 ## Nextcloud 32: Migrating Existing ExApps from DSP to HaRP

haproxy.cfg.template

@@ -47,6 +47,7 @@ frontend ex_apps
     http-request silent-drop if { var(txn.exapps.bad_request) -m int eq 1 }
     http-request return status 401 content-type text/plain string "401 Unauthorized" if { var(txn.exapps.unauthorized) -m int eq 1 }
     http-request return status 403 content-type text/plain string "403 Forbidden" if { var(txn.exapps.forbidden) -m int eq 1 }
+    http-request return status 405 content-type text/plain string "405 Method Not Allowed" hdr Allow "%[var(txn.exapps.allow)]" if { var(txn.exapps.method_not_allowed) -m int eq 1 }
     http-request return status 404 content-type text/plain string "404 Not Found" if { var(txn.exapps.not_found) -m int eq 1 }
     http-request return status 503 content-type text/plain string "503 Service Unavailable (HaRP)" if !{ var(txn.exapps.backend) -m found }
     use_backend %[var(txn.exapps.backend)]
@@ -62,6 +63,7 @@ _HTTPS_FRONTEND_     filter spoe engine exapps-spoe config /etc/haproxy/spoe-age
 _HTTPS_FRONTEND_     http-request silent-drop if { var(txn.exapps.bad_request) -m int eq 1 }
 _HTTPS_FRONTEND_     http-request return status 401 content-type text/plain string "401 Unauthorized" if { var(txn.exapps.unauthorized) -m int eq 1 }
 _HTTPS_FRONTEND_     http-request return status 403 content-type text/plain string "403 Forbidden" if { var(txn.exapps.forbidden) -m int eq 1 }
+_HTTPS_FRONTEND_     http-request return status 405 content-type text/plain string "405 Method Not Allowed" hdr Allow "%[var(txn.exapps.allow)]" if { var(txn.exapps.method_not_allowed) -m int eq 1 }
 _HTTPS_FRONTEND_     http-request return status 404 content-type text/plain string "404 Not Found" if { var(txn.exapps.not_found) -m int eq 1 }
 _HTTPS_FRONTEND_     http-request return status 503 content-type text/plain string "503 Service Unavailable (HaRP)" if !{ var(txn.exapps.backend) -m found }
 _HTTPS_FRONTEND_     use_backend %[var(txn.exapps.backend)]

haproxy_agent.py

@@ -342,6 +342,7 @@ class AccessLevel(IntEnum):
 
 class ExAppRoute(BaseModel):
     url: str = Field(..., description="REGEX for URL, e.g. r'^/private/.*'")
+    verb: str | None = Field(None, description="HTTP methods, e.g. 'GET,POST'. Older AppAPI versions do not send it.")
     access_level: AccessLevel = Field(..., description="ADMIN(2), USER(1), or PUBLIC(0)")
     bruteforce_protection: list[int] = Field(
         [], description="List with HTTP statuses to trigger the bruteforce protection."
@@ -355,6 +356,19 @@ def encode_bruteforce_protection_values(self) -> Self:
         self.str_bruteforce_protection = json.dumps(self.bruteforce_protection) if self.bruteforce_protection else ""
         return self
 
+    def methods(self) -> frozenset[str] | None:
+        """The methods the route lists, or None (every method) when AppAPI did not send a verb."""
+        if self.verb is None:
+            return None
+        return frozenset(token.upper() for token in re.split(r"[\s,]+", self.verb) if token)
+
+    def accepts(self, method: str) -> bool:
+        """HEAD is accepted wherever GET is. Methods are case-sensitive: a backend may treat `get` as not GET."""
+        methods = self.methods()
+        if methods is None:
+            return True
+        return method in methods or (method == "HEAD" and "GET" in methods)
+
 
 class ExApp(BaseModel):
     exapp_token: str = Field(..., repr=False)  # the ExApp's secret; records are written to the log
@@ -677,20 +691,57 @@ async def get_session(pass_cookie: str) -> NcUser | None:
 ###############################################################################
 
 
+def find_route(
+    routes: list[ExAppRoute], target_path: str, method: str, exapp_id: str
+) -> tuple[ExAppRoute | None, list[ExAppRoute]]:
+    """The first route matching both path and method, as in AppAPI's PHP proxy, or None and the routes of the path.
+
+    A request without a method (a custom spoe-agent.conf without `method`) gets the first route of the path.
+    """
+    path_routes = []
+    for route in routes:
+        try:
+            if not re.match(route.url, target_path):
+                continue
+        except re.error as err:
+            LOGGER.error("Invalid regex %s in route for exapp %s: %s", route.url, exapp_id, err)
+            continue
+        if not method or route.accepts(method):
+            return route, path_routes
+        path_routes.append(route)
+    return None, path_routes
+
+
+def allowed_methods(routes: list[ExAppRoute]) -> str:
+    """The `Allow` header of a 405: the methods of these routes, with HEAD wherever GET is."""
+    methods = set().union(*(route.methods() or frozenset() for route in routes))
+    if "GET" in methods:
+        methods.add("HEAD")
+    return ", ".join(sorted(methods))
+
+
 @SPOA_AGENT.handler("exapps_msg")
 async def exapps_msg(
-    path: str, headers: str, client_ip: ipaddress.IPv4Address | ipaddress.IPv6Address, pass_cookie: str
+    path: str,
+    headers: str,
+    client_ip: ipaddress.IPv4Address | ipaddress.IPv6Address,
+    pass_cookie: str,
+    method: str = "",
 ) -> AckPayload:
     """An exception would kill the SPOA connection; a zero-action ACK hangs the stream until `timeout processing`."""
     try:
-        return await _exapps_msg(path, headers, client_ip, pass_cookie)
+        return await _exapps_msg(path, headers, client_ip, pass_cookie, method)
     except Exception:
         LOGGER.exception("Unhandled error while processing request to path=%s", path)
         return AckPayload().set_txn_var("agent_error", 1)
 
 
 async def _exapps_msg(
-    path: str, headers: str, client_ip: ipaddress.IPv4Address | ipaddress.IPv6Address, pass_cookie: str
+    path: str,
+    headers: str,
+    client_ip: ipaddress.IPv4Address | ipaddress.IPv6Address,
+    pass_cookie: str,
+    method: str = "",
 ) -> AckPayload:
     reply = AckPayload()
     request_headers = parse_headers(headers)
@@ -783,6 +834,17 @@ async def _exapps_msg(
         await record_ip_failure(client_ip_str)
         return reply.set_txn_var("bad_request", 1)
     else:
+        route, path_routes = find_route(exapp_record.routes, target_path, method, exapp_id)
+        if route is None and path_routes:
+            # Not counted toward the IP ban: link checkers, monitors and CORS preflights send such requests. So it is
+            # decided before the user lookup, or random credentials would get checked by Nextcloud without limit.
+            # not_found is also set, so that a custom /haproxy.cfg without the 405 rule answers 404.
+            allow = allowed_methods(path_routes)
+            LOGGER.info("No route of %s allows %s %s; allowed: %s", exapp_id, method, target_path, allow)
+            reply = reply.set_txn_var("allow", allow)
+            reply = reply.set_txn_var("method_not_allowed", 1)
+            return reply.set_txn_var("not_found", 1)
+
         nc_user = None
         if pass_cookie or "authorization" in request_headers:
             # We also pass requests with "authorization" to the Nextcloud to support App Passwords and Basic Auth.
@@ -799,24 +861,14 @@ async def _exapps_msg(
                     LOGGER.exception("Failed to fetch user info from Nextcloud", exc_info=e)
                     return reply.set_txn_var("unauthorized", 1)
 
-        for route in exapp_record.routes:
-            try:
-                if re.match(route.url, target_path):
-                    if route.access_level == AccessLevel.PUBLIC:
-                        exapp_route_bruteforce_protection = route.str_bruteforce_protection
-                        route_allowed = True
-                        break
-
-                    if nc_user and route.access_level <= nc_user.access_level:
-                        exapp_route_bruteforce_protection = route.str_bruteforce_protection
-                        route_allowed = True
-                        break
-
-                    LOGGER.error("Access denied for '%s' to %s", nc_user.user_id if nc_user else "", target_path)
-                    await record_ip_failure(client_ip_str)
-                    return reply.set_txn_var("forbidden", 1)
-            except re.error as err:
-                LOGGER.error("Invalid regex %s in route for exapp %s: %s", route.url, exapp_id, err)
+        if route is not None:
+            if route.access_level == AccessLevel.PUBLIC or (nc_user and route.access_level <= nc_user.access_level):
+                exapp_route_bruteforce_protection = route.str_bruteforce_protection
+                route_allowed = True
+            else:
+                LOGGER.error("Access denied for '%s' to %s", nc_user.user_id if nc_user else "", target_path)
+                await record_ip_failure(client_ip_str)
+                return reply.set_txn_var("forbidden", 1)
 
     if not route_allowed:
         LOGGER.error("No defined route for handling %s", target_path)

spoe-agent.conf

@@ -13,7 +13,7 @@ spoe-agent exapps-agent
 
 spoe-message exapps_msg
     event on-frontend-http-request
-    args path=path headers=req.hdrs client_ip=src pass_cookie=req.cook('oc_sessionPassphrase')
+    args path=path headers=req.hdrs client_ip=src pass_cookie=req.cook('oc_sessionPassphrase') method=method
 
 [exapps-bruteforce-protection-spoe]
 spoe-agent exapps-agent

tests/unit/test_route_verbs.py

@@ -0,0 +1,189 @@
+# SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+# SPDX-License-Identifier: AGPL-3.0-or-later
+"""Route verbs: first match by path and method, HEAD implied by GET, 405 with Allow for a method no route lists."""
+
+import asyncio
+import ipaddress
+
+from haproxyspoa.payloads.ack import AckPayload
+
+import haproxy_agent as agent
+
+IP = "192.0.2.7"
+
+SAME_URL = [  # a public GET route listed before an admin POST route of the same URL
+    {"url": "^/x$", "verb": "GET", "access_level": 0},
+    {"url": "^/x$", "verb": "POST", "access_level": 2},
+]
+
+
+def txn_vars(reply: AckPayload) -> dict:
+    return {action.name: action.value for action in reply.actions}
+
+
+def exapp(*routes) -> agent.ExApp:
+    return agent.ExApp.model_validate(
+        {"exapp_token": "t", "exapp_version": "1", "host": "127.0.0.1", "port": 23000, "routes": list(routes)}
+    )
+
+
+def decide(monkeypatch, record, method, path, level=None, lookups=None, headers="") -> dict:
+    """The txn vars the agent sets for this request; `level` sends credentials of a user with that access level.
+
+    `method=None` calls the handler without it, as for a custom spoe-agent.conf without `method`.
+    """
+
+    async def fetch(_app_id):
+        return record
+
+    async def user(_app_id, _headers):
+        if lookups is not None:
+            lookups.append(path)
+        return agent.NcUser(user_id="u", access_level=level)
+
+    monkeypatch.setattr(agent, "_get_or_fetch_exapp", fetch)
+    monkeypatch.setattr(agent, "nc_get_user", user)
+    if level is not None:
+        headers = "authorization: Basic eDp5"
+    args = {"path": f"/exapps/app{path}", "headers": headers, "client_ip": ipaddress.ip_address(IP), "pass_cookie": ""}
+    if method is not None:
+        args["method"] = method
+    return txn_vars(asyncio.run(agent.exapps_msg(**args)))  # the handler, called the way haproxyspoa calls it
+
+
+def outcome(found: dict) -> str:
+    if "backend" in found:
+        return "ALLOW"
+    for name, code in (("method_not_allowed", "405"), ("forbidden", "403"), ("not_found", "404")):
+        if found.get(name) == 1:
+            return code
+    return repr(found)
+
+
+def request(monkeypatch, record, method, path, level=None) -> str:
+    return outcome(decide(monkeypatch, record, method, path, level))
+
+
+def strikes():
+    return len(agent.BLACKLIST_CACHE.get(IP, []))
+
+
+def test_first_match_by_path_and_method(monkeypatch):
+    assert request(monkeypatch, exapp(*SAME_URL), "GET", "/x") == "ALLOW"
+    assert request(monkeypatch, exapp(*SAME_URL), "POST", "/x") == "403"
+    assert strikes() == 1
+    assert request(monkeypatch, exapp(*SAME_URL), "POST", "/x", level=2) == "ALLOW"
+
+
+def test_bruteforce_protection_comes_from_the_deciding_route(monkeypatch):
+    record = exapp(
+        {"url": "^/login$", "verb": "GET", "access_level": 0},
+        {"url": "^/login$", "verb": "POST", "access_level": 0, "bruteforce_protection": [401]},
+    )
+    found = decide(monkeypatch, record, "GET", "/login")
+    assert found["backend"] == "ex_apps_backend"
+    assert "statuses_to_trigger_bp" not in found
+    found = decide(monkeypatch, record, "POST", "/login")
+    assert found["backend"] == "ex_apps_backend_w_bruteforce"
+    assert found["statuses_to_trigger_bp"] == "[401]"
+
+
+def test_unlisted_method_is_405_with_allow_and_no_ban_strike(monkeypatch):
+    other_url = {"url": "^/other$", "verb": "PUT", "access_level": 0}  # not part of the Allow header of /x
+    found = decide(monkeypatch, exapp(*SAME_URL, other_url), "DELETE", "/x")
+    assert found["method_not_allowed"] == 1
+    assert found["not_found"] == 1  # a custom /haproxy.cfg without the 405 rule answers 404
+    assert found["allow"] == "GET, HEAD, POST"
+    for _ in range(20):  # a link checker or monitor cannot get its address banned this way
+        assert request(monkeypatch, exapp(*SAME_URL), "PATCH", "/x") == "405"
+    assert strikes() == 0
+    assert request(monkeypatch, exapp(*SAME_URL), "GET", "/x") == "ALLOW"
+
+
+def test_unlisted_method_is_refused_before_the_user_lookup(monkeypatch):
+    # Not counted toward the ban, so it must not let a client make Nextcloud check credentials without limit.
+    lookups = []
+    found = decide(monkeypatch, exapp(*SAME_URL), "DELETE", "/x", level=1, lookups=lookups)
+    assert outcome(found) == "405"
+    assert lookups == []
+    assert outcome(decide(monkeypatch, exapp(*SAME_URL), "POST", "/x", level=2, lookups=lookups)) == "ALLOW"
+    assert lookups == ["/x"]
+
+
+def test_head_is_implied_by_get(monkeypatch):
+    assert request(monkeypatch, exapp(*SAME_URL), "HEAD", "/x") == "ALLOW"
+    only_post = exapp({"url": "^/y$", "verb": "POST", "access_level": 0})
+    assert request(monkeypatch, only_post, "HEAD", "/y") == "405"
+
+
+def test_options_is_not_special(monkeypatch):
+    assert request(monkeypatch, exapp(*SAME_URL), "OPTIONS", "/x") == "405"
+    listed = exapp({"url": "^/y$", "verb": "POST,OPTIONS", "access_level": 0})
+    assert request(monkeypatch, listed, "OPTIONS", "/y") == "ALLOW"
+
+
+def test_route_with_several_verbs(monkeypatch):
+    record = exapp({"url": "^/z$", "verb": "GET,POST", "access_level": 0})
+    assert request(monkeypatch, record, "POST", "/z") == "ALLOW"
+    assert request(monkeypatch, record, "PUT", "/z") == "405"
+
+
+def test_verb_as_appapi_before_35_stores_it(monkeypatch):
+    # AppAPI before 35 stores the text of <verb> as it is in info.xml, line breaks and case included.
+    record = exapp({"url": "^/z$", "verb": "\n      get, POST\n    ", "access_level": 0})
+    assert request(monkeypatch, record, "GET", "/z") == "ALLOW"
+    assert request(monkeypatch, record, "POST", "/z") == "ALLOW"
+    assert decide(monkeypatch, record, "PUT", "/z")["allow"] == "GET, HEAD, POST"
+
+
+def test_method_is_case_sensitive(monkeypatch):
+    assert request(monkeypatch, exapp(*SAME_URL), "get", "/x") == "405"
+
+
+def test_methods_are_compared_as_tokens(monkeypatch):
+    record = exapp({"url": "^/z$", "verb": "GET,POST", "access_level": 0})
+    for method in ("ET", "OS", "GETPOST"):  # a client may send any method token
+        assert request(monkeypatch, record, method, "/z") == "405"
+
+
+def test_route_without_verb_matches_every_method(monkeypatch):
+    older_appapi = exapp({"url": "^/x$", "access_level": 0})
+    assert request(monkeypatch, older_appapi, "DELETE", "/x") == "ALLOW"
+
+
+def test_request_without_method_keeps_the_first_path_match(monkeypatch):
+    # A custom spoe-agent.conf without `method`: the first route of the path decides, as before.
+    assert request(monkeypatch, exapp(*SAME_URL), None, "/x") == "ALLOW"
+    assert request(monkeypatch, exapp(*reversed(SAME_URL)), None, "/x") == "403"
+
+
+def test_request_signed_by_appapi_skips_the_route_check(monkeypatch):
+    signed = "\n".join(
+        [
+            "harp-shared-key: unit-test-shared-key",
+            "authorization-app-api: dXNlcjpzZWNyZXQ=",
+            "ex-app-version: 1",
+            "ex-app-id: app",
+            "ex-app-host: 127.0.0.1",
+            "ex-app-port: 23000",
+        ]
+    )
+    assert outcome(decide(monkeypatch, exapp(*SAME_URL), "DELETE", "/x", headers=signed)) == "ALLOW"
+    assert outcome(decide(monkeypatch, exapp(*SAME_URL), "PUT", "/enabled", headers=signed)) == "ALLOW"
+
+
+def test_public_route_still_tells_the_exapp_who_the_user_is(monkeypatch):
+    found = decide(monkeypatch, exapp(*SAME_URL), "GET", "/x", level=1)
+    assert found["exapp_token"] == agent.b64encode(b"u:t")
+
+
+def test_unknown_path_is_still_404_and_counted(monkeypatch):
+    assert request(monkeypatch, exapp(*SAME_URL), "GET", "/nope") == "404"
+    assert strikes() == 1
+
+
+def test_route_with_invalid_regex_is_not_a_path_match(monkeypatch):
+    # AppAPI does not check route regexes, and Python's re rejects some PCRE patterns.
+    record = exapp({"url": "^/x(", "verb": "PUT", "access_level": 0})
+    assert request(monkeypatch, record, "GET", "/x") == "404"
+    assert strikes() == 1