Skip to content

Commit 7fd795e

Browse files
ssddanbrownssddanbrown
committed
OIDC: Extracted user detail handling to own OidcUserDetails class
Allows a proper defined object instead of an array an extracts related logic out of OidcService. Updated userinfo to only be called if we're missing details.
1 parent 9183e7f commit 7fd795e

File tree

2 files changed

+114
-91
lines changed

2 files changed

+114
-91
lines changed

‎app/Access/Oidc/OidcService.php‎

Lines changed: 31 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -159,69 +159,6 @@ protected function getAdditionalScopes(): array
159159
return array_filter($scopeArr);
160160
}
161161

162-
/**
163-
* Calculate the display name.
164-
*/
165-
protected function getUserDisplayName(OidcIdToken $token, string $defaultValue): string
166-
{
167-
$displayNameAttrString = $this->config()['display_name_claims'] ?? '';
168-
$displayNameAttrs = explode('|', $displayNameAttrString);
169-
170-
$displayName = [];
171-
foreach ($displayNameAttrs as $dnAttr) {
172-
$dnComponent = $token->getClaim($dnAttr) ?? '';
173-
if ($dnComponent !== '') {
174-
$displayName[] = $dnComponent;
175-
}
176-
}
177-
178-
if (count($displayName) == 0) {
179-
$displayName[] = $defaultValue;
180-
}
181-
182-
return implode(' ', $displayName);
183-
}
184-
185-
/**
186-
* Extract the assigned groups from the id token.
187-
*
188-
* @return string[]
189-
*/
190-
protected function getUserGroups(OidcIdToken $token): array
191-
{
192-
$groupsAttr = $this->config()['groups_claim'];
193-
if (empty($groupsAttr)) {
194-
return [];
195-
}
196-
197-
$groupsList = Arr::get($token->getAllClaims(), $groupsAttr);
198-
if (!is_array($groupsList)) {
199-
return [];
200-
}
201-
202-
return array_values(array_filter($groupsList, function ($val) {
203-
return is_string($val);
204-
}));
205-
}
206-
207-
/**
208-
* Extract the details of a user from an ID token.
209-
*
210-
* @return array{name: string, email: string, external_id: string, groups: string[]}
211-
*/
212-
protected function getUserDetails(OidcIdToken $token): array
213-
{
214-
$idClaim = $this->config()['external_id_claim'];
215-
$id = $token->getClaim($idClaim);
216-
217-
return [
218-
'external_id' => $id,
219-
'email' => $token->getClaim('email'),
220-
'name' => $this->getUserDisplayName($token, $id),
221-
'groups' => $this->getUserGroups($token),
222-
];
223-
}
224-
225162
/**
226163
* Processes a received access token for a user. Login the user when
227164
* they exist, optionally registering them automatically.
@@ -241,26 +178,6 @@ protected function processAccessTokenCallback(OidcAccessToken $accessToken, Oidc
241178

242179
session()->put("oidc_id_token", $idTokenText);
243180

244-
// TODO - This should not affect id token validation
245-
// TODO - Should only call if we're missing properties
246-
if (!empty($settings->userinfoEndpoint)) {
247-
$provider = $this->getProvider($settings);
248-
$request = $provider->getAuthenticatedRequest('GET', $settings->userinfoEndpoint, $accessToken->getToken());
249-
$response = $provider->getParsedResponse($request);
250-
// TODO - Ensure response content-type is "application/json" before using in this way (5.3.2)
251-
// TODO - The sub Claim in the UserInfo Response MUST be verified to exactly match the sub Claim in the ID Token; if they do not match, the UserInfo Response values MUST NOT be used. (5.3.2)
252-
// TODO - Response validation (5.3.4)
253-
// TODO - Verify that the OP that responded was the intended OP through a TLS server certificate check, per RFC 6125 [RFC6125].
254-
// TODO - If the Client has provided a userinfo_encrypted_response_alg parameter during Registration, decrypt the UserInfo Response using the keys specified during Registration.
255-
// TODO - If the response was signed, the Client SHOULD validate the signature according to JWS [JWS].
256-
$claims = $idToken->getAllClaims();
257-
foreach ($response as $key => $value) {
258-
$claims[$key] = $value;
259-
}
260-
// TODO - Should maybe remain separate from IdToken completely
261-
$idToken->replaceClaims($claims);
262-
}
263-
264181
$returnClaims = Theme::dispatch(ThemeEvents::OIDC_ID_TOKEN_PRE_VALIDATE, $idToken->getAllClaims(), [
265182
'access_token' => $accessToken->getToken(),
266183
'expires_in' => $accessToken->getExpires(),
@@ -281,31 +198,54 @@ protected function processAccessTokenCallback(OidcAccessToken $accessToken, Oidc
281198
throw new OidcException("ID token validate failed with error: {$exception->getMessage()}");
282199
}
283200

284-
$userDetails = $this->getUserDetails($idToken);
285-
$isLoggedIn = auth()->check();
201+
$userDetails = OidcUserDetails::fromToken(
202+
$idToken,
203+
$this->config()['external_id_claim'],
204+
$this->config()['display_name_claims'] ?? '',
205+
$this->config()['groups_claim'] ?? ''
206+
);
286207

287-
if (empty($userDetails['email'])) {
208+
// TODO - This should not affect id token validation
209+
if (!$userDetails->isFullyPopulated($this->shouldSyncGroups()) && !empty($settings->userinfoEndpoint)) {
210+
$provider = $this->getProvider($settings);
211+
$request = $provider->getAuthenticatedRequest('GET', $settings->userinfoEndpoint, $accessToken->getToken());
212+
$response = $provider->getParsedResponse($request);
213+
// TODO - Ensure response content-type is "application/json" before using in this way (5.3.2)
214+
// TODO - The sub Claim in the UserInfo Response MUST be verified to exactly match the sub Claim in the ID Token; if they do not match, the UserInfo Response values MUST NOT be used. (5.3.2)
215+
// TODO - Response validation (5.3.4)
216+
// TODO - Verify that the OP that responded was the intended OP through a TLS server certificate check, per RFC 6125 [RFC6125].
217+
// TODO - If the Client has provided a userinfo_encrypted_response_alg parameter during Registration, decrypt the UserInfo Response using the keys specified during Registration.
218+
// TODO - If the response was signed, the Client SHOULD validate the signature according to JWS [JWS].
219+
$claims = $idToken->getAllClaims();
220+
foreach ($response as $key => $value) {
221+
$claims[$key] = $value;
222+
}
223+
// TODO - Should maybe remain separate from IdToken completely
224+
$idToken->replaceClaims($claims);
225+
}
226+
227+
if (empty($userDetails->email)) {
288228
throw new OidcException(trans('errors.oidc_no_email_address'));
289229
}
290230

231+
$isLoggedIn = auth()->check();
291232
if ($isLoggedIn) {
292233
throw new OidcException(trans('errors.oidc_already_logged_in'));
293234
}
294235

295236
try {
296237
$user = $this->registrationService->findOrRegister(
297-
$userDetails['name'],
298-
$userDetails['email'],
299-
$userDetails['external_id']
238+
$userDetails->name,
239+
$userDetails->email,
240+
$userDetails->externalId
300241
);
301242
} catch (UserRegistrationException $exception) {
302243
throw new OidcException($exception->getMessage());
303244
}
304245

305246
if ($this->shouldSyncGroups()) {
306-
$groups = $userDetails['groups'];
307247
$detachExisting = $this->config()['remove_from_groups'];
308-
$this->groupService->syncUserWithFoundGroups($user, $groups, $detachExisting);
248+
$this->groupService->syncUserWithFoundGroups($user, $userDetails->groups ?? [], $detachExisting);
309249
}
310250

311251
$this->loginService->login($user, 'oidc');

‎app/Access/Oidc/OidcUserDetails.php‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
<?php
2+
3+
namespace BookStack\Access\Oidc;
4+
5+
use Illuminate\Support\Arr;
6+
7+
class OidcUserDetails
8+
{
9+
public function __construct(
10+
public ?string $externalId = null,
11+
public ?string $email = null,
12+
public ?string $name = null,
13+
public ?array $groups = null,
14+
) {
15+
}
16+
17+
/**
18+
* Check if the user details are fully populated for our usage.
19+
*/
20+
public function isFullyPopulated(bool $groupSyncActive): bool
21+
{
22+
$hasEmpty = empty($this->externalId)
23+
|| empty($this->email)
24+
|| empty($this->name)
25+
|| ($groupSyncActive && empty($this->groups));
26+
27+
return !$hasEmpty;
28+
}
29+
30+
/**
31+
* Populate user details from OidcIdToken data.
32+
*/
33+
public static function fromToken(
34+
OidcIdToken $token,
35+
string $idClaim,
36+
string $displayNameClaims,
37+
string $groupsClaim,
38+
): static {
39+
$id = $token->getClaim($idClaim);
40+
41+
return new self(
42+
externalId: $id,
43+
email: $token->getClaim('email'),
44+
name: static::getUserDisplayName($displayNameClaims, $token, $id),
45+
groups: static::getUserGroups($groupsClaim, $token),
46+
);
47+
}
48+
49+
protected static function getUserDisplayName(string $displayNameClaims, OidcIdToken $token, string $defaultValue): string
50+
{
51+
$displayNameClaimParts = explode('|', $displayNameClaims);
52+
53+
$displayName = [];
54+
foreach ($displayNameClaimParts as $claim) {
55+
$component = $token->getClaim(trim($claim)) ?? '';
56+
if ($component !== '') {
57+
$displayName[] = $component;
58+
}
59+
}
60+
61+
if (count($displayName) === 0) {
62+
$displayName[] = $defaultValue;
63+
}
64+
65+
return implode(' ', $displayName);
66+
}
67+
68+
protected static function getUserGroups(string $groupsClaim, OidcIdToken $token): array
69+
{
70+
if (empty($groupsClaim)) {
71+
return [];
72+
}
73+
74+
$groupsList = Arr::get($token->getAllClaims(), $groupsClaim);
75+
if (!is_array($groupsList)) {
76+
return [];
77+
}
78+
79+
return array_values(array_filter($groupsList, function ($val) {
80+
return is_string($val);
81+
}));
82+
}
83+
}

0 commit comments

Comments
 (0)