From e2dae991c64a67315f334b630a03eed8eec3a078 Mon Sep 17 00:00:00 2001 From: Dan Webb Date: Mon, 2 Dec 2024 10:43:40 +0000 Subject: [PATCH 1/2] chore: Add OIDC debug logging Adds logging of all received claims Adds validation for required claims that are present but empty Signed-off-by: Dan Webb --- .../security/providers/openid_provider.py | 11 +++++++ .../providers/test_openid_provider.py | 29 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/mealie/core/security/providers/openid_provider.py b/mealie/core/security/providers/openid_provider.py index 5c2a4b87632..f487f9ef414 100644 --- a/mealie/core/security/providers/openid_provider.py +++ b/mealie/core/security/providers/openid_provider.py @@ -27,6 +27,11 @@ def authenticate(self) -> tuple[str, timedelta] | None: self._logger.error("[OIDC] No claims in the id_token") return None + # Log all claims for debugging + self._logger.debug("[OIDC] Received claims:") + for key, value in claims.items(): + self._logger.debug("[OIDC] %s: %s", key, value) + if not self.required_claims.issubset(claims.keys()): self._logger.error( "[OIDC] Required claims not present. Expected: %s Actual: %s", @@ -35,6 +40,12 @@ def authenticate(self) -> tuple[str, timedelta] | None: ) return None + # Check for empty required claims + for claim in self.required_claims: + if not claims.get(claim): + self._logger.error("[OIDC] Required claim '%s' is empty", claim) + return None + repos = get_repositories(self.session, group_id=None, household_id=None) is_admin = False diff --git a/tests/unit_tests/core/security/providers/test_openid_provider.py b/tests/unit_tests/core/security/providers/test_openid_provider.py index 94fca7d9b16..88cfba7baa4 100644 --- a/tests/unit_tests/core/security/providers/test_openid_provider.py +++ b/tests/unit_tests/core/security/providers/test_openid_provider.py @@ -1,5 +1,6 @@ import pytest from pytest import MonkeyPatch, Session +import logging from mealie.core.config import get_app_settings from mealie.core.security.providers.openid_provider import OpenIDProvider @@ -20,6 +21,18 @@ def test_empty_claims(): assert auth_provider.authenticate() is None +def test_empty_required_claims(): + data = { + "preferred_username": "dude1", + "email": "", # Empty required claim + "name": "Firstname Lastname", + "groups": ["mealie_user"], + } + auth_provider = OpenIDProvider(None, data) + + assert auth_provider.authenticate() is None + + def test_missing_claims(): data = {"preferred_username": "dude1"} auth_provider = OpenIDProvider(None, data) @@ -162,3 +175,19 @@ def test_ldap_user_creation_invalid_group_or_household( assert user is not None else: assert user is None + + +def test_claims_logging(caplog): + caplog.set_level(logging.DEBUG) + data = { + "preferred_username": "testuser", + "email": "test@example.com", + "name": "Test User", + "groups": ["mealie_user"], + } + auth_provider = OpenIDProvider(None, data) + auth_provider.authenticate() + + # Verify that all claims are logged + for key, value in data.items(): + assert f"{key}: {value}" in caplog.text From 5c6cb1ac3b844bff2f3a48cdad89eab3e5433e2d Mon Sep 17 00:00:00 2001 From: Dan Webb Date: Mon, 2 Dec 2024 13:47:35 +0000 Subject: [PATCH 2/2] Add session fixture Signed-off-by: Dan Webb --- .../core/security/providers/test_openid_provider.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/core/security/providers/test_openid_provider.py b/tests/unit_tests/core/security/providers/test_openid_provider.py index 88cfba7baa4..7973ed65500 100644 --- a/tests/unit_tests/core/security/providers/test_openid_provider.py +++ b/tests/unit_tests/core/security/providers/test_openid_provider.py @@ -177,7 +177,7 @@ def test_ldap_user_creation_invalid_group_or_household( assert user is None -def test_claims_logging(caplog): +def test_claims_logging(caplog, session: Session): caplog.set_level(logging.DEBUG) data = { "preferred_username": "testuser", @@ -185,7 +185,7 @@ def test_claims_logging(caplog): "name": "Test User", "groups": ["mealie_user"], } - auth_provider = OpenIDProvider(None, data) + auth_provider = OpenIDProvider(session, data) auth_provider.authenticate() # Verify that all claims are logged