Extend the access token to every endpoint, not just the web UI

The management token only gated / and /api/* -- every device-facing
/frame/* endpoint (including /frame/image, which serves the actual
photo bytes) stayed open regardless. That was fine while the server
was assumed LAN-only, but defeats the point now that HTTPS exists
specifically to let this sit behind a public hostname.

build_url() (frame_client.c) is the one chokepoint all firmware-side
URL construction already went through, so it now appends ?token= to
every request it builds -- device fetches and QR-embedded links alike
-- instead of that being bolted on per-callsite. Server-side, the
former require_management_token dependency (renamed require_access_token)
is applied to /frame/config, /frame/image, /frame/advance,
/frame/photo-info, /frame/face-labels, and /frame/share/{asset_id} too.
/health stays open -- pure liveness, nothing sensitive to protect.

firmware/README.md

@@ -76,11 +76,11 @@ two-step setup screen:
    portal's config page (`http://192.168.4.1/` by default), for a
    one-scan shortcut once you've joined the AP.
 
-The config page asks for your home WiFi SSID/password and the "Tools
+The config page asks for your home WiFi SSID/password, the "Tools
 Server" address (`host:port` of the [server](../server/) -- **not** your
-Immich server; see below for the `https://` form). Saving reboots the
-device, which then connects to your home network and starts its normal
-fetch/sleep cycle.
+Immich server; see below for the `https://` form), and an optional
+"Access Token" (see below). Saving reboots the device, which then
+connects to your home network and starts its normal fetch/sleep cycle.
 
 ## HTTP vs HTTPS
 
@@ -114,6 +114,18 @@ CA cert never covers a raw IP. Use whatever hostname the certificate's
 SAN list actually covers (e.g. a local DNS/hosts entry pointing at the
 frame's LAN IP, or the same public hostname the proxy is issued for).
 
+## Access token
+
+If the server has `MANAGEMENT_TOKEN` set (see
+[`server/README.md`](../server/README.md)), it requires that same value
+on every request -- the web UI *and* every device-facing request the
+frame itself makes. Paste it into the captive portal's "Access Token"
+field and the device sends it (`?token=...`) on every request
+automatically, and bakes it into the manage-menu/share QR codes so
+scanning them just works too. Leave it blank if the server has no
+`MANAGEMENT_TOKEN` configured -- the default, unauthenticated-on-a-
+trusted-LAN behavior from before.
+
 ## Skipping to the next photo
 
 Wire a momentary push button between GPIO2 and GND (internal pull-up,

firmware/main/frame_client.c

@@ -32,17 +32,27 @@ static EventGroupHandle_t s_sta_event_group;
  * Cloudflare-issued origin certificate. See firmware/main/certs/. */
 extern const char cloudflare_origin_ca_pem_start[] asm("_binary_cloudflare_origin_ca_pem_start");
 
-/* Builds a full URL from cfg->toolsserver + a path (no leading slash).
- * toolsserver is normally a bare "host:port", defaulting to plain http;
- * it may instead carry an explicit "http://" or "https://" prefix to
- * pick the scheme, e.g. "https://frame.example.com" if a reverse proxy
- * is terminating TLS in front of the tools server. */
-static void build_url(char *out, size_t out_size, const char *toolsserver, const char *path)
+/* Builds a full URL from cfg->toolsserver + a path (no leading slash),
+ * appending cfg->access_token as ?token= if one's set. toolsserver is
+ * normally a bare "host:port", defaulting to plain http; it may instead
+ * carry an explicit "http://" or "https://" prefix to pick the scheme,
+ * e.g. "https://frame.example.com" if a reverse proxy is terminating
+ * TLS in front of the tools server. The token, once the server has
+ * MANAGEMENT_TOKEN set, is required on every request the server
+ * receives (device-facing endpoints included, not just the web UI) --
+ * this is the one chokepoint all of them go through, so every caller
+ * gets it for free instead of needing to remember to add it. */
+static void build_url(char *out, size_t out_size, const frame_config_t *cfg, const char *path)
 {
+    const char *toolsserver = cfg->toolsserver;
+    size_t len;
     if (strncmp(toolsserver, "http://", 7) == 0 || strncmp(toolsserver, "https://", 8) == 0) {
-        snprintf(out, out_size, "%s/%s", toolsserver, path);
+        len = (size_t)snprintf(out, out_size, "%s/%s", toolsserver, path);
     } else {
-        snprintf(out, out_size, "http://%s/%s", toolsserver, path);
+        len = (size_t)snprintf(out, out_size, "http://%s/%s", toolsserver, path);
+    }
+    if (cfg->access_token[0] != '\0' && len < out_size) {
+        snprintf(out + len, out_size - len, "?token=%s", cfg->access_token);
     }
 }
 
@@ -213,15 +223,15 @@ static bool json_extract_string(const char *json, const char *key, char *out, si
 /* GETs the server's /frame/config -- doubles as both the reachability
  * check (any completed HTTP response means the socket-level connection
  * succeeded) and the source of the server-configurable refresh interval. */
-static frame_server_config_t fetch_frame_config(const char *toolsserver)
+static frame_server_config_t fetch_frame_config(const frame_config_t *cfg)
 {
     frame_server_config_t result = {
         .reachable = false,
         .refresh_interval_s = CONFIG_FRAME_SLEEP_INTERVAL_S,
     };
 
-    char url[160];
-    build_url(url, sizeof(url), toolsserver, "frame/config");
+    char url[256];
+    build_url(url, sizeof(url), cfg, "frame/config");
 
     esp_http_client_config_t config = {
         .url = url,
@@ -233,7 +243,7 @@ static frame_server_config_t fetch_frame_config(const char *toolsserver)
 
     esp_err_t err = esp_http_client_open(client, 0);
     if (err != ESP_OK) {
-        ESP_LOGW(TAG, "Server '%s' not reachable: %s", toolsserver, esp_err_to_name(err));
+        ESP_LOGW(TAG, "Server '%s' not reachable: %s", cfg->toolsserver, esp_err_to_name(err));
         esp_http_client_cleanup(client);
         return result;
     }
@@ -272,7 +282,7 @@ static frame_server_config_t fetch_frame_config(const char *toolsserver)
  * current photo, etc.) just leaves all outputs empty -- the caller
  * treats that as "skip these optional overlay regions", not a hard
  * error, since the base "scan to manage" QR should still show. */
-static void fetch_photo_info(const char *toolsserver, char *location_line1, size_t location_line1_size,
+static void fetch_photo_info(const frame_config_t *cfg, char *location_line1, size_t location_line1_size,
                               char *location_line2, size_t location_line2_size, char *taken_at, size_t taken_at_size,
                               char *share_url, size_t share_url_size)
 {
@@ -281,8 +291,8 @@ static void fetch_photo_info(const char *toolsserver, char *location_line1, size
     taken_at[0] = '\0';
     share_url[0] = '\0';
 
-    char url[160];
-    build_url(url, sizeof(url), toolsserver, "frame/photo-info");
+    char url[256];
+    build_url(url, sizeof(url), cfg, "frame/photo-info");
 
     esp_http_client_config_t config = {
         .url = url,
@@ -327,7 +337,7 @@ static void fetch_photo_info(const char *toolsserver, char *location_line1, size
     if (json_extract_string(body, "asset_id", asset_id, sizeof(asset_id))) {
         char path[80];
         snprintf(path, sizeof(path), "frame/share/%s", asset_id);
-        build_url(share_url, share_url_size, toolsserver, path);
+        build_url(share_url, share_url_size, cfg, path);
     }
 }
 
@@ -339,10 +349,10 @@ static void fetch_photo_info(const char *toolsserver, char *location_line1, size
  * this file instead of needing an actual array parser. Any failure
  * (unreachable, malformed response, etc.) just returns 0 -- named faces
  * are a "nice to have" addition to the menu, not worth failing it over. */
-static int fetch_face_labels(const char *toolsserver, manage_face_label_t *out, int max_labels)
+static int fetch_face_labels(const frame_config_t *cfg, manage_face_label_t *out, int max_labels)
 {
-    char url[160];
-    build_url(url, sizeof(url), toolsserver, "frame/face-labels");
+    char url[256];
+    build_url(url, sizeof(url), cfg, "frame/face-labels");
 
     esp_http_client_config_t config = {
         .url = url,
@@ -481,8 +491,8 @@ static size_t http_read_fn(uint8_t *chunk, size_t chunk_size, void *ctx_)
 static esp_err_t fetch_and_display(const frame_config_t *cfg, bool force_advance,
                                     const manage_overlay_set_t *overlay)
 {
-    char url[160];
-    build_url(url, sizeof(url), cfg->toolsserver, force_advance ? "frame/advance" : "frame/image");
+    char url[256];
+    build_url(url, sizeof(url), cfg, force_advance ? "frame/advance" : "frame/image");
 
     esp_http_client_config_t config = {
         .url = url,
@@ -578,25 +588,19 @@ static bool wait_for_button_press(uint32_t timeout_ms)
 static esp_err_t show_menu_level(const frame_config_t *cfg, bool force_advance, int level)
 {
     char management_url[256];
-    build_url(management_url, sizeof(management_url), cfg->toolsserver, "");
-    if (cfg->access_token[0] != '\0') {
-        /* Embeds the token so scanning the QR just works -- matches the
-         * server's MANAGEMENT_TOKEN gate on GET / (see server/README.md). */
-        size_t len = strlen(management_url);
-        snprintf(management_url + len, sizeof(management_url) - len, "?token=%s", cfg->access_token);
-    }
+    build_url(management_url, sizeof(management_url), cfg, "");
 
     char location_line1[32];
     char location_line2[32];
     char taken_at[32];
-    char share_url[160];
-    fetch_photo_info(cfg->toolsserver, location_line1, sizeof(location_line1), location_line2,
+    char share_url[256];
+    fetch_photo_info(cfg, location_line1, sizeof(location_line1), location_line2,
                       sizeof(location_line2), taken_at, sizeof(taken_at), share_url, sizeof(share_url));
 
     manage_face_label_t face_labels[MANAGE_FACE_LABELS_MAX];
     int face_label_count = 0;
     if (level >= 2) {
-        face_label_count = fetch_face_labels(cfg->toolsserver, face_labels, MANAGE_FACE_LABELS_MAX);
+        face_label_count = fetch_face_labels(cfg, face_labels, MANAGE_FACE_LABELS_MAX);
     }
 
     manage_overlay_content_t content = {
@@ -727,7 +731,7 @@ void frame_client_run(const frame_config_t *cfg, bool force_advance, bool show_m
      * just be discarded. */
     uint32_t sleep_seconds = CONFIG_FRAME_RETRY_INTERVAL_S;
     if (image_ok) {
-        frame_server_config_t server_cfg = fetch_frame_config(cfg->toolsserver);
+        frame_server_config_t server_cfg = fetch_frame_config(cfg);
         sleep_seconds = server_cfg.reachable ? server_cfg.refresh_interval_s : CONFIG_FRAME_RETRY_INTERVAL_S;
     }
 

server/README.md

@@ -33,13 +33,17 @@ algorithm itself -- it just streams the response straight to the panel.
    for HTTPS, put a TLS-terminating reverse proxy (e.g. nginx) in front of
    it and enter the proxy's `https://` address instead (see
    `firmware/README.md`'s HTTPS section for what the ESP32 side needs).
-6. **Optional: set `MANAGEMENT_TOKEN`** in `docker-compose.yml` to gate the
-   web UI behind a shared secret (leave unset to keep it open, the
-   previous default -- fine on a trusted LAN). If set, paste the same
-   value into the ESP32's captive portal setup form's **Access Token**
-   field so the manage-menu's "scan to manage" QR code embeds it
-   automatically (`?token=...`); visiting the page without a valid token
-   in the URL shows a plain token-entry prompt instead of the config UI.
+6. **Optional: set `MANAGEMENT_TOKEN`** in `docker-compose.yml` to gate
+   the *entire server* -- the web UI (`/`, `/api/*`) and every
+   device-facing `/frame/*` endpoint -- behind a shared secret (leave
+   unset to keep it all open, the previous default -- fine on a trusted
+   LAN). If set, paste the same value into the ESP32's captive portal
+   setup form's **Access Token** field: the device then sends it on
+   every request it makes, and the manage-menu/share QR codes embed it
+   automatically (`?token=...`) so scanning them just works. Visiting
+   the web UI without a valid token in the URL shows a plain token-entry
+   prompt instead of the config UI; `/health` stays open regardless
+   (pure liveness, nothing sensitive in it).
 
 ## Endpoints
 
@@ -116,15 +120,15 @@ algorithm itself -- it just streams the response straight to the panel.
   in sequential or shuffle order per the Order setting. Dragging photos
   in the web UI (or using "Show next") only rearranges what's already in
   that lookahead; it doesn't add or remove photos from the album.
-- `/frame/image`, `/frame/advance`, `/frame/photo-info`, `/frame/face-labels`,
-  and `/frame/share/{asset_id}` -- the device-facing endpoints -- aren't
-  authenticated. That's fine on a trusted home LAN for now, but worth
-  revisiting if this ever needs to sit somewhere less trusted.
-  `/frame/share` at least is scoped to only ever create a link for a
-  photo this frame is actually showing or has queued, not any Immich
-  asset ID someone might guess. The web UI (`/`, `/api/*`) is separately
-  gated by `MANAGEMENT_TOKEN` if set (see Setup above) -- these are two
-  independent trust boundaries, not one shared mechanism.
+- Every endpoint except `/` and `/health` -- the web UI's `/api/*` and
+  every device-facing `/frame/*` -- requires `?token=` (or the
+  `mgmt_token` cookie the web UI sets after a valid one) once
+  `MANAGEMENT_TOKEN` is set (see Setup above); unset, everything stays
+  open like before, which is still fine on a trusted home LAN. `/frame/share`
+  additionally stays scoped to only ever create a link for a photo this
+  frame is actually showing or has queued, not any Immich asset ID
+  someone might guess -- a second layer a leaked token alone wouldn't
+  bypass.
 - The 6-color palette RGB values in `app/image_pipeline.py` are
   approximations, not measured values (Waveshare doesn't publish exact
   color primaries for this panel) -- tune them once you can compare a

server/app/main.py

@@ -34,25 +34,32 @@ MANAGEMENT_TOKEN_COOKIE = "mgmt_token"
 
 def _token_valid(request: Request, cfg: config.FrameConfig) -> bool:
     """No management_token configured (MANAGEMENT_TOKEN env var, see
-    docker-compose.yml.example) means the management page stays open on a
-    trusted LAN, matching this project's existing default. Once one's
+    docker-compose.yml.example) means the whole server stays open on a
+    trusted LAN, matching this project's original default. Once one's
     set, a request is authorized by either a ?token= query param (what
-    the manage-menu QR code embeds) or the cookie index() sets after a
-    valid query-param hit (so the page's own fetch()/<img> calls, which
-    carry no query string, stay authorized for the rest of the visit)."""
+    the ESP32 sends on every device request, and what the manage-menu/
+    share QR codes embed for a human scanning them) or the cookie
+    index() sets after a valid query-param hit (so the web UI's own
+    fetch()/<img> calls, which carry no query string, stay authorized
+    for the rest of that browsing visit)."""
     if not cfg.management_token:
         return True
     supplied = request.query_params.get("token") or request.cookies.get(MANAGEMENT_TOKEN_COOKIE)
     return supplied is not None and supplied == cfg.management_token
 
 
-def require_management_token(request: Request) -> None:
-    """Dependency for the /api/* routes behind the management page. index()
-    below handles the unauthorized case itself (a friendlier HTML prompt,
-    not a bare 401) since that's the one route an unauthorized visitor is
-    actually meant to land on."""
+def require_access_token(request: Request) -> None:
+    """Dependency for every route except / and /health: the web UI's
+    /api/* and every device-facing /frame/*. index() handles the
+    unauthorized case itself (a friendlier HTML prompt, not a bare 401)
+    since that's the one route a human is actually meant to land on with
+    no token yet; the ESP32 sends its token as ?token= on every request
+    it makes (see frame_client.c's build_url()), so device endpoints
+    just 401 outright on a missing/wrong one. /health stays open -- it
+    reveals nothing but process liveness, and gating it would break
+    plain infra/uptime monitoring for no real security benefit."""
     if not _token_valid(request, config.load()):
-        raise HTTPException(401, "Missing or invalid management token")
+        raise HTTPException(401, "Missing or invalid access token")
 
 
 @app.get("/health")
@@ -60,7 +67,7 @@ def health() -> dict:
     return {"status": "ok"}
 
 
-@app.get("/frame/config")
+@app.get("/frame/config", dependencies=[Depends(require_access_token)])
 def frame_config():
     """Device-facing settings, polled by the frame alongside its
     reachability check. Always returns 200 with current settings
@@ -91,7 +98,7 @@ def index(request: Request):
     return response
 
 
-@app.get("/api/albums", dependencies=[Depends(require_management_token)])
+@app.get("/api/albums", dependencies=[Depends(require_access_token)])
 def api_albums():
     cfg = config.load()
     if not cfg.immich_url or not cfg.immich_api_key:
@@ -103,7 +110,7 @@ def api_albums():
     return [{"id": a["id"], "name": a["albumName"], "count": a.get("assetCount", 0)} for a in albums]
 
 
-@app.post("/api/config", dependencies=[Depends(require_management_token)])
+@app.post("/api/config", dependencies=[Depends(require_access_token)])
 def api_config_save(
     album_id: str = Form(""),
     order: str = Form("sequential"),
@@ -168,7 +175,7 @@ def _render_asset(client: ImmichClient, cfg: config.FrameConfig, asset_id: str)
     return render_frame(source, faces=faces)
 
 
-@app.get("/frame/image")
+@app.get("/frame/image", dependencies=[Depends(require_access_token)])
 def frame_image():
     """Returns the current photo. Idempotent: only actually advances to
     the next photo once refresh_interval_s has elapsed since the current
@@ -187,7 +194,7 @@ def frame_image():
     return Response(content=_render_asset(client, cfg, cfg.current_asset_id), media_type="application/octet-stream")
 
 
-@app.post("/frame/advance")
+@app.post("/frame/advance", dependencies=[Depends(require_access_token)])
 def frame_advance():
     """Forces an immediate advance to the next photo, ignoring
     refresh_interval_s, and resets the interval clock from now. Used by
@@ -274,7 +281,7 @@ def _format_taken_at(exif: dict) -> str | None:
         return None
 
 
-@app.get("/frame/photo-info")
+@app.get("/frame/photo-info", dependencies=[Depends(require_access_token)])
 def frame_photo_info():
     """Location/date-taken text for the manage-button overlay, plus the
     asset id used to build the share-QR's target URL. Read-only, same
@@ -307,16 +314,17 @@ def frame_photo_info():
     }
 
 
-@app.get("/frame/share/{asset_id}")
+@app.get("/frame/share/{asset_id}", dependencies=[Depends(require_access_token)])
 def frame_share(asset_id: str):
     """Creates a 30-minute public Immich share link for asset_id and
     redirects to it -- what the manage overlay's bottom-left QR code
-    points to. The link is created lazily, when this actually gets hit
-    (i.e. when someone scans it), not when the manage button was
-    pressed, so the 30-minute window starts when it's actually used.
-    Scoped to the photo currently showing or queued -- not any arbitrary
-    Immich asset id -- since this is otherwise an unauthenticated
-    endpoint (see server/README.md)."""
+    points to (the firmware bakes ?token= into that QR the same way it
+    does for the management QR, see frame_client.c's build_url()). The
+    link is created lazily, when this actually gets hit (i.e. when
+    someone scans it), not when the manage button was pressed, so the
+    30-minute window starts when it's actually used. Also scoped to the
+    photo currently showing or queued -- not any arbitrary Immich asset
+    id -- as a second layer even a leaked token wouldn't bypass."""
     cfg = config.load()
     _require_configured(cfg)
 
@@ -332,7 +340,7 @@ def frame_share(asset_id: str):
     return RedirectResponse(share_url)
 
 
-@app.get("/frame/face-labels")
+@app.get("/frame/face-labels", dependencies=[Depends(require_access_token)])
 def frame_face_labels():
     """Named-face positions for the manage button's escalated "level 2"
     menu -- who's in the current photo, per Immich's own face
@@ -381,7 +389,7 @@ def frame_face_labels():
     return result
 
 
-@app.get("/api/queue", dependencies=[Depends(require_management_token)])
+@app.get("/api/queue", dependencies=[Depends(require_access_token)])
 def api_queue():
     cfg = config.load()
     _require_configured(cfg)
@@ -408,7 +416,7 @@ class QueueReorderRequest(BaseModel):
     queue: list[str]
 
 
-@app.post("/api/queue/reorder", dependencies=[Depends(require_management_token)])
+@app.post("/api/queue/reorder", dependencies=[Depends(require_access_token)])
 def api_queue_reorder(body: QueueReorderRequest):
     """Applies the client's requested order, tolerating drift between the
     browser's last-fetched snapshot and the server's current queue (e.g.
@@ -429,7 +437,7 @@ class QueuePromoteRequest(BaseModel):
     asset_id: str
 
 
-@app.post("/api/queue/promote", dependencies=[Depends(require_management_token)])
+@app.post("/api/queue/promote", dependencies=[Depends(require_access_token)])
 def api_queue_promote(body: QueuePromoteRequest):
     """Moves a single photo to the front of the queue -- "Show next" in
     the web UI. Unlike /api/queue/reorder, this doesn't depend on the
@@ -444,7 +452,7 @@ def api_queue_promote(body: QueuePromoteRequest):
     return {"status": "saved"}
 
 
-@app.get("/api/photo-thumbnail/{asset_id}", dependencies=[Depends(require_management_token)])
+@app.get("/api/photo-thumbnail/{asset_id}", dependencies=[Depends(require_access_token)])
 def api_photo_thumbnail(asset_id: str):
     cfg = config.load()
     _require_configured(cfg)