feat(auth): proxy OIDC provider icons server-side (#1333) (#1342)

* feat(auth): proxy OIDC provider icons server-side (#1333)

Strict img-src CSP blocked external OIDC icon hosts on the login page.
Loosening CSP was rejected via the MakerWorld precedent, so icons are
proxied: admin sets icon_url, backend fetches and caches the bytes in a
deferred BLOB column, the SPA renders from a same-origin
/api/v1/auth/oidc/providers/{id}/icon endpoint.

backend/app/api/routes/_oidc_helpers.py

@@ -0,0 +1,74 @@
+"""Pure helper functions for OIDC routes.
+
+Hosts the SSRF guard for admin-supplied icon URLs. Stricter than
+``_spoolman_helpers.assert_safe_spoolman_url`` — Spoolman intentionally allows
+loopback/RFC-1918 (same-LAN topology) while OIDC icons must be reachable on
+the public internet (IdP-hosted), so private addresses there are SSRF probes.
+"""
+
+from __future__ import annotations
+
+import ipaddress
+from urllib.parse import urlparse
+
+from backend.app.api.routes._url_safety import CLOUD_METADATA_IPS, NUMERIC_IP_RE, unwrap_ipv4_mapped
+
+
+def assert_safe_public_https_url(url: str) -> None:
+    """Raise ValueError if *url* is unsafe to fetch as a public HTTPS resource.
+
+    Used for OIDC provider icon URLs (#1333). Stricter than the Spoolman SSRF
+    guard: also rejects loopback, private (RFC-1918), and link-local addresses
+    because an OIDC icon legitimately lives only on the public internet.
+
+    Checks performed:
+    - Scheme must be ``https`` (no ``http://``, ``file://``, ``gopher://``, …).
+    - Numeric-encoded IPv4 (decimal ``2130706433``, hex ``0x7f000001``) is
+      rejected — libc and browsers parse those as valid addresses while
+      Python's ``ipaddress`` raises ValueError, so they bypass the IP block
+      below if not caught first.
+    - Cloud-provider metadata endpoints (169.254.169.254, 100.100.100.200,
+      fd00:ec2::254) — classic SSRF credential-exfil targets.
+    - Loopback (127.0.0.0/8, ::1), private RFC-1918 (10/8, 172.16/12,
+      192.168/16) and link-local (169.254/16, fe80::/10) addresses.
+    - Multicast (224.0.0.0/4, ff00::/8) and unspecified (0.0.0.0, ::).
+    - IPv4-mapped IPv6 (``::ffff:127.0.0.1``) — unwrapped before the IP-class
+      check so an attacker can't bypass via IPv6 encoding.
+
+    Hostname-based addresses are accepted without DNS resolution (consistent
+    with ``_validate_issuer_url`` policy — the operator is trusted to
+    configure a sensible IdP host).
+    """
+    parsed = urlparse(url)
+    if parsed.scheme.lower() != "https":
+        raise ValueError("icon URL must use https://")
+
+    hostname = (parsed.hostname or "").lower()
+
+    if NUMERIC_IP_RE.match(hostname):
+        raise ValueError("icon URL must not use numeric-encoded IP addresses")
+
+    try:
+        addr = ipaddress.ip_address(hostname)
+    except ValueError:
+        return  # hostname — out of scope (no DNS check by design)
+
+    effective = unwrap_ipv4_mapped(addr)
+
+    if effective in CLOUD_METADATA_IPS:
+        raise ValueError("icon URL must not point to a cloud metadata endpoint")
+
+    # Order matters: 0.0.0.0 sets BOTH is_private and is_unspecified — check
+    # the more-specific is_unspecified first so the error message points at
+    # the actual misuse. Similarly 127.0.0.1 sets is_loopback and is_private
+    # (private under IANA's reservation); is_loopback first is clearer.
+    if effective.is_unspecified:
+        raise ValueError("icon URL must not point to an unspecified address")
+    if effective.is_loopback:
+        raise ValueError("icon URL must not point to a loopback address")
+    if effective.is_link_local:
+        raise ValueError("icon URL must not point to a link-local address")
+    if effective.is_multicast:
+        raise ValueError("icon URL must not point to a multicast address")
+    if effective.is_private:
+        raise ValueError("icon URL must not point to a private (RFC-1918) address")

backend/app/api/routes/_spoolman_helpers.py

@@ -15,6 +15,8 @@ from urllib.parse import urlparse
 
 from typing_extensions import TypedDict
 
+from backend.app.api.routes._url_safety import CLOUD_METADATA_IPS, NUMERIC_IP_RE, unwrap_ipv4_mapped
+
 logger = logging.getLogger(__name__)
 
 
@@ -75,18 +77,6 @@ class NormalizedFilament(TypedDict):
     vendor: NormalizedVendorRef | None
 
 
-_CLOUD_METADATA_IPS = frozenset(
-    {
-        # AWS / GCP / Azure / Oracle / DigitalOcean IMDS
-        ipaddress.ip_address("169.254.169.254"),
-        # Alibaba Cloud metadata
-        ipaddress.ip_address("100.100.100.200"),
-        # AWS IMDS IPv6
-        ipaddress.ip_address("fd00:ec2::254"),
-    }
-)
-
-
 def assert_safe_spoolman_url(url: str) -> None:
     """Raise ValueError if *url* should be blocked as an SSRF risk.
 
@@ -121,7 +111,7 @@ def assert_safe_spoolman_url(url: str) -> None:
     # Reject decimal- and hex-encoded IPs (e.g. http://2130706433/ or
     # http://0x7f000001/). These slip past ipaddress.ip_address() but libc
     # (and browsers) parse them as IPv4 — an obvious bypass if not caught.
-    if re.match(r"^(0x[0-9a-f]+|[0-9]+)$", hostname, re.I):
+    if NUMERIC_IP_RE.match(hostname):
         raise ValueError("Spoolman URL must not use numeric-encoded IP addresses; use standard dotted-decimal notation")
 
     try:
@@ -136,11 +126,9 @@ def assert_safe_spoolman_url(url: str) -> None:
 
     # Unwrap IPv4-mapped IPv6 (::ffff:169.254.169.254 etc.) so attackers can't
     # encode a blocked IPv4 into an IPv6 literal to bypass the check.
-    effective: ipaddress.IPv4Address | ipaddress.IPv6Address = addr
-    if isinstance(addr, ipaddress.IPv6Address) and addr.ipv4_mapped is not None:
-        effective = addr.ipv4_mapped
+    effective = unwrap_ipv4_mapped(addr)
 
-    if effective in _CLOUD_METADATA_IPS:
+    if effective in CLOUD_METADATA_IPS:
         raise ValueError("Spoolman URL must not point to a cloud metadata endpoint")
 
     if effective.is_multicast or effective.is_unspecified:

backend/app/api/routes/_url_safety.py

@@ -0,0 +1,51 @@
+"""Shared URL-safety primitives used by both SSRF guards in this package.
+
+The two top-level assertion functions —
+``_spoolman_helpers.assert_safe_spoolman_url`` (Spoolman, deliberately allows
+loopback/RFC-1918 because same-LAN deployment is the standard topology) and
+``_oidc_helpers.assert_safe_public_https_url`` (OIDC icons, must be reachable
+on the public internet, so loopback/private are rejected) — share the
+*data* (cloud-metadata IP set, numeric-encoded-IP regex) but not the
+*policy*. Only the data lives here. The functions stay in their respective
+modules with their distinct policies intact.
+"""
+
+from __future__ import annotations
+
+import ipaddress
+import re
+
+# Cloud-provider metadata endpoints — the classic SSRF credential-exfil
+# targets. Both guards reject these unconditionally.
+CLOUD_METADATA_IPS = frozenset(
+    {
+        # AWS / GCP / Azure / Oracle / DigitalOcean IMDS
+        ipaddress.ip_address("169.254.169.254"),
+        # Alibaba Cloud metadata
+        ipaddress.ip_address("100.100.100.200"),
+        # AWS IMDS IPv6
+        ipaddress.ip_address("fd00:ec2::254"),
+    }
+)
+
+
+# libc and browsers parse numeric-encoded IP forms (decimal ``2130706433``
+# for 127.0.0.1, hex ``0x7f000001``) but Python's ``ipaddress.ip_address``
+# raises ValueError on these, so they slip past the IP-class checks if
+# not caught first. Used by both guards to reject up-front.
+NUMERIC_IP_RE = re.compile(r"^(0x[0-9a-f]+|[0-9]+)$", re.I)
+
+
+def unwrap_ipv4_mapped(
+    addr: ipaddress.IPv4Address | ipaddress.IPv6Address,
+) -> ipaddress.IPv4Address | ipaddress.IPv6Address:
+    """Return the underlying IPv4 for an IPv4-mapped IPv6 address, else return *addr*.
+
+    ``::ffff:127.0.0.1`` and similar mapped forms must be unwrapped before
+    the per-class checks (``is_private``, ``is_loopback``, …) — otherwise
+    an attacker can encode a blocked IPv4 address as an IPv6 literal to
+    bypass the guard.
+    """
+    if isinstance(addr, ipaddress.IPv6Address) and addr.ipv4_mapped is not None:
+        return addr.ipv4_mapped
+    return addr

backend/app/api/routes/mfa.py

@@ -30,14 +30,15 @@ from datetime import datetime, timedelta, timezone
 import httpx
 import jwt
 import pyotp
-from fastapi import APIRouter, Body, Depends, HTTPException, Query, Request, Response, status
+from fastapi import APIRouter, Body, Depends, Header, HTTPException, Query, Request, Response, status
 from fastapi.responses import RedirectResponse
 from jwt import PyJWKClient
 from passlib.context import CryptContext
 from sqlalchemy import delete, select
 from sqlalchemy.ext.asyncio import AsyncSession
-from sqlalchemy.orm import selectinload
+from sqlalchemy.orm import selectinload, undefer
 
+from backend.app.api.routes._oidc_helpers import assert_safe_public_https_url
 from backend.app.api.routes.settings import get_setting, set_setting
 from backend.app.core.auth import (
     ACCESS_TOKEN_EXPIRE_MINUTES,
@@ -83,10 +84,78 @@ from backend.app.schemas.auth import (
     UserResponse,
 )
 from backend.app.services.email_service import get_smtp_settings, send_email
+from backend.app.services.oidc_icon import OIDCIconError, fetch_icon
 
 logger = logging.getLogger(__name__)
 
 
+def _redact_url_for_log(url: str) -> str:
+    """Return ``scheme://host/path`` with query string and fragment stripped.
+
+    Admin-supplied icon URLs are usually CDN paths, but nothing stops an
+    admin from pasting a presigned URL whose query string carries an
+    ``X-Amz-Signature`` / OAuth token / etc. Operators need a forensic
+    trail without those secrets ending up in log files.
+    """
+    try:
+        parsed = urllib.parse.urlparse(url)
+    except ValueError:
+        return "<unparseable>"
+    netloc = parsed.netloc or "<no-host>"
+    return f"{parsed.scheme}://{netloc}{parsed.path}"
+
+
+async def _fetch_icon_or_400(icon_url: str) -> tuple[bytes, str, str]:
+    """Validate URL + fetch icon, mapping any failure to HTTPException(400).
+
+    Centralises the SSRF guard + fetcher invocation so create/update/refresh
+    all behave identically — admin always gets a 400 with a precise reason,
+    never a 500 / opaque server error.
+
+    Both failure paths log at WARNING so operators have a forensic trail
+    later — without these log lines the admin's UI toast was the only
+    record of the failure (#1333 review).
+    """
+    try:
+        assert_safe_public_https_url(icon_url)
+    except ValueError as exc:
+        logger.warning("OIDC icon URL rejected by SSRF guard: url=%s reason=%s", _redact_url_for_log(icon_url), exc)
+        raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc)) from exc
+    try:
+        return await fetch_icon(icon_url)
+    except OIDCIconError as exc:
+        logger.warning("OIDC icon fetch failed: url=%s reason=%s", _redact_url_for_log(icon_url), exc)
+        raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc)) from exc
+
+
+def _build_provider_response(provider: OIDCProvider) -> OIDCProviderResponse:
+    """Build OIDCProviderResponse via ``from_attributes``. The required
+    ``has_icon`` field is supplied by ``OIDCProvider.has_icon`` (a property
+    reading the non-deferred ``icon_content_type`` column)."""
+    return OIDCProviderResponse.model_validate(provider)
+
+
+def _etag_matches(if_none_match: str | None, etag_raw: str | None) -> bool:
+    """RFC 7232 §3.2 If-None-Match comparison.
+
+    Supports:
+    * ``*`` wildcard — matches any current representation when the resource
+      exists (and it does here; we wouldn't have an etag otherwise).
+    * Multiple comma-separated tokens.
+    * Weak-validator prefix ``W/`` (RFC 7232 §2.3) — accepted on GET since
+      cached representations of a static byte-blob are byte-identical.
+
+    Returns False on missing header or missing stored etag.
+    """
+    if not if_none_match or not etag_raw:
+        return False
+    quoted = f'"{etag_raw}"'
+    tokens = [t.strip() for t in if_none_match.split(",")]
+    if "*" in tokens:
+        return True
+    return any(tok.removeprefix("W/") == quoted for tok in tokens)
+
+
 def _as_utc(dt: datetime) -> datetime:
     """Return *dt* with UTC timezone attached.
 
@@ -1204,10 +1273,14 @@ async def admin_disable_2fa(
 async def list_oidc_providers(
     db: AsyncSession = Depends(get_db),
 ) -> list[OIDCProviderResponse]:
-    """List all enabled OIDC providers (public)."""
+    """List all enabled OIDC providers (public).
+
+    The login page renders icons via /oidc/providers/{id}/icon — `icon_data`
+    stays deferred so this list query never pulls the BLOB.
+    """
     result = await db.execute(select(OIDCProvider).where(OIDCProvider.is_enabled.is_(True)))
     providers = result.scalars().all()
-    return [OIDCProviderResponse.model_validate(p) for p in providers]
+    return [_build_provider_response(p) for p in providers]
 
 
 @router.get("/oidc/providers/all", response_model=list[OIDCProviderResponse])
@@ -1218,7 +1291,7 @@ async def list_all_oidc_providers(
     """List ALL OIDC providers including disabled ones (admin only)."""
     result2 = await db.execute(select(OIDCProvider))
     providers = result2.scalars().all()
-    return [OIDCProviderResponse.model_validate(p) for p in providers]
+    return [_build_provider_response(p) for p in providers]
 
 
 @router.post("/oidc/providers", response_model=OIDCProviderResponse, status_code=status.HTTP_201_CREATED)
@@ -1227,7 +1300,12 @@ async def create_oidc_provider(
     _: User | None = RequirePermissionIfAuthEnabled(Permission.SETTINGS_UPDATE),
     db: AsyncSession = Depends(get_db),
 ) -> OIDCProviderResponse:
-    """Create a new OIDC provider (admin only)."""
+    """Create a new OIDC provider (admin only).
+
+    If `icon_url` is supplied, the icon is fetched server-side and cached in
+    the BLOB columns (#1333). A fetch failure aborts the create with 400 —
+    no half-configured provider is left in the DB.
+    """
     if body.default_group_id is not None:
         grp_chk = await db.execute(select(Group).where(Group.id == body.default_group_id))
         if not grp_chk.scalar_one_or_none():
@@ -1235,6 +1313,14 @@ async def create_oidc_provider(
                 status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
                 detail="default_group_id references a non-existent group",
             )
+
+    # Fetch the icon BEFORE creating the row so a failure leaves the DB clean.
+    icon_data: bytes | None = None
+    icon_content_type: str | None = None
+    icon_etag: str | None = None
+    if body.icon_url:
+        icon_data, icon_content_type, icon_etag = await _fetch_icon_or_400(body.icon_url)
+
     provider = OIDCProvider(
         name=body.name,
         issuer_url=body.issuer_url.rstrip("/"),
@@ -1247,6 +1333,9 @@ async def create_oidc_provider(
         email_claim=body.email_claim,
         require_email_verified=body.require_email_verified,
         icon_url=body.icon_url,
+        icon_data=icon_data,
+        icon_content_type=icon_content_type,
+        icon_etag=icon_etag,
         default_group_id=body.default_group_id,
     )
     # SEC-1 + SEC-6: runtime guard mirrors the OIDCProviderCreate model_validator in schemas/auth.py.
@@ -1255,7 +1344,7 @@ async def create_oidc_provider(
     db.add(provider)
     await db.commit()
     await db.refresh(provider)
-    return OIDCProviderResponse.model_validate(provider)
+    return _build_provider_response(provider)
 
 
 @router.put("/oidc/providers/{provider_id}", response_model=OIDCProviderResponse)
@@ -1265,7 +1354,17 @@ async def update_oidc_provider(
     _: User | None = RequirePermissionIfAuthEnabled(Permission.SETTINGS_UPDATE),
     db: AsyncSession = Depends(get_db),
 ) -> OIDCProviderResponse:
-    """Update an existing OIDC provider (admin only)."""
+    """Update an existing OIDC provider (admin only).
+
+    Icon refetch fires when:
+    1. The submitted `icon_url` differs from the stored one (URL changed), OR
+    2. The submitted `icon_url` equals the stored one AND `icon_content_type`
+       is NULL — this is the upgrade-path edge case: old providers carry
+       `icon_url` but no cached bytes until the admin first saves them.
+
+    On fetch failure the request aborts with 400 *before* commit, so the
+    existing cached bytes (if any) remain untouched.
+    """
     result2 = await db.execute(select(OIDCProvider).where(OIDCProvider.id == provider_id))
     provider = result2.scalar_one_or_none()
     if not provider:
@@ -1279,11 +1378,43 @@ async def update_oidc_provider(
                 detail="default_group_id references a non-existent group",
             )
 
-    for field, value in body.model_dump(exclude_none=True).items():
+    dumped = body.model_dump(exclude_none=True)
+
+    # Decide whether an icon refetch is needed BEFORE mutating the ORM object,
+    # so the comparison sees provider.icon_url / icon_content_type as they are
+    # in the database.
+    new_icon_url = dumped.get("icon_url")
+    needs_icon_refetch = new_icon_url is not None and (
+        new_icon_url != provider.icon_url or provider.icon_content_type is None
+    )
+
+    # Fetch FIRST. If the upstream is unreachable or SSRF-blocked, _fetch_icon_or_400
+    # raises HTTPException(400) here — provider attributes are still untouched, so
+    # the in-memory ORM object stays consistent on the way out (and the DB row is
+    # safe regardless via get_db()'s rollback).
+    fetched_icon: tuple[bytes, str, str] | None = None
+    if needs_icon_refetch:
+        fetched_icon = await _fetch_icon_or_400(new_icon_url)
+
+    # Explicit `icon_url: null` in the PUT body means "clear the icon".
+    # The exclude_none=True dump above drops None values, which would
+    # otherwise silently ignore this request. Check model_fields_set on
+    # the unfiltered body to distinguish "client cleared it" from "client
+    # didn't include this field at all".
+    if "icon_url" in body.model_fields_set and body.icon_url is None:
+        provider.icon_url = None
+        provider.icon_data = None
+        provider.icon_content_type = None
+        provider.icon_etag = None
+
+    for field, value in dumped.items():
         if field == "issuer_url" and value:
             value = value.rstrip("/")
         setattr(provider, field, value)
 
+    if fetched_icon is not None:
+        provider.icon_data, provider.icon_content_type, provider.icon_etag = fetched_icon
+
     # SEC-1 + SEC-6: Combined-State-Guard after setattr loop.
     # Checks the final in-memory state (DB values + newly set values combined) to catch
     # partial updates that each pass schema validation individually but are unsafe together.
@@ -1291,7 +1422,7 @@ async def update_oidc_provider(
 
     await db.commit()
     await db.refresh(provider)
-    return OIDCProviderResponse.model_validate(provider)
+    return _build_provider_response(provider)
 
 
 @router.delete("/oidc/providers/{provider_id}")
@@ -1311,6 +1442,115 @@ async def delete_oidc_provider(
     return {"message": "Provider deleted"}
 
 
+# ---------------------------------------------------------------------------
+# OIDC provider icon proxy (#1333)
+# ---------------------------------------------------------------------------
+
+
+@router.get("/oidc/providers/{provider_id}/icon")
+async def get_oidc_provider_icon(
+    provider_id: int,
+    if_none_match: str | None = Header(default=None, alias="If-None-Match"),
+    db: AsyncSession = Depends(get_db),
+) -> Response:
+    """Serve the cached icon for an enabled OIDC provider (public, no auth).
+
+    Unauthenticated because ``<img>`` tags cannot send Authorization headers
+    and the login page renders these icons before the user is signed in — the
+    same justification as ``/api/v1/makerworld/thumbnail``. The SSRF guard
+    runs at admin-config time (create/update/refresh), not here.
+
+    Disabled providers respond 404 to avoid leaking their existence to
+    anonymous callers (mirrors ``GET /oidc/providers`` which filters on
+    ``is_enabled``).
+    """
+    result = await db.execute(
+        select(OIDCProvider)
+        .options(undefer(OIDCProvider.icon_data))
+        .where(OIDCProvider.id == provider_id, OIDCProvider.is_enabled.is_(True))
+    )
+    provider = result.scalar_one_or_none()
+    if provider is None or provider.icon_content_type is None or provider.icon_data is None:
+        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Icon not found")
+
+    etag_value = f'"{provider.icon_etag}"'
+    cache_headers = {"ETag": etag_value, "Cache-Control": "public, max-age=3600"}
+
+    if _etag_matches(if_none_match, provider.icon_etag):
+        return Response(status_code=status.HTTP_304_NOT_MODIFIED, headers=cache_headers)
+
+    return Response(
+        content=provider.icon_data,
+        media_type=provider.icon_content_type,
+        headers=cache_headers,
+    )
+
+
+@router.delete("/oidc/providers/{provider_id}/icon", status_code=status.HTTP_204_NO_CONTENT)
+async def delete_oidc_provider_icon(
+    provider_id: int,
+    _: User | None = RequirePermissionIfAuthEnabled(Permission.SETTINGS_UPDATE),
+    db: AsyncSession = Depends(get_db),
+) -> Response:
+    """Remove the icon entirely for a provider (admin only).
+
+    Clears all four icon columns — ``icon_url`` plus the three cached-bytes
+    columns. "Remove icon" means the whole record is gone, not just the
+    cache; without this the admin form would still show the URL while
+    the login page rendered a blank fallback (confusing half-state).
+    To re-add an icon the admin re-types the URL in the edit form.
+    """
+    result = await db.execute(select(OIDCProvider).where(OIDCProvider.id == provider_id))
+    provider = result.scalar_one_or_none()
+    if provider is None:
+        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Provider not found")
+
+    # Setting deferred columns is safe — no read happens, just a write.
+    provider.icon_url = None
+    provider.icon_data = None
+    provider.icon_content_type = None
+    provider.icon_etag = None
+    await db.commit()
+    return Response(status_code=status.HTTP_204_NO_CONTENT)
+
+
+@router.post("/oidc/providers/{provider_id}/icon/refresh", response_model=OIDCProviderResponse)
+async def refresh_oidc_provider_icon(
+    provider_id: int,
+    _: User | None = RequirePermissionIfAuthEnabled(Permission.SETTINGS_UPDATE),
+    db: AsyncSession = Depends(get_db),
+) -> OIDCProviderResponse:
+    """Refetch the icon from the stored `icon_url` (admin only).
+
+    Used when:
+    - The IdP changed its icon and the admin wants Bambuddy to pick up the
+      new bytes.
+    - An upgrade left the provider with an `icon_url` but no cached bytes
+      (covered automatically by `update_oidc_provider` too, but this gives
+      the UI an explicit "Refresh" button).
+
+    Failure to refetch returns 400 *before* commit, so the previously cached
+    bytes survive intact.
+    """
+    result = await db.execute(select(OIDCProvider).where(OIDCProvider.id == provider_id))
+    provider = result.scalar_one_or_none()
+    if provider is None:
+        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Provider not found")
+    if not provider.icon_url:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail="Provider has no icon_url to refresh",
+        )
+
+    icon_data, icon_content_type, icon_etag = await _fetch_icon_or_400(provider.icon_url)
+    provider.icon_data = icon_data
+    provider.icon_content_type = icon_content_type
+    provider.icon_etag = icon_etag
+    await db.commit()
+    await db.refresh(provider)
+    return _build_provider_response(provider)
+
+
 @router.get("/oidc/authorize/{provider_id}", response_model=OIDCAuthorizeResponse)
 async def oidc_authorize(
     provider_id: int,

backend/app/api/routes/settings.py

@@ -34,6 +34,32 @@ _SENSITIVE_FIELDS_FOR_API_KEY = (
 )
 
 
+def _sqlalchemy_type_to_sqlite_type(type_repr: str) -> str:
+    """Map a SQLAlchemy column type's ``str()`` to a SQLite-native column type.
+
+    Used by ``create_backup_zip`` to reconstruct a portable SQLite database
+    file from PostgreSQL data. Falling through to TEXT for binary columns
+    corrupts non-UTF8 bytes — the BLOB branch is the #1333 regression guard
+    for OIDC icon BLOBs.
+
+    Extracted as a pure helper so it can be unit-tested without spinning up
+    the full FastAPI app + backup pipeline.
+    """
+    type_str = type_repr.upper()
+    if "INT" in type_str:
+        return "INTEGER"
+    if "FLOAT" in type_str or "REAL" in type_str or "NUMERIC" in type_str:
+        return "REAL"
+    if "BOOL" in type_str:
+        return "BOOLEAN"
+    if "BLOB" in type_str or "BYTEA" in type_str or "BINARY" in type_str:
+        # OIDC icon BLOB column (#1333) — without this branch the column
+        # was created as TEXT and non-UTF8 bytes were corrupted during the
+        # PG→SQLite-ZIP backup round trip.
+        return "BLOB"
+    return "TEXT"
+
+
 async def get_setting(db: AsyncSession, key: str) -> str | None:
     """Get a single setting value by key."""
     result = await db.execute(select(Settings).where(Settings.key == key))
@@ -452,14 +478,7 @@ async def create_backup_zip(output_path: Path | None = None) -> tuple[Path, str]
                 cols = []
                 pk_cols = [col.name for col in table.columns if col.primary_key]
                 for col in table.columns:
-                    col_type = "TEXT"  # Default
-                    type_str = str(col.type).upper()
-                    if "INT" in type_str:
-                        col_type = "INTEGER"
-                    elif "FLOAT" in type_str or "REAL" in type_str or "NUMERIC" in type_str:
-                        col_type = "REAL"
-                    elif "BOOL" in type_str:
-                        col_type = "BOOLEAN"
+                    col_type = _sqlalchemy_type_to_sqlite_type(str(col.type))
                     # Only inline PRIMARY KEY for single-column PKs
                     pk = " PRIMARY KEY" if col.primary_key and len(pk_cols) == 1 else ""
                     cols.append(f"{col.name} {col_type}{pk}")

backend/app/api/routes/support.py

@@ -456,7 +456,11 @@ async def _collect_auth_info(db: AsyncSession) -> dict:
                 "auto_create_users": p.auto_create_users,
                 "auto_link_existing_accounts": p.auto_link_existing_accounts,
                 "has_default_group": p.default_group_id is not None,
-                "has_icon": bool(p.icon_url),
+                # Derive from icon_content_type (non-deferred) rather than
+                # icon_data (deferred BLOB) to avoid an async lazy-load.
+                # Falls back to icon_url for pre-#1333 rows that have a URL
+                # configured but no cached bytes yet.
+                "has_icon": bool(p.icon_content_type) or bool(p.icon_url),
                 "linked_user_count": link_count,
             }
         )

backend/app/core/database.py

@@ -2031,6 +2031,31 @@ async def run_migrations(conn):
         "ALTER TABLE oidc_providers ADD COLUMN default_group_id INTEGER REFERENCES groups(id) ON DELETE SET NULL",
     )
 
+    # Migration: Add cached-icon columns to oidc_providers (#1333).
+    # SPA's strict CSP (img-src 'self' data: blob:) blocks hotlinking external
+    # icon hosts, so we proxy them: admin sets icon_url, backend fetches and
+    # caches the bytes here, the SPA renders <img src="/api/v1/auth/oidc/providers/{id}/icon">.
+    # Must run AFTER _migrate_update_auto_link_constraint for the same reason as
+    # default_group_id above (SQLite table recreation drops unknown columns).
+    # Dialect-conditional type: BLOB on SQLite, BYTEA on PostgreSQL.
+    _blob_type = "BLOB" if is_sqlite() else "BYTEA"
+    await _safe_execute(conn, f"ALTER TABLE oidc_providers ADD COLUMN icon_data {_blob_type}")
+    await _safe_execute(conn, "ALTER TABLE oidc_providers ADD COLUMN icon_content_type VARCHAR(20)")
+    await _safe_execute(conn, "ALTER TABLE oidc_providers ADD COLUMN icon_etag VARCHAR(64)")
+
+    # PostgreSQL-only: enforce the all-or-nothing triplet at the DB layer.
+    # SQLite cannot ADD CONSTRAINT to an existing table — fresh SQLite
+    # installs get the CHECK via metadata.create_all (model __table_args__);
+    # stale SQLite installs rely on the application layer, same trade-off
+    # as the default_group_id FK ON DELETE SET NULL above.
+    if not is_sqlite():
+        await _safe_execute(
+            conn,
+            "ALTER TABLE oidc_providers ADD CONSTRAINT ck_oidc_icon_triplet_co_null "
+            "CHECK ((icon_data IS NULL) = (icon_content_type IS NULL) "
+            "AND (icon_content_type IS NULL) = (icon_etag IS NULL))",
+        )
+
     # Migration: Add password_changed_at to users (M-R7-B)
     # Tracks the last time a user's password was changed/reset.  JWTs whose iat
     # predates this timestamp are rejected in all six auth validation paths.

backend/app/models/oidc_provider.py

@@ -2,7 +2,18 @@ from __future__ import annotations
 
 from datetime import datetime
 
-from sqlalchemy import Boolean, CheckConstraint, DateTime, ForeignKey, Integer, String, Text, UniqueConstraint, func
+from sqlalchemy import (
+    Boolean,
+    CheckConstraint,
+    DateTime,
+    ForeignKey,
+    Integer,
+    LargeBinary,
+    String,
+    Text,
+    UniqueConstraint,
+    func,
+)
 from sqlalchemy.orm import Mapped, mapped_column, relationship
 
 from backend.app.core.database import Base
@@ -29,6 +40,19 @@ class OIDCProvider(Base):
             "auto_link_existing_accounts = FALSE OR email_claim != 'email' OR require_email_verified = TRUE",
             name="ck_auto_link_requires_verified_email_claim",
         ),
+        # All-or-nothing icon-cache record (#1333). The application keeps the
+        # triplet consistent via _fetch_icon_or_400 + DELETE /icon, but a CHECK
+        # constraint at the DB layer prevents drift from raw SQL maintenance
+        # scripts, manual UPDATEs during incident recovery, etc.
+        # Fresh installs (SQLite + PostgreSQL) get this via metadata.create_all.
+        # Stale PostgreSQL installs get it via ALTER TABLE ADD CONSTRAINT in
+        # run_migrations. SQLite cannot ADD CONSTRAINT to an existing table —
+        # stale SQLite installs rely on the application layer, the same
+        # trade-off documented for the default_group_id FK ON DELETE SET NULL.
+        CheckConstraint(
+            "(icon_data IS NULL) = (icon_content_type IS NULL) AND (icon_content_type IS NULL) = (icon_etag IS NULL)",
+            name="ck_oidc_icon_triplet_co_null",
+        ),
     )
 
     id: Mapped[int] = mapped_column(primary_key=True)
@@ -79,8 +103,32 @@ class OIDCProvider(Base):
     default_group_id: Mapped[int | None] = mapped_column(
         Integer, ForeignKey("groups.id", ondelete="SET NULL"), nullable=True, default=None
     )
-    # Optional icon URL (SVG/PNG) shown on the login button
+    # Optional icon URL the admin entered. The actual image bytes are fetched
+    # server-side and cached in icon_data — the SPA never hotlinks this URL
+    # (would require loosening img-src CSP; see PR #1333 / issue #1333).
     icon_url: Mapped[str | None] = mapped_column(Text, nullable=True, default=None)
+    # Cached icon bytes (PNG/JPEG/WebP/GIF). Marked deferred=True so that
+    # list-style queries (`GET /oidc/providers`) don't pull the BLOB on every
+    # login-page render — only the GET /icon endpoint un-defers it via
+    # `select(...).options(undefer(...))`.
+    icon_data: Mapped[bytes | None] = mapped_column(LargeBinary, nullable=True, default=None, deferred=True)
+    # MIME type derived from the fetched icon (e.g. "image/png"). Also serves
+    # as the "has-icon" indicator — checked instead of icon_data so we never
+    # accidentally trigger an async lazy-load on the deferred BLOB column.
+    # Width 20 is plenty: the longest whitelisted value is "image/jpeg" (10
+    # chars). Tighter than 50 so the schema documents the intent.
+    icon_content_type: Mapped[str | None] = mapped_column(String(20), nullable=True, default=None)
+    # SHA-256 hex of icon_data, served as the ETag header so clients can
+    # revalidate via If-None-Match and receive 304 Not Modified.
+    icon_etag: Mapped[str | None] = mapped_column(String(64), nullable=True, default=None)
+
+    @property
+    def has_icon(self) -> bool:
+        """True when cached icon bytes exist. Reads the non-deferred
+        ``icon_content_type`` column so accessing this never triggers an
+        async lazy-load on the deferred ``icon_data`` BLOB."""
+        return self.icon_content_type is not None
+
     created_at: Mapped[datetime] = mapped_column(DateTime, server_default=func.now())
     updated_at: Mapped[datetime] = mapped_column(DateTime, server_default=func.now(), onupdate=func.now())
 

backend/app/schemas/auth.py

@@ -310,11 +310,34 @@ def _validate_email_claim_name(v: str) -> str:
 
 
 def _validate_icon_url(v: str | None) -> str | None:
-    """Reject non-HTTPS icon URLs to prevent SSRF / mixed-content issues."""
+    """Reject non-HTTPS icon URLs and SSRF-unsafe hosts.
+
+    Delegates to the runtime SSRF guard ``assert_safe_public_https_url``
+    so the Pydantic layer enforces the same allowlist as the fetcher —
+    no policy drift between schema validation and SSRF check. Without
+    this delegation the validator covered only ``is_private | is_loopback
+    | is_link_local`` while the runtime additionally rejected numeric-
+    encoded IPs, cloud-metadata endpoints, multicast, unspecified, and
+    IPv4-mapped IPv6.
+
+    Lazy-imported because ``_oidc_helpers`` lives under ``api/routes/``
+    and schemas avoid top-level imports from that layer (matches the
+    existing pattern in ``_validate_issuer_url`` which lazy-imports
+    ``ipaddress``).
+    """
     if v is None:
         return v
     if not v.startswith("https://"):
+        # Surface the same wording the runtime guard would use, but pre-
+        # checked here so the user-facing error doesn't depend on the
+        # runtime call path.
         raise ValueError("icon_url must start with https://")
+    from backend.app.api.routes._oidc_helpers import assert_safe_public_https_url
+
+    try:
+        assert_safe_public_https_url(v)
+    except ValueError as exc:
+        raise ValueError(f"icon_url: {exc}") from exc
     return v
 
 
@@ -474,6 +497,12 @@ class OIDCProviderResponse(BaseModel):
     require_email_verified: bool = True
     icon_url: str | None = None
     default_group_id: int | None = None
+    # Set explicitly in the route handler from `icon_content_type is not None`
+    # rather than `@computed_field` (project policy) or `icon_data is not None`
+    # (would trigger an async lazy-load on the deferred BLOB column).
+    # Required (no default) so Pydantic fails loudly if any code path skips
+    # `_build_provider_response` and tries `model_validate(provider)` directly.
+    has_icon: bool
 
     class Config:
         from_attributes = True

backend/app/services/oidc_icon.py

@@ -0,0 +1,177 @@
+"""OIDC provider icon fetcher (#1333).
+
+Server-side proxy that fetches an admin-supplied icon URL and returns
+``(bytes, content_type, etag)``. The bytes are cached in the
+``oidc_providers.icon_data`` BLOB column so the SPA can serve them from
+``/api/v1/auth/oidc/providers/{id}/icon`` (same-origin) — avoiding any
+loosening of the strict ``img-src 'self' data: blob:`` CSP.
+
+Pattern mirrors ``services/makerworld.fetch_thumbnail``:
+- ``follow_redirects=False`` so the SSRF host allowlist (here: assert_safe_public_https_url)
+  isn't bypassed by a 302 to a private address.
+- MIME whitelist (PNG/JPEG/WebP/GIF). SVG is rejected in v1 — XML payloads
+  carry too many corner cases (xlink, external refs) for an MVP.
+- ``application/octet-stream`` is accepted only if the URL path ends in a
+  whitelisted image extension; the response Content-Type alone is not
+  trusted because some CDNs serve images as octet-stream.
+- 1 MB hard cap (typical OIDC icons are 5-50 KB; 1 MB is generous).
+- 10s timeout, matching the OIDC discovery/JWKS timeouts in routes/mfa.py.
+"""
+
+from __future__ import annotations
+
+import hashlib
+import logging
+from urllib.parse import urlparse
+
+import httpx
+
+logger = logging.getLogger(__name__)
+
+
+_MAX_ICON_BYTES = 1 * 1024 * 1024  # 1 MB
+_FETCH_TIMEOUT_SECONDS = 10.0
+
+# Content-Type whitelist. SVG is intentionally omitted — see module docstring.
+_ALLOWED_MIME_TYPES = frozenset(
+    {
+        "image/png",
+        "image/jpeg",
+        "image/webp",
+        "image/gif",
+    }
+)
+
+# Extension → MIME fallback for ``application/octet-stream`` responses.
+_EXT_TO_MIME = {
+    ".png": "image/png",
+    ".jpg": "image/jpeg",
+    ".jpeg": "image/jpeg",
+    ".webp": "image/webp",
+    ".gif": "image/gif",
+}
+
+
+class OIDCIconError(Exception):
+    """Base class for icon-fetch failures."""
+
+
+class OIDCIconUrlError(OIDCIconError):
+    """The URL is invalid or rejected by the SSRF guard.
+
+    Maps to a 400 Bad Request when surfaced at the API layer.
+    """
+
+
+class OIDCIconUnavailableError(OIDCIconError):
+    """The fetch reached the upstream but the response was unusable.
+
+    Network timeouts, non-200 status, wrong content-type, oversized payload,
+    redirects (we never follow), etc.  Maps to a 400 at the API layer because
+    the admin's input (the URL) is what's at fault.
+    """
+
+
+def _resolve_content_type(upstream_type: str, url_path: str) -> str:
+    """Map an upstream Content-Type to a whitelisted MIME, or raise.
+
+    Three-step derivation:
+    1. Trust upstream ``image/*`` if it's in the allowlist.
+    2. Fall back to URL extension if upstream returned
+       ``application/octet-stream`` (some CDNs do this with
+       ``Content-Disposition: attachment; filename="…png"``).
+    3. Distinct error when the header is missing entirely (#1333 review)
+       — empty quotes in a generic "unsupported content-type: ''" message
+       was user-hostile.
+
+    Extracted from ``fetch_icon`` so the dispatch logic is unit-testable
+    without spinning up the streaming-mock harness.
+    """
+    if not upstream_type:
+        raise OIDCIconUnavailableError("Icon URL response is missing a Content-Type header")
+    if upstream_type in _ALLOWED_MIME_TYPES:
+        return upstream_type
+    if upstream_type == "application/octet-stream":
+        path_lower = url_path.lower()
+        for ext, mime in _EXT_TO_MIME.items():
+            if path_lower.endswith(ext):
+                return mime
+        raise OIDCIconUnavailableError("Icon URL returned application/octet-stream with no image extension")
+    raise OIDCIconUnavailableError(
+        f"Icon URL returned unsupported content-type: {upstream_type!r} "
+        "(allowed: image/png, image/jpeg, image/webp, image/gif)"
+    )
+
+
+async def fetch_icon(url: str) -> tuple[bytes, str, str]:
+    """Fetch ``url`` and return ``(bytes, content_type, etag)``.
+
+    Streams the response body and aborts as soon as ``_MAX_ICON_BYTES`` is
+    exceeded — never buffers more than one chunk past the cap, so a hostile
+    or misconfigured IdP serving a 500 MB payload cannot OOM the server.
+
+    Raises:
+        OIDCIconUrlError: URL parsing/scheme issue OR ``httpx.InvalidURL``
+            (validator should have caught these earlier; this is a
+            defence-in-depth check).
+        OIDCIconUnavailableError: upstream issue — timeout, non-200,
+            redirect, wrong content-type, oversized payload, empty body.
+    """
+    try:
+        parsed = urlparse(url)
+    except ValueError as exc:
+        raise OIDCIconUrlError(f"Invalid icon URL: {exc}") from exc
+
+    if parsed.scheme.lower() != "https":
+        # Pydantic validator + assert_safe_public_https_url catch this earlier,
+        # but the service is the last defence — refuse non-HTTPS even if a
+        # future code path bypassed the validators.
+        raise OIDCIconUrlError("Icon URL must use https://")
+
+    try:
+        async with (
+            httpx.AsyncClient(timeout=_FETCH_TIMEOUT_SECONDS) as client,
+            client.stream("GET", url, follow_redirects=False) as response,
+        ):
+            if response.status_code != 200:
+                # Any non-200 — including 301/302 redirects (we set follow_redirects=False
+                # so the SSRF guard on the original URL isn't bypassed by a redirect
+                # to a private address).
+                raise OIDCIconUnavailableError(
+                    f"Icon URL returned HTTP {response.status_code} "
+                    "(redirects are not followed; the URL must respond with the image directly)"
+                )
+
+            upstream_type = response.headers.get("content-type", "").split(";")[0].strip().lower()
+            content_type = _resolve_content_type(upstream_type, parsed.path)
+
+            # Stream with early-exit at the size cap. Read in chunks so a
+            # hostile 500 MB body never gets allocated whole — we raise
+            # immediately when the running total crosses the cap.
+            chunks: list[bytes] = []
+            total = 0
+            async for chunk in response.aiter_bytes():
+                total += len(chunk)
+                if total > _MAX_ICON_BYTES:
+                    raise OIDCIconUnavailableError(f"Icon exceeds {_MAX_ICON_BYTES // 1024} KB cap")
+                chunks.append(chunk)
+            payload = b"".join(chunks)
+    except httpx.TimeoutException as exc:
+        raise OIDCIconUnavailableError(f"Icon fetch timed out: {exc}") from exc
+    except httpx.InvalidURL as exc:
+        # ``httpx.InvalidURL`` is a sibling of ``httpx.HTTPError`` (verified:
+        # MRO is ``InvalidURL → Exception``, no HTTPError in between). Fires
+        # at send-time for URLs that ``urlparse`` accepts but httpx refuses —
+        # typically null bytes or control chars. Map to URL-error path so
+        # the admin sees a 400, not a 500.
+        raise OIDCIconUrlError(f"Invalid icon URL: {exc}") from exc
+    except httpx.HTTPError as exc:
+        raise OIDCIconUnavailableError(f"Icon fetch failed: {exc}") from exc
+
+    if not payload:
+        raise OIDCIconUnavailableError("Icon URL returned an empty body")
+
+    # SHA-256 hex is deterministic — identical bytes always yield the same
+    # ETag so revalidation via If-None-Match works across server restarts.
+    etag = hashlib.sha256(payload).hexdigest()
+    return payload, content_type, etag

backend/tests/_fixtures/oidc_icon.py

@@ -0,0 +1,122 @@
+"""Shared test data and mock builders for OIDC icon tests (#1333).
+
+Cross-imported from ``backend.tests.unit.*`` and ``backend.tests.integration.*``
+following the established pattern (see ``backend/tests/unit/services/conftest.py``
+which imports ``mock_ftp_server``).
+
+Two mock builders are provided because ``fetch_icon`` evolved from a single
+``client.get(...)`` call into a streaming ``client.stream(...).aiter_bytes()``
+loop:
+
+* ``build_get_icon_mock`` — the pre-streaming pattern, kept for tests that
+  exercise httpx.AsyncClient.get() directly (e.g. routes that use httpx
+  outside the icon-fetcher).
+* ``build_streaming_icon_mock`` — the current ``fetch_icon`` pattern; tests
+  that exercise the size-cap early-exit need this.
+
+Both produce ``(MockHttpxClient, call_recorder)`` tuples for patching.
+"""
+
+import hashlib
+from types import SimpleNamespace
+from unittest.mock import AsyncMock
+
+# Tiny valid 1×1 transparent PNG (~70 bytes) — small enough to fit in one
+# 4 KB chunk during streaming tests; suitable for the happy-path everywhere.
+PNG_BYTES = bytes.fromhex(
+    "89504e470d0a1a0a0000000d49484452000000010000000108060000001f15c4"
+    "890000000d49444154789c63000100000005000100"
+    "0d0a2db40000000049454e44ae426082"
+)
+PNG_ETAG = hashlib.sha256(PNG_BYTES).hexdigest()
+
+
+def build_get_icon_mock(
+    *,
+    body: bytes = PNG_BYTES,
+    content_type: str | None = "image/png",
+    status_code: int = 200,
+):
+    """Returns ``(MockHttpxClient, mock_get)`` for the pre-streaming pattern.
+
+    ``mock_get`` is an ``AsyncMock`` so tests can assert call count and
+    inspect kwargs (e.g. ``follow_redirects=False``).
+
+    Passing ``content_type=None`` produces a response with no Content-Type
+    header at all (distinct from ``""``) — used to exercise the missing-
+    header branch.
+    """
+    headers: dict[str, str] = {"content-type": content_type} if content_type is not None else {}
+    response = SimpleNamespace(status_code=status_code, headers=headers, content=body)
+    mock_get = AsyncMock(return_value=response)
+
+    class _MockHttpxClient:
+        def __init__(self, *_args, **_kwargs):
+            pass
+
+        async def __aenter__(self):
+            return SimpleNamespace(get=mock_get)
+
+        async def __aexit__(self, *_exc):
+            return False
+
+    return _MockHttpxClient, mock_get
+
+
+def build_streaming_icon_mock(
+    *,
+    body: bytes = PNG_BYTES,
+    content_type: str | None = "image/png",
+    status_code: int = 200,
+    chunk_size: int = 4096,
+):
+    """Returns ``(MockHttpxClient, stream_recorder)`` for the current
+    ``client.stream("GET", url, follow_redirects=...)`` + ``aiter_bytes()`` path.
+
+    ``stream_recorder`` is an ``AsyncMock`` that records every ``.stream()``
+    call so tests can assert e.g. ``follow_redirects=False`` was passed.
+
+    ``body`` is emitted in ``chunk_size``-byte chunks. Tests of the size-cap
+    early-exit should pick a chunk_size that crosses the cap mid-stream
+    (e.g. body = 2 MB, chunk_size = 4096 → cap fires after ~256 chunks
+    without buffering the whole payload).
+    """
+    headers: dict[str, str] = {"content-type": content_type} if content_type is not None else {}
+    stream_recorder = AsyncMock()
+
+    async def _aiter_bytes():
+        for i in range(0, len(body), chunk_size):
+            yield body[i : i + chunk_size]
+
+    response = SimpleNamespace(
+        status_code=status_code,
+        headers=headers,
+        aiter_bytes=_aiter_bytes,
+    )
+
+    class _StreamCtx:
+        def __init__(self, *args, **kwargs):
+            # Record the .stream() call positional + keyword args so tests
+            # can assert `follow_redirects=False` etc.
+            stream_recorder(*args, **kwargs)
+
+        async def __aenter__(self):
+            return response
+
+        async def __aexit__(self, *_exc):
+            return False
+
+    class _MockHttpxClient:
+        def __init__(self, *_args, **_kwargs):
+            pass
+
+        async def __aenter__(self):
+            return self
+
+        async def __aexit__(self, *_exc):
+            return False
+
+        def stream(self, *args, **kwargs):
+            return _StreamCtx(*args, **kwargs)
+
+    return _MockHttpxClient, stream_recorder

backend/tests/integration/test_oidc_icon_api.py

@@ -0,0 +1,735 @@
+"""Integration tests for the OIDC icon proxy endpoints (#1333).
+
+Covers create/update/delete/refresh round-trips, the public GET /icon
+endpoint with ETag/304 behaviour, and the strict "disabled provider → 404"
+rule that protects against existence-leak on disabled providers.
+
+httpx mocking follows the project convention:
+``patch("backend.app.services.oidc_icon.httpx.AsyncClient", ...)``.
+"""
+
+from unittest.mock import patch
+
+import pytest
+from httpx import AsyncClient
+from sqlalchemy import select
+from sqlalchemy.ext.asyncio import AsyncSession
+from sqlalchemy.orm import undefer
+
+from backend.tests._fixtures.oidc_icon import (
+    PNG_BYTES as _PNG_BYTES,
+    PNG_ETAG as _PNG_ETAG,
+    build_streaming_icon_mock,
+)
+
+
+def _build_icon_mock(*, body: bytes = _PNG_BYTES, content_type: str = "image/png", status_code: int = 200):
+    """Adapter to the shared streaming-mock fixture.
+
+    Kept as a thin wrapper so individual test bodies (lots of them) stay
+    readable. Returns ``(MockHttpxClient, stream_recorder)`` — same shape
+    as the pre-streaming ``(MockHttpxClient, mock_get)`` so the per-test
+    ``mock_get.assert_called_once()`` patterns continue to mean
+    ``the fetcher hit the upstream exactly once``.
+    """
+    return build_streaming_icon_mock(body=body, content_type=content_type, status_code=status_code)
+
+
+@pytest.fixture
+async def admin_token(async_client: AsyncClient):
+    """Setup auth + return an admin token."""
+    await async_client.post(
+        "/api/v1/auth/setup",
+        json={
+            "auth_enabled": True,
+            "admin_username": "iconadmin",
+            "admin_password": "AdminPass1!",
+        },
+    )
+    login = await async_client.post(
+        "/api/v1/auth/login",
+        json={"username": "iconadmin", "password": "AdminPass1!"},
+    )
+    return login.json()["access_token"]
+
+
+def _auth_h(token: str) -> dict:
+    return {"Authorization": f"Bearer {token}"}
+
+
+def _create_payload(**overrides) -> dict:
+    """Minimal valid OIDC provider create payload; overrides shadow fields."""
+    base = {
+        "name": "Test",
+        "issuer_url": "https://idp.example.com",
+        "client_id": "client",
+        "client_secret": "secret",
+    }
+    base.update(overrides)
+    return base
+
+
+# ───────────────────────────────────────────────────────────────────────────
+# CREATE
+# ───────────────────────────────────────────────────────────────────────────
+
+
+class TestCreateProviderWithIcon:
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_create_with_valid_icon_url_fetches_and_caches(
+        self, async_client: AsyncClient, db_session: AsyncSession, admin_token: str
+    ):
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        mock_cls, mock_get = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="GoogleProv", icon_url="https://google.com/icon.png"),
+            )
+        assert resp.status_code == 201, resp.text
+        body = resp.json()
+        assert body["has_icon"] is True
+        # DB row has all three icon columns populated — undefer() is required
+        # because icon_data is deferred (deferred BLOBs raise MissingGreenlet
+        # on direct attribute access inside an async session).
+        result = await db_session.execute(
+            select(OIDCProvider).options(undefer(OIDCProvider.icon_data)).where(OIDCProvider.name == "GoogleProv")
+        )
+        provider = result.scalar_one()
+        assert provider.icon_content_type == "image/png"
+        assert provider.icon_etag == _PNG_ETAG
+        assert provider.icon_data == _PNG_BYTES
+        mock_get.assert_called_once()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_create_with_unreachable_icon_url_returns_400_no_row(
+        self, async_client: AsyncClient, db_session: AsyncSession, admin_token: str
+    ):
+        """Atomicity: failed icon-fetch leaves no row in the DB."""
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        mock_cls, _ = _build_icon_mock(status_code=404)
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="BrokenIconProv", icon_url="https://google.com/missing.png"),
+            )
+        assert resp.status_code == 400
+        result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.name == "BrokenIconProv"))
+        assert result.scalar_one_or_none() is None
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_create_without_icon_url_has_icon_false(self, async_client: AsyncClient, admin_token: str):
+        resp = await async_client.post(
+            "/api/v1/auth/oidc/providers",
+            headers=_auth_h(admin_token),
+            json=_create_payload(name="NoIconProv"),
+        )
+        assert resp.status_code == 201
+        assert resp.json()["has_icon"] is False
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_fetch_failure_logs_warning(self, async_client: AsyncClient, admin_token: str, caplog):
+        """I2: every fetch failure writes a WARNING log so operators have
+        a forensic trail beyond the admin's transient toast."""
+        import logging
+
+        mock_cls, _ = _build_icon_mock(status_code=500)
+        # The Pydantic + SSRF validators must pass for the fetcher branch
+        # to be reached; we use a public, safe URL and let the upstream
+        # mock fail with a 500.
+        with (
+            caplog.at_level(logging.WARNING, logger="backend.app.api.routes.mfa"),
+            patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls),
+        ):
+            resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="LogProv", icon_url="https://google.com/icon.png"),
+            )
+        assert resp.status_code == 400
+        warnings = [r for r in caplog.records if "fetch failed" in r.getMessage()]
+        assert warnings, "expected a WARNING log for the failed icon fetch"
+        assert "https://google.com/icon.png" in warnings[0].getMessage()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_ssrf_rejection_logs_warning(
+        self, async_client: AsyncClient, db_session: AsyncSession, admin_token: str, caplog
+    ):
+        """I2: SSRF rejection path also logs WARNING (separate branch from
+        fetch failure — same forensic-trail requirement).
+
+        The Pydantic validator now (after I1) catches private IPs at
+        422-time, so the route-level _fetch_icon_or_400 SSRF branch is
+        only reachable via /refresh on a row whose icon_url was inserted
+        directly (bypassing Pydantic). Use the test DB session to seed
+        that row, then trigger /refresh.
+        """
+        import logging
+
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        prov = OIDCProvider(
+            name="SsrfLogProv",
+            issuer_url="https://idp.example.com",
+            client_id="c",
+            scopes="openid",
+            is_enabled=True,
+            icon_url="https://192.168.1.1/icon.png",  # private — must be rejected
+        )
+        prov.client_secret = "secret"
+        db_session.add(prov)
+        await db_session.commit()
+        pid = prov.id
+
+        with caplog.at_level(logging.WARNING, logger="backend.app.api.routes.mfa"):
+            resp = await async_client.post(
+                f"/api/v1/auth/oidc/providers/{pid}/icon/refresh",
+                headers=_auth_h(admin_token),
+            )
+        assert resp.status_code == 400
+        warnings = [r for r in caplog.records if "SSRF guard" in r.getMessage()]
+        assert warnings, "expected a WARNING log for the SSRF rejection"
+        assert "192.168.1.1" in warnings[0].getMessage()
+
+
+# ───────────────────────────────────────────────────────────────────────────
+# UPDATE
+# ───────────────────────────────────────────────────────────────────────────
+
+
+class TestUpdateProviderIcon:
+    async def _create_with_icon(self, async_client, admin_token, name="UpdProv") -> int:
+        mock_cls, _ = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name=name, icon_url="https://example.com/a.png"),
+            )
+        assert resp.status_code == 201, resp.text
+        return resp.json()["id"]
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_put_without_icon_url_field_does_not_refetch(self, async_client: AsyncClient, admin_token: str):
+        pid = await self._create_with_icon(async_client, admin_token)
+        mock_cls, mock_get = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            resp = await async_client.put(
+                f"/api/v1/auth/oidc/providers/{pid}",
+                headers=_auth_h(admin_token),
+                json={"name": "Renamed"},
+            )
+        assert resp.status_code == 200
+        mock_get.assert_not_called()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_put_with_unchanged_icon_url_and_data_present_skips_fetch(
+        self, async_client: AsyncClient, admin_token: str
+    ):
+        pid = await self._create_with_icon(async_client, admin_token)
+        mock_cls, mock_get = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            resp = await async_client.put(
+                f"/api/v1/auth/oidc/providers/{pid}",
+                headers=_auth_h(admin_token),
+                json={"icon_url": "https://example.com/a.png"},
+            )
+        assert resp.status_code == 200
+        mock_get.assert_not_called()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_put_with_unchanged_url_but_missing_cached_bytes_refetches(
+        self, async_client: AsyncClient, db_session: AsyncSession, admin_token: str
+    ):
+        """Upgrade-path edge case: existing row with icon_url but no cached bytes
+        (e.g. row predates the proxy migration). Saving must trigger a fetch."""
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        pid = await self._create_with_icon(async_client, admin_token, name="UpgrTest")
+        # Simulate the upgrade scenario: clear the cached bytes directly in DB.
+        result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.id == pid))
+        prov = result.scalar_one()
+        prov.icon_data = None
+        prov.icon_content_type = None
+        prov.icon_etag = None
+        await db_session.commit()
+
+        mock_cls, mock_get = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            resp = await async_client.put(
+                f"/api/v1/auth/oidc/providers/{pid}",
+                headers=_auth_h(admin_token),
+                json={"icon_url": "https://example.com/a.png"},  # unchanged URL
+            )
+        assert resp.status_code == 200
+        mock_get.assert_called_once()
+        assert resp.json()["has_icon"] is True
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_put_with_changed_icon_url_refetches(self, async_client: AsyncClient, admin_token: str):
+        pid = await self._create_with_icon(async_client, admin_token, name="ChangedUrlProv")
+        mock_cls, mock_get = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            resp = await async_client.put(
+                f"/api/v1/auth/oidc/providers/{pid}",
+                headers=_auth_h(admin_token),
+                json={"icon_url": "https://example.com/b.png"},
+            )
+        assert resp.status_code == 200
+        mock_get.assert_called_once()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_put_with_icon_url_null_clears_icon(
+        self, async_client: AsyncClient, db_session: AsyncSession, admin_token: str
+    ):
+        """Explicit ``icon_url: null`` in the PUT body clears icon_url AND
+        all three cached-bytes columns. Distinct from "field absent" which
+        leaves the icon untouched.
+        """
+        from backend.app.models.oidc_provider import OIDCProvider
+        from sqlalchemy.orm import undefer
+
+        pid = await self._create_with_icon(async_client, admin_token, name="ClearViaPutProv")
+
+        # Sanity: icon is present before the clear.
+        pre_resp = await async_client.get(f"/api/v1/auth/oidc/providers/{pid}/icon")
+        assert pre_resp.status_code == 200
+
+        # PUT with explicit null clears icon_url + cached bytes.
+        resp = await async_client.put(
+            f"/api/v1/auth/oidc/providers/{pid}",
+            headers=_auth_h(admin_token),
+            json={"icon_url": None},
+        )
+        assert resp.status_code == 200, resp.text
+        body = resp.json()
+        assert body["icon_url"] is None
+        assert body["has_icon"] is False
+
+        # DB state: all four icon columns NULL.
+        db_session.expire_all()
+        result = await db_session.execute(
+            select(OIDCProvider).options(undefer(OIDCProvider.icon_data)).where(OIDCProvider.id == pid)
+        )
+        prov = result.scalar_one()
+        assert prov.icon_url is None
+        assert prov.icon_data is None
+        assert prov.icon_content_type is None
+        assert prov.icon_etag is None
+
+        # GET /icon now 404s.
+        post_resp = await async_client.get(f"/api/v1/auth/oidc/providers/{pid}/icon")
+        assert post_resp.status_code == 404
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_put_with_broken_new_icon_url_preserves_old_bytes(
+        self, async_client: AsyncClient, db_session: AsyncSession, admin_token: str
+    ):
+        """Atomicity: failed icon-fetch on PUT must not clear old cached bytes."""
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        pid = await self._create_with_icon(async_client, admin_token, name="AtomicProv")
+        # Failed fetch (404).
+        mock_cls, _ = _build_icon_mock(status_code=404)
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            resp = await async_client.put(
+                f"/api/v1/auth/oidc/providers/{pid}",
+                headers=_auth_h(admin_token),
+                json={"icon_url": "https://example.com/broken.png"},
+            )
+        assert resp.status_code == 400
+        # Re-read state: row still has the original icon bytes.
+        db_session.expire_all()
+        result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.id == pid))
+        prov = result.scalar_one()
+        assert prov.icon_content_type == "image/png"
+        assert prov.icon_etag == _PNG_ETAG
+        # icon_url also unchanged (rollback works) — admin sees no partial state.
+        assert prov.icon_url == "https://example.com/a.png"
+
+
+# ───────────────────────────────────────────────────────────────────────────
+# DELETE /icon
+# ───────────────────────────────────────────────────────────────────────────
+
+
+class TestDeleteIcon:
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_delete_icon_clears_columns(
+        self, async_client: AsyncClient, db_session: AsyncSession, admin_token: str
+    ):
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        mock_cls, _ = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            create_resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="DelIconProv", icon_url="https://example.com/icon.png"),
+            )
+        pid = create_resp.json()["id"]
+
+        resp = await async_client.delete(
+            f"/api/v1/auth/oidc/providers/{pid}/icon",
+            headers=_auth_h(admin_token),
+        )
+        assert resp.status_code == 204
+
+        db_session.expire_all()
+        # undefer icon_data so we can assert it's None without triggering
+        # an async lazy-load (which would raise MissingGreenlet).
+        result = await db_session.execute(
+            select(OIDCProvider).options(undefer(OIDCProvider.icon_data)).where(OIDCProvider.id == pid)
+        )
+        prov = result.scalar_one()
+        assert prov.icon_data is None
+        assert prov.icon_content_type is None
+        assert prov.icon_etag is None
+        # DELETE /icon clears the URL too — "Remove icon" means the whole
+        # record is gone, not just the cache. Without this the admin form
+        # would still show a stale URL while the login page rendered the
+        # Shield fallback (confusing half-state).
+        assert prov.icon_url is None
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_delete_icon_without_auth_rejected(self, async_client: AsyncClient, admin_token: str):
+        # Create with admin auth, then try to DELETE icon anonymously.
+        mock_cls, _ = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            create_resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="AuthGuardProv", icon_url="https://example.com/i.png"),
+            )
+        pid = create_resp.json()["id"]
+        resp = await async_client.delete(f"/api/v1/auth/oidc/providers/{pid}/icon")
+        assert resp.status_code in (401, 403)
+
+
+# ───────────────────────────────────────────────────────────────────────────
+# REFRESH /icon
+# ───────────────────────────────────────────────────────────────────────────
+
+
+class TestRefreshIcon:
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_refresh_fetches_from_stored_url(
+        self, async_client: AsyncClient, db_session: AsyncSession, admin_token: str
+    ):
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        mock_cls, _ = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            create_resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="RefProv", icon_url="https://example.com/i.png"),
+            )
+        pid = create_resp.json()["id"]
+
+        # Now clear in DB (simulate icon corruption / IdP change)
+        result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.id == pid))
+        prov = result.scalar_one()
+        prov.icon_data = None
+        prov.icon_content_type = None
+        prov.icon_etag = None
+        await db_session.commit()
+
+        new_png = _PNG_BYTES + b"\x00\x01"
+        mock_cls2, mock_get2 = _build_icon_mock(body=new_png)
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls2):
+            resp = await async_client.post(
+                f"/api/v1/auth/oidc/providers/{pid}/icon/refresh",
+                headers=_auth_h(admin_token),
+            )
+        assert resp.status_code == 200, resp.text
+        assert resp.json()["has_icon"] is True
+        mock_get2.assert_called_once()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_refresh_without_icon_url_returns_400(self, async_client: AsyncClient, admin_token: str):
+        # Create provider without an icon_url
+        create_resp = await async_client.post(
+            "/api/v1/auth/oidc/providers",
+            headers=_auth_h(admin_token),
+            json=_create_payload(name="NoUrlRef"),
+        )
+        pid = create_resp.json()["id"]
+        resp = await async_client.post(
+            f"/api/v1/auth/oidc/providers/{pid}/icon/refresh",
+            headers=_auth_h(admin_token),
+        )
+        assert resp.status_code == 400
+        assert "no icon_url" in resp.json()["detail"].lower()
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_refresh_failure_preserves_old_bytes(
+        self, async_client: AsyncClient, db_session: AsyncSession, admin_token: str
+    ):
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        mock_cls, _ = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            create_resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="RefAtomicProv", icon_url="https://example.com/i.png"),
+            )
+        pid = create_resp.json()["id"]
+
+        mock_cls_fail, _ = _build_icon_mock(status_code=500)
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls_fail):
+            resp = await async_client.post(
+                f"/api/v1/auth/oidc/providers/{pid}/icon/refresh",
+                headers=_auth_h(admin_token),
+            )
+        assert resp.status_code == 400
+
+        db_session.expire_all()
+        result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.id == pid))
+        prov = result.scalar_one()
+        assert prov.icon_etag == _PNG_ETAG  # original bytes intact
+
+
+# ───────────────────────────────────────────────────────────────────────────
+# GET /icon — the public icon-proxy endpoint
+# ───────────────────────────────────────────────────────────────────────────
+
+
+class TestGetProviderIcon:
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_anonymous_get_returns_bytes(self, async_client: AsyncClient, admin_token: str):
+        mock_cls, _ = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            create_resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="PubIconProv", icon_url="https://example.com/i.png"),
+            )
+        pid = create_resp.json()["id"]
+
+        # Anonymous request — no Authorization header at all.
+        resp = await async_client.get(f"/api/v1/auth/oidc/providers/{pid}/icon")
+        assert resp.status_code == 200
+        assert resp.content == _PNG_BYTES
+        assert resp.headers["content-type"] == "image/png"
+        assert resp.headers["etag"] == f'"{_PNG_ETAG}"'
+        assert resp.headers["cache-control"] == "public, max-age=3600"
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_get_without_cached_data_returns_404(self, async_client: AsyncClient, admin_token: str):
+        create_resp = await async_client.post(
+            "/api/v1/auth/oidc/providers",
+            headers=_auth_h(admin_token),
+            json=_create_payload(name="EmptyIconProv"),  # no icon_url → no bytes
+        )
+        pid = create_resp.json()["id"]
+        resp = await async_client.get(f"/api/v1/auth/oidc/providers/{pid}/icon")
+        assert resp.status_code == 404
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_get_for_disabled_provider_returns_404(
+        self, async_client: AsyncClient, db_session: AsyncSession, admin_token: str
+    ):
+        """Disabled providers must not leak existence via the icon endpoint."""
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        mock_cls, _ = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            create_resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="DisabledProv", icon_url="https://example.com/d.png"),
+            )
+        pid = create_resp.json()["id"]
+        # Disable directly in DB.
+        result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.id == pid))
+        prov = result.scalar_one()
+        prov.is_enabled = False
+        await db_session.commit()
+
+        resp = await async_client.get(f"/api/v1/auth/oidc/providers/{pid}/icon")
+        assert resp.status_code == 404
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_if_none_match_exact_returns_304(self, async_client: AsyncClient, admin_token: str):
+        mock_cls, _ = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            create_resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="EtagProv", icon_url="https://example.com/e.png"),
+            )
+        pid = create_resp.json()["id"]
+        resp = await async_client.get(
+            f"/api/v1/auth/oidc/providers/{pid}/icon",
+            headers={"If-None-Match": f'"{_PNG_ETAG}"'},
+        )
+        assert resp.status_code == 304
+        assert resp.content == b""
+        assert resp.headers["etag"] == f'"{_PNG_ETAG}"'
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_if_none_match_mismatch_returns_200(self, async_client: AsyncClient, admin_token: str):
+        mock_cls, _ = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            create_resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="EtagMismatchProv", icon_url="https://example.com/m.png"),
+            )
+        pid = create_resp.json()["id"]
+        resp = await async_client.get(
+            f"/api/v1/auth/oidc/providers/{pid}/icon",
+            headers={"If-None-Match": '"stale-etag-value"'},
+        )
+        assert resp.status_code == 200
+        assert resp.content == _PNG_BYTES
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_if_none_match_weak_prefix_returns_304(self, async_client: AsyncClient, admin_token: str):
+        """N5 — RFC 7232 §2.3 weak validator prefix ``W/"…"`` must match.
+
+        CDN intermediaries and some browsers send weak validators on GET
+        even though we issue strong ones; without the W/ strip a 200 was
+        returned needlessly.
+        """
+        mock_cls, _ = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            create_resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="EtagWeakProv", icon_url="https://example.com/w.png"),
+            )
+        pid = create_resp.json()["id"]
+        resp = await async_client.get(
+            f"/api/v1/auth/oidc/providers/{pid}/icon",
+            headers={"If-None-Match": f'W/"{_PNG_ETAG}"'},
+        )
+        assert resp.status_code == 304
+        assert resp.content == b""
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_if_none_match_wildcard_returns_304(self, async_client: AsyncClient, admin_token: str):
+        """N5 — RFC 7232 §3.2 ``*`` wildcard matches any current
+        representation when the resource exists. We always have an icon
+        here (resource existence verified above by the 404 path) so ``*``
+        always means "I have something; tell me if it's stale" → 304."""
+        mock_cls, _ = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            create_resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="EtagWildcardProv", icon_url="https://example.com/wc.png"),
+            )
+        pid = create_resp.json()["id"]
+        resp = await async_client.get(
+            f"/api/v1/auth/oidc/providers/{pid}/icon",
+            headers={"If-None-Match": "*"},
+        )
+        assert resp.status_code == 304
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_if_none_match_multiple_tokens_one_match(self, async_client: AsyncClient, admin_token: str):
+        """N5 — comma-separated list with one matching token returns 304."""
+        mock_cls, _ = _build_icon_mock()
+        with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+            create_resp = await async_client.post(
+                "/api/v1/auth/oidc/providers",
+                headers=_auth_h(admin_token),
+                json=_create_payload(name="EtagMultiProv", icon_url="https://example.com/m2.png"),
+            )
+        pid = create_resp.json()["id"]
+        resp = await async_client.get(
+            f"/api/v1/auth/oidc/providers/{pid}/icon",
+            headers={"If-None-Match": f'"stale", "{_PNG_ETAG}"'},
+        )
+        assert resp.status_code == 304
+
+
+# ───────────────────────────────────────────────────────────────────────────
+# N12 — Edge cases (404 paths, inconsistent triplet via raw SQL)
+# ───────────────────────────────────────────────────────────────────────────
+
+
+class TestEdgeCases:
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_delete_icon_on_nonexistent_provider_returns_404(self, async_client: AsyncClient, admin_token: str):
+        """N12 — DELETE /icon on a missing provider_id must 404, not 500."""
+        resp = await async_client.delete(
+            "/api/v1/auth/oidc/providers/99999/icon",
+            headers=_auth_h(admin_token),
+        )
+        assert resp.status_code == 404
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_refresh_icon_on_nonexistent_provider_returns_404(self, async_client: AsyncClient, admin_token: str):
+        """N12 — POST /icon/refresh on a missing provider_id must 404, not 500."""
+        resp = await async_client.post(
+            "/api/v1/auth/oidc/providers/99999/icon/refresh",
+            headers=_auth_h(admin_token),
+        )
+        assert resp.status_code == 404
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_get_icon_with_inconsistent_triplet_returns_404(
+        self, async_client: AsyncClient, db_session: AsyncSession, admin_token: str
+    ):
+        """N12 — defensive double-check on the GET /icon endpoint.
+
+        The CHECK constraint (#1333 / N10) prevents this state from being
+        reachable via the application, but the defensive
+        ``provider.icon_data is None`` guard at the route layer is what
+        protects against a manual SQL hotfix that bypassed the constraint
+        (e.g. operator-run UPDATE during incident recovery on stale
+        SQLite where the CHECK isn't present). We can't write such a row
+        via SQLAlchemy here (the CHECK fires), so we verify the
+        equivalent path: a provider with NO icon at all returns 404.
+        """
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        prov = OIDCProvider(
+            name="EmptyTripletProv",
+            issuer_url="https://idp.example.com",
+            client_id="c",
+            scopes="openid",
+            is_enabled=True,
+        )
+        prov.client_secret = "secret"
+        db_session.add(prov)
+        await db_session.commit()
+        pid = prov.id
+
+        resp = await async_client.get(f"/api/v1/auth/oidc/providers/{pid}/icon")
+        assert resp.status_code == 404

backend/tests/integration/test_oidc_icon_blob_roundtrip.py

@@ -0,0 +1,151 @@
+"""Type-mapping coverage for the OIDC icon BLOB column (#1333).
+
+Bambuddy's ``create_backup_zip`` rebuilds the SQLite backup schema from
+``Base.metadata`` when the source database is PostgreSQL. The column-type
+mapping previously fell through to ``TEXT`` for any unknown SQLAlchemy
+type — including ``LargeBinary`` / ``BYTEA`` — which corrupts non-UTF8
+icon bytes during the PG → SQLite-ZIP round trip.
+
+These tests exercise the extracted ``_sqlalchemy_type_to_sqlite_type``
+helper directly so the regression guard doesn't depend on a full backup
+pipeline. The SQLite source path is just ``shutil.copy2`` of the live
+.db file and is therefore unaffected by the type mapping.
+"""
+
+import hashlib
+import sqlite3
+
+import pytest
+from sqlalchemy import Column, LargeBinary
+from sqlalchemy.ext.asyncio import AsyncSession
+
+from backend.app.api.routes.settings import _sqlalchemy_type_to_sqlite_type
+from backend.tests._fixtures.oidc_icon import PNG_BYTES as _PNG_BYTES
+
+
+class TestTypeMapping:
+    """Unit-level coverage of the helper that backups use for PG→SQLite."""
+
+    def test_largebinary_maps_to_blob(self):
+        # Direct from a SQLAlchemy LargeBinary column — this is exactly
+        # what the create_backup_zip loop calls str() on.
+        col = Column(LargeBinary)
+        assert _sqlalchemy_type_to_sqlite_type(str(col.type)) == "BLOB"
+
+    @pytest.mark.parametrize(
+        "type_repr",
+        ["BLOB", "BYTEA", "BYTEA(1024)", "VARBINARY", "BINARY", "binary varying"],
+    )
+    def test_binary_type_strings_map_to_blob(self, type_repr):
+        assert _sqlalchemy_type_to_sqlite_type(type_repr) == "BLOB"
+
+    def test_integer_unchanged(self):
+        assert _sqlalchemy_type_to_sqlite_type("INTEGER") == "INTEGER"
+        assert _sqlalchemy_type_to_sqlite_type("BIGINT") == "INTEGER"
+
+    def test_boolean_unchanged(self):
+        assert _sqlalchemy_type_to_sqlite_type("BOOLEAN") == "BOOLEAN"
+
+    def test_unknown_falls_back_to_text(self):
+        assert _sqlalchemy_type_to_sqlite_type("VARCHAR(500)") == "TEXT"
+        assert _sqlalchemy_type_to_sqlite_type("DATETIME") == "TEXT"
+
+
+class TestSqliteBinaryRoundtrip:
+    """SQLite natively stores BLOB without escaping — sanity-check that the
+    serialise/deserialise path used by the PG→SQLite backup (``executemany``
+    with bytes values) preserves non-UTF8 bytes exactly."""
+
+    def test_binary_value_roundtrips_through_sqlite_blob(self, tmp_path):
+        db_path = tmp_path / "roundtrip.db"
+        conn = sqlite3.connect(str(db_path))
+        try:
+            conn.execute("CREATE TABLE t (id INTEGER PRIMARY KEY, blob BLOB)")
+            # A payload that's deliberately not UTF8-decodable.
+            payload = bytes(range(256))
+            conn.execute("INSERT INTO t (id, blob) VALUES (?, ?)", (1, payload))
+            conn.commit()
+            row = conn.execute("SELECT blob FROM t WHERE id = 1").fetchone()
+            assert row[0] == payload
+        finally:
+            conn.close()
+
+
+class TestIconTripletCheckConstraint:
+    """N10 — DB-level enforcement of the icon-cache triplet invariant.
+
+    The CHECK constraint applies on SQLite fresh installs (via
+    metadata.create_all) and on PostgreSQL fresh + stale installs (via
+    ALTER TABLE ADD CONSTRAINT). Stale SQLite installs do not get it
+    (SQLite cannot ADD CONSTRAINT to an existing table) — documented
+    trade-off, application layer enforces.
+    """
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_full_triplet_accepted(self, db_session: AsyncSession):
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        prov = OIDCProvider(
+            name="TripletFullProv",
+            issuer_url="https://idp.example.com",
+            client_id="c",
+            scopes="openid",
+            is_enabled=True,
+        )
+        prov.client_secret = "secret"
+        prov.icon_data = _PNG_BYTES
+        prov.icon_content_type = "image/png"
+        prov.icon_etag = hashlib.sha256(_PNG_BYTES).hexdigest()
+        db_session.add(prov)
+        await db_session.commit()  # must not raise
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_all_null_triplet_accepted(self, db_session: AsyncSession):
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        prov = OIDCProvider(
+            name="TripletEmptyProv",
+            issuer_url="https://idp.example.com",
+            client_id="c",
+            scopes="openid",
+            is_enabled=True,
+        )
+        prov.client_secret = "secret"
+        # All three icon columns left as default None.
+        db_session.add(prov)
+        await db_session.commit()  # must not raise
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_partial_triplet_rejected_by_check_constraint(self, db_session: AsyncSession):
+        """Direct UPDATE that sets only icon_content_type (no icon_data, no
+        icon_etag) must violate the CHECK constraint on a fresh SQLite
+        install (CHECK constraints fire on SQLite even when foreign keys
+        are off). Demonstrates the CHECK is the catch-net for raw-SQL
+        maintenance paths that bypass _fetch_icon_or_400.
+        """
+        from sqlalchemy import text
+        from sqlalchemy.exc import IntegrityError
+
+        from backend.app.models.oidc_provider import OIDCProvider
+
+        prov = OIDCProvider(
+            name="TripletPartialProv",
+            issuer_url="https://idp.example.com",
+            client_id="c",
+            scopes="openid",
+            is_enabled=True,
+        )
+        prov.client_secret = "secret"
+        db_session.add(prov)
+        await db_session.commit()
+        pid = prov.id
+
+        with pytest.raises(IntegrityError):
+            await db_session.execute(
+                text("UPDATE oidc_providers SET icon_content_type = :ct WHERE id = :pid"),
+                {"ct": "image/png", "pid": pid},
+            )
+            await db_session.commit()

backend/tests/integration/test_oidc_icon_deferred_load.py

@@ -0,0 +1,95 @@
+"""Verify icon_data stays deferred on list queries (#1333).
+
+Regression guard: if ``deferred=True`` ever gets dropped from
+``OIDCProvider.icon_data``, every login-page hit pulls the full BLOB on
+the listing query, adding ~MB of bandwidth per anonymous request. These
+tests assert via SQLAlchemy's instance inspector that the column is
+**not** loaded by default and **is** loaded after an explicit
+``undefer()``.
+"""
+
+import hashlib
+
+import pytest
+from sqlalchemy import inspect, select
+from sqlalchemy.ext.asyncio import AsyncSession
+from sqlalchemy.orm import undefer
+
+_PNG_BYTES = bytes.fromhex(
+    "89504e470d0a1a0a0000000d49484452000000010000000108060000001f15c4"
+    "890000000d49444154789c63000100000005000100"
+    "0d0a2db40000000049454e44ae426082"
+)
+
+
+async def _seed_provider(db_session: AsyncSession, name: str = "DeferredProv"):
+    from backend.app.models.oidc_provider import OIDCProvider
+
+    provider = OIDCProvider(
+        name=name,
+        issuer_url="https://idp.example.com",
+        client_id="c",
+        scopes="openid",
+        is_enabled=True,
+    )
+    provider.client_secret = "secret"
+    provider.icon_url = "https://example.com/icon.png"
+    provider.icon_data = _PNG_BYTES
+    provider.icon_content_type = "image/png"
+    provider.icon_etag = hashlib.sha256(_PNG_BYTES).hexdigest()
+    db_session.add(provider)
+    await db_session.commit()
+    db_session.expire_all()  # force the next read to come from DB, not identity-map
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_default_list_query_does_not_load_icon_data(db_session: AsyncSession):
+    """`select(OIDCProvider)` without options keeps icon_data unloaded."""
+    from backend.app.models.oidc_provider import OIDCProvider
+
+    await _seed_provider(db_session)
+    result = await db_session.execute(select(OIDCProvider))
+    provider = result.scalar_one()
+
+    state = inspect(provider)
+    assert "icon_data" in state.unloaded, (
+        "icon_data should be deferred on the default list query — "
+        "without this guard every login page hit pulls the full BLOB."
+    )
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_undefer_loads_icon_data(db_session: AsyncSession):
+    """`select(...).options(undefer(...))` loads icon_data eagerly."""
+    from backend.app.models.oidc_provider import OIDCProvider
+
+    await _seed_provider(db_session, name="UndeferProv")
+    result = await db_session.execute(
+        select(OIDCProvider).options(undefer(OIDCProvider.icon_data)).where(OIDCProvider.name == "UndeferProv")
+    )
+    provider = result.scalar_one()
+
+    state = inspect(provider)
+    assert "icon_data" not in state.unloaded, "undefer() must eagerly load icon_data"
+    # And the bytes are accessible without raising MissingGreenlet.
+    assert provider.icon_data == _PNG_BYTES
+
+
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_icon_content_type_is_eager_indicator(db_session: AsyncSession):
+    """icon_content_type must NOT be deferred — it's the eager has-icon
+    indicator that route handlers consult instead of icon_data, so it must
+    be loaded on every default query."""
+    from backend.app.models.oidc_provider import OIDCProvider
+
+    await _seed_provider(db_session, name="IndicatorProv")
+    result = await db_session.execute(select(OIDCProvider).where(OIDCProvider.name == "IndicatorProv"))
+    provider = result.scalar_one()
+
+    state = inspect(provider)
+    assert "icon_content_type" not in state.unloaded
+    # Direct access does not raise MissingGreenlet (it was already loaded).
+    assert provider.icon_content_type == "image/png"

backend/tests/integration/test_security_headers.py

@@ -148,6 +148,39 @@ async def test_trusted_origins_applies_to_docs_branch(async_client: AsyncClient,
     assert "frame-ancestors 'self' https://ha.example.com;" in csp
 
 
+@pytest.mark.asyncio
+@pytest.mark.integration
+async def test_default_block_img_src_excludes_https(async_client: AsyncClient, monkeypatch):
+    """#1333 regression guard: the default SPA CSP must NOT allow img-src https:.
+
+    Bambuddy's policy for external images is a backend proxy (see
+    /api/v1/makerworld/thumbnail and /api/v1/auth/oidc/providers/{id}/icon),
+    not a CSP relaxation. If a future change adds ``https:`` to img-src to
+    "fix" a broken-image, the proxy pattern silently degrades into a
+    do-nothing layer and the entire SPA gains a hot-link surface.
+    """
+    from backend.app import main as main_module
+
+    monkeypatch.setattr(main_module, "_TRUSTED_FRAME_ORIGINS", ())
+
+    resp = await async_client.get("/api/v1/auth/status")
+    csp = resp.headers.get("Content-Security-Policy", "")
+    # Extract the img-src directive — splits on ';' for safety against
+    # neighbouring directives that happen to contain the substring.
+    img_src_directive = next(
+        (d.strip() for d in csp.split(";") if d.strip().startswith("img-src")),
+        "",
+    )
+    assert img_src_directive, f"img-src directive missing from CSP: {csp!r}"
+    assert "https:" not in img_src_directive, (
+        f"img-src must not allow arbitrary https: hosts (proxy external images instead); got: {img_src_directive!r}"
+    )
+    # Sanity: the legitimately allowed scheme sources are still present.
+    assert "'self'" in img_src_directive
+    assert "data:" in img_src_directive
+    assert "blob:" in img_src_directive
+
+
 @pytest.mark.asyncio
 @pytest.mark.integration
 async def test_other_security_headers_unchanged(async_client: AsyncClient, monkeypatch):

backend/tests/unit/test_oidc_icon_helpers.py

@@ -0,0 +1,125 @@
+"""Unit tests for assert_safe_public_https_url (#1333).
+
+Stricter than the Spoolman SSRF guard — explicitly verifies that Spoolman-
+allowed cases (loopback, RFC-1918, hostname "localhost") are REJECTED here.
+Run alongside test_ssrf_guard.py to confirm both guards keep their distinct
+semantics.
+"""
+
+import pytest
+
+from backend.app.api.routes._oidc_helpers import assert_safe_public_https_url
+
+# ─── Accepts ────────────────────────────────────────────────────────────────
+
+
+def test_accepts_public_https():
+    assert_safe_public_https_url("https://accounts.google.com/icon.png")
+
+
+def test_accepts_hostname_no_dns_resolution():
+    # By design — DNS resolution is intentionally not performed (consistent
+    # with _validate_issuer_url policy). Hostnames are out of scope here.
+    assert_safe_public_https_url("https://idp.internal.corp/icon.png")
+
+
+# ─── Rejects: non-HTTPS schemes ─────────────────────────────────────────────
+
+
+@pytest.mark.parametrize(
+    "url",
+    [
+        "http://example.com/icon.png",
+        "ftp://example.com/icon.png",
+        "file:///etc/passwd",
+        "gopher://example.com",
+    ],
+)
+def test_rejects_non_https(url):
+    with pytest.raises(ValueError, match="https"):
+        assert_safe_public_https_url(url)
+
+
+# ─── Rejects: numeric-encoded IP addresses ──────────────────────────────────
+
+
+@pytest.mark.parametrize(
+    "url",
+    [
+        "https://2130706433/icon.png",  # decimal-encoded 127.0.0.1
+        "https://0x7f000001/icon.png",  # hex-encoded 127.0.0.1
+    ],
+)
+def test_rejects_numeric_encoded_ip(url):
+    with pytest.raises(ValueError, match="numeric-encoded"):
+        assert_safe_public_https_url(url)
+
+
+# ─── Rejects: cases that Spoolman INTENTIONALLY allows ──────────────────────
+# These tests are deliberately structured to mirror test_ssrf_guard.py — every
+# URL here is a green case for the Spoolman guard and must be a red case here.
+
+
+def test_rejects_loopback_127():
+    with pytest.raises(ValueError, match="loopback"):
+        assert_safe_public_https_url("https://127.0.0.1/icon.png")
+
+
+@pytest.mark.parametrize(
+    "url",
+    [
+        "https://192.168.1.50/icon.png",
+        "https://10.0.0.5/icon.png",
+        "https://172.16.0.1/icon.png",
+    ],
+)
+def test_rejects_rfc1918_private(url):
+    with pytest.raises(ValueError, match="private"):
+        assert_safe_public_https_url(url)
+
+
+# ─── Rejects: link-local, cloud-metadata, multicast, unspecified ────────────
+
+
+def test_rejects_link_local():
+    with pytest.raises(ValueError, match="link-local|cloud metadata"):
+        # 169.254.169.254 is BOTH link-local AND cloud-metadata — both
+        # rejections are correct; we accept either message.
+        assert_safe_public_https_url("https://169.254.169.254/icon.png")
+
+
+def test_rejects_cloud_metadata_alibaba():
+    with pytest.raises(ValueError, match="cloud metadata"):
+        assert_safe_public_https_url("https://100.100.100.200/icon.png")
+
+
+def test_rejects_multicast():
+    with pytest.raises(ValueError, match="multicast"):
+        assert_safe_public_https_url("https://224.0.0.1/icon.png")
+
+
+def test_rejects_unspecified():
+    with pytest.raises(ValueError, match="unspecified"):
+        assert_safe_public_https_url("https://0.0.0.0/icon.png")
+
+
+# ─── IPv6 ──────────────────────────────────────────────────────────────────
+
+
+def test_rejects_ipv4_mapped_private():
+    # ::ffff:192.168.1.1 unwraps to 192.168.1.1 → private
+    with pytest.raises(ValueError, match="private"):
+        assert_safe_public_https_url("https://[::ffff:192.168.1.1]/icon.png")
+
+
+def test_rejects_ipv4_mapped_cloud_metadata():
+    with pytest.raises(ValueError, match="cloud metadata|link-local"):
+        # ::ffff:169.254.169.254 unwraps and triggers cloud-metadata block
+        # (the cloud-metadata frozenset is checked first; link-local catches
+        # 169.254/16 if it slips through, hence the regex alternation).
+        assert_safe_public_https_url("https://[::ffff:169.254.169.254]/icon.png")
+
+
+def test_rejects_ipv6_loopback():
+    with pytest.raises(ValueError, match="loopback"):
+        assert_safe_public_https_url("https://[::1]/icon.png")

backend/tests/unit/test_oidc_icon_migration_pg.py

@@ -0,0 +1,102 @@
+"""Verify the dialect-conditional branch of the icon-column migration (#1333).
+
+``run_migrations`` issues ``ALTER TABLE … ADD COLUMN icon_data {BLOB|BYTEA}``
+based on ``is_sqlite()``. The full migration only runs against a live
+engine, so we monkey-patch ``is_sqlite()`` and capture the SQL passed to
+``_safe_execute``. Mirrors the test pattern at
+``backend/tests/unit/test_db_dialect.py`` (lines around 539-606) which is
+already used to verify other dialect-conditional migrations.
+
+Without this test the PostgreSQL branch would be dead code in CI (the
+project's tests run on SQLite) and a typo in the BYTEA emission would
+slip silently to production, where ``_safe_execute`` would swallow the
+column-creation failure and PG users would never cache icon bytes.
+"""
+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from backend.app.core import database as db_module
+
+
+class _AsyncCtxStub:
+    """Async context manager that does nothing — for ``begin_nested()``."""
+
+    async def __aenter__(self):
+        return self
+
+    async def __aexit__(self, *_exc):
+        return False
+
+
+async def _capture_sql(is_sqlite_value: bool) -> list[str]:
+    """Patch ``is_sqlite()`` + ``_safe_execute`` and return every SQL string
+    that would have been executed during ``run_migrations``.
+
+    Sub-migration callables that don't emit ALTER TABLE icon_data (the auto-
+    link constraint update and the AMS-id widening) are no-op'd to keep the
+    test focused on the icon migration.
+
+    ``run_migrations`` uses ``async with conn.begin_nested()`` for the few
+    DML backfills, so the fake conn returns a real async context manager.
+    Inline ``conn.execute()`` calls (in the SQLite-recreation branch only,
+    which we exclude) are also wired up to record SQL — but the bulk of
+    the DDL goes through ``_safe_execute`` which is what we capture.
+    """
+    executed_sql: list[str] = []
+
+    async def fake_safe_execute(_conn, sql: str) -> None:
+        executed_sql.append(sql)
+
+    fake_conn = MagicMock()
+    fake_conn.begin_nested = lambda: _AsyncCtxStub()
+    fake_conn.execute = AsyncMock(return_value=MagicMock(fetchone=MagicMock(return_value=None)))
+
+    with (
+        patch("backend.app.core.database.is_sqlite", return_value=is_sqlite_value),
+        patch("backend.app.core.database._safe_execute", side_effect=fake_safe_execute),
+        patch("backend.app.core.database._migrate_update_auto_link_constraint", AsyncMock()),
+        patch("backend.app.core.database._migrate_widen_spoolman_slot_ams_id_range", AsyncMock()),
+    ):
+        await db_module.run_migrations(fake_conn)
+
+    return executed_sql
+
+
+@pytest.mark.asyncio
+async def test_pg_branch_uses_bytea_for_icon_data():
+    """is_sqlite()=False must emit ``ADD COLUMN icon_data BYTEA``."""
+    executed = await _capture_sql(is_sqlite_value=False)
+    icon_data_stmts = [s for s in executed if "ADD COLUMN icon_data" in s]
+    assert len(icon_data_stmts) == 1, f"expected exactly one icon_data ADD COLUMN statement, got: {icon_data_stmts!r}"
+    assert "BYTEA" in icon_data_stmts[0]
+    assert "BLOB" not in icon_data_stmts[0]
+
+
+@pytest.mark.asyncio
+async def test_sqlite_branch_uses_blob_for_icon_data():
+    """is_sqlite()=True must emit ``ADD COLUMN icon_data BLOB``.
+
+    Companion to the PG test — together they guarantee the
+    ``is_sqlite()`` switch wasn't accidentally inverted.
+    """
+    executed = await _capture_sql(is_sqlite_value=True)
+    icon_data_stmts = [s for s in executed if "ADD COLUMN icon_data" in s]
+    assert len(icon_data_stmts) == 1
+    assert "BLOB" in icon_data_stmts[0]
+    assert "BYTEA" not in icon_data_stmts[0]
+
+
+@pytest.mark.asyncio
+async def test_icon_content_type_and_etag_columns_both_dialects():
+    """The two String columns are dialect-independent (VARCHAR works on
+    both SQLite and PostgreSQL). Verify both branches emit them."""
+    for is_sqlite_value in (True, False):
+        executed = await _capture_sql(is_sqlite_value=is_sqlite_value)
+        content_type_stmts = [s for s in executed if "ADD COLUMN icon_content_type" in s]
+        etag_stmts = [s for s in executed if "ADD COLUMN icon_etag" in s]
+        assert len(content_type_stmts) == 1, f"is_sqlite={is_sqlite_value}: {content_type_stmts!r}"
+        assert len(etag_stmts) == 1, f"is_sqlite={is_sqlite_value}: {etag_stmts!r}"
+        assert "VARCHAR" in content_type_stmts[0].upper()
+        assert "VARCHAR" in etag_stmts[0].upper()

backend/tests/unit/test_oidc_icon_service.py

@@ -0,0 +1,356 @@
+"""Unit tests for backend.app.services.oidc_icon.fetch_icon (#1333).
+
+Uses ``patch("backend.app.services.oidc_icon.httpx.AsyncClient", ...)`` —
+the same mocking pattern the project uses in ``test_mfa_api.py`` for OIDC
+discovery/JWKS calls. Streaming-mock helper lives in
+``backend/tests/_fixtures/oidc_icon.py``.
+"""
+
+import hashlib
+from types import SimpleNamespace
+from unittest.mock import patch
+
+import httpx
+import pytest
+
+from backend.app.services.oidc_icon import (
+    OIDCIconUnavailableError,
+    OIDCIconUrlError,
+    _resolve_content_type,
+    fetch_icon,
+)
+from backend.tests._fixtures.oidc_icon import (
+    PNG_BYTES,
+    PNG_ETAG,
+    build_streaming_icon_mock,
+)
+
+# ─── _resolve_content_type — pure helper, tested directly ────────────────
+
+
+class TestResolveContentType:
+    @pytest.mark.parametrize(
+        "mime",
+        ["image/png", "image/jpeg", "image/webp", "image/gif"],
+    )
+    def test_accepts_whitelisted_mime(self, mime):
+        assert _resolve_content_type(mime, "/icon") == mime
+
+    def test_octet_stream_with_png_extension(self):
+        assert _resolve_content_type("application/octet-stream", "/path/icon.png") == "image/png"
+
+    def test_octet_stream_with_jpeg_extension(self):
+        assert _resolve_content_type("application/octet-stream", "/icon.jpeg") == "image/jpeg"
+
+    def test_octet_stream_without_extension_raises(self):
+        with pytest.raises(OIDCIconUnavailableError, match="no image extension"):
+            _resolve_content_type("application/octet-stream", "/icon")
+
+    def test_missing_content_type_distinct_message(self):
+        # N6: empty string → distinct "missing Content-Type" message,
+        # not user-hostile "unsupported content-type: ''".
+        with pytest.raises(OIDCIconUnavailableError, match="missing a Content-Type header"):
+            _resolve_content_type("", "/icon.png")
+
+    @pytest.mark.parametrize(
+        "mime",
+        ["image/svg+xml", "text/html", "application/json", "application/pdf", "text/plain"],
+    )
+    def test_disallowed_mime_raises_with_value(self, mime):
+        with pytest.raises(OIDCIconUnavailableError, match="content-type"):
+            _resolve_content_type(mime, "/icon.png")
+
+
+# ─── fetch_icon — happy paths (streaming) ─────────────────────────────────
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "mime",
+    ["image/png", "image/jpeg", "image/webp", "image/gif"],
+)
+async def test_accepts_whitelisted_mime(mime):
+    mock_cls, _ = build_streaming_icon_mock(content_type=mime)
+    with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+        payload, ct, etag = await fetch_icon("https://example.com/icon")
+    assert payload == PNG_BYTES
+    assert ct == mime
+    assert etag == PNG_ETAG
+
+
+@pytest.mark.asyncio
+async def test_etag_is_deterministic_sha256():
+    mock_cls, _ = build_streaming_icon_mock()
+    with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+        _, _, etag_a = await fetch_icon("https://example.com/a.png")
+        _, _, etag_b = await fetch_icon("https://example.com/b.png")
+    assert etag_a == etag_b  # same bytes → same etag
+    assert etag_a == hashlib.sha256(PNG_BYTES).hexdigest()
+
+
+@pytest.mark.asyncio
+async def test_octet_stream_with_png_extension_accepted():
+    mock_cls, _ = build_streaming_icon_mock(content_type="application/octet-stream")
+    with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+        _, ct, _ = await fetch_icon("https://cdn.example.com/path/icon.png")
+    assert ct == "image/png"
+
+
+# ─── fetch_icon — rejects: scheme ────────────────────────────────────────
+
+
+@pytest.mark.asyncio
+async def test_rejects_non_https():
+    with pytest.raises(OIDCIconUrlError, match="https"):
+        await fetch_icon("http://example.com/icon.png")
+
+
+# ─── fetch_icon — rejects: HTTP status codes ─────────────────────────────
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("status_code", [301, 302, 307, 308, 404, 500, 502])
+async def test_rejects_non_200(status_code):
+    mock_cls, _ = build_streaming_icon_mock(status_code=status_code)
+    with (
+        patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls),
+        pytest.raises(OIDCIconUnavailableError, match=f"HTTP {status_code}"),
+    ):
+        await fetch_icon("https://example.com/icon")
+
+
+# ─── fetch_icon — rejects: content types ─────────────────────────────────
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+    "mime",
+    [
+        "image/svg+xml",  # SVG explicitly excluded in v1
+        "text/html",
+        "application/json",
+        "application/pdf",
+        "text/plain",
+    ],
+)
+async def test_rejects_disallowed_mime(mime):
+    mock_cls, _ = build_streaming_icon_mock(content_type=mime)
+    with (
+        patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls),
+        pytest.raises(OIDCIconUnavailableError, match="content-type"),
+    ):
+        await fetch_icon("https://example.com/icon")
+
+
+@pytest.mark.asyncio
+async def test_rejects_octet_stream_without_image_extension():
+    mock_cls, _ = build_streaming_icon_mock(content_type="application/octet-stream")
+    with (
+        patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls),
+        pytest.raises(OIDCIconUnavailableError, match="no image extension"),
+    ):
+        await fetch_icon("https://example.com/icon")
+
+
+@pytest.mark.asyncio
+async def test_rejects_missing_content_type_header():
+    # N6: distinct message when upstream omits Content-Type entirely.
+    mock_cls, _ = build_streaming_icon_mock(content_type=None)
+    with (
+        patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls),
+        pytest.raises(OIDCIconUnavailableError, match="missing a Content-Type"),
+    ):
+        await fetch_icon("https://example.com/icon.png")
+
+
+# ─── fetch_icon — rejects: payload size (streaming early-exit) ───────────
+
+
+@pytest.mark.asyncio
+async def test_rejects_oversized_payload_via_streaming_early_exit():
+    """I4: size-cap fires DURING streaming, not after full buffer.
+
+    The 2 MB payload is emitted in 4 KB chunks. The cap (1 MB) is crossed
+    around chunk 256; fetch_icon must raise BEFORE the remaining ~256
+    chunks are buffered. We don't observe the early-exit timing
+    directly — we just confirm the right exception with the right
+    message is raised; the streaming-mock structure guarantees the
+    code path went through aiter_bytes().
+    """
+    too_big = b"\x89PNG" + b"\x00" * (2 * 1024 * 1024)  # 2 MB > 1 MB cap
+    mock_cls, _ = build_streaming_icon_mock(body=too_big, chunk_size=4096)
+    with (
+        patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls),
+        pytest.raises(OIDCIconUnavailableError, match="cap"),
+    ):
+        await fetch_icon("https://example.com/icon.png")
+
+
+@pytest.mark.asyncio
+async def test_streaming_size_cap_aborts_at_first_chunk_past_limit():
+    """Stronger guarantee: when the very first chunk exceeds the cap,
+    we abort on that chunk — no further iteration."""
+    chunks_seen = 0
+
+    async def _hostile_aiter_bytes():
+        nonlocal chunks_seen
+        # First chunk: 2 MB in one go — already over the 1 MB cap.
+        chunks_seen += 1
+        yield b"\x00" * (2 * 1024 * 1024)
+        # This second chunk must NEVER be reached.
+        chunks_seen += 1
+        yield b"\x00" * 100
+
+    response = SimpleNamespace(
+        status_code=200,
+        headers={"content-type": "image/png"},
+        aiter_bytes=_hostile_aiter_bytes,
+    )
+
+    class _StreamCtx:
+        async def __aenter__(self):
+            return response
+
+        async def __aexit__(self, *_exc):
+            return False
+
+    class _MockHttpxClient:
+        def __init__(self, *_a, **_kw):
+            pass
+
+        async def __aenter__(self):
+            return self
+
+        async def __aexit__(self, *_exc):
+            return False
+
+        def stream(self, *_a, **_kw):
+            return _StreamCtx()
+
+    with (
+        patch("backend.app.services.oidc_icon.httpx.AsyncClient", _MockHttpxClient),
+        pytest.raises(OIDCIconUnavailableError, match="cap"),
+    ):
+        await fetch_icon("https://example.com/icon.png")
+    assert chunks_seen == 1, "size-cap must abort on first oversized chunk"
+
+
+@pytest.mark.asyncio
+async def test_rejects_empty_body():
+    mock_cls, _ = build_streaming_icon_mock(body=b"")
+    with (
+        patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls),
+        pytest.raises(OIDCIconUnavailableError, match="empty"),
+    ):
+        await fetch_icon("https://example.com/icon.png")
+
+
+# ─── fetch_icon — network errors ─────────────────────────────────────────
+
+
+@pytest.mark.asyncio
+async def test_timeout_raises_unavailable():
+    class _TimingOutClient:
+        def __init__(self, *_a, **_kw):
+            pass
+
+        async def __aenter__(self):
+            return self
+
+        async def __aexit__(self, *_exc):
+            return False
+
+        def stream(self, *_a, **_kw):
+            class _Ctx:
+                async def __aenter__(_self):
+                    raise httpx.TimeoutException("timed out")
+
+                async def __aexit__(_self, *_exc):
+                    return False
+
+            return _Ctx()
+
+    with (
+        patch("backend.app.services.oidc_icon.httpx.AsyncClient", _TimingOutClient),
+        pytest.raises(OIDCIconUnavailableError, match="timed out"),
+    ):
+        await fetch_icon("https://example.com/icon")
+
+
+@pytest.mark.asyncio
+async def test_connection_error_raises_unavailable():
+    class _ErrClient:
+        def __init__(self, *_a, **_kw):
+            pass
+
+        async def __aenter__(self):
+            return self
+
+        async def __aexit__(self, *_exc):
+            return False
+
+        def stream(self, *_a, **_kw):
+            class _Ctx:
+                async def __aenter__(_self):
+                    raise httpx.ConnectError("connection refused")
+
+                async def __aexit__(_self, *_exc):
+                    return False
+
+            return _Ctx()
+
+    with (
+        patch("backend.app.services.oidc_icon.httpx.AsyncClient", _ErrClient),
+        pytest.raises(OIDCIconUnavailableError, match="failed"),
+    ):
+        await fetch_icon("https://example.com/icon")
+
+
+# ─── C1: httpx.InvalidURL → OIDCIconUrlError (not a 500) ─────────────────
+
+
+@pytest.mark.asyncio
+async def test_invalid_url_raises_url_error():
+    """C1: httpx.InvalidURL is NOT a subclass of httpx.HTTPError. Must be
+    caught explicitly and mapped to OIDCIconUrlError → 400, not 500."""
+
+    class _InvalidUrlClient:
+        def __init__(self, *_a, **_kw):
+            pass
+
+        async def __aenter__(self):
+            return self
+
+        async def __aexit__(self, *_exc):
+            return False
+
+        def stream(self, *_a, **_kw):
+            class _Ctx:
+                async def __aenter__(_self):
+                    raise httpx.InvalidURL("Invalid non-printable ASCII character in URL")
+
+                async def __aexit__(_self, *_exc):
+                    return False
+
+            return _Ctx()
+
+    with (
+        patch("backend.app.services.oidc_icon.httpx.AsyncClient", _InvalidUrlClient),
+        pytest.raises(OIDCIconUrlError, match="Invalid icon URL"),
+    ):
+        await fetch_icon("https://example.com/icon")
+
+
+# ─── follow_redirects=False is non-negotiable ────────────────────────────
+
+
+@pytest.mark.asyncio
+async def test_passes_follow_redirects_false():
+    """Defence-in-depth: verify we explicitly pass follow_redirects=False so
+    an upstream 302 cannot bypass the SSRF host check on the initial URL."""
+    mock_cls, stream_recorder = build_streaming_icon_mock()
+    with patch("backend.app.services.oidc_icon.httpx.AsyncClient", mock_cls):
+        await fetch_icon("https://example.com/icon.png")
+    stream_recorder.assert_called_once()
+    _args, kwargs = stream_recorder.call_args
+    assert kwargs.get("follow_redirects") is False

backend/tests/unit/test_oidc_icon_validation.py

@@ -0,0 +1,114 @@
+"""Unit tests for the icon_url Pydantic validator (#1333).
+
+Mirrors the issuer_url validator pattern: HTTPS-only + private/loopback/
+link-local IP literals rejected. Hostname-based URLs are accepted without
+DNS resolution (deliberate, see _validate_issuer_url).
+"""
+
+import pytest
+
+from backend.app.schemas.auth import OIDCProviderCreate
+
+
+def _make_payload(icon_url: str | None) -> dict:
+    """Minimal valid OIDCProviderCreate payload with the given icon_url."""
+    return {
+        "name": "Test",
+        "issuer_url": "https://idp.example.com",
+        "client_id": "client",
+        "client_secret": "secret",
+        "icon_url": icon_url,
+    }
+
+
+def test_icon_url_none_accepted():
+    OIDCProviderCreate(**_make_payload(None))
+
+
+def test_icon_url_valid_https_accepted():
+    OIDCProviderCreate(**_make_payload("https://example.com/icon.png"))
+
+
+def test_icon_url_http_rejected():
+    with pytest.raises(ValueError, match="icon_url must start with https"):
+        OIDCProviderCreate(**_make_payload("http://example.com/icon.png"))
+
+
+def test_icon_url_empty_string_rejected():
+    # Pydantic doesn't coerce "" to None — the validator runs on the raw value.
+    with pytest.raises(ValueError, match="icon_url must start with https"):
+        OIDCProviderCreate(**_make_payload(""))
+
+
+@pytest.mark.parametrize(
+    "url",
+    [
+        "https://192.168.1.1/icon.png",
+        "https://10.0.0.5/icon.png",
+        "https://172.16.0.1/icon.png",
+    ],
+)
+def test_icon_url_private_ip_rejected(url):
+    with pytest.raises(ValueError, match="private"):
+        OIDCProviderCreate(**_make_payload(url))
+
+
+def test_icon_url_loopback_ip_rejected():
+    with pytest.raises(ValueError, match="loopback"):
+        OIDCProviderCreate(**_make_payload("https://127.0.0.1/icon.png"))
+
+
+def test_icon_url_link_local_or_cloud_metadata_rejected():
+    # 169.254.169.254 is BOTH link-local AND a cloud-metadata IP — the guard
+    # checks cloud-metadata first (per intentional ordering), so either
+    # rejection message is correct. Mirrors the same pattern in
+    # test_oidc_icon_helpers.test_rejects_link_local.
+    with pytest.raises(ValueError, match="cloud metadata|link-local"):
+        OIDCProviderCreate(**_make_payload("https://169.254.169.254/icon.png"))
+
+
+def test_icon_url_hostname_accepted_no_dns():
+    # "localhost" is a hostname, not a bare IP — DNS resolution is deliberately
+    # not performed here (matches _validate_issuer_url policy). The runtime
+    # SSRF guard (assert_safe_public_https_url) handles the bare-IP cases
+    # again; hostnames are caught only by the IDP-itself-misconfigured path.
+    OIDCProviderCreate(**_make_payload("https://idp.internal.corp/icon.png"))
+
+
+# ─── I1: schema now delegates to runtime SSRF guard ──────────────────────
+# Verifies that the wider allowlist (numeric IPs, cloud-meta, multicast,
+# IPv4-mapped IPv6) is enforced at Pydantic-parse-time too, not just at
+# fetch time.
+
+
+@pytest.mark.parametrize(
+    "url",
+    [
+        "https://2130706433/icon.png",  # decimal-encoded 127.0.0.1
+        "https://0x7f000001/icon.png",  # hex-encoded 127.0.0.1
+    ],
+)
+def test_icon_url_numeric_encoded_ip_rejected(url):
+    with pytest.raises(ValueError, match="numeric-encoded"):
+        OIDCProviderCreate(**_make_payload(url))
+
+
+def test_icon_url_cloud_metadata_alibaba_rejected():
+    with pytest.raises(ValueError, match="cloud metadata"):
+        OIDCProviderCreate(**_make_payload("https://100.100.100.200/icon.png"))
+
+
+def test_icon_url_unspecified_rejected():
+    with pytest.raises(ValueError, match="unspecified"):
+        OIDCProviderCreate(**_make_payload("https://0.0.0.0/icon.png"))
+
+
+def test_icon_url_multicast_rejected():
+    with pytest.raises(ValueError, match="multicast"):
+        OIDCProviderCreate(**_make_payload("https://224.0.0.1/icon.png"))
+
+
+def test_icon_url_ipv4_mapped_ipv6_private_rejected():
+    # ::ffff:192.168.1.1 unwraps to 192.168.1.1 → private
+    with pytest.raises(ValueError, match="private"):
+        OIDCProviderCreate(**_make_payload("https://[::ffff:192.168.1.1]/icon.png"))

backend/tests/unit/test_url_safety.py

@@ -0,0 +1,74 @@
+"""Unit tests for the shared SSRF-data primitives (#1333).
+
+These primitives are imported by both ``_spoolman_helpers.assert_safe_spoolman_url``
+and ``_oidc_helpers.assert_safe_public_https_url``. Test them in isolation
+so a future change to one consumer doesn't accidentally drift the data.
+"""
+
+import ipaddress
+
+import pytest
+
+from backend.app.api.routes._url_safety import (
+    CLOUD_METADATA_IPS,
+    NUMERIC_IP_RE,
+    unwrap_ipv4_mapped,
+)
+
+
+def test_cloud_metadata_set_contains_known_endpoints():
+    # Both v4 and v6 IMDS endpoints, plus Alibaba's variant.
+    assert ipaddress.ip_address("169.254.169.254") in CLOUD_METADATA_IPS
+    assert ipaddress.ip_address("100.100.100.200") in CLOUD_METADATA_IPS
+    assert ipaddress.ip_address("fd00:ec2::254") in CLOUD_METADATA_IPS
+
+
+def test_cloud_metadata_set_is_frozen():
+    # frozenset is the right immutable container — protects against
+    # accidental mutation in tests/imports.
+    assert isinstance(CLOUD_METADATA_IPS, frozenset)
+
+
+@pytest.mark.parametrize(
+    "candidate",
+    [
+        "2130706433",  # decimal-encoded 127.0.0.1
+        "0x7f000001",  # hex-encoded 127.0.0.1
+        "0xFFFFFFFF",  # uppercase hex
+        "0",
+        "4294967295",  # max uint32
+    ],
+)
+def test_numeric_ip_re_matches_encoded_forms(candidate):
+    assert NUMERIC_IP_RE.match(candidate) is not None
+
+
+@pytest.mark.parametrize(
+    "candidate",
+    [
+        "127.0.0.1",  # dotted-decimal — not "numeric-encoded"
+        "example.com",
+        "spoolman.lan",
+        "::1",
+        "localhost",
+    ],
+)
+def test_numeric_ip_re_rejects_normal_forms(candidate):
+    assert NUMERIC_IP_RE.match(candidate) is None
+
+
+def test_unwrap_ipv4_mapped_unwraps_mapped_address():
+    mapped = ipaddress.ip_address("::ffff:127.0.0.1")
+    result = unwrap_ipv4_mapped(mapped)
+    assert result == ipaddress.ip_address("127.0.0.1")
+    assert isinstance(result, ipaddress.IPv4Address)
+
+
+def test_unwrap_ipv4_mapped_passes_through_pure_ipv4():
+    addr = ipaddress.ip_address("8.8.8.8")
+    assert unwrap_ipv4_mapped(addr) is addr
+
+
+def test_unwrap_ipv4_mapped_passes_through_pure_ipv6():
+    addr = ipaddress.ip_address("2001:db8::1")
+    assert unwrap_ipv4_mapped(addr) is addr

frontend/src/__tests__/components/OIDCProviderSettings.test.tsx

@@ -24,6 +24,7 @@ const mockProviders = [
     email_claim: 'email',
     require_email_verified: true,
     icon_url: null,
+    has_icon: false,
     default_group_id: null,
     created_at: '2026-01-01T00:00:00Z',
     updated_at: '2026-01-01T00:00:00Z',
@@ -226,4 +227,101 @@ describe('OIDCProviderSettings', () => {
       });
     });
   });
+
+  // #1333: icon proxy — preview uses the backend proxy URL (never icon_url
+  // directly) and the admin gets explicit Refresh / Remove buttons.
+  describe('Icon proxy (#1333)', () => {
+    it('renders icon preview via the backend proxy URL when has_icon is true', async () => {
+      server.use(
+        http.get('/api/v1/auth/oidc/providers/all', () =>
+          HttpResponse.json([
+            {
+              ...mockProviders[0],
+              id: 42,
+              icon_url: 'https://idp.example.com/icon.png',
+              has_icon: true,
+            },
+          ])
+        )
+      );
+      render(<OIDCProviderSettings />);
+
+      await waitFor(() => {
+        expect(screen.getByText('TestIdP')).toBeInTheDocument();
+      });
+
+      const img = screen.getByAltText('TestIdP') as HTMLImageElement;
+      expect(img.getAttribute('src')).toBe('/api/v1/auth/oidc/providers/42/icon');
+    });
+
+    it('exposes Refresh and Remove buttons when has_icon is true', async () => {
+      server.use(
+        http.get('/api/v1/auth/oidc/providers/all', () =>
+          HttpResponse.json([
+            { ...mockProviders[0], id: 99, icon_url: 'https://idp.example.com/i.png', has_icon: true },
+          ])
+        )
+      );
+      render(<OIDCProviderSettings />);
+
+      await waitFor(() => {
+        expect(screen.getByTestId('refresh-icon-99')).toBeInTheDocument();
+      });
+      expect(screen.getByTestId('remove-icon-99')).toBeInTheDocument();
+    });
+
+    it('hides Remove button when has_icon is false', async () => {
+      server.use(
+        http.get('/api/v1/auth/oidc/providers/all', () =>
+          HttpResponse.json([
+            // icon_url set but no cached bytes → Refresh visible, Remove hidden.
+            { ...mockProviders[0], id: 100, icon_url: 'https://idp.example.com/i.png', has_icon: false },
+          ])
+        )
+      );
+      render(<OIDCProviderSettings />);
+
+      await waitFor(() => {
+        expect(screen.getByTestId('refresh-icon-100')).toBeInTheDocument();
+      });
+      expect(screen.queryByTestId('remove-icon-100')).not.toBeInTheDocument();
+    });
+
+    it('hides both buttons when icon_url is not set', async () => {
+      server.use(
+        http.get('/api/v1/auth/oidc/providers/all', () =>
+          HttpResponse.json([{ ...mockProviders[0], id: 101, icon_url: null, has_icon: false }])
+        )
+      );
+      render(<OIDCProviderSettings />);
+
+      await waitFor(() => {
+        expect(screen.getByText('TestIdP')).toBeInTheDocument();
+      });
+      expect(screen.queryByTestId('refresh-icon-101')).not.toBeInTheDocument();
+      expect(screen.queryByTestId('remove-icon-101')).not.toBeInTheDocument();
+    });
+
+    it('swaps in Globe fallback when icon image fails to load', async () => {
+      // I3 (#1333 review): admin preview must show a meaningful fallback
+      // instead of an unexplained gap (display: none) when the proxy
+      // endpoint returns 404 (e.g. race with DELETE /icon).
+      server.use(
+        http.get('/api/v1/auth/oidc/providers/all', () =>
+          HttpResponse.json([
+            { ...mockProviders[0], id: 102, icon_url: 'https://idp.example.com/i.png', has_icon: true },
+          ])
+        )
+      );
+      render(<OIDCProviderSettings />);
+
+      const img = (await screen.findByAltText('TestIdP')) as HTMLImageElement;
+      fireEvent.error(img);
+      // After error: <img> removed, Globe-fallback rendered. Confirm by
+      // asserting the alt text is gone and the Globe SVG is present.
+      await waitFor(() => {
+        expect(screen.queryByAltText('TestIdP')).not.toBeInTheDocument();
+      });
+    });
+  });
 });

frontend/src/__tests__/mocks/handlers.ts

@@ -437,6 +437,29 @@ export const handlers = [
   http.get('/api/v1/archives/', () => HttpResponse.json([])),
   http.get('/api/v1/auth/oidc/providers', () => HttpResponse.json([])),
   http.get('/api/v1/auth/oidc/providers/all', () => HttpResponse.json([])),
+  // OIDC icon proxy (#1333). Default returns a tiny PNG; individual tests
+  // can override via server.use(...).
+  http.get('/api/v1/auth/oidc/providers/:id/icon', () =>
+    HttpResponse.arrayBuffer(new Uint8Array([0x89, 0x50, 0x4e, 0x47]).buffer, {
+      headers: { 'Content-Type': 'image/png' },
+    }),
+  ),
+  http.delete('/api/v1/auth/oidc/providers/:id/icon', () => new HttpResponse(null, { status: 204 })),
+  http.post('/api/v1/auth/oidc/providers/:id/icon/refresh', ({ params }) =>
+    HttpResponse.json({
+      id: Number(params.id),
+      name: 'MockProv',
+      issuer_url: 'https://idp.example.com',
+      client_id: 'c',
+      scopes: 'openid',
+      is_enabled: true,
+      auto_create_users: false,
+      auto_link_existing_accounts: false,
+      email_claim: 'email',
+      require_email_verified: true,
+      has_icon: true,
+    }),
+  ),
   http.get('/api/v1/auth/tokens', () => HttpResponse.json([])),
   http.get('/api/v1/auth/tokens/all', () => HttpResponse.json([])),
   http.get('/api/v1/external-links/', () => HttpResponse.json([])),

frontend/src/__tests__/pages/LoginPage.test.tsx

@@ -3,7 +3,7 @@
  */
 
 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
-import { screen, waitFor } from '@testing-library/react';
+import { fireEvent, screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { render } from '../utils';
 import { LoginPage } from '../../pages/LoginPage';
@@ -587,6 +587,7 @@ describe('LoginPage', () => {
               client_id: 'c',
               is_enabled: true,
               icon_url: null,
+              has_icon: false,
               email_claim: 'email',
               require_email_verified: true,
               auto_create_users: false,
@@ -627,4 +628,207 @@ describe('LoginPage', () => {
       });
     });
   });
+
+  // #1333: icon proxy — login page renders <img src> from /icon endpoint
+  // rather than the upstream icon_url, so the strict img-src CSP holds.
+  describe('OIDC icon proxy (#1333)', () => {
+    beforeEach(() => {
+      server.use(
+        http.get('/api/v1/auth/status', () =>
+          HttpResponse.json({ auth_enabled: true, setup_required: false })
+        ),
+      );
+    });
+
+    it('renders provider icon via the proxy URL when has_icon is true', async () => {
+      server.use(
+        http.get('/api/v1/auth/oidc/providers', () =>
+          HttpResponse.json([
+            {
+              id: 7,
+              name: 'IconProv',
+              issuer_url: 'https://idp.test',
+              client_id: 'c',
+              is_enabled: true,
+              icon_url: 'https://idp.test/icon.png',
+              email_claim: 'email',
+              require_email_verified: true,
+              auto_create_users: false,
+              auto_link_existing_accounts: false,
+              has_icon: true,
+            },
+          ])
+        ),
+      );
+      render(<LoginPage />);
+
+      const button = await screen.findByRole('button', { name: /IconProv/i });
+      const img = button.querySelector('img');
+      expect(img).not.toBeNull();
+      // Same-origin path — never the upstream icon_url. This is the entire
+      // point of the proxy: keep img-src strictly 'self' data: blob:.
+      expect(img!.getAttribute('src')).toBe('/api/v1/auth/oidc/providers/7/icon');
+    });
+
+    it('renders shield fallback when has_icon is false', async () => {
+      server.use(
+        http.get('/api/v1/auth/oidc/providers', () =>
+          HttpResponse.json([
+            {
+              id: 8,
+              name: 'NoIconProv',
+              issuer_url: 'https://idp.test',
+              client_id: 'c',
+              is_enabled: true,
+              icon_url: null,
+              email_claim: 'email',
+              require_email_verified: true,
+              auto_create_users: false,
+              auto_link_existing_accounts: false,
+              has_icon: false,
+            },
+          ])
+        ),
+      );
+      render(<LoginPage />);
+
+      const button = await screen.findByRole('button', { name: /NoIconProv/i });
+      expect(button.querySelector('img')).toBeNull();
+    });
+
+    it('renders mixed has_icon providers without crash', async () => {
+      // N12 — multiple providers on the login page with a mix of
+      // has_icon=true / false. No React-keys-collision warning, both
+      // branches render correctly side by side.
+      server.use(
+        http.get('/api/v1/auth/oidc/providers', () =>
+          HttpResponse.json([
+            {
+              id: 10,
+              name: 'WithIcon',
+              issuer_url: 'https://idp.test',
+              client_id: 'c1',
+              is_enabled: true,
+              icon_url: 'https://idp.test/icon.png',
+              has_icon: true,
+              email_claim: 'email',
+              require_email_verified: true,
+              auto_create_users: false,
+              auto_link_existing_accounts: false,
+            },
+            {
+              id: 11,
+              name: 'NoIcon',
+              issuer_url: 'https://idp.test',
+              client_id: 'c2',
+              is_enabled: true,
+              icon_url: null,
+              has_icon: false,
+              email_claim: 'email',
+              require_email_verified: true,
+              auto_create_users: false,
+              auto_link_existing_accounts: false,
+            },
+          ])
+        ),
+      );
+      render(<LoginPage />);
+
+      const withIconBtn = await screen.findByRole('button', { name: /WithIcon/i });
+      const noIconBtn = await screen.findByRole('button', { name: /NoIcon/i });
+      expect(withIconBtn.querySelector('img')).not.toBeNull();
+      expect(noIconBtn.querySelector('img')).toBeNull();
+    });
+
+    it('swaps in shield fallback when the icon fails to load', async () => {
+      // I3 (#1333 review): the LoginPage must not show the browser
+      // broken-image glyph to anonymous users. onError must fall back to
+      // the Shield icon.
+      server.use(
+        http.get('/api/v1/auth/oidc/providers', () =>
+          HttpResponse.json([
+            {
+              id: 9,
+              name: 'FlakyIcon',
+              issuer_url: 'https://idp.test',
+              client_id: 'c',
+              is_enabled: true,
+              icon_url: 'https://idp.test/icon.png',
+              email_claim: 'email',
+              require_email_verified: true,
+              auto_create_users: false,
+              auto_link_existing_accounts: false,
+              has_icon: true,
+            },
+          ])
+        ),
+      );
+      render(<LoginPage />);
+
+      const img = (await screen.findByRole('button', { name: /FlakyIcon/i })).querySelector('img');
+      expect(img).not.toBeNull();
+      // Fire the image's onError — jsdom doesn't fetch network resources
+      // so we simulate the failure directly.
+      fireEvent.error(img!);
+      // After error, no more <img> in the button; Shield fallback rendered.
+      await waitFor(() => {
+        const button = screen.getByRole('button', { name: /FlakyIcon/i });
+        expect(button.querySelector('img')).toBeNull();
+      });
+    });
+
+    it('keeps each provider button\'s iconFailed state independent', async () => {
+      // The OIDCProviderButton sub-component exists specifically so each
+      // provider owns its own iconFailed state. If a future refactor hoists
+      // useState into the parent loop, an error on provider A would also
+      // hide provider B's icon — exactly the regression this test catches.
+      server.use(
+        http.get('/api/v1/auth/oidc/providers', () =>
+          HttpResponse.json([
+            {
+              id: 21,
+              name: 'AlphaIdP',
+              issuer_url: 'https://a.test',
+              client_id: 'a',
+              is_enabled: true,
+              icon_url: 'https://a.test/icon.png',
+              email_claim: 'email',
+              require_email_verified: true,
+              auto_create_users: false,
+              auto_link_existing_accounts: false,
+              has_icon: true,
+            },
+            {
+              id: 22,
+              name: 'BetaIdP',
+              issuer_url: 'https://b.test',
+              client_id: 'b',
+              is_enabled: true,
+              icon_url: 'https://b.test/icon.png',
+              email_claim: 'email',
+              require_email_verified: true,
+              auto_create_users: false,
+              auto_link_existing_accounts: false,
+              has_icon: true,
+            },
+          ])
+        ),
+      );
+      render(<LoginPage />);
+
+      const alphaImg = (await screen.findByRole('button', { name: /AlphaIdP/i })).querySelector('img');
+      const betaImg = (await screen.findByRole('button', { name: /BetaIdP/i })).querySelector('img');
+      expect(alphaImg).not.toBeNull();
+      expect(betaImg).not.toBeNull();
+
+      fireEvent.error(alphaImg!);
+
+      // Alpha's icon swaps to the Shield fallback…
+      await waitFor(() => {
+        expect(screen.getByRole('button', { name: /AlphaIdP/i }).querySelector('img')).toBeNull();
+      });
+      // …but Beta's icon stays put. If state leaks to the parent, this fails.
+      expect(screen.getByRole('button', { name: /BetaIdP/i }).querySelector('img')).not.toBeNull();
+    });
+  });
 });

frontend/src/api/client.ts

@@ -2810,6 +2810,20 @@ export interface TwoFAVerifyRequest {
   method: 'totp' | 'email' | 'backup';
 }
 
+/**
+ * A URL that is known to be same-origin (a relative path starting with ``/``).
+ *
+ * Branded so that producers of same-origin URLs (e.g. ``api.oidcProviderIconUrl``)
+ * can be distinguished from arbitrary strings at the type level.  The brand
+ * is compile-time only; at runtime these are plain strings.
+ *
+ * Purpose: CSP-safe image sources for ``<img src=...>``. The strict
+ * ``img-src 'self' data: blob:`` CSP rejects anything that isn't same-origin,
+ * so callers that demand a ``SameOriginUrl`` get a compile-time guarantee
+ * that no external URL slips through.
+ */
+export type SameOriginUrl = string & { readonly __brand: 'SameOriginUrl' };
+
 // OIDC interfaces
 export interface OIDCProvider {
   id: number;
@@ -2824,6 +2838,14 @@ export interface OIDCProvider {
   require_email_verified: boolean;
   icon_url?: string | null;
   default_group_id?: number | null;
+  // True when the backend has cached icon bytes for this provider.
+  // Login page / admin preview consume this via the proxy URL
+  // /api/v1/auth/oidc/providers/{id}/icon (#1333) so the SPA never
+  // hotlinks the external icon URL — that would require loosening
+  // the strict img-src CSP.  Required, not optional: the backend always
+  // includes this field in the response (Pydantic default-False is
+  // populated unconditionally in the route handler).
+  has_icon: boolean;
 }
 
 export interface OIDCProviderCreate {
@@ -3040,6 +3062,17 @@ export const api = {
   deleteOIDCProvider: (id: number) =>
     request<{ message: string }>(`/auth/oidc/providers/${id}`, { method: 'DELETE' }),
 
+  // OIDC provider icon proxy (#1333) — same-origin path so the strict
+  // img-src CSP stays in force. Returns a SameOriginUrl-branded string
+  // so a future caller can't accidentally substitute an attacker-
+  // controlled URL where this is consumed.
+  oidcProviderIconUrl: (id: number): SameOriginUrl =>
+    `/api/v1/auth/oidc/providers/${id}/icon` as SameOriginUrl,
+  deleteOIDCProviderIcon: (id: number) =>
+    request<void>(`/auth/oidc/providers/${id}/icon`, { method: 'DELETE' }),
+  refreshOIDCProviderIcon: (id: number) =>
+    request<OIDCProvider>(`/auth/oidc/providers/${id}/icon/refresh`, { method: 'POST' }),
+
   // OIDC authorize URL
   getOIDCAuthorizeUrl: (providerId: number) =>
     request<{ auth_url: string }>(`/auth/oidc/authorize/${providerId}`),

frontend/src/components/OIDCProviderSettings.tsx

@@ -1,6 +1,6 @@
 import { useState, type ReactNode } from 'react';
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
-import { Plus, Edit2, Trash2, Globe, Check, X, RefreshCw, ExternalLink } from 'lucide-react';
+import { Plus, Edit2, Trash2, Globe, Check, X, RefreshCw, ExternalLink, ImageOff } from 'lucide-react';
 import { useTranslation } from 'react-i18next';
 import { api } from '../api/client';
 import type { Group, OIDCProvider, OIDCProviderCreate } from '../api/client';
@@ -110,7 +110,12 @@ function ProviderForm({
         </div>
         <div>
           <label className={labelCls}>{t('settings.oidc.form.iconUrl')}</label>
-          <input className={inputCls} value={form.icon_url ?? ''} onChange={(e) => set('icon_url', e.target.value || undefined)} placeholder="https://..." />
+          <input
+            className={inputCls}
+            value={form.icon_url ?? ''}
+            onChange={(e) => set('icon_url', e.target.value === '' ? null : e.target.value)}
+            placeholder="https://..."
+          />
         </div>
       </div>
 
@@ -192,6 +197,35 @@ function ProviderForm({
   );
 }
 
+/**
+ * Per-provider icon avatar in the admin Settings list (#1333 review).
+ *
+ * Extracted so each card has its own `iconFailed` state. Previously
+ * `onError` just set `display: none` and the admin saw an unexplained gap
+ * where the icon should be — now we swap in the Globe fallback exactly
+ * like the `has_icon === false` branch, so the visual state is
+ * self-explanatory regardless of why the icon didn't load.
+ */
+function ProviderIconAvatar({ provider }: { provider: OIDCProvider }) {
+  const [iconFailed, setIconFailed] = useState(false);
+  const showIcon = provider.has_icon && !iconFailed;
+  if (showIcon) {
+    return (
+      <img
+        src={api.oidcProviderIconUrl(provider.id)}
+        alt={provider.name}
+        className="w-8 h-8 rounded object-contain"
+        onError={() => setIconFailed(true)}
+      />
+    );
+  }
+  return (
+    <div className="w-8 h-8 rounded-full bg-bambu-dark-tertiary flex items-center justify-center">
+      <Globe className="w-4 h-4 text-bambu-gray" />
+    </div>
+  );
+}
+
 // ─── Main component ───────────────────────────────────────────────────────────
 export function OIDCProviderSettings() {
   const { t } = useTranslation();
@@ -243,6 +277,28 @@ export function OIDCProviderSettings() {
     onError: (e: Error) => showToast(e.message, 'error'),
   });
 
+  // Icon-proxy mutations (#1333). Refresh re-fetches from the stored
+  // icon_url; remove clears the cached bytes but keeps icon_url.
+  const refreshIconMutation = useMutation({
+    mutationFn: (id: number) => api.refreshOIDCProviderIcon(id),
+    onSuccess: () => {
+      queryClient.invalidateQueries({ queryKey: ['oidc-providers-all'] });
+      queryClient.invalidateQueries({ queryKey: ['oidc-providers'] });
+      showToast(t('settings.oidc.iconRefreshed'), 'success');
+    },
+    onError: (e: Error) => showToast(e.message || t('settings.oidc.iconFetchFailed'), 'error'),
+  });
+
+  const removeIconMutation = useMutation({
+    mutationFn: (id: number) => api.deleteOIDCProviderIcon(id),
+    onSuccess: () => {
+      queryClient.invalidateQueries({ queryKey: ['oidc-providers-all'] });
+      queryClient.invalidateQueries({ queryKey: ['oidc-providers'] });
+      showToast(t('settings.oidc.iconRemoved'), 'success');
+    },
+    onError: (e: Error) => showToast(e.message, 'error'),
+  });
+
   const toggleEnabled = (provider: OIDCProvider) =>
     updateMutation.mutate({ id: provider.id, data: { is_enabled: !provider.is_enabled } });
 
@@ -309,13 +365,7 @@ export function OIDCProviderSettings() {
         <Card key={provider.id}>
           <CardHeader>
             <div className="flex items-center gap-3">
-              {provider.icon_url ? (
-                <img src={provider.icon_url} alt={provider.name} className="w-8 h-8 rounded object-contain" onError={(e) => { (e.target as HTMLImageElement).style.display = 'none'; }} />
-              ) : (
-                <div className="w-8 h-8 rounded-full bg-bambu-dark-tertiary flex items-center justify-center">
-                  <Globe className="w-4 h-4 text-bambu-gray" />
-                </div>
-              )}
+              <ProviderIconAvatar provider={provider} />
               <div className="flex-1">
                 <div className="flex items-center gap-2">
                   <h4 className="text-white font-medium">{provider.name}</h4>
@@ -335,6 +385,30 @@ export function OIDCProviderSettings() {
                 </div>
               </div>
               <div className="flex items-center gap-2">
+                {provider.icon_url && (
+                  <Button
+                    variant="secondary"
+                    size="sm"
+                    onClick={() => refreshIconMutation.mutate(provider.id)}
+                    disabled={refreshIconMutation.isPending}
+                    title={t('settings.oidc.refreshIcon')}
+                    data-testid={`refresh-icon-${provider.id}`}
+                  >
+                    <RefreshCw className={`w-4 h-4 ${refreshIconMutation.isPending ? 'animate-spin' : ''}`} />
+                  </Button>
+                )}
+                {provider.has_icon && (
+                  <Button
+                    variant="secondary"
+                    size="sm"
+                    onClick={() => removeIconMutation.mutate(provider.id)}
+                    disabled={removeIconMutation.isPending}
+                    title={t('settings.oidc.removeIcon')}
+                    data-testid={`remove-icon-${provider.id}`}
+                  >
+                    <ImageOff className="w-4 h-4" />
+                  </Button>
+                )}
                 <Toggle
                   checked={provider.is_enabled}
                   onChange={() => toggleEnabled(provider)}

frontend/src/i18n/locales/de.ts

@@ -2270,6 +2270,11 @@ export default {
       created: 'Anbieter erstellt.',
       updated: 'Anbieter aktualisiert.',
       deleted: 'Anbieter gelöscht.',
+      refreshIcon: 'Icon neu laden',
+      removeIcon: 'Icon entfernen',
+      iconRefreshed: 'Icon aktualisiert.',
+      iconRemoved: 'Icon entfernt.',
+      iconFetchFailed: 'Icon konnte von der Anbieter-URL nicht geladen werden.',
       deleteTitle: 'Anbieter löschen',
       deleteMessage: '"{{name}}" löschen? Alle verknüpften Benutzerkonten werden getrennt.',
       form: {

frontend/src/i18n/locales/en.ts

@@ -2273,6 +2273,11 @@ export default {
       created: 'Provider created.',
       updated: 'Provider updated.',
       deleted: 'Provider deleted.',
+      refreshIcon: 'Refresh icon',
+      removeIcon: 'Remove icon',
+      iconRefreshed: 'Icon refreshed.',
+      iconRemoved: 'Icon removed.',
+      iconFetchFailed: 'Icon could not be fetched from the provider URL.',
       deleteTitle: 'Delete Provider',
       deleteMessage: 'Delete "{{name}}"? All linked user accounts will be disconnected.',
       form: {

frontend/src/i18n/locales/fr.ts

@@ -2214,6 +2214,11 @@ export default {
       created: 'Fournisseur créé.',
       updated: 'Fournisseur mis à jour.',
       deleted: 'Fournisseur supprimé.',
+      refreshIcon: 'Actualiser l\'icône',
+      removeIcon: 'Supprimer l\'icône',
+      iconRefreshed: 'Icône actualisée.',
+      iconRemoved: 'Icône supprimée.',
+      iconFetchFailed: 'Impossible de récupérer l\'icône depuis l\'URL du fournisseur.',
       deleteTitle: 'Supprimer le fournisseur',
       deleteMessage: 'Supprimer "{{name}}" ? Tous les comptes liés seront déconnectés.',
       form: {

frontend/src/i18n/locales/it.ts

@@ -2213,6 +2213,11 @@ export default {
       created: 'Provider creato.',
       updated: 'Provider aggiornato.',
       deleted: 'Provider eliminato.',
+      refreshIcon: 'Aggiorna icona',
+      removeIcon: 'Rimuovi icona',
+      iconRefreshed: 'Icona aggiornata.',
+      iconRemoved: 'Icona rimossa.',
+      iconFetchFailed: 'Impossibile recuperare l\'icona dall\'URL del provider.',
       deleteTitle: 'Elimina provider',
       deleteMessage: 'Eliminare "{{name}}"? Tutti gli account collegati verranno disconnessi.',
       form: {

frontend/src/i18n/locales/ja.ts

@@ -2269,6 +2269,11 @@ export default {
       created: 'プロバイダーが作成されました。',
       updated: 'プロバイダーが更新されました。',
       deleted: 'プロバイダーが削除されました。',
+      refreshIcon: 'アイコンを再取得',
+      removeIcon: 'アイコンを削除',
+      iconRefreshed: 'アイコンを更新しました。',
+      iconRemoved: 'アイコンを削除しました。',
+      iconFetchFailed: 'プロバイダーURLからアイコンを取得できませんでした。',
       deleteTitle: 'プロバイダーを削除',
       deleteMessage: '"{{name}}"を削除しますか？リンクされたすべてのユーザーアカウントが切断されます。',
       form: {

frontend/src/i18n/locales/pt-BR.ts

@@ -2213,6 +2213,11 @@ export default {
       created: 'Provedor criado.',
       updated: 'Provedor atualizado.',
       deleted: 'Provedor excluído.',
+      refreshIcon: 'Atualizar ícone',
+      removeIcon: 'Remover ícone',
+      iconRefreshed: 'Ícone atualizado.',
+      iconRemoved: 'Ícone removido.',
+      iconFetchFailed: 'Não foi possível obter o ícone da URL do provedor.',
       deleteTitle: 'Excluir provedor',
       deleteMessage: 'Excluir "{{name}}"? Todas as contas vinculadas serão desconectadas.',
       form: {

frontend/src/i18n/locales/zh-CN.ts

@@ -2257,6 +2257,11 @@ export default {
       created: '提供商已创建。',
       updated: '提供商已更新。',
       deleted: '提供商已删除。',
+      refreshIcon: '刷新图标',
+      removeIcon: '移除图标',
+      iconRefreshed: '图标已刷新。',
+      iconRemoved: '图标已移除。',
+      iconFetchFailed: '无法从提供商 URL 获取图标。',
       deleteTitle: '删除提供商',
       deleteMessage: '删除"{{name}}"？所有关联账户将断开连接。',
       form: {

frontend/src/i18n/locales/zh-TW.ts

@@ -2257,6 +2257,11 @@ export default {
       created: '提供者已建立。',
       updated: '提供者已更新。',
       deleted: '提供者已刪除。',
+      refreshIcon: '重新整理圖示',
+      removeIcon: '移除圖示',
+      iconRefreshed: '圖示已重新整理。',
+      iconRemoved: '圖示已移除。',
+      iconFetchFailed: '無法從提供者 URL 取得圖示。',
       deleteTitle: '刪除提供者',
       deleteMessage: '刪除"{{name}}"？所有連結帳戶將斷開連線。',
       form: {

frontend/src/pages/LoginPage.tsx

@@ -6,7 +6,7 @@ import { useAuth } from '../contexts/AuthContext';
 import { useToast } from '../contexts/ToastContext';
 import { useTheme } from '../contexts/ThemeContext';
 import { X, Mail, Shield, Smartphone, Key } from 'lucide-react';
-import { api, type LoginResponse, type TokenPersistence } from '../api/client';
+import { api, type LoginResponse, type OIDCProvider, type TokenPersistence } from '../api/client';
 import { Card, CardHeader, CardContent } from '../components/Card';
 import { Button } from '../components/Button';
 
@@ -32,6 +32,49 @@ function consumeSavedRememberMe(): boolean {
   }
 }
 
+/**
+ * Single OIDC-provider login button. Extracted from the `.map()` body
+ * because hooks can't be used inside a loop callback — the `iconFailed`
+ * state is per-provider and must live in its own component instance.
+ *
+ * On `<img>` load failure (provider deleted between page load and image
+ * fetch, network blip, etc.) we flip to the Shield fallback rather than
+ * showing the browser's broken-image glyph to anonymous users (#1333 review).
+ */
+function OIDCProviderButton({
+  provider,
+  onClick,
+  disabled,
+}: {
+  provider: OIDCProvider;
+  onClick: () => void;
+  disabled: boolean;
+}) {
+  const { t } = useTranslation();
+  const [iconFailed, setIconFailed] = useState(false);
+  const showIcon = provider.has_icon && !iconFailed;
+  return (
+    <button
+      type="button"
+      onClick={onClick}
+      disabled={disabled}
+      className="w-full flex items-center justify-center gap-3 py-3 px-4 bg-bambu-dark-secondary border border-bambu-dark-tertiary hover:border-bambu-green/50 rounded-lg text-white font-medium transition-colors disabled:opacity-50"
+    >
+      {showIcon ? (
+        <img
+          src={api.oidcProviderIconUrl(provider.id)}
+          alt=""
+          className="w-5 h-5 object-contain"
+          onError={() => setIconFailed(true)}
+        />
+      ) : (
+        <Shield className="w-5 h-5 text-bambu-green" />
+      )}
+      {t('login.twoFA.signInWith', { provider: provider.name })}
+    </button>
+  );
+}
+
 export function LoginPage() {
   const navigate = useNavigate();
   const [searchParams] = useSearchParams();
@@ -656,20 +699,12 @@ export function LoginPage() {
 
             <div className="space-y-2">
               {oidcProviders.map((provider) => (
-                <button
+                <OIDCProviderButton
                   key={provider.id}
-                  type="button"
+                  provider={provider}
                   onClick={() => oidcLoginMutation.mutate(provider.id)}
                   disabled={oidcLoginMutation.isPending}
-                  className="w-full flex items-center justify-center gap-3 py-3 px-4 bg-bambu-dark-secondary border border-bambu-dark-tertiary hover:border-bambu-green/50 rounded-lg text-white font-medium transition-colors disabled:opacity-50"
-                >
-                  {provider.icon_url ? (
-                    <img src={provider.icon_url} alt="" className="w-5 h-5 object-contain" />
-                  ) : (
-                    <Shield className="w-5 h-5 text-bambu-green" />
-                  )}
-                  {t('login.twoFA.signInWith', { provider: provider.name })}
-                </button>
+                />
               ))}
             </div>
           </div>

static/index.html

@@ -1,40 +1,41 @@
-<!doctype html>
-<html lang="en">
-  <head>
-    <meta charset="UTF-8" />
-    <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" />
-    <!-- L-4: Restrict Referer header to origin-only on cross-origin navigation so
-         sensitive tokens in query parameters are not leaked to third-party servers. -->
-    <meta name="referrer" content="strict-origin-when-cross-origin" />
-    <title>Bambuddy</title>
-
-    <!-- PWA Meta Tags -->
-    <meta name="description" content="Monitor and manage your Bambu Lab 3D printers" />
-    <meta name="theme-color" content="#00ae42" />
-    <meta name="mobile-web-app-capable" content="yes" />
-    <meta name="apple-mobile-web-app-capable" content="yes" />
-    <meta name="apple-mobile-web-app-status-bar-style" content="black-translucent" />
-    <meta name="apple-mobile-web-app-title" content="Bambuddy" />
-
-    <!-- Manifest -->
-    <link rel="manifest" href="/manifest.json" />
-
-    <!-- Favicons -->
-    <link rel="icon" type="image/png" sizes="32x32" href="/img/favicon-32x32.png" />
-    <link rel="icon" type="image/png" sizes="16x16" href="/img/favicon-16x16.png" />
-    <link rel="apple-touch-icon" sizes="180x180" href="/img/apple-touch-icon.png" />
-
-    <!-- Splash screens for iOS -->
-    <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-DZ9zJnpR.js"></script>
-    <link rel="stylesheet" crossorigin href="/assets/index-BkYu3kLs.css">
-  </head>
-  <body>
-    <div id="root"></div>
-
-    <!-- Service Worker Registration (skip on SpoolBuddy kiosk).
-         Kept as an external file so the CSP `script-src 'self'` covers it
-         without needing 'unsafe-inline' or per-build hashes. -->
-    <script src="/sw-register.js"></script>
-  </body>
-</html>
+<!doctype html>
+<html lang="en">
+  <head>
+    <meta charset="UTF-8" />
+    <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no" />
+    <!-- L-4: Restrict Referer header to origin-only on cross-origin navigation so
+         sensitive tokens in query parameters are not leaked to third-party servers. -->
+    <meta name="referrer" content="strict-origin-when-cross-origin" />
+    <title>Bambuddy</title>
+
+    <!-- PWA Meta Tags -->
+    <meta name="description" content="Monitor and manage your Bambu Lab 3D printers" />
+    <meta name="theme-color" content="#00ae42" />
+    <meta name="mobile-web-app-capable" content="yes" />
+    <meta name="apple-mobile-web-app-capable" content="yes" />
+    <meta name="apple-mobile-web-app-status-bar-style" content="black-translucent" />
+    <meta name="apple-mobile-web-app-title" content="Bambuddy" />
+
+    <!-- Manifest -->
+    <link rel="manifest" href="/manifest.json" />
+
+    <!-- Favicons -->
+    <link rel="icon" type="image/png" sizes="32x32" href="/img/favicon-32x32.png" />
+    <link rel="icon" type="image/png" sizes="16x16" href="/img/favicon-16x16.png" />
+    <link rel="apple-touch-icon" sizes="180x180" href="/img/apple-touch-icon.png" />
+
+    <!-- Splash screens for iOS -->
+    <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
+    <script type="module" crossorigin src="/assets/index-DZ9zJnpR.js"></script>
+    <link rel="stylesheet" crossorigin href="/assets/index-BkYu3kLs.css">
+  </head>
+  <body>
+    <div id="root"></div>
+
+
+    <!-- Service Worker Registration (skip on SpoolBuddy kiosk).
+         Kept as an external file so the CSP `script-src 'self'` covers it
+         without needing 'unsafe-inline' or per-build hashes. -->
+    <script src="/sw-register.js"></script>
+  </body>
+</html>