Merge branch 'internetstandards:main' into main

Author: apio-sys

checks/tasks/dispatcher.py

@@ -44,6 +44,7 @@ def check_results(url, checks_registry, remote_addr, get_results=False) -> Probe
     If the task is not registered and the user has not passed the task limit,
     start the task.
     """
+
     url = url.lower()
     cache_id = redis_id.dom_task.id.format(url, checks_registry.name)
     cache_ttl = redis_id.dom_task.ttl
@@ -53,7 +54,8 @@ def check_results(url, checks_registry, remote_addr, get_results=False) -> Probe
         log.debug("No task found for task in cache. Creating a new redis task.")
         # Task is not yet available (not running AND not in cache)
         # Limit concurrent task launches per IP
-        req_limit_id = redis_id.req_limit.id.format(remote_addr)
+        # replace colons with dots for IPv6 addresses as colons are used as namespaces
+        req_limit_id = redis_id.req_limit.id.format(remote_addr.replace(":", ".") if remote_addr else None)
         req_limit_ttl = redis_id.req_limit.ttl
         if user_limit_exceeded(req_limit_id):
             log.debug("User limit exceeded. Too many requests from this IP. Blocked.")

docker/compose.development.yaml

@@ -17,8 +17,17 @@ services:
     environment:
       - INTERNETNL_DOMAINNAME
 
-  app:
+  webserver:
+    build:
+      context: ..
+      dockerfile: docker/webserver.Dockerfile
+    develop:
+      watch:
+        # auto rebuild/reload when config files change
+        - path: ./webserver/
+          action: rebuild
 
+  app:
     develop:
       watch:
         # auto rebuild/reload when CSS/JS changes

docker/webserver/nginx_templates/app.conf.template

@@ -178,6 +178,9 @@ server {
 
         proxy_set_header REMOTE-USER $remote_user;
 
+        # forward client IP for rate limiting on tasks (see `redis_id.py` `req_limit`)
+        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
+
         include /etc/nginx/conf.d/basic_auth.include;
         # set proxy_pass argument for 'app' container as variable, this way nginx doesn't fail when 'app' is unresolvable at startup
         set $app http://app:8080;
@@ -217,6 +220,8 @@ server {
         # pass host for Django's allowed_hosts
         proxy_set_header Host $host;
         proxy_set_header X-Forwarded-Proto $scheme;
+        # forward client IP for rate limiting on tasks (see `redis_id.py` `req_limit`)
+        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
         # set proxy_pass argument for 'app' container as variable, this way nginx doesn't fail when 'app' is unresolvable at startup
         set $app http://app:8080;
         proxy_pass $app;

integration_tests/integration/test_website.py

@@ -73,3 +73,20 @@ def test_ipv6_ns_with_bad_connectivity(page, app_url, unique_id):
 
     # but some of them are not resolvable
     expect(page.get_by_text("Not all name servers that have an IPv6 address are reachable over IPv6."))
+
+
+def test_rate_limit(page, app_url, test_domain, docker_compose_exec):
+    """Test if correct rate limit keys are created when starting a test."""
+
+    test_domain = "www." + test_domain
+
+    page.goto(app_url)
+
+    page.locator("#web-url").fill(test_domain)
+    page.locator("section.websitetest button").click()
+
+    rate_limit_redis_entry = docker_compose_exec("redis", "redis-cli keys dom:req_limit:*").decode("utf8").strip()
+    assert rate_limit_redis_entry, "there should be a redis entry for rate limiting"
+    assert not rate_limit_redis_entry.endswith(
+        "None"
+    ), "there should be no rate limit key created with `None` as IP address"