adam/meshcore-hub
1
0
Fork 0
mirror of https://github.com/ipnet-mesh/meshcore-hub.git synced 2026-07-06 01:41:27 +02:00
Code Issues Packages Projects Releases Wiki Activity

Fix OIDC logout redirect and username display for LogTo

Browse Source 
- Pass client_id in logout redirect so LogTo can validate post_logout_redirect_uri
- Add OIDC_POST_LOGOUT_REDIRECT_URI config option with fallback derivation
- Move session.clear() after logout_redirect() to allow state data save
- Add 'username' to strip_userinfo() name fallback chain (LogTo uses this)
- Strip quotes from OIDC_SCOPES and pass as list to Authlib (fixes direnv
  quoting issue where literal quotes were sent in the authorization URL)
- Add OIDC_POST_LOGOUT_REDIRECT_URI to config, app state, and docs
- Add INFO-level logging to callback and logout handlers for diagnostics
- Update .env.example, README.md, AGENTS.md, docs/upgrading.md
This commit is contained in:
Louis King
2026-04-28 22:44:01 +01:00
parent 02c0a8f1b7
commit d1b6f0d0a7
8 changed files with 254 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.env.example
+15 -2
View File
@@ -23,6 +23,13 @@
#
# Serial ports are typically /dev/ttyUSB[0-9] or /dev/ttyACM[0-9] on Linux.
# -----------------------------------------------------------------------------
# NOTE: If using direnv, values with spaces MUST be quoted:
# OIDC_SCOPES="openid email profile"
# The application strips surrounding quotes automatically. Do NOT use
# quotes for values loaded via Docker Compose map syntax:
# environment:
# OIDC_SCOPES: openid email profile
# -----------------------------------------------------------------------------
# =============================================================================
# COMMON SETTINGS
@@ -359,9 +366,15 @@ WEB_PORT=8080
# Example: https://hub.example.com/auth/callback
# OIDC_REDIRECT_URI=
# OAuth scopes to request
# Post-logout redirect URI (must match Sign-out redirect URIs configured in IdP)
# Falls back to OIDC_REDIRECT_URI base or request.base_url if not set
# Example: https://hub.example.com/
# OIDC_POST_LOGOUT_REDIRECT_URI=
# OAuth scopes to request. The 'openid' scope is required for ID tokens
# and userinfo endpoint access. Quotes around the value are stripped automatically.
# Default: openid email profile
# OIDC_SCOPES=openid email profile
# OIDC_SCOPES="openid email profile"
# ID token claim name containing user roles
# Default: roles
AGENTS.md
+2 -1
View File
@@ -642,7 +642,8 @@ Key variables:
- `OIDC_CLIENT_SECRET` - OIDC client secret (required if OIDC_ENABLED=true)
- `OIDC_DISCOVERY_URL` - OIDC discovery URL (required if OIDC_ENABLED=true)
- `OIDC_REDIRECT_URI` - Explicit callback URL (overrides auto-derivation)
- `OIDC_SCOPES` - OAuth scopes (default: `openid email profile`)
- `OIDC_POST_LOGOUT_REDIRECT_URI` - Post-logout redirect URI (must match IdP sign-out URIs, falls back to `OIDC_REDIRECT_URI` base)
- `OIDC_SCOPES` - OAuth scopes (default: `openid email profile`). The `openid` scope is required for ID tokens and userinfo. Quotes are stripped automatically for direnv compatibility.
- `OIDC_ROLES_CLAIM` - ID token claim for roles (default: `roles`)
- `OIDC_ADMIN_ROLE` - Role value for admin access (default: `admin`)
- `OIDC_MEMBER_ROLE` - Role value for member access (default: `member`)
README.md
+2 -1
View File
@@ -374,7 +374,8 @@ The collector automatically cleans up old event data and inactive nodes:
| `OIDC_CLIENT_SECRET` | _(none)_ | OIDC client secret (from IdP, required when OIDC_ENABLED=true) |
| `OIDC_DISCOVERY_URL` | _(none)_ | IdP's `.well-known/openid-configuration` URL (required when OIDC_ENABLED=true) |
| `OIDC_REDIRECT_URI` | _(auto-derived)_ | Explicit callback URL (overrides auto-derivation from request) |
| `OIDC_SCOPES` | `openid email profile` | OAuth scopes to request |
| `OIDC_POST_LOGOUT_REDIRECT_URI` | _(auto-derived)_ | Post-logout redirect URI (must match Sign-out redirect URIs in IdP). Falls back to `OIDC_REDIRECT_URI` base or `request.base_url` |
| `OIDC_SCOPES` | `openid email profile` | OAuth scopes to request. The `openid` scope is required for ID tokens. Quotes are stripped automatically. |
| `OIDC_ROLES_CLAIM` | `roles` | ID token claim name containing user roles |
| `OIDC_ADMIN_ROLE` | `admin` | Role value granting admin access |
| `OIDC_MEMBER_ROLE` | `member` | Role value granting member access |
docs/upgrading.md
+3 -1
View File
@@ -38,7 +38,9 @@ This release includes **breaking changes** to the admin authentication model.
### New Variables
See the OIDC section in `.env.example` for the full list of 12 new environment variables.
See the OIDC section in `.env.example` for the full list of environment variables.
**Important for LogTo users:** You must pass `client_id` in the logout request for the post-logout redirect to work. This is handled automatically by the application. You also need to register your app's URL as a **Sign-out redirect URI** in the LogTo admin console (e.g. `https://ipnt.uk`). If the redirect still doesn't work after updating, set `OIDC_POST_LOGOUT_REDIRECT_URI` explicitly to match your registered URI.
## v0.9.0
src/meshcore_hub/common/config.py
+7
View File
@@ -293,6 +293,13 @@ class WebSettings(CommonSettings):
default=None,
description="OIDC callback URL (overrides auto-derivation)",
)
oidc_post_logout_redirect_uri: Optional[str] = Field(
default=None,
description=(
"OIDC post-logout redirect URI (must match Sign-out redirect URIs "
"in IdP config). Falls back to OIDC_REDIRECT_URI base or request.base_url."
),
)
oidc_scopes: str = Field(
default="openid email profile", description="OAuth scopes to request"
)
src/meshcore_hub/web/app.py
+61 -10
View File
@@ -288,6 +288,9 @@ def create_app(
scopes=settings.oidc_scopes,
)
app.state.oidc_enabled = True
app.state.oidc_client_id = settings.oidc_client_id
app.state.oidc_redirect_uri = settings.oidc_redirect_uri
app.state.oidc_post_logout_redirect_uri = settings.oidc_post_logout_redirect_uri
app.state.oidc_roles_claim = settings.oidc_roles_claim
app.state.oidc_admin_role = settings.oidc_admin_role
app.state.oidc_member_role = settings.oidc_member_role
@@ -803,7 +806,12 @@ def create_app(
redirect_uri = getattr(request.app.state, "oidc_redirect_uri", None) or str(
request.url_for("auth_callback")
)
return await oauth.oidc.authorize_redirect(request, redirect_uri) # type: ignore[no-any-return]
response: Response = await oauth.oidc.authorize_redirect(request, redirect_uri)
logger.info(
"OIDC login: authorization URL=%s",
response.headers.get("location", "unknown"),
)
return response
@app.get("/auth/callback", tags=["Auth"], name="auth_callback")
async def auth_callback(request: Request) -> Response:
@@ -811,9 +819,29 @@ def create_app(
if not request.app.state.oidc_enabled:
return JSONResponse({"detail": "OIDC not enabled"}, status_code=400)
token = await oauth.oidc.authorize_access_token(request)
userinfo = token.get("userinfo", {})
logger.info(
"OIDC callback: token keys=%s, granted scope=%s",
list(token.keys()),
token.get("scope"),
)
userinfo = token.get("userinfo") or {}
logger.info("OIDC callback: ID token userinfo=%s", dict(userinfo))
if not userinfo.get("name") and not userinfo.get("email"):
try:
userinfo = await oauth.oidc.userinfo(token=token)
logger.info("OIDC callback: /userinfo endpoint=%s", dict(userinfo))
except Exception:
logger.exception(
"OIDC userinfo fetch failed, using ID token claims only"
)
roles_claim = request.app.state.oidc_roles_claim
request.session["user"] = strip_userinfo(userinfo, roles_claim)
session_user = strip_userinfo(userinfo, roles_claim)
logger.info("OIDC callback: session user=%s", session_user)
request.session["user"] = session_user
request.session["id_token"] = token.get("id_token")
next_url = request.session.pop("next", "/")
from starlette.responses import RedirectResponse
@@ -824,17 +852,40 @@ def create_app(
"""Clear session and redirect to IdP end_session_endpoint."""
if not request.app.state.oidc_enabled:
return JSONResponse({"detail": "OIDC not enabled"}, status_code=400)
request.session.clear()
from starlette.responses import RedirectResponse
id_token_hint = request.session.get("id_token")
client_id = request.app.state.oidc_client_id
post_logout_uri = request.app.state.oidc_post_logout_redirect_uri
if not post_logout_uri:
redirect_uri = getattr(request.app.state, "oidc_redirect_uri", None)
if redirect_uri:
base = redirect_uri.rsplit("/auth/callback", 1)[0]
post_logout_uri = base.rstrip("/") + "/"
else:
post_logout_uri = str(request.base_url).rstrip("/")
logger.info(
"OIDC logout: client_id=%s, id_token_hint=%s, post_logout_redirect_uri=%s",
client_id,
"present" if id_token_hint else "MISSING",
post_logout_uri,
)
try:
metadata = await oauth.oidc.load_server_metadata()
end_session = metadata.get("end_session_endpoint")
response: Response = await oauth.oidc.logout_redirect(
request,
post_logout_redirect_uri=post_logout_uri,
id_token_hint=id_token_hint,
client_id=client_id,
)
except Exception:
end_session = None
if end_session:
return RedirectResponse(url=end_session)
return RedirectResponse(url="/")
logger.exception("OIDC logout_redirect failed, redirecting to /")
response = RedirectResponse(url="/")
request.session.clear()
return response
@app.get("/auth/user", tags=["Auth"])
async def auth_user(request: Request) -> JSONResponse:
src/meshcore_hub/web/oidc.py
+11 -2
View File
@@ -15,12 +15,15 @@ def init_oidc(
client_id: str, client_secret: str, discovery_url: str, scopes: str
) -> None:
"""Register the OIDC client on the OAuth registry."""
if not discovery_url.endswith("/.well-known/openid-configuration"):
discovery_url = discovery_url.rstrip("/") + "/.well-known/openid-configuration"
scope_list = scopes.strip('"').strip("'").split()
oauth.register(
name="oidc",
client_id=client_id,
client_secret=client_secret,
server_metadata_url=discovery_url,
client_kwargs={"scope": scopes},
client_kwargs={"scope": scope_list},
)
@@ -56,9 +59,15 @@ def get_user_roles(
def strip_userinfo(userinfo: dict[str, Any], roles_claim: str) -> dict[str, Any]:
"""Strip userinfo to essential fields for session storage."""
name = (
userinfo.get("name")
or userinfo.get("preferred_username")
or userinfo.get("username")
or userinfo.get("nickname")
)
return {
"sub": userinfo.get("sub"),
"name": userinfo.get("name"),
"name": name,
"email": userinfo.get("email"),
"picture": userinfo.get("picture"),
roles_claim: userinfo.get(roles_claim, []),
tests/test_web/test_oidc.py
+153 -6
View File
@@ -6,6 +6,8 @@ from unittest.mock import AsyncMock, patch
from fastapi.testclient import TestClient
from meshcore_hub.web.oidc import init_oidc, strip_userinfo
class TestOIDCSettingsValidation:
"""Test OIDC configuration validation."""
@@ -78,18 +80,44 @@ class TestAuthLogout:
def test_logout_clears_session(
self, client_with_oidc_admin_session: TestClient
) -> None:
"""Test logout clears session and redirects."""
"""Test logout clears session and redirects via IdP."""
with patch(
"meshcore_hub.web.app.oauth.oidc.load_server_metadata",
"meshcore_hub.web.app.oauth.oidc.logout_redirect",
new_callable=AsyncMock,
) as mock_metadata:
mock_metadata.return_value = {
"end_session_endpoint": "https://idp.example.com/logout"
}
) as mock_logout:
from starlette.responses import RedirectResponse
mock_logout.return_value = RedirectResponse(
url="https://idp.example.com/logout"
)
response = client_with_oidc_admin_session.get(
"/auth/logout", follow_redirects=False
)
assert response.status_code == 307
mock_logout.assert_called_once()
call_kwargs = mock_logout.call_args[1]
assert call_kwargs["client_id"] == "test-client-id"
assert "post_logout_redirect_uri" in call_kwargs
def test_logout_falls_back_to_base_url(
self, client_with_oidc_admin_session: TestClient
) -> None:
"""Test logout uses request.base_url when no redirect URI configured."""
with patch(
"meshcore_hub.web.app.oauth.oidc.logout_redirect",
new_callable=AsyncMock,
) as mock_logout:
from starlette.responses import RedirectResponse
mock_logout.return_value = RedirectResponse(
url="https://idp.example.com/logout"
)
response = client_with_oidc_admin_session.get(
"/auth/logout", follow_redirects=False
)
assert response.status_code == 307
call_kwargs = mock_logout.call_args[1]
assert "post_logout_redirect_uri" in call_kwargs
class TestAuthUser:
@@ -257,6 +285,125 @@ class TestConfigInjection:
assert config["is_member"] is False
class TestStripUserinfo:
"""Test strip_userinfo helper function."""
def test_name_from_name_claim(self) -> None:
"""Test name extracted from 'name' claim."""
userinfo = {"sub": "user-1", "name": "John Doe", "email": "john@example.com"}
result = strip_userinfo(userinfo, "roles")
assert result["name"] == "John Doe"
def test_name_from_preferred_username(self) -> None:
"""Test name falls back to 'preferred_username'."""
userinfo = {"sub": "user-1", "preferred_username": "johndoe"}
result = strip_userinfo(userinfo, "roles")
assert result["name"] == "johndoe"
def test_name_from_username(self) -> None:
"""Test name falls back to 'username' (LogTo-style)."""
userinfo = {"sub": "user-1", "username": "johndoe"}
result = strip_userinfo(userinfo, "roles")
assert result["name"] == "johndoe"
def test_name_from_nickname(self) -> None:
"""Test name falls back to 'nickname'."""
userinfo = {"sub": "user-1", "nickname": "johnny"}
result = strip_userinfo(userinfo, "roles")
assert result["name"] == "johnny"
def test_name_priority_order(self) -> None:
"""Test name claim priority: name > preferred_username > username > nickname."""
userinfo = {
"sub": "user-1",
"name": "Full Name",
"preferred_username": "pref",
"username": "user",
"nickname": "nick",
}
result = strip_userinfo(userinfo, "roles")
assert result["name"] == "Full Name"
def test_name_prefers_username_over_nickname(self) -> None:
"""Test username is preferred over nickname when name is absent."""
userinfo = {"sub": "user-1", "username": "logto_user", "nickname": "nick"}
result = strip_userinfo(userinfo, "roles")
assert result["name"] == "logto_user"
def test_name_none_when_all_missing(self) -> None:
"""Test name is None when no name-like claims present."""
userinfo = {"sub": "user-1", "email": "user@example.com"}
result = strip_userinfo(userinfo, "roles")
assert result["name"] is None
def test_roles_extracted(self) -> None:
"""Test roles are extracted from configured claim."""
userinfo = {"sub": "user-1", "custom_roles": ["admin", "member"]}
result = strip_userinfo(userinfo, "custom_roles")
assert result["custom_roles"] == ["admin", "member"]
def test_preserves_sub_email_picture(self) -> None:
"""Test sub, email, and picture are preserved."""
userinfo = {
"sub": "user-1",
"email": "user@example.com",
"picture": "https://example.com/avatar.png",
}
result = strip_userinfo(userinfo, "roles")
assert result["sub"] == "user-1"
assert result["email"] == "user@example.com"
assert result["picture"] == "https://example.com/avatar.png"
class TestInitOidcScopeParsing:
"""Test that init_oidc handles quoted and unquoted scope strings."""
def test_plain_scope_string(self) -> None:
"""Test unquoted scope string is split into list."""
with patch("meshcore_hub.web.oidc.oauth") as mock_oauth:
init_oidc(
"id", "secret", "https://idp.example.com/oidc", "openid email profile"
)
call_kwargs = mock_oauth.register.call_args[1]
assert call_kwargs["client_kwargs"]["scope"] == [
"openid",
"email",
"profile",
]
def test_double_quoted_scope_string(self) -> None:
"""Test double-quoted scope string (from Docker env) is stripped and split."""
with patch("meshcore_hub.web.oidc.oauth") as mock_oauth:
init_oidc(
"id",
"secret",
"https://idp.example.com/oidc",
'"openid email profile"',
)
call_kwargs = mock_oauth.register.call_args[1]
assert call_kwargs["client_kwargs"]["scope"] == [
"openid",
"email",
"profile",
]
def test_single_quoted_scope_string(self) -> None:
"""Test single-quoted scope string is stripped and split."""
with patch("meshcore_hub.web.oidc.oauth") as mock_oauth:
init_oidc(
"id",
"secret",
"https://idp.example.com/oidc",
"'openid email profile'",
)
call_kwargs = mock_oauth.register.call_args[1]
assert call_kwargs["client_kwargs"]["scope"] == [
"openid",
"email",
"profile",
]
def _extract_config(text: str) -> dict[str, Any]:
"""Extract __APP_CONFIG__ from SPA HTML."""
start = text.find("window.__APP_CONFIG__ = ") + len("window.__APP_CONFIG__ = ")