From 430afb2e8c4c4f6c06492519ebdb5d40deb6aa3c Mon Sep 17 00:00:00 2001 From: Oleksandr Piskun Date: Fri, 23 Oct 2026 07:58:28 +0000 Subject: [PATCH 1/7] feat: route checks honour the route verb Signed-off-by: Oleksandr Piskun --- README.md | 2 + haproxy.cfg.template | 2 + haproxy_agent.py | 94 ++++++++++++++----- spoe-agent.conf | 2 +- tests/unit/test_route_verbs.py | 161 +++++++++++++++++++++++++++++++++ 5 files changed, 239 insertions(+), 22 deletions(-) create mode 100644 tests/unit/test_route_verbs.py diff --git a/README.md b/README.md index b86c3b6..b62be92 100644 --- a/README.md +++ b/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 `` of each route in your `info.xml` every HTTP method that route serves. 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`. If browsers send CORS preflight requests to your ExApp, list `OPTIONS` in a separate `PUBLIC` route, as preflight requests carry no credentials. + That's it! Your ExApp is now adapted to Nextcloud 32. ## Nextcloud 32: Migrating Existing ExApps from DSP to HaRP diff --git a/haproxy.cfg.template b/haproxy.cfg.template index 7718674..7088245 100644 --- a/haproxy.cfg.template +++ b/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)] diff --git a/haproxy_agent.py b/haproxy_agent.py index ced4eda..19ba48b 100644 --- a/haproxy_agent.py +++ b/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.findall(r"[A-Za-z][A-Za-z-]*", self.verb)) + + 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) diff --git a/spoe-agent.conf b/spoe-agent.conf index 830c314..5f25274 100644 --- a/spoe-agent.conf +++ b/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 diff --git a/tests/unit/test_route_verbs.py b/tests/unit/test_route_verbs.py new file mode 100644 index 0000000..683526a --- /dev/null +++ b/tests/unit/test_route_verbs.py @@ -0,0 +1,161 @@ +# 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" + +BUG = [ # 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) -> dict: + """The txn vars the agent sets for this request; `level` sends credentials of a user with that access level.""" + + 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) + headers = "authorization: Basic eDp5" if level is not None else "" + reply = agent.exapps_msg( # the handler, called the way haproxyspoa calls it + path=f"/exapps/app{path}", headers=headers, client_ip=ipaddress.ip_address(IP), pass_cookie="", method=method + ) + return txn_vars(asyncio.run(reply)) + + +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(*BUG), "GET", "/x") == "ALLOW" + assert request(monkeypatch, exapp(*BUG), "POST", "/x") == "403" + assert strikes() == 1 + assert request(monkeypatch, exapp(*BUG), "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): + found = decide(monkeypatch, exapp(*BUG), "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(*BUG), "PATCH", "/x") == "405" + assert strikes() == 0 + assert request(monkeypatch, exapp(*BUG), "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(*BUG), "DELETE", "/x", level=1, lookups=lookups) + assert outcome(found) == "405" + assert lookups == [] + assert outcome(decide(monkeypatch, exapp(*BUG), "POST", "/x", level=2, lookups=lookups)) == "ALLOW" + assert lookups == ["/x"] + + +def test_head_is_implied_by_get(monkeypatch): + assert request(monkeypatch, exapp(*BUG), "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(*BUG), "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_method_is_case_sensitive(monkeypatch): + assert request(monkeypatch, exapp(*BUG), "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(*BUG), "", "/x") == "ALLOW" + assert request(monkeypatch, exapp(*reversed(BUG)), "", "/x") == "403" + + +def test_public_route_still_tells_the_exapp_who_the_user_is(monkeypatch): + found = decide(monkeypatch, exapp(*BUG), "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(*BUG), "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 From 5f905d819cf6a1b7a8102d139bff82988d24fa35 Mon Sep 17 00:00:00 2001 From: Oleksandr Piskun Date: Fri, 23 Oct 2026 10:09:28 +0000 Subject: [PATCH 2/7] docs: an OPTIONS route goes before the other routes of its path Signed-off-by: Oleksandr Piskun --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b62be92..b531305 100644 --- a/README.md +++ b/README.md @@ -717,7 +717,7 @@ 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 `` of each route in your `info.xml` every HTTP method that route serves. 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`. If browsers send CORS preflight requests to your ExApp, list `OPTIONS` in a separate `PUBLIC` route, as preflight requests carry no credentials. +5. List in the `` of each route in your `info.xml` every HTTP method that route serves. 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`. If browsers send CORS preflight requests to your ExApp, list `OPTIONS` only in a `PUBLIC` route placed before the other routes of that path, as preflight requests carry no credentials. That's it! Your ExApp is now adapted to Nextcloud 32. From c860bf6e0b5012b8bd75389441651b8d952a2157 Mon Sep 17 00:00:00 2001 From: Oleksandr Piskun Date: Fri, 23 Oct 2026 10:10:11 +0000 Subject: [PATCH 3/7] fix: split the route verb into whole methods Signed-off-by: Oleksandr Piskun --- haproxy_agent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/haproxy_agent.py b/haproxy_agent.py index 19ba48b..2825e56 100644 --- a/haproxy_agent.py +++ b/haproxy_agent.py @@ -360,7 +360,7 @@ 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.findall(r"[A-Za-z][A-Za-z-]*", self.verb)) + 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.""" From 27e0080aa2738161d919598eefabf050c4dc3bda Mon Sep 17 00:00:00 2001 From: Oleksandr Piskun Date: Fri, 23 Oct 2026 10:41:29 +0000 Subject: [PATCH 4/7] docs: drop the OPTIONS route advice Signed-off-by: Oleksandr Piskun --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b531305..30f4f1c 100644 --- a/README.md +++ b/README.md @@ -717,7 +717,7 @@ 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 `` of each route in your `info.xml` every HTTP method that route serves. 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`. If browsers send CORS preflight requests to your ExApp, list `OPTIONS` only in a `PUBLIC` route placed before the other routes of that path, as preflight requests carry no credentials. +5. List in the `` of each route in your `info.xml` every HTTP method that route serves. 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`. That's it! Your ExApp is now adapted to Nextcloud 32. From 64558e93c6c71d471e4537b67bbf0ac1bff02678 Mon Sep 17 00:00:00 2001 From: Oleksandr Piskun Date: Fri, 23 Oct 2026 11:11:26 +0000 Subject: [PATCH 5/7] docs: route matching on older HaRP and AppAPI Signed-off-by: Oleksandr Piskun --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 30f4f1c..ca78f66 100644 --- a/README.md +++ b/README.md @@ -717,7 +717,7 @@ 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 `` of each route in your `info.xml` every HTTP method that route serves. 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`. +5. List in the `` 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. From bbc2109fb3a97d428086d9d0b0eef1795a83ef4b Mon Sep 17 00:00:00 2001 From: Oleksandr Piskun Date: Fri, 23 Oct 2026 11:11:26 +0000 Subject: [PATCH 6/7] test: the verb as AppAPI before 35 stores it Signed-off-by: Oleksandr Piskun --- tests/unit/test_route_verbs.py | 40 ++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/tests/unit/test_route_verbs.py b/tests/unit/test_route_verbs.py index 683526a..bde1462 100644 --- a/tests/unit/test_route_verbs.py +++ b/tests/unit/test_route_verbs.py @@ -11,7 +11,7 @@ IP = "192.0.2.7" -BUG = [ # a public GET route listed before an admin POST route of the same URL +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}, ] @@ -65,10 +65,10 @@ def strikes(): def test_first_match_by_path_and_method(monkeypatch): - assert request(monkeypatch, exapp(*BUG), "GET", "/x") == "ALLOW" - assert request(monkeypatch, exapp(*BUG), "POST", "/x") == "403" + 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(*BUG), "POST", "/x", level=2) == "ALLOW" + assert request(monkeypatch, exapp(*SAME_URL), "POST", "/x", level=2) == "ALLOW" def test_bruteforce_protection_comes_from_the_deciding_route(monkeypatch): @@ -85,34 +85,34 @@ def test_bruteforce_protection_comes_from_the_deciding_route(monkeypatch): def test_unlisted_method_is_405_with_allow_and_no_ban_strike(monkeypatch): - found = decide(monkeypatch, exapp(*BUG), "DELETE", "/x") + found = decide(monkeypatch, exapp(*SAME_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(*BUG), "PATCH", "/x") == "405" + assert request(monkeypatch, exapp(*SAME_URL), "PATCH", "/x") == "405" assert strikes() == 0 - assert request(monkeypatch, exapp(*BUG), "GET", "/x") == "ALLOW" + 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(*BUG), "DELETE", "/x", level=1, lookups=lookups) + found = decide(monkeypatch, exapp(*SAME_URL), "DELETE", "/x", level=1, lookups=lookups) assert outcome(found) == "405" assert lookups == [] - assert outcome(decide(monkeypatch, exapp(*BUG), "POST", "/x", level=2, lookups=lookups)) == "ALLOW" + 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(*BUG), "HEAD", "/x") == "ALLOW" + 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(*BUG), "OPTIONS", "/x") == "405" + 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" @@ -123,8 +123,16 @@ def test_route_with_several_verbs(monkeypatch): assert request(monkeypatch, record, "PUT", "/z") == "405" +def test_verb_as_appapi_before_35_stores_it(monkeypatch): + # AppAPI before 35 stores the text of 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 request(monkeypatch, record, "PUT", "/z") == "405" + + def test_method_is_case_sensitive(monkeypatch): - assert request(monkeypatch, exapp(*BUG), "get", "/x") == "405" + assert request(monkeypatch, exapp(*SAME_URL), "get", "/x") == "405" def test_methods_are_compared_as_tokens(monkeypatch): @@ -140,17 +148,17 @@ def test_route_without_verb_matches_every_method(monkeypatch): 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(*BUG), "", "/x") == "ALLOW" - assert request(monkeypatch, exapp(*reversed(BUG)), "", "/x") == "403" + assert request(monkeypatch, exapp(*SAME_URL), "", "/x") == "ALLOW" + assert request(monkeypatch, exapp(*reversed(SAME_URL)), "", "/x") == "403" def test_public_route_still_tells_the_exapp_who_the_user_is(monkeypatch): - found = decide(monkeypatch, exapp(*BUG), "GET", "/x", level=1) + 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(*BUG), "GET", "/nope") == "404" + assert request(monkeypatch, exapp(*SAME_URL), "GET", "/nope") == "404" assert strikes() == 1 From ae47f46973e2b2c5227353cb635971afc8e129ea Mon Sep 17 00:00:00 2001 From: Oleksandr Piskun Date: Fri, 23 Oct 2026 11:22:12 +0000 Subject: [PATCH 7/7] test: cover the route verb in the handler call, signed requests and the deploy test Signed-off-by: Oleksandr Piskun --- .github/workflows/tests-deploy-docker.yml | 4 +++ tests/unit/test_route_verbs.py | 42 +++++++++++++++++------ 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/.github/workflows/tests-deploy-docker.yml b/.github/workflows/tests-deploy-docker.yml index 2147edf..5de3d80 100644 --- a/.github/workflows/tests-deploy-docker.yml +++ b/.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) diff --git a/tests/unit/test_route_verbs.py b/tests/unit/test_route_verbs.py index bde1462..18166b7 100644 --- a/tests/unit/test_route_verbs.py +++ b/tests/unit/test_route_verbs.py @@ -27,8 +27,11 @@ def exapp(*routes) -> agent.ExApp: ) -def decide(monkeypatch, record, method, path, level=None, lookups=None) -> dict: - """The txn vars the agent sets for this request; `level` sends credentials of a user with that access level.""" +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 @@ -40,11 +43,12 @@ async def user(_app_id, _headers): monkeypatch.setattr(agent, "_get_or_fetch_exapp", fetch) monkeypatch.setattr(agent, "nc_get_user", user) - headers = "authorization: Basic eDp5" if level is not None else "" - reply = agent.exapps_msg( # the handler, called the way haproxyspoa calls it - path=f"/exapps/app{path}", headers=headers, client_ip=ipaddress.ip_address(IP), pass_cookie="", method=method - ) - return txn_vars(asyncio.run(reply)) + 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: @@ -85,7 +89,8 @@ def test_bruteforce_protection_comes_from_the_deciding_route(monkeypatch): def test_unlisted_method_is_405_with_allow_and_no_ban_strike(monkeypatch): - found = decide(monkeypatch, exapp(*SAME_URL), "DELETE", "/x") + 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" @@ -128,7 +133,7 @@ def test_verb_as_appapi_before_35_stores_it(monkeypatch): 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 request(monkeypatch, record, "PUT", "/z") == "405" + assert decide(monkeypatch, record, "PUT", "/z")["allow"] == "GET, HEAD, POST" def test_method_is_case_sensitive(monkeypatch): @@ -148,8 +153,23 @@ def test_route_without_verb_matches_every_method(monkeypatch): 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), "", "/x") == "ALLOW" - assert request(monkeypatch, exapp(*reversed(SAME_URL)), "", "/x") == "403" + 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):