From c9fee702f0f9549c9e67275b9732f6eee8af854c Mon Sep 17 00:00:00 2001 From: codebude Date: Fri, 30 Oct 2026 14:05:34 +0100 Subject: [PATCH 1/2] Fix OIDC redirects for stale loopback PUBLIC_APP_URL --- .env.example | 4 +- backend/app/main.py | 2 + backend/app/routers/oidc.py | 112 ++++++++++++++++++++++++++---------- backend/tests/test_oidc.py | 92 ++++++++++++++++++++++++++++- docs/guide/configuration.md | 6 +- docs/releases.md | 3 + 6 files changed, 186 insertions(+), 33 deletions(-) diff --git a/.env.example b/.env.example index e2850145..57c29dc6 100644 --- a/.env.example +++ b/.env.example @@ -93,7 +93,9 @@ MAIL_FROM= MAIL_FROM_NAME=LibrisLog MAIL_STARTTLS=True MAIL_SSL_TLS=False -PUBLIC_APP_URL=http://localhost:5173 +# Required for password reset emails. Also needed when the frontend runs on a +# different origin than the backend (local dev: http://localhost:5173). +PUBLIC_APP_URL= PASSWORD_RESET_TOKEN_MAX_AGE=3600 # Anonymous installation telemetry (see docs: guide/telemetry) diff --git a/backend/app/main.py b/backend/app/main.py index 4ecd169c..d11b347e 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -130,6 +130,8 @@ async def lifespan(app: FastAPI): "See .env.example for mail setup instructions." ) + oidc.warn_about_loopback_public_app_url() + maintenance_task = asyncio.create_task(_periodic_maintenance()) telemetry_task = ( asyncio.create_task(_telemetry_heartbeat()) if not settings.telemetry_disabled else None diff --git a/backend/app/routers/oidc.py b/backend/app/routers/oidc.py index 728b4e3f..4a344797 100644 --- a/backend/app/routers/oidc.py +++ b/backend/app/routers/oidc.py @@ -1,8 +1,7 @@ """OIDC SSO endpoints โ€” login, callback, link, unlink, and status.""" import logging -from urllib.parse import quote_plus -from typing import Any +from urllib.parse import quote_plus, urlparse from fastapi import APIRouter, Depends, HTTPException, Request from fastapi.responses import RedirectResponse @@ -31,39 +30,91 @@ def _provider_id() -> str: return settings.oidc_provider_id -def _frontend_base_url() -> str: +_LOOPBACK_HOSTS = {"localhost", "127.0.0.1", "::1"} + + +def _is_loopback_host(hostname: str | None) -> bool: + """Return True for the hostnames browsers use to reach a local machine.""" + return (hostname or "").lower() in _LOOPBACK_HOSTS + + +def _is_loopback_url(url: str) -> bool: + """Return True when a configured URL points at a loopback host.""" + try: + return _is_loopback_host(urlparse(url).hostname) + except ValueError: + return False + + +def _frontend_base_url(request: Request | None = None) -> str: """Base URL of the frontend app, used for browser redirects. Empty in single-origin (production) deployments, where the backend serves the frontend and relative redirects resolve correctly. Set ``PUBLIC_APP_URL`` in split-origin (local dev) setups. + + Older ``.env.example`` files shipped ``http://localhost:5173``. If such a + loopback URL is still configured, it is honored only for requests that + themselves arrive on a loopback host. Production requests fall back to + relative redirects instead of sending users to their own localhost. """ - return (settings.public_app_url or "").rstrip("/") + base = (settings.public_app_url or "").strip().rstrip("/") + if not base: + return "" + if request is not None and _is_loopback_url(base): + if not _is_loopback_host(request.url.hostname): + return "" + return base + + +def warn_about_loopback_public_app_url() -> None: + """Log a startup hint for an old loopback ``PUBLIC_APP_URL`` value.""" + if not (settings.oidc_enabled or settings.mail_server.strip()): + return + if _is_loopback_url(settings.public_app_url or ""): + logger.warning( + "PUBLIC_APP_URL=%s points to a loopback address. It will only be used " + "for requests from loopback hosts; production requests use relative " + "redirects. Remove it or set the public frontend URL.", + settings.public_app_url, + ) -def _with_frontend_base(path: str) -> str: - base = _frontend_base_url() +def _with_frontend_base(path: str, request: Request | None = None) -> str: + base = _frontend_base_url(request) return f"{base}{path}" if base else path -def _frontend_warning_redirect(message: str) -> RedirectResponse: +def _frontend_warning_redirect(message: str, request: Request) -> RedirectResponse: """Redirect to the frontend login page with an OIDC error message.""" - return RedirectResponse(url=_with_frontend_base(f"/login?oidc_error={quote_plus(message)}"), status_code=302) + return RedirectResponse( + url=_with_frontend_base(f"/login?oidc_error={quote_plus(message)}", request), + status_code=302, + ) -def _frontend_success_redirect() -> RedirectResponse: +def _frontend_success_redirect(request: Request) -> RedirectResponse: """Redirect to the frontend callback page on successful login.""" - return RedirectResponse(url=_with_frontend_base("/auth/oidc/callback"), status_code=302) + return RedirectResponse( + url=_with_frontend_base("/auth/oidc/callback", request), + status_code=302, + ) -def _link_success_redirect() -> RedirectResponse: +def _link_success_redirect(request: Request) -> RedirectResponse: """Redirect to the frontend link callback with success status.""" - return RedirectResponse(url=_with_frontend_base("/auth/oidc/link-callback?status=success"), status_code=302) + return RedirectResponse( + url=_with_frontend_base("/auth/oidc/link-callback?status=success", request), + status_code=302, + ) -def _link_error_redirect(message: str) -> RedirectResponse: +def _link_error_redirect(message: str, request: Request) -> RedirectResponse: """Redirect to the frontend link callback with an error message.""" - return RedirectResponse(url=_with_frontend_base(f"/auth/oidc/link-callback?error={quote_plus(message)}"), status_code=302) + return RedirectResponse( + url=_with_frontend_base(f"/auth/oidc/link-callback?error={quote_plus(message)}", request), + status_code=302, + ) def _resolve_callback_redirect_uri(request: Request, route_name: str) -> str: @@ -106,7 +157,8 @@ async def oidc_login(request: Request): ) return _frontend_warning_redirect( f"{_provider_name()} login is currently unavailable. " - f"Please use email and password, then try again later." + f"Please use email and password, then try again later.", + request, ) @@ -123,19 +175,19 @@ async def oidc_callback( client = get_oidc_client() if not client: logger.warning("OIDC callback called while OIDC disabled") - return _frontend_warning_redirect("OIDC is not enabled") + return _frontend_warning_redirect("OIDC is not enabled", request) try: token = await client.authorize_access_token(request) except Exception: logger.exception("OIDC authorize_access_token failed during login callback") - return _frontend_warning_redirect("OIDC login failed") + return _frontend_warning_redirect("OIDC login failed", request) userinfo = token.get("userinfo") or {} oidc_sub = userinfo.get("sub") if not oidc_sub: logger.warning("OIDC callback missing sub claim: token_keys=%s", list(token.keys())) - return _frontend_warning_redirect("OIDC response is missing subject") + return _frontend_warning_redirect("OIDC response is missing subject", request) link = session.exec( select(OidcLink).where( @@ -150,17 +202,18 @@ async def oidc_callback( ) return _frontend_warning_redirect( f"Your {_provider_name()} account is not linked. " - f"Please log in with email and password first, then link it in your profile." + f"Please log in with email and password first, then link it in your profile.", + request, ) user = session.get(User, link.user_id) if not user: logger.error("OIDC link points to missing user: link_id=%s user_id=%s", link.id, link.user_id) - return _frontend_warning_redirect("Linked user account no longer exists") + return _frontend_warning_redirect("Linked user account no longer exists", request) assert user.id is not None start_browser_session(request, user.id, user.credentials_version) - return _frontend_success_redirect() + return _frontend_success_redirect(request) @router.get("/link-status", response_model=OidcLinkRead) @@ -206,7 +259,7 @@ async def oidc_link_authorize(request: Request): if not request.session.get("oidc_link_user_id"): logger.warning("OIDC link authorize called without link session") - return _link_error_redirect("Missing link session. Please start linking again.") + return _link_error_redirect("Missing link session. Please start linking again.", request) redirect_uri = _resolve_callback_redirect_uri(request, "oidc_link_callback") try: @@ -217,7 +270,8 @@ async def oidc_link_authorize(request: Request): _provider_id(), redirect_uri, ) return _link_error_redirect( - "OIDC provider is currently unavailable. Please try again later." + "OIDC provider is currently unavailable. Please try again later.", + request, ) @@ -233,24 +287,24 @@ async def oidc_link_callback( client = get_oidc_client() if not client: logger.warning("OIDC link callback called while OIDC disabled") - return _link_error_redirect("OIDC is not enabled") + return _link_error_redirect("OIDC is not enabled", request) user_id = request.session.pop("oidc_link_user_id", None) if not user_id: logger.warning("OIDC link callback missing link session") - return _link_error_redirect("Missing link session. Please start linking again.") + return _link_error_redirect("Missing link session. Please start linking again.", request) try: token = await client.authorize_access_token(request) except Exception: logger.exception("OIDC authorize_access_token failed during link callback") - return _link_error_redirect("OIDC linking failed") + return _link_error_redirect("OIDC linking failed", request) userinfo = token.get("userinfo") or {} oidc_sub = userinfo.get("sub") if not oidc_sub: logger.warning("OIDC link callback missing sub claim: token_keys=%s", list(token.keys())) - return _link_error_redirect("OIDC response is missing subject") + return _link_error_redirect("OIDC response is missing subject", request) existing_by_sub = session.exec( select(OidcLink).where( @@ -263,7 +317,7 @@ async def oidc_link_callback( "OIDC link conflict: provider=%s sub=%s existing_user_id=%s requested_user_id=%s", _provider_id(), oidc_sub, existing_by_sub.user_id, user_id, ) - return _link_error_redirect("This OIDC account is already linked to another user") + return _link_error_redirect("This OIDC account is already linked to another user", request) existing_link = session.exec(select(OidcLink).where(OidcLink.user_id == user_id)).first() if existing_link: @@ -285,7 +339,7 @@ async def oidc_link_callback( ) session.commit() - return _link_success_redirect() + return _link_success_redirect(request) @router.delete("/unlink", status_code=204) diff --git a/backend/tests/test_oidc.py b/backend/tests/test_oidc.py index c40b17b9..22816817 100644 --- a/backend/tests/test_oidc.py +++ b/backend/tests/test_oidc.py @@ -65,7 +65,7 @@ def test_get_oidc_client_returns_none_when_disabled(monkeypatch: MonkeyPatch) -> def test_get_oidc_client_registers_and_returns_client_when_enabled(monkeypatch: MonkeyPatch) -> None: """Lines 26-36: when enabled and not yet registered, oauth.register is called.""" - from app.oidc import get_oidc_client, _registered + from app.oidc import get_oidc_client monkeypatch.setattr("app.oidc.oidc_is_enabled", lambda: True) monkeypatch.setattr(settings, "oidc_provider_id", "test-oidc") @@ -549,3 +549,93 @@ def test_oidc_link_callback_updates_existing_link( assert link.oidc_sub == "new-sub" assert link.oidc_email == "new@example.com" assert link.oidc_name == "New Name" + + +def _request_with_host(host: str): + """Build a minimal request so redirect bases can be tested directly.""" + from starlette.requests import Request + + return Request( + { + "type": "http", + "method": "GET", + "path": "/", + "headers": [(b"host", host.encode())], + "scheme": "http", + "server": ("testserver", 80), + } + ) + + +def test_frontend_base_url_ignores_loopback_for_remote_host(monkeypatch: MonkeyPatch) -> None: + """An old localhost PUBLIC_APP_URL must not redirect production users.""" + from app.routers import oidc as oidc_router + + monkeypatch.setattr(settings, "public_app_url", "http://localhost:5173") + + request = _request_with_host("books.example.com") + assert oidc_router._frontend_base_url(request) == "" + response = oidc_router._frontend_success_redirect(request) + assert response.headers["location"] == "/auth/oidc/callback" + + +def test_frontend_base_url_uses_loopback_for_local_host(monkeypatch: MonkeyPatch) -> None: + """Local split-origin development still gets the configured frontend URL.""" + from app.routers import oidc as oidc_router + + monkeypatch.setattr(settings, "public_app_url", "http://localhost:5173") + + request = _request_with_host("localhost:8000") + assert oidc_router._frontend_base_url(request) == "http://localhost:5173" + response = oidc_router._frontend_success_redirect(request) + assert response.headers["location"] == "http://localhost:5173/auth/oidc/callback" + + +def test_frontend_base_url_uses_configured_public_url(monkeypatch: MonkeyPatch) -> None: + """A real public frontend URL is used for remote requests.""" + from app.routers import oidc as oidc_router + + monkeypatch.setattr(settings, "public_app_url", "https://books.example.com/") + + request = _request_with_host("books.example.com") + assert oidc_router._frontend_base_url(request) == "https://books.example.com" + response = oidc_router._frontend_success_redirect(request) + assert response.headers["location"] == "https://books.example.com/auth/oidc/callback" + + +def test_frontend_base_url_ignores_other_loopback_urls(monkeypatch: MonkeyPatch) -> None: + """The safety net covers 127.0.0.1 and IPv6 loopback, not just localhost:5173.""" + from app.routers import oidc as oidc_router + + monkeypatch.setattr(settings, "public_app_url", "http://127.0.0.1:4173") + + request = _request_with_host("books.example.com") + assert oidc_router._frontend_base_url(request) == "" + + +def test_loopback_public_app_url_warns(monkeypatch: MonkeyPatch) -> None: + """Startup should explain why a loopback PUBLIC_APP_URL is being ignored.""" + from app.routers import oidc as oidc_router + + monkeypatch.setattr(settings, "public_app_url", "http://localhost:5173") + monkeypatch.setattr(settings, "oidc_enabled", True) + with patch("app.routers.oidc.logger.warning") as mock_warning: + oidc_router.warn_about_loopback_public_app_url() + + mock_warning.assert_called_once() + assert "loopback" in str(mock_warning.call_args) + + +def test_loopback_public_app_url_warning_is_skipped_when_unused( + monkeypatch: MonkeyPatch, +) -> None: + """Do not warn about a loopback URL when neither OIDC nor mail uses it.""" + from app.routers import oidc as oidc_router + + monkeypatch.setattr(settings, "public_app_url", "http://localhost:5173") + monkeypatch.setattr(settings, "oidc_enabled", False) + monkeypatch.setattr(settings, "mail_server", "") + with patch("app.routers.oidc.logger.warning") as mock_warning: + oidc_router.warn_about_loopback_public_app_url() + + mock_warning.assert_not_called() diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index d889d2b4..d0104fba 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -141,7 +141,7 @@ Password reset is optional. If mail is not configured, the "Forgot password?" li | `MAIL_FROM_NAME` | Sender display name | `LibrisLog` | | `MAIL_STARTTLS` | Use STARTTLS (`True`/`False`) | `True` | | `MAIL_SSL_TLS` | Use implicit SSL/TLS (`True`/`False`) | `False` | -| `PUBLIC_APP_URL` | Public URL of the frontend. Required for password reset emails; needed for OIDC redirects in split-origin (local dev) setups. Leave empty when the backend serves the frontend. | (none) | +| `PUBLIC_APP_URL` | Public URL of the frontend. Required for password reset emails; needed for OIDC redirects in split-origin (local dev) setups. Leave empty when the backend serves the frontend. A loopback URL such as `http://localhost:5173` is ignored for non-local requests so older `.env` files do not redirect production users to localhost. | (none) | | `PASSWORD_RESET_TOKEN_MAX_AGE` | Reset token validity in seconds | `3600` (1 hour) | ::: tip Local Development @@ -307,6 +307,8 @@ MAIL_FROM= MAIL_FROM_NAME=LibrisLog MAIL_STARTTLS=True MAIL_SSL_TLS=False -PUBLIC_APP_URL=http://localhost:5173 +# Required for password reset emails. Also needed when the frontend runs on a +# different origin than the backend (local dev: http://localhost:5173). +PUBLIC_APP_URL= PASSWORD_RESET_TOKEN_MAX_AGE=3600 ``` \ No newline at end of file diff --git a/docs/releases.md b/docs/releases.md index fb9fe30d..f314f750 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -68,6 +68,9 @@ LibrisLog v1.11.0 lets books be read more than once. Each reading is tracked as **Dependencies** - โฌ†๏ธ Updated backend and frontend dependencies, including FastAPI, Pydantic, SQLAlchemy, Alembic, Starlette, pytest, Playwright, Scrapling, curl-cffi, SvelteKit, Svelte, Vite, Vitest, and Chart.js. TypeScript 7 was intentionally deferred because it is a major upgrade. +**Bug fixes** +- ๐Ÿ” **OIDC redirect safety net.** A loopback `PUBLIC_APP_URL` such as the old `.env.example` value `http://localhost:5173` is ignored for non-local requests, so existing installations do not redirect users to localhost after upgrading. A startup warning explains how to set it correctly. + **Breaking changes:** None. No data migration. Existing endpoints keep their behaviour; only two optional query parameters and new endpoints were added. Both authentication features are disabled by default. Existing installations keep password login and behave exactly as before. [Compare with v1.11.0](https://github.com/codebude/librislog/compare/v1.11.0...HEAD) From 52fb697b84ffd3537342b4d2215c5d7e35cf8681 Mon Sep 17 00:00:00 2001 From: codebude Date: Fri, 30 Oct 2026 14:18:51 +0100 Subject: [PATCH 2/2] Add automatic OIDC login redirect --- .env.example | 3 ++ backend/app/config.py | 1 + backend/app/routers/oidc.py | 1 + backend/app/schemas.py | 1 + backend/tests/test_oidc.py | 28 +++++++++++ docs/guide/configuration.md | 1 + docs/guide/using-librislog/administration.md | 4 ++ docs/releases.md | 1 + frontend/src/lib/types.ts | 1 + frontend/src/lib/utils/oidcRedirect.test.ts | 35 ++++++++++++++ frontend/src/lib/utils/oidcRedirect.ts | 19 ++++++++ frontend/src/routes/login/+page.svelte | 25 +++++++++- frontend/src/routes/login/page.test.ts | 50 ++++++++++++++++++-- frontend/src/routes/profile/+page.svelte | 2 +- frontend/src/routes/setup/page.test.ts | 3 +- 15 files changed, 169 insertions(+), 6 deletions(-) create mode 100644 frontend/src/lib/utils/oidcRedirect.test.ts create mode 100644 frontend/src/lib/utils/oidcRedirect.ts diff --git a/.env.example b/.env.example index 57c29dc6..93200c63 100644 --- a/.env.example +++ b/.env.example @@ -26,6 +26,9 @@ TRUSTED_PROXY_AUTH_USER_MAP= # Optional OIDC settings (leave OIDC_ENABLED=false to disable) OIDC_ENABLED=false +# Redirect the login page straight to the provider. Only takes effect when +# PASSWORD_LOGIN_ENABLED=false. Failed attempts return to the login page. +OIDC_AUTO_REDIRECT=false OIDC_PROVIDER_ID=oidc OIDC_PROVIDER_NAME=Single Sign-On OIDC_CLIENT_ID= diff --git a/backend/app/config.py b/backend/app/config.py index 3299a544..782a79ce 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -26,6 +26,7 @@ class Settings(BaseSettings): trusted_proxy_auth_secret: str = "" trusted_proxy_auth_user_map: str = "" oidc_enabled: bool = False + oidc_auto_redirect: bool = False oidc_provider_id: str = "oidc" oidc_provider_name: str = "Single Sign-On" oidc_client_id: str = "" diff --git a/backend/app/routers/oidc.py b/backend/app/routers/oidc.py index 4a344797..0f405f05 100644 --- a/backend/app/routers/oidc.py +++ b/backend/app/routers/oidc.py @@ -137,6 +137,7 @@ def oidc_config() -> OidcConfigRead: enabled=True, provider_id=settings.oidc_provider_id, provider_name=_provider_name(), + auto_redirect=settings.oidc_auto_redirect and not settings.password_login_enabled, ) diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 064e5bd6..2d472090 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -658,6 +658,7 @@ class OidcConfigRead(SQLModel): enabled: bool provider_id: Optional[str] = None provider_name: Optional[str] = None + auto_redirect: bool = False class AppConfigRead(SQLModel): diff --git a/backend/tests/test_oidc.py b/backend/tests/test_oidc.py index 22816817..e3f9f2fd 100644 --- a/backend/tests/test_oidc.py +++ b/backend/tests/test_oidc.py @@ -113,6 +113,33 @@ def test_oidc_config_enabled(client: TestClient, monkeypatch: MonkeyPatch) -> No assert data["enabled"] is True assert data["provider_id"] == "test-oidc" assert data["provider_name"] == "Test SSO" + assert data["auto_redirect"] is False + + +def test_oidc_config_auto_redirect_when_password_login_disabled( + client: TestClient, monkeypatch: MonkeyPatch +) -> None: + """Auto-redirect is exposed only when password login is unavailable.""" + _set_oidc_enabled(monkeypatch) + monkeypatch.setattr(settings, "oidc_auto_redirect", True) + monkeypatch.setattr(settings, "password_login_enabled", False) + + data = client.get("/api/oidc/config").json() + + assert data["auto_redirect"] is True + + +def test_oidc_config_auto_redirect_ignored_when_password_login_enabled( + client: TestClient, monkeypatch: MonkeyPatch +) -> None: + """The backend refuses auto-redirect while password login is available.""" + _set_oidc_enabled(monkeypatch) + monkeypatch.setattr(settings, "oidc_auto_redirect", True) + monkeypatch.setattr(settings, "password_login_enabled", True) + + data = client.get("/api/oidc/config").json() + + assert data["auto_redirect"] is False def test_oidc_login_returns_404_when_disabled(client: TestClient, monkeypatch: MonkeyPatch) -> None: @@ -336,6 +363,7 @@ def test_oidc_config_disabled(client: TestClient, monkeypatch: MonkeyPatch) -> N assert data["enabled"] is False assert data["provider_id"] is None assert data["provider_name"] is None + assert data["auto_redirect"] is False def test_oidc_login_with_x_forwarded_proto(client: TestClient, monkeypatch: MonkeyPatch) -> None: diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index d0104fba..a653bd96 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -123,6 +123,7 @@ controlled by the proxy, not by LibrisLog. | Variable | Description | |----------|-------------| | `OIDC_ENABLED` | Enable OIDC authentication (`true`/`false`) | +| `OIDC_AUTO_REDIRECT` | Redirect the login page straight to the provider. Only effective when `OIDC_ENABLED=true` and `PASSWORD_LOGIN_ENABLED=false`. Failed attempts return to the login page with an error instead of redirecting again. | `false` | | `OIDC_CLIENT_ID` | OIDC client ID | | `OIDC_CLIENT_SECRET` | OIDC client secret | | `OIDC_WELL_KNOWN_URL` | OIDC well-known configuration URL | diff --git a/docs/guide/using-librislog/administration.md b/docs/guide/using-librislog/administration.md index eb99c761..707f03a2 100644 --- a/docs/guide/using-librislog/administration.md +++ b/docs/guide/using-librislog/administration.md @@ -37,6 +37,10 @@ open **Profile โ†’ Single Sign-On โ†’ Link account**. Admin-created users withou link cannot sign in while password login is disabled. If you get locked out, set `PASSWORD_LOGIN_ENABLED=true` and restart the backend. +Set `OIDC_AUTO_REDIRECT=true` to send visitors straight to the provider instead of +showing the login page. This only takes effect while password login is disabled; a failed +attempt returns to the login page with the error instead of redirecting again. + For account management, admins can still create users and set passwords through **Administration โ†’ Users**. These actions do not enable password login while the flag is off; they only store the password for later use or recovery. The profile password diff --git a/docs/releases.md b/docs/releases.md index f314f750..d596e974 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -57,6 +57,7 @@ LibrisLog v1.11.0 lets books be read more than once. Each reading is tracked as - ๐Ÿงน **Orphan tag cleanup.** Tags left on no book are deleted automatically. - ๐Ÿ” **Disable password login.** `PASSWORD_LOGIN_ENABLED=false` hides the email/password form and "Forgot password?" link and makes the login, forgot-password, and reset-password endpoints return `403`. First-admin setup still works, so a new instance can always be bootstrapped. The setup page warns when single sign-on should be linked before logging out. - ๐ŸŒ **Trusted reverse-proxy authentication.** `TRUSTED_PROXY_AUTH_ENABLED=true` signs a user in automatically when a trusted proxy forwards an identity in a configurable header. The header, trusted IPs or CIDR ranges, and an optional shared secret are configurable, and `TRUSTED_PROXY_AUTH_USER_MAP` can translate proxy usernames to account emails when the proxy name is not the email itself. The feature fails closed when misconfigured, rejects ambiguous email matches, and ignores unknown values. +- ๐Ÿ” **Automatic OIDC redirect.** `OIDC_AUTO_REDIRECT=true` sends the login page straight to the provider when `PASSWORD_LOGIN_ENABLED=false`. Failed attempts return to the login page with an error instead of looping. - โš™๏ธ **Config API.** `GET /api/config` now exposes `password_login_enabled` and the effective `trusted_proxy_auth_enabled` state so the login page can adapt. The profile password field is hidden while password login is disabled. - ๐ŸŒ **Translations.** New login and setup messages in English, German, Spanish, French, and Chinese. diff --git a/frontend/src/lib/types.ts b/frontend/src/lib/types.ts index 7fb008a5..795a4364 100644 --- a/frontend/src/lib/types.ts +++ b/frontend/src/lib/types.ts @@ -321,6 +321,7 @@ export interface OidcConfig { enabled: boolean; provider_id: string | null; provider_name: string | null; + auto_redirect: boolean; } export interface AppConfig { diff --git a/frontend/src/lib/utils/oidcRedirect.test.ts b/frontend/src/lib/utils/oidcRedirect.test.ts new file mode 100644 index 00000000..6dc5c2da --- /dev/null +++ b/frontend/src/lib/utils/oidcRedirect.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect } from 'vitest'; +import { shouldAutoRedirectToOidc } from './oidcRedirect.js'; + +const base = { + oidcEnabled: true, + oidcAutoRedirect: true, + passwordLoginEnabled: false, + hasOidcError: false +}; + +describe('shouldAutoRedirectToOidc', () => { + it('redirects when OIDC is the only sign-in method and auto-redirect is on', () => { + expect(shouldAutoRedirectToOidc(base)).toBe(true); + }); + + it('does not redirect while password login is enabled', () => { + expect(shouldAutoRedirectToOidc({ ...base, passwordLoginEnabled: true })).toBe(false); + }); + + it('does not redirect while the login configuration is still loading', () => { + expect(shouldAutoRedirectToOidc({ ...base, passwordLoginEnabled: null })).toBe(false); + }); + + it('does not redirect when OIDC is disabled', () => { + expect(shouldAutoRedirectToOidc({ ...base, oidcEnabled: false })).toBe(false); + }); + + it('does not redirect when auto-redirect is disabled', () => { + expect(shouldAutoRedirectToOidc({ ...base, oidcAutoRedirect: false })).toBe(false); + }); + + it('does not redirect after an OIDC error', () => { + expect(shouldAutoRedirectToOidc({ ...base, hasOidcError: true })).toBe(false); + }); +}); diff --git a/frontend/src/lib/utils/oidcRedirect.ts b/frontend/src/lib/utils/oidcRedirect.ts new file mode 100644 index 00000000..7512b6c3 --- /dev/null +++ b/frontend/src/lib/utils/oidcRedirect.ts @@ -0,0 +1,19 @@ +export interface OidcAutoRedirectOptions { + oidcEnabled: boolean; + oidcAutoRedirect: boolean; + passwordLoginEnabled: boolean | null; + hasOidcError: boolean; +} + +/** + * Automatic OIDC redirect is only safe when OIDC is the only usable sign-in + * method and the previous attempt did not already fail with an error. + */ +export function shouldAutoRedirectToOidc(options: OidcAutoRedirectOptions): boolean { + return ( + options.oidcEnabled && + options.oidcAutoRedirect && + options.passwordLoginEnabled === false && + !options.hasOidcError + ); +} diff --git a/frontend/src/routes/login/+page.svelte b/frontend/src/routes/login/+page.svelte index e7b8ee5b..3552b4e3 100644 --- a/frontend/src/routes/login/+page.svelte +++ b/frontend/src/routes/login/+page.svelte @@ -6,6 +6,7 @@ import { currentUser, csrfToken } from '#lib/stores/auth.js'; import { _, locale, setLocale, SUPPORTED_LOCALES, type AppLocale } from '#lib/i18n/index.js'; import { setTimezone, detectTimezone } from '#lib/stores/timezone.js'; + import { shouldAutoRedirectToOidc } from '#lib/utils/oidcRedirect.js'; import { setThemeMode, setCustomTheme, applyThemeToDocument, saveThemeToStorage, sanitizeThemeMode, THEME_MODE_KEY, CUSTOM_THEME_KEY @@ -19,6 +20,8 @@ let error = $state(''); let oidcEnabled = $state(false); let oidcProviderName = $state('Single Sign-On'); + let oidcAutoRedirect = $state(false); + let autoRedirectStarted = false; let passwordLoginEnabled = $state(null); let trustedProxyAuthEnabled = $state(false); let configError = $state(false); @@ -32,6 +35,7 @@ if (oidcResult.status === 'fulfilled') { oidcEnabled = oidcResult.value.enabled; oidcProviderName = oidcResult.value.provider_name ?? 'Single Sign-On'; + oidcAutoRedirect = oidcResult.value.auto_redirect ?? false; } else { oidcEnabled = false; } @@ -41,6 +45,9 @@ } else { configError = true; } + if (oidcResult.status === 'fulfilled' && appResult.status === 'fulfilled') { + maybeAutoRedirectToOidc(); + } })(); }); @@ -126,8 +133,24 @@ forgotDialog?.showModal(); } + function maybeAutoRedirectToOidc() { + if (autoRedirectStarted) return; + const hasOidcError = new URLSearchParams(window.location.search).has('oidc_error'); + if ( + shouldAutoRedirectToOidc({ + oidcEnabled, + oidcAutoRedirect, + passwordLoginEnabled, + hasOidcError + }) + ) { + autoRedirectStarted = true; + startOidcLogin(); + } + } + function startOidcLogin() { - window.location.href = api.oidc.loginUrl(); + window.location.assign(api.oidc.loginUrl()); } $effect(() => { diff --git a/frontend/src/routes/login/page.test.ts b/frontend/src/routes/login/page.test.ts index ba488093..cfd9dbb6 100644 --- a/frontend/src/routes/login/page.test.ts +++ b/frontend/src/routes/login/page.test.ts @@ -29,12 +29,14 @@ vi.mock('#lib/api.js', () => ({ function mockConfig({ passwordLoginEnabled = true, oidcEnabled = true, - trustedProxyAuthEnabled = false + trustedProxyAuthEnabled = false, + autoRedirect = false } = {}) { mockOidcConfig.mockResolvedValue({ enabled: oidcEnabled, provider_id: oidcEnabled ? 'oidc' : null, - provider_name: oidcEnabled ? 'Pocket ID' : null + provider_name: oidcEnabled ? 'Pocket ID' : null, + auto_redirect: autoRedirect }); mockAppConfig.mockResolvedValue({ embed_enabled: true, @@ -85,7 +87,12 @@ describe('Login page authentication methods', () => { }); it('shows an error instead of the form when config cannot be loaded', async () => { - mockOidcConfig.mockResolvedValue({ enabled: false, provider_id: null, provider_name: null }); + mockOidcConfig.mockResolvedValue({ + enabled: false, + provider_id: null, + provider_name: null, + auto_redirect: false + }); mockAppConfig.mockRejectedValue(new Error('config unavailable')); render(LoginPage); @@ -95,6 +102,43 @@ describe('Login page authentication methods', () => { expect(screen.queryByLabelText('Password')).not.toBeInTheDocument(); }); + it('auto-redirects to OIDC when password login is disabled', async () => { + mockConfig({ passwordLoginEnabled: false, oidcEnabled: true, autoRedirect: true }); + const assign = vi.spyOn(window.location, 'assign').mockImplementation(() => {}); + render(LoginPage); + + await waitFor(() => { + expect(assign).toHaveBeenCalledWith('/api/oidc/login'); + }); + assign.mockRestore(); + }); + + it('does not auto-redirect when password login is enabled', async () => { + mockConfig({ passwordLoginEnabled: true, oidcEnabled: true, autoRedirect: true }); + const assign = vi.spyOn(window.location, 'assign').mockImplementation(() => {}); + render(LoginPage); + + await waitFor(() => { + expect(screen.getByText('Continue with Pocket ID')).toBeInTheDocument(); + }); + expect(assign).not.toHaveBeenCalled(); + assign.mockRestore(); + }); + + it('does not auto-redirect after an OIDC error', async () => { + window.history.replaceState({}, '', '/login?oidc_error=OIDC%20failed'); + mockConfig({ passwordLoginEnabled: false, oidcEnabled: true, autoRedirect: true }); + const assign = vi.spyOn(window.location, 'assign').mockImplementation(() => {}); + render(LoginPage); + + await waitFor(() => { + expect(screen.getByText(/OIDC failed/i)).toBeInTheDocument(); + }); + expect(assign).not.toHaveBeenCalled(); + assign.mockRestore(); + window.history.replaceState({}, '', '/login'); + }); + it('mentions proxy-managed access when proxy auth is enabled', async () => { mockConfig({ passwordLoginEnabled: false, oidcEnabled: false, trustedProxyAuthEnabled: true }); render(LoginPage); diff --git a/frontend/src/routes/profile/+page.svelte b/frontend/src/routes/profile/+page.svelte index a1ef6dd5..c364d914 100644 --- a/frontend/src/routes/profile/+page.svelte +++ b/frontend/src/routes/profile/+page.svelte @@ -41,7 +41,7 @@ import ShareLinkDialog from '#lib/components/ShareLinkDialog.svelte'; let keyCopied = $state(false); let keys = $state([]); let pendingDeleteKeyId = $state(null); - let oidcConfig = $state({ enabled: false, provider_id: null, provider_name: null }); + let oidcConfig = $state({ enabled: false, provider_id: null, provider_name: null, auto_redirect: false }); let appConfig = $state({ embed_enabled: true, dashboard_quote_enabled: true, thalia_cover_search_enabled: false, password_login_enabled: true, trusted_proxy_auth_enabled: false }); let oidcLink = $state({ linked: false, provider_name: null, oidc_email: null, oidc_name: null }); let oidcLoading = $state(false); diff --git a/frontend/src/routes/setup/page.test.ts b/frontend/src/routes/setup/page.test.ts index 82c061d8..725105b4 100644 --- a/frontend/src/routes/setup/page.test.ts +++ b/frontend/src/routes/setup/page.test.ts @@ -27,7 +27,8 @@ function mockConfig({ passwordLoginEnabled = true, oidcEnabled = true } = {}) { mockOidcConfig.mockResolvedValue({ enabled: oidcEnabled, provider_id: oidcEnabled ? 'oidc' : null, - provider_name: oidcEnabled ? 'Pocket ID' : null + provider_name: oidcEnabled ? 'Pocket ID' : null, + auto_redirect: false }); mockAppConfig.mockResolvedValue({ embed_enabled: true,