Fixed incorrect URL generation in bootstrap API endpoint that server config files (#599)

* Added function to construct netloc by parsing request headers before defaulting to parsing the Request object's properties
* Removed templates and endpoints that are no longer needed
* Replaced 'url_for()' function with calculated 'proxy_path' value instead; allows correct URL generation when server is behind NGINX server and accessed via Ingress or LoadBalancer, and when server is standalone
* Try testing '/session_info/connections' instead now that '/session_info/' has been removed

Author: tieneupin

src/murfey/server/api/bootstrap.py

@@ -70,6 +70,32 @@ def _sanitise_str(input: str) -> str:
     return input_clean
 
 
+def resolve_netloc(request: Request) -> str:
+    """
+    Helper function to construct the correct netloc (hostname[:port]) to use based on
+    the request received. It will prioritise parsing the request headers for the host,
+    port, protocol and using them to construct the netloc before defaulting to parsing
+    the FastAPI Request object to do so.
+    """
+
+    # Prefer headers added by reverse proxies
+    host = request.headers.get("X-Forwarded-Host", request.url.hostname)
+    port = request.headers.get(
+        "X-Forwarded-Port", str(request.url.port) if request.url.port else None
+    )
+    proto = request.headers.get("X-Forwarded-Proto", request.url.scheme)
+
+    # Default ports shouldn't be included; if no ports are found, return just the host
+    if (
+        (proto == "http" and port == "80")
+        or (proto == "https" and port == "443")
+        or not port
+    ):
+        return host
+
+    return f"{host}:{port}"
+
+
 """
 =======================================================================================
 VERSION-RELATED API ENDPOINTS
@@ -126,15 +152,10 @@ def get_bootstrap_instructions(request: Request):
     machine with no internet access.
     """
 
-    # Constructs the netloc (hostname + port) and proxy path depending on if the
-    # request was forwarded via proxy
-    netloc = (
-        f"{request.headers['X-Forwarded-Host']}:{request.headers['X-Forwarded-Port']}"
-        if request.headers.get("X-Forwarded-Host")
-        and request.headers.get("X-Forwarded-Port")
-        else request.url.netloc
-    )
-    # Additional bit in URL path after the netloc caused by the proxy reroute
+    # Check if this is a forwarded request from somewhere else and construct netloc
+    netloc = resolve_netloc(request)
+
+    # Find path to 'bootstrap' router using current URL path
     proxy_path = request.url.path.removesuffix(f"{bootstrap.prefix}/")
 
     return respond_with_template(
@@ -362,12 +383,7 @@ def get_pacman_mirrors(request: Request):
     """
 
     # Check if this is a forwarded request from somewhere else and construct netloc
-    netloc = (
-        f"{request.headers['X-Forwarded-Host']}:{request.headers['X-Forwarded-Port']}"
-        if request.headers.get("X-Forwarded-Host")
-        and request.headers.get("X-Forwarded-Port")
-        else request.url.netloc
-    )
+    netloc = resolve_netloc(request)
 
     # Find path to Rust router using current URL Path
     path_to_router = request.url.path.removesuffix("/config/pacman.d.zip")
@@ -641,12 +657,7 @@ def get_cargo_config(request: Request):
     """
 
     # Check if this is a forwarded request from somewhere else and construct netloc
-    netloc = (
-        f"{request.headers['X-Forwarded-Host']}:{request.headers['X-Forwarded-Port']}"
-        if request.headers.get("X-Forwarded-Host")
-        and request.headers.get("X-Forwarded-Port")
-        else request.url.netloc
-    )
+    netloc = resolve_netloc(request)
 
     # Find path to Rust router using current URL Path
     path_to_router = request.url.path.removesuffix("/cargo/config.toml")
@@ -723,12 +734,7 @@ def get_index_config(request: Request):
     """
 
     # Check if this is a forwarded request from somewhere else and construct netloc
-    netloc = (
-        f"{request.headers['X-Forwarded-Host']}:{request.headers['X-Forwarded-Port']}"
-        if request.headers.get("X-Forwarded-Host")
-        and request.headers.get("X-Forwarded-Port")
-        else request.url.netloc
-    )
+    netloc = resolve_netloc(request)
 
     # Find path to Rust router using current URL Path
     path_to_router = request.url.path.removesuffix("/index/config.json")

src/murfey/server/api/session_info.py

@@ -4,13 +4,12 @@
 from typing import Dict, List, Optional
 
 from fastapi import APIRouter, Depends, Request
-from fastapi.responses import FileResponse, HTMLResponse
+from fastapi.responses import FileResponse
 from pydantic import BaseModel
 from sqlalchemy import func
 from sqlmodel import select
 from werkzeug.utils import secure_filename
 
-import murfey
 import murfey.server.api.websocket as ws
 from murfey.server import _transport_object
 from murfey.server.api import templates
@@ -32,12 +31,7 @@
 from murfey.server.ispyb import get_all_ongoing_visits
 from murfey.server.murfey_db import murfey_db
 from murfey.util import sanitise
-from murfey.util.config import (
-    MachineConfig,
-    get_hostname,
-    get_machine_config,
-    get_microscope,
-)
+from murfey.util.config import MachineConfig, get_machine_config
 from murfey.util.db import (
     ClientEnvironment,
     DataCollection,
@@ -64,20 +58,6 @@
 )
 
 
-# This will be the homepage for a given microscope.
-@router.get("/", response_class=HTMLResponse)
-async def root(request: Request):
-    return templates.TemplateResponse(
-        request=request,
-        name="home.html",
-        context={
-            "hostname": get_hostname(),
-            "microscope": get_microscope(),
-            "version": murfey.__version__,
-        },
-    )
-
-
 @router.get("/health/")
 def health_check(db=ispyb_db):
     conn = db.connection()

src/murfey/templates/activevisits.html

@@ -1,9 +1,9 @@
 <html>
   <head>
-    <title>{{ microscope }}</title>
-    <link href="{{ url_for('static', path='/styles.css') }}" rel="stylesheet" />
+    <title>Visits {{ "({})".format(microscope) if microscope else "" }}</title>
+    <link href="{{ proxy_path }}/static/styles.css" rel="stylesheet" />
     <link
-      href="{{ url_for('images', path='/icon_268.png') }}"
+      href="{{ proxy_path }}/images/icon_268.png"
       rel="icon"
       type="image/png"
     />

src/murfey/templates/base.html

@@ -1,26 +1,20 @@
 <html>
   <head>
     <title>Murfey - {% block title %}{% endblock %}</title>
+    <link href="{{ proxy_path }}/static/styles.css" rel="stylesheet" />
     <link
-      href="{{ proxy_path }}{{ url_for('static', path='/styles.css') }}"
-      rel="stylesheet"
-    />
-    <link
-      href="{{ proxy_path }}{{ url_for('images', path='/icon_268.png') }}"
+      href="{{ proxy_path }}/images/icon_268.png"
       rel="icon"
       type="image/png"
     />
   </head>
   <body>
     <div class="topnav">
       <a href="{{ proxy_path }}/docs">Home</a>
-      <a href="{{ proxy_path }}/bootstrap">Installation Instructions</a>
-      <a href="{{ proxy_path }}/pypi/fastapi">FastAPI (PyPI)</a>
+      <a href="{{ proxy_path }}/bootstrap/">Installation Instructions</a>
+      <a href="{{ proxy_path }}/pypi/fastapi/">FastAPI (PyPI)</a>
       <p style="text-align: right; margin: 8px 10px 0 auto">
-        <img
-          src="{{ proxy_path }}{{ url_for('images', path='/diamond.png') }}"
-          style="height: 25px"
-        />
+        <img src="{{ proxy_path }}/images/diamond.png" style="height: 25px" />
       </p>
     </div>
 
@@ -31,7 +25,8 @@
         For help please contact your local contact or the EM Data Analysis team.
       </p>
       <p style="font-size: 60%">
-        Murfey v{{ version }} running on {{ hostname }} ({{ microscope }})
+        Murfey v{{ version }} running on {{ hostname }} {{
+        "({})".format(microscope) if microscope else "" }}
       </p>
     </div>
   </body>

src/murfey/templates/home.html

@@ -30,10 +30,11 @@ def login(test_user):
 @patch("murfey.server.api.auth.check_user", return_value=True)
 def test_read_main(mock_check, test_user):
     token = login(test_user)
-    response = client.get("/session_info", headers={"Authorization": f"Bearer {token}"})
+    response = client.get(
+        "/session_info/connections", headers={"Authorization": f"Bearer {token}"}
+    )
     assert mock_check.called_once()
     assert response.status_code == 200
-    assert "<title>" in response.text.lower()
 
 
 def test_pypi_proxy():