Fix OIDC infinite loop if user is not in OIDC_USER_GROUP (#3487)

Author: cmintey

frontend/pages/login.vue

@@ -201,7 +201,7 @@ export default defineComponent({
     }
 
     function isDirectLogin() {
-        return router.currentRoute.query.direct
+        return Object.keys(router.currentRoute.query).includes("direct")
     }
 
     async function oidcAuthenticate() {

frontend/schemes/DynamicOpenIDConnectScheme.js

@@ -16,7 +16,6 @@ export default class DynamicOpenIDConnectScheme extends OpenIDConnectScheme {
             this.$auth.$storage
         )
 
-
         // eslint-disable-next-line @typescript-eslint/no-unsafe-return
         return await super.mounted()
     }
@@ -79,6 +78,7 @@ export default class DynamicOpenIDConnectScheme extends OpenIDConnectScheme {
         // Update tokens with mealie token
         this.updateTokens(response)
       } catch {
+        this.$auth.reset()
         const currentUrl = new URL(window.location.href)
         if (currentUrl.pathname === "/login" && currentUrl.searchParams.has("direct")) {
           return

mealie/core/security/providers/openid_provider.py

@@ -35,17 +35,19 @@ async def authenticate(self) -> tuple[str, timedelta] | None:
         repos = get_repositories(self.session)
 
         user = self.try_get_user(claims.get(settings.OIDC_USER_CLAIM))
-        group_claim = claims.get("groups", [])
-        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
-
-        if not is_valid_user:
-            self._logger.debug(
-                "[OIDC] User does not have the required group. Found: %s - Required: %s",
-                group_claim,
-                settings.OIDC_USER_GROUP,
-            )
-            return None
+        is_admin = False
+        if settings.OIDC_USER_GROUP or settings.OIDC_ADMIN_GROUP:
+            group_claim = claims.get("groups", [])
+            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
+
+            if not is_valid_user:
+                self._logger.debug(
+                    "[OIDC] User does not have the required group. Found: %s - Required: %s",
+                    group_claim,
+                    settings.OIDC_USER_GROUP,
+                )
+                return None
 
         if not user:
             if not settings.OIDC_SIGNUP_ENABLED:
@@ -68,7 +70,7 @@ async def authenticate(self) -> tuple[str, timedelta] | None:
             return self.get_access_token(user, settings.OIDC_REMEMBER_ME)  # type: ignore
 
         if user:
-            if user.admin != is_admin:
+            if settings.OIDC_ADMIN_GROUP and user.admin != is_admin:
                 self._logger.debug(f"[OIDC] {'Setting' if is_admin else 'Removing'} user as admin")
                 user.admin = is_admin
                 repos.users.update(user.id, user)
@@ -91,6 +93,7 @@ def get_claims(self, settings: AppSettings) -> JWTClaims | None:
             self._logger.error(
                 f"[OIDC] Unsupported algorithm '{algorithm}'. Unable to decode id token due to mismatched algorithm."
             )
+            return None
 
         try:
             claims.validate()

tests/e2e/docker/docker-compose.yml

@@ -30,6 +30,7 @@ services:
 
       OIDC_AUTH_ENABLED: True
       OIDC_SIGNUP_ENABLED: True
+      OIDC_USER_GROUP: user
       OIDC_ADMIN_GROUP: admin
       OIDC_CONFIGURATION_URL: http://localhost:8080/default/.well-known/openid-configuration
       OIDC_CLIENT_ID: default

tests/e2e/login.spec.ts

@@ -55,7 +55,8 @@ test('oidc initial login', async ({ page }) => {
         "sub": username,
         "email": `${username}@example.com`,
         "preferred_username": username,
-        "name": name
+        "name": name,
+        "groups": ["user"]
     }
 
     await page.goto('http://localhost:9000/login');
@@ -67,14 +68,35 @@ test('oidc initial login', async ({ page }) => {
     await expect(page.getByRole('link', { name: 'Settings' })).not.toBeVisible();
 });
 
+test('oidc login with user not in propery group', async ({ page }) => {
+    const username = "testUserNoGroup"
+    const name = "Test User No Group"
+    const claims = {
+        "sub": username,
+        "email": `${username}@example.com`,
+        "preferred_username": username,
+        "name": name,
+        "groups": []
+    }
+
+    await page.goto('http://localhost:9000/login');
+    await page.getByRole('button', { name: 'Login with OAuth' }).click();
+    await page.getByPlaceholder('Enter any user/subject').fill(username);
+    await page.getByPlaceholder('Optional claims JSON value,').fill(JSON.stringify(claims));
+    await page.getByRole('button', { name: 'Sign-in' }).click();
+    await expect(page).toHaveURL(/.*\/login\/?\?direct=1/)
+    await expect(page.getByRole('button', { name: 'Login with OAuth' })).toBeVisible()
+});
+
 test('oidc sequential login', async ({ page }) => {
     const username = "testUser2"
     const name = "Test User 2"
     const claims = {
         "sub": username,
         "email": `${username}@example.com`,
         "preferred_username": username,
-        "name": name
+        "name": name,
+        "groups": ["user"]
     }
 
     await page.goto('http://localhost:9000/login');
@@ -100,7 +122,8 @@ test('settings page verify oidc', async ({ page }) => {
         "sub": username,
         "email": `${username}@example.com`,
         "preferred_username": username,
-        "name": name
+        "name": name,
+        "groups": ["user"]
     }
 
     await page.goto('http://localhost:9000/login');
@@ -133,7 +156,7 @@ test('oidc admin user', async ({ page }) => {
         "email": `${username}@example.com`,
         "preferred_username": username,
         "name": name,
-        "groups": ["admin"]
+        "groups": ["user", "admin"]
     }
 
     await page.goto('http://localhost:9000/login');