colin
/
bookstack
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

OIDC: Updated picture fetch implementation during review 

Review of #5429
This commit is contained in:
Dan Brown 2025-05-24 14:02:37 +01:00
parent 05f7f4cb17
commit f9dbbe5d70
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
4 changed files with 38 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
app/Access/Oidc/OidcService.php
View File

@ -222,6 +222,10 @@ class OidcService
throw new OidcException($exception->getMessage());
}
if ($this->config()['fetch_avatar'] && $user->wasRecentlyCreated && $userDetails->picture) {
$this->userAvatars->assignToUserFromUrl($user, $userDetails->picture);
}
if ($this->shouldSyncGroups()) {
$detachExisting = $this->config()['remove_from_groups'];
$this->groupService->syncUserWithFoundGroups($user, $userDetails->groups ?? [], $detachExisting);
@ -229,10 +233,6 @@ class OidcService
$this->loginService->login($user, 'oidc');
if ($this->config()['fetch_avatars'] && $userDetails->picture) {
$this->userAvatars->assignToUserFromUrl($user, $userDetails->picture, $accessToken->getToken());
}
return $user;
}

20
app/Access/Oidc/OidcUserDetails.php
View File

@ -41,16 +41,16 @@ class OidcUserDetails
$this->email = $claims->getClaim('email') ?? $this->email;
$this->name = static::getUserDisplayName($displayNameClaims, $claims) ?? $this->name;
$this->groups = static::getUserGroups($groupsClaim, $claims) ?? $this->groups;
$this->picture = $claims->getClaim('picture') ?: $this->picture;
$this->picture = static::getPicture($claims) ?: $this->picture;
}
protected static function getUserDisplayName(string $displayNameClaims, ProvidesClaims $token): string
protected static function getUserDisplayName(string $displayNameClaims, ProvidesClaims $claims): string
{
$displayNameClaimParts = explode('|', $displayNameClaims);
$displayName = [];
foreach ($displayNameClaimParts as $claim) {
$component = $token->getClaim(trim($claim)) ?? '';
$component = $claims->getClaim(trim($claim)) ?? '';
if ($component !== '') {
$displayName[] = $component;
}
@ -59,13 +59,13 @@ class OidcUserDetails
return implode(' ', $displayName);
}
protected static function getUserGroups(string $groupsClaim, ProvidesClaims $token): ?array
protected static function getUserGroups(string $groupsClaim, ProvidesClaims $claims): ?array
{
if (empty($groupsClaim)) {
return null;
}
$groupsList = Arr::get($token->getAllClaims(), $groupsClaim);
$groupsList = Arr::get($claims->getAllClaims(), $groupsClaim);
if (!is_array($groupsList)) {
return null;
}
@ -74,4 +74,14 @@ class OidcUserDetails
return is_string($val);
}));
}
protected static function getPicture(ProvidesClaims $claims): ?string
{
$picture = $claims->getClaim('picture');
if (is_string($picture) && str_starts_with($picture, 'http')) {
return $picture;
}
return null;
}
}

8
app/Config/oidc.php
View File

@ -47,6 +47,11 @@ return [
// Multiple values can be provided comma seperated.
'additional_scopes' => env('OIDC_ADDITIONAL_SCOPES', null),
// Enable fetching of the user's avatar from the 'picture' claim on initial login.
// This can be a security risk due to performing server-side fetching of data from external URLs.
// Only enable if you trust the OIDC auth provider to provide safe URLs for user images.
'fetch_avatar' => env('OIDC_FETCH_AVATAR', false),
// Group sync options
// Enable syncing, upon login, of OIDC groups to BookStack roles
'user_to_groups' => env('OIDC_USER_TO_GROUPS', false),
@ -54,7 +59,4 @@ return [
'groups_claim' => env('OIDC_GROUPS_CLAIM', 'groups'),
// When syncing groups, remove any groups that no longer match. Otherwise, sync only adds new groups.
'remove_from_groups' => env('OIDC_REMOVE_FROM_GROUPS', false),
// When enabled, BookStack will fetch the user’s avatar from the 'picture' claim (SSRF risk if URLs are untrusted).
'fetch_avatars' => env('OIDC_FETCH_AVATARS', false),
];

30
app/Uploads/UserAvatars.php
View File

@ -5,6 +5,7 @@ namespace BookStack\Uploads;
use BookStack\Exceptions\HttpFetchException;
use BookStack\Http\HttpRequestService;
use BookStack\Users\Models\User;
use BookStack\Util\WebSafeMimeSniffer;
use Exception;
use GuzzleHttp\Psr7\Request;
use Illuminate\Support\Facades\Log;
@ -56,17 +57,19 @@ class UserAvatars
/**
* Assign a new avatar image to the given user by fetching from a remote URL.
*/
public function assignToUserFromUrl(User $user, string $avatarUrl, ?string $accessToken = null): void
public function assignToUserFromUrl(User $user, string $avatarUrl): void
{
// Quickly skip invalid or non-HTTP URLs
if (!$avatarUrl || !str_starts_with($avatarUrl, 'http')) {
return;
}
try {
$this->destroyAllForUser($user);
$imageData = $this->getAvatarImageData($avatarUrl, $accessToken);
$avatar = $this->createAvatarImageFromData($user, $imageData, 'png');
$imageData = $this->getAvatarImageData($avatarUrl);
$mime = (new WebSafeMimeSniffer())->sniff($imageData);
[$format, $type] = explode('/', $mime, 2);
if ($format !== 'image' || ImageService::isExtensionSupported($type)) {
return;
}
$avatar = $this->createAvatarImageFromData($user, $imageData, $type);
$user->avatar()->associate($avatar);
$user->save();
} catch (Exception $e) {
@ -130,20 +133,15 @@ class UserAvatars
}
/**
* Gets an image from a URL (public or private) and returns it as a string of image data.
* Get an image from a URL and return it as a string of image data.
*
* @throws HttpFetchException
*/
protected function getAvatarImageData(string $url, ?string $accessToken = null): string
protected function getAvatarImageData(string $url): string
{
try {
$headers = [];
if (!empty($accessToken)) {
$headers['Authorization'] = 'Bearer ' . $accessToken;
}
$client = $this->http->buildClient(5);
$response = $client->sendRequest(new Request('GET', $url, $headers));
$response = $client->sendRequest(new Request('GET', $url));
if ($response->getStatusCode() !== 200) {
throw new HttpFetchException(trans('errors.cannot_get_image_from_url', ['url' => $url]));