Skip to content

Commit 767699a

Browse files
committed
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
1 parent 7161f22 commit 767699a

File tree

2 files changed

+24
-4
lines changed

2 files changed

+24
-4
lines changed

app/Access/Oidc/OidcUserDetails.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public function isFullyPopulated(bool $groupSyncActive): bool
2222
$hasEmpty = empty($this->externalId)
2323
|| empty($this->email)
2424
|| empty($this->name)
25-
|| ($groupSyncActive && empty($this->groups));
25+
|| ($groupSyncActive && $this->groups === null);
2626

2727
return !$hasEmpty;
2828
}
@@ -57,15 +57,15 @@ protected static function getUserDisplayName(string $displayNameClaims, Provides
5757
return implode(' ', $displayName);
5858
}
5959

60-
protected static function getUserGroups(string $groupsClaim, ProvidesClaims $token): array
60+
protected static function getUserGroups(string $groupsClaim, ProvidesClaims $token): ?array
6161
{
6262
if (empty($groupsClaim)) {
63-
return [];
63+
return null;
6464
}
6565

6666
$groupsList = Arr::get($token->getAllClaims(), $groupsClaim);
6767
if (!is_array($groupsList)) {
68-
return [];
68+
return null;
6969
}
7070

7171
return array_values(array_filter($groupsList, function ($val) {

tests/Auth/OidcTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,26 @@ public function test_userinfo_endpoint_response_with_invalid_content_type_throws
849849
$this->assertSessionError('Userinfo endpoint response validation failed with error: No valid subject value found in userinfo data');
850850
}
851851

852+
public function test_userinfo_endpoint_not_called_if_empty_groups_array_provided_in_id_token()
853+
{
854+
config()->set([
855+
'oidc.user_to_groups' => true,
856+
'oidc.groups_claim' => 'groups',
857+
'oidc.remove_from_groups' => false,
858+
]);
859+
860+
$this->post('/oidc/login');
861+
$state = session()->get('oidc_state');
862+
$client = $this->mockHttpClient([$this->getMockAuthorizationResponse([
863+
'groups' => [],
864+
])]);
865+
866+
$resp = $this->get('/oidc/callback?code=SplxlOBeZQQYbYS6WxSbIA&state=' . $state);
867+
$resp->assertRedirect('/');
868+
$this->assertEquals(1, $client->requestCount());
869+
$this->assertTrue(auth()->check());
870+
}
871+
852872
protected function withAutodiscovery(): void
853873
{
854874
config()->set([

0 commit comments

Comments
 (0)