From 767699a0664c13fea1293654578f571ec4a08fa3 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 14 Jul 2024 14:21:16 +0100
Subject: [PATCH] OIDC: Fixed incorrect detection of group detail population

An empty (but valid formed) groups list provided via the OIDC ID token
would be considered as a lacking detail, and therefore trigger a lookup
to the userinfo endpoint in an attempt to get that information.

This fixes this to properly distinguish between not-provided and empty
state, to avoid userinfo where provided as valid but empty.

Includes test to cover.
For #5101
---
 app/Access/Oidc/OidcUserDetails.php |  8 ++++----
 tests/Auth/OidcTest.php             | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/app/Access/Oidc/OidcUserDetails.php b/app/Access/Oidc/OidcUserDetails.php
index bccc49ee4..fae20de0b 100644
--- a/app/Access/Oidc/OidcUserDetails.php
+++ b/app/Access/Oidc/OidcUserDetails.php
@@ -22,7 +22,7 @@ class OidcUserDetails
         $hasEmpty = empty($this->externalId)
             || empty($this->email)
             || empty($this->name)
-            || ($groupSyncActive && empty($this->groups));
+            || ($groupSyncActive && $this->groups === null);
 
         return !$hasEmpty;
     }
@@ -57,15 +57,15 @@ class OidcUserDetails
         return implode(' ', $displayName);
     }
 
-    protected static function getUserGroups(string $groupsClaim, ProvidesClaims $token): array
+    protected static function getUserGroups(string $groupsClaim, ProvidesClaims $token): ?array
     {
         if (empty($groupsClaim)) {
-            return [];
+            return null;
         }
 
         $groupsList = Arr::get($token->getAllClaims(), $groupsClaim);
         if (!is_array($groupsList)) {
-            return [];
+            return null;
         }
 
         return array_values(array_filter($groupsList, function ($val) {
diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php
index 9bde71c80..201f67b53 100644
--- a/tests/Auth/OidcTest.php
+++ b/tests/Auth/OidcTest.php
@@ -849,6 +849,26 @@ class OidcTest extends TestCase
         $this->assertSessionError('Userinfo endpoint response validation failed with error: No valid subject value found in userinfo data');
     }
 
+    public function test_userinfo_endpoint_not_called_if_empty_groups_array_provided_in_id_token()
+    {
+        config()->set([
+            'oidc.user_to_groups'     => true,
+            'oidc.groups_claim'       => 'groups',
+            'oidc.remove_from_groups' => false,
+        ]);
+
+        $this->post('/oidc/login');
+        $state = session()->get('oidc_state');
+        $client = $this->mockHttpClient([$this->getMockAuthorizationResponse([
+            'groups' => [],
+        ])]);
+
+        $resp = $this->get('/oidc/callback?code=SplxlOBeZQQYbYS6WxSbIA&state=' . $state);
+        $resp->assertRedirect('/');
+        $this->assertEquals(1, $client->requestCount());
+        $this->assertTrue(auth()->check());
+    }
+
     protected function withAutodiscovery(): void
     {
         config()->set([