captive portal: acknowledge on Continue, not on view

iOS fetches the portal page in the background before opening the sheet,
then re-probes after the sheet's own navigation.  Acknowledging on view
made that re-probe return "Success" as the sheet's content.  Phones only
re-check the network (and offer "Done") after a full-page navigation, so
the welcome page now has a Continue button leading to /portal/continue,
which is the only place a client is acknowledged.  The Continue page says
to tap Done and links to WROLPi and the Controller.

Also hijack clients3.google.com (older Android probe host).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

controller/api/captive_portal.py

@@ -14,6 +14,7 @@ from fastapi.templating import Jinja2Templates
 
 from controller.lib.admin import get_hotspot_device, get_hotspot_ssid
 from controller.lib.captive_portal import (
+    PORTAL_CONTINUE_PATH,
     PORTAL_PATH,
     PROBE_PATHS,
     PROBE_SUCCESS_RESPONSES,
@@ -34,21 +35,35 @@ templates = Jinja2Templates(directory=str(Path(__file__).parent.parent / "templa
 NO_STORE = {"Cache-Control": "no-store"}
 
 
-@router.get(PORTAL_PATH, include_in_schema=False)
-async def portal(request: Request):
-    """The landing page: this WROLPi's address and links to WROLPi and the Controller."""
+def _render_portal(request: Request, connected: bool):
     if is_docker_mode():
         raise HTTPException(status_code=404)
     ip = get_hotspot_ip(get_hotspot_device())
     if not ip:
         # Hotspot is down; a page with the wrong address is worse than none.
         raise HTTPException(status_code=404)
-    # The phone has seen the address; let its next probe succeed so it offers "Done".
-    acknowledge_client(request.client.host if request.client else None)
-    context = {"ip": ip, "ssid": get_hotspot_ssid()}
+    context = {"ip": ip, "ssid": get_hotspot_ssid(), "connected": connected,
+               "continue_path": PORTAL_CONTINUE_PATH}
     return templates.TemplateResponse(request, "portal.html", context, headers=NO_STORE)
 
 
+@router.get(PORTAL_PATH, include_in_schema=False)
+async def portal(request: Request):
+    """The welcome page: this WROLPi's address and a Continue button."""
+    return _render_portal(request, connected=False)
+
+
+@router.get(PORTAL_CONTINUE_PATH, include_in_schema=False)
+async def portal_continue(request: Request):
+    """
+    The page after Continue.  The phone re-probes after this navigation; the client is now
+    acknowledged so that probe succeeds and the sheet offers "Done".
+    """
+    response = _render_portal(request, connected=True)  # 404s first if the hotspot is down
+    acknowledge_client(request.client.host if request.client else None)
+    return response
+
+
 async def probe(request: Request):
     """
     A connectivity probe.  Anything but the expected reply makes the phone show the portal, so

controller/lib/captive_portal.py

@@ -19,6 +19,7 @@ PROBE_HOSTS = (
     'captive.g.aaplimg.com',  # the CDN name captive.apple.com aliases to
     'connectivitycheck.gstatic.com',  # Android
     'connectivitycheck.android.com',  # Android (older)
+    'clients3.google.com',  # Android (older)
     'www.msftconnecttest.com',  # Windows 10+
     'www.msftncsi.com',  # Windows 7/8
     'detectportal.firefox.com',  # Firefox
@@ -55,6 +56,9 @@ INCLUDE_FILE = DNSMASQ_SHARED_DIR / '50-wrolpi-captive-portal.conf'
 DEFAULT_HOTSPOT_IP = '10.42.0.1'
 
 PORTAL_PATH = '/portal'
+# A full-page navigation the user makes from the portal.  Phones only re-probe (and offer
+# "Done") after a navigation, so the client is acknowledged here, never on viewing the page.
+PORTAL_CONTINUE_PATH = '/portal/continue'
 
 
 def is_captive_portal_enabled() -> bool:
@@ -135,13 +139,13 @@ def dnsmasq_config_ip() -> Optional[str]:
     return None
 
 
-# Clients (by hotspot IP) which have loaded the portal page.  Their later probes get the success
-# reply.  Reset whenever the hotspot starts so a new session shows the page again.
+# Clients (by hotspot IP) which tapped Continue on the portal page.  Their later probes get the
+# success reply.  Reset whenever the hotspot starts so a new session shows the page again.
 _acknowledged_clients: set[str] = set()
 
 
 def acknowledge_client(ip: str):
-    """The client at `ip` has seen the portal page; its probes now say "online"."""
+    """The client at `ip` tapped Continue on the portal page; its probes now say "online"."""
     if ip:
         _acknowledged_clients.add(ip)
 

controller/templates/portal.html

@@ -49,18 +49,28 @@
     </style>
 </head>
 <body>
+    {% if connected %}
+    <h1>You are connected</h1>
+    <p class="muted">Tap <strong>Done</strong> in the corner of this window to keep using the
+        <strong>{{ ssid }}</strong> hotspot.</p>
+    {% else %}
     <h1>This is your WROLPi</h1>
     <p class="muted">You are connected to the <strong>{{ ssid }}</strong> hotspot.</p>
+    {% endif %}
 
     <p>The address of this WROLPi is</p>
     <div class="address">{{ ip }}</div>
 
+    {% if connected %}
     <a class="btn" href="https://{{ ip }}/">Open WROLPi</a>
     <a class="btn secondary" href="https://{{ ip }}/controller/">Open the Controller</a>
+    {% else %}
+    {# A real navigation: phones only re-check the network (and offer "Done") after one. #}
+    <a class="btn" href="{{ continue_path }}">Continue</a>
+    {% endif %}
 
     <div class="note muted">
-        <p>Tap <strong>Done</strong> to stay connected, then open your browser to
-            <strong>https://{{ ip }}</strong>.</p>
+        <p>Open your browser to <strong>https://{{ ip }}</strong> to use WROLPi.</p>
         <p>Your browser will warn about the certificate the first time. That is expected for an
             offline WROLPi; continue to the site.</p>
         <p>This hotspot does not provide Internet access.</p>

controller/test/test_captive_portal.py

@@ -12,6 +12,7 @@ from controller.lib.captive_portal import (
     DEFAULT_HOTSPOT_IP,
     FRIENDLY_NAME,
     PROBE_HOSTS,
+    PORTAL_CONTINUE_PATH,
     PROBE_PATHS,
     PROBE_SUCCESS_RESPONSES,
     acknowledge_client,
@@ -169,12 +170,37 @@ class TestProbeRedirects:
 
 
 class TestAcknowledgedClients:
-    """Once a client has seen the portal page, its probes say "online" so the OS offers Done."""
+    """Once a client taps Continue, its probes say "online" so the OS offers Done."""
 
-    @pytest.mark.parametrize("path", PROBE_PATHS)
-    def test_probe_succeeds_after_portal_seen(self, test_client, path):
+    def test_viewing_the_portal_does_not_acknowledge(self, test_client):
+        """iOS fetches the page in the background before showing it; only a tap counts."""
         with mock.patch("controller.api.captive_portal.get_hotspot_ip", return_value='10.42.0.1'):
             assert test_client.get('/portal').status_code == 200
+        assert not is_client_acknowledged('testclient')
+        assert test_client.get('/hotspot-detect.html', follow_redirects=False).status_code == 302
+
+    def test_continue_page(self, test_client):
+        with mock.patch("controller.api.captive_portal.get_hotspot_ip", return_value='10.42.0.1'):
+            response = test_client.get(PORTAL_CONTINUE_PATH)
+        assert response.status_code == 200
+        assert response.headers['cache-control'] == 'no-store'
+        assert 'Done' in response.text
+        assert '10.42.0.1' in response.text
+        assert 'https://10.42.0.1/' in response.text
+        assert is_client_acknowledged('testclient')
+
+    def test_continue_404_when_hotspot_down(self, test_client):
+        with mock.patch("controller.api.captive_portal.get_hotspot_ip", return_value=None):
+            assert test_client.get(PORTAL_CONTINUE_PATH).status_code == 404
+        assert not is_client_acknowledged('testclient')
+
+    def test_continue_404_in_docker(self, test_client_docker_mode):
+        assert test_client_docker_mode.get(PORTAL_CONTINUE_PATH).status_code == 404
+
+    @pytest.mark.parametrize("path", PROBE_PATHS)
+    def test_probe_succeeds_after_continue(self, test_client, path):
+        with mock.patch("controller.api.captive_portal.get_hotspot_ip", return_value='10.42.0.1'):
+            assert test_client.get(PORTAL_CONTINUE_PATH).status_code == 200
         response = test_client.get(path, follow_redirects=False)
         status, media_type, body = PROBE_SUCCESS_RESPONSES[path]
         assert response.status_code == status
@@ -190,12 +216,6 @@ class TestAcknowledgedClients:
         assert response.status_code == 200
         assert '<BODY>Success</BODY>' in response.text
 
-    def test_portal_404_does_not_acknowledge(self, test_client):
-        """A hotspot-down 404 is not the client seeing the address."""
-        with mock.patch("controller.api.captive_portal.get_hotspot_ip", return_value=None):
-            assert test_client.get('/portal').status_code == 404
-        assert test_client.get('/generate_204', follow_redirects=False).status_code == 302
-
     def test_other_clients_still_redirected(self, test_client):
         acknowledge_client('10.42.0.99')
         assert not is_client_acknowledged('testclient')
@@ -220,8 +240,7 @@ class TestPortalPage:
         assert response.headers['cache-control'] == 'no-store'
         body = response.text
         assert '10.42.0.1' in body
-        assert 'https://10.42.0.1/' in body
-        assert 'https://10.42.0.1/controller/' in body
+        assert f'href="{PORTAL_CONTINUE_PATH}"' in body
         assert 'WROLPi' in body  # the SSID
 
     def test_404_when_hotspot_down(self, test_client):