mirror of
				https://github.com/mealie-recipes/mealie.git
				synced 2025-10-31 02:03:35 -04:00 
			
		
		
		
	fix: Handle missing OIDC groups claim (#6054)
This commit is contained in:
		| @@ -16,8 +16,9 @@ class OpenIDProvider(AuthProvider[UserInfo]): | |||||||
|  |  | ||||||
|     _logger = root_logger.get_logger("openid_provider") |     _logger = root_logger.get_logger("openid_provider") | ||||||
|  |  | ||||||
|     def __init__(self, session: Session, data: UserInfo) -> None: |     def __init__(self, session: Session, data: UserInfo, use_default_groups: bool = False) -> None: | ||||||
|         super().__init__(session, data) |         super().__init__(session, data) | ||||||
|  |         self.use_default_groups = use_default_groups | ||||||
|  |  | ||||||
|     def authenticate(self) -> tuple[str, timedelta] | None: |     def authenticate(self) -> tuple[str, timedelta] | None: | ||||||
|         """Attempt to authenticate a user given a username and password""" |         """Attempt to authenticate a user given a username and password""" | ||||||
| @@ -51,6 +52,15 @@ class OpenIDProvider(AuthProvider[UserInfo]): | |||||||
|  |  | ||||||
|         is_admin = False |         is_admin = False | ||||||
|         if settings.OIDC_REQUIRES_GROUP_CLAIM: |         if settings.OIDC_REQUIRES_GROUP_CLAIM: | ||||||
|  |             # We explicitly allow the groups claim to be missing to account for the behaviour of some IdPs: | ||||||
|  |             # https://github.com/keycloak/keycloak/issues/22340 | ||||||
|  |             # We still log a warning though | ||||||
|  |             if settings.OIDC_GROUPS_CLAIM not in claims: | ||||||
|  |                 self._logger.warning( | ||||||
|  |                     "[OIDC] claims did not include a %s claim%s", | ||||||
|  |                     settings.OIDC_GROUPS_CLAIM, | ||||||
|  |                     ", using an empty list as default" if self.use_default_groups else "", | ||||||
|  |                 ) | ||||||
|             group_claim = claims.get(settings.OIDC_GROUPS_CLAIM, []) or [] |             group_claim = claims.get(settings.OIDC_GROUPS_CLAIM, []) or [] | ||||||
|             is_admin = settings.OIDC_ADMIN_GROUP in group_claim if settings.OIDC_ADMIN_GROUP else False |             is_admin = settings.OIDC_ADMIN_GROUP in group_claim if settings.OIDC_ADMIN_GROUP else False | ||||||
|             is_valid_user = settings.OIDC_USER_GROUP in group_claim if settings.OIDC_USER_GROUP else True |             is_valid_user = settings.OIDC_USER_GROUP in group_claim if settings.OIDC_USER_GROUP else True | ||||||
| @@ -111,6 +121,6 @@ class OpenIDProvider(AuthProvider[UserInfo]): | |||||||
|         settings = get_app_settings() |         settings = get_app_settings() | ||||||
|  |  | ||||||
|         claims = {settings.OIDC_NAME_CLAIM, "email", settings.OIDC_USER_CLAIM} |         claims = {settings.OIDC_NAME_CLAIM, "email", settings.OIDC_USER_CLAIM} | ||||||
|         if settings.OIDC_REQUIRES_GROUP_CLAIM: |         if settings.OIDC_REQUIRES_GROUP_CLAIM and not self.use_default_groups: | ||||||
|             claims.add(settings.OIDC_GROUPS_CLAIM) |             claims.add(settings.OIDC_GROUPS_CLAIM) | ||||||
|         return claims |         return claims | ||||||
|   | |||||||
| @@ -138,7 +138,7 @@ async def oauth_callback(request: Request, response: Response, session: Session | |||||||
|         try: |         try: | ||||||
|             logger.debug("[OIDC] Claims not present in the ID token, pulling user info") |             logger.debug("[OIDC] Claims not present in the ID token, pulling user info") | ||||||
|             userinfo = await client.userinfo(token=token) |             userinfo = await client.userinfo(token=token) | ||||||
|             auth_provider = OpenIDProvider(session, userinfo) |             auth_provider = OpenIDProvider(session, userinfo, use_default_groups=True) | ||||||
|             auth = auth_provider.authenticate() |             auth = auth_provider.authenticate() | ||||||
|         except MissingClaimException: |         except MissingClaimException: | ||||||
|             auth = None |             auth = None | ||||||
|   | |||||||
| @@ -61,6 +61,49 @@ def test_missing_groups_claim(monkeypatch: MonkeyPatch): | |||||||
|         auth_provider.authenticate() |         auth_provider.authenticate() | ||||||
|  |  | ||||||
|  |  | ||||||
|  | def test_missing_groups_claim_admin(monkeypatch: MonkeyPatch): | ||||||
|  |     monkeypatch.setenv("OIDC_ADMIN_GROUP", "mealie_admin") | ||||||
|  |     get_app_settings.cache_clear() | ||||||
|  |  | ||||||
|  |     data = { | ||||||
|  |         "preferred_username": "dude1", | ||||||
|  |         "email": "email@email.com", | ||||||
|  |         "name": "Firstname Lastname", | ||||||
|  |     } | ||||||
|  |     auth_provider = OpenIDProvider(None, data) | ||||||
|  |  | ||||||
|  |     with pytest.raises(MissingClaimException): | ||||||
|  |         auth_provider.authenticate() | ||||||
|  |  | ||||||
|  |  | ||||||
|  | def test_missing_groups_claim_with_default(monkeypatch: MonkeyPatch): | ||||||
|  |     monkeypatch.setenv("OIDC_USER_GROUP", "mealie_user") | ||||||
|  |     get_app_settings.cache_clear() | ||||||
|  |  | ||||||
|  |     data = { | ||||||
|  |         "preferred_username": "dude1", | ||||||
|  |         "email": "email@email.com", | ||||||
|  |         "name": "Firstname Lastname", | ||||||
|  |     } | ||||||
|  |     auth_provider = OpenIDProvider(None, data, True) | ||||||
|  |  | ||||||
|  |     assert auth_provider.authenticate() is None | ||||||
|  |  | ||||||
|  |  | ||||||
|  | def test_missing_groups_claim_admin_group_with_default(monkeypatch: MonkeyPatch, unique_user: TestUser): | ||||||
|  |     monkeypatch.setenv("OIDC_ADMIN_GROUP", "mealie_admin") | ||||||
|  |     get_app_settings.cache_clear() | ||||||
|  |  | ||||||
|  |     data = { | ||||||
|  |         "preferred_username": "dude1", | ||||||
|  |         "email": unique_user.email, | ||||||
|  |         "name": "Firstname Lastname", | ||||||
|  |     } | ||||||
|  |     auth_provider = OpenIDProvider(unique_user.repos.session, data, True) | ||||||
|  |  | ||||||
|  |     assert auth_provider.authenticate() is not None | ||||||
|  |  | ||||||
|  |  | ||||||
| def test_missing_user_group(monkeypatch: MonkeyPatch): | def test_missing_user_group(monkeypatch: MonkeyPatch): | ||||||
|     monkeypatch.setenv("OIDC_USER_GROUP", "mealie_user") |     monkeypatch.setenv("OIDC_USER_GROUP", "mealie_user") | ||||||
|     get_app_settings.cache_clear() |     get_app_settings.cache_clear() | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user