Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LDAP: Error thrown when no CN provided by LDAP server #5443

Closed
dofeldsc opened this issue Jan 24, 2025 · 3 comments
Closed

LDAP: Error thrown when no CN provided by LDAP server #5443

dofeldsc opened this issue Jan 24, 2025 · 3 comments

Comments

@dofeldsc
Copy link

Describe the Bug

Null default value causes exception in App/Access
/LdapService.php on login.

If this is done with anonymous bind and cn is not accessible, argument 3 (userCn) for getUserDisplayName is null, which is not allowed.

$userCn = $this->getUserResponseProperty($user, 'cn', null);
        $formatted = [
            'uid'   => $this->getUserResponseProperty($user, $idAttr, $user['dn']),
            'name'  => $this->getUserDisplayName($user, $displayNameAttrs, $userCn),
            'dn'    => $user['dn'],
            'email' => $this->getUserResponseProperty($user, $emailAttr, null),
            'avatar' => $thumbnailAttr ? $this->getUserResponseProperty($user, $thumbnailAttr, null) : null,
        ];

This change came with Version 24.12, before that login with this config worked fine.
Not a breaking change, but thught you should know :)

Best regards and thanks for your work!

Steps to Reproduce

  1. LDAP-Config without LDAP_DN/LDAP_PASS
  2. LDAP where cn is not readable with anonymous
  3. Try to Login

Expected Behaviour

Login

Screenshots or Additional Context

No response

Browser Details

No response

Exact BookStack Version

24.12.1

@thereiam
Copy link

thereiam commented Feb 14, 2025

Hi,
I had the same issue on v24.12.1.

I had to change https://github.com/BookStackApp/BookStack/blob/release/app/Access/LdapService.php#L108

from

'name'  => $this->getUserDisplayName($user, $displayNameAttrs, $userCn),

to

'name'  => $this->getUserDisplayName($user, $displayNameAttrs, $idAttr),

because there seems to be a bug

with loading this var from .env https://github.com/BookStackApp/BookStack/blob/release/app/Config/services.php#L130

However in my case it seems unrelated to how ldap binding is done (anon or not).

Below is the error with the original code:

BookStack\Access\LdapService::getUserDisplayName(): Argument #3 ($defaultValue) must be of type string, null given, called in /var/www/bookstack/app/Access/LdapService.php on line 118

Stacktrace:

#0 /var/www/bookstack/app/Access/LdapService.php(118): BookStack\Access\LdapService->getUserDisplayName()
#1 /var/www/bookstack/app/Access/Guards/LdapSessionGuard.php(72): BookStack\Access\LdapService->getUserDetails()
#2 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Auth/AuthManager.php(340): BookStack\Access\Guards\LdapSessionGuard->attempt()
#3 /var/www/bookstack/app/Access/LoginService.php(164): Illuminate\Auth\AuthManager->__call()
#4 /var/www/bookstack/app/Access/Controllers/LoginController.php(137): BookStack\Access\LoginService->attempt()
#5 /var/www/bookstack/app/Access/Controllers/LoginController.php(74): BookStack\Access\Controllers\LoginController->attemptLogin()
#6 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Routing/Controller.php(54): BookStack\Access\Controllers\LoginController->login()
#7 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(43): Illuminate\Routing\Controller->callAction()
#8 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Routing/Route.php(260): Illuminate\Routing\ControllerDispatcher->dispatch()
#9 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Routing/Route.php(205): Illuminate\Routing\Route->runController()
#10 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Routing/Router.php(806): Illuminate\Routing\Route->run()
#11 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(144): Illuminate\Routing\Router->Illuminate\Routing\{closure}()
#12 /var/www/bookstack/app/Http/Middleware/CheckGuard.php(27): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#13 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): BookStack\Http\Middleware\CheckGuard->handle()
#14 /var/www/bookstack/app/Http/Middleware/RedirectIfAuthenticated.php(28): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#15 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): BookStack\Http\Middleware\RedirectIfAuthenticated->handle()
#16 /var/www/bookstack/app/Http/Middleware/Localization.php(32): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#17 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): BookStack\Http\Middleware\Localization->handle()
#18 /var/www/bookstack/app/Http/Middleware/RunThemeActions.php(26): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#19 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): BookStack\Http\Middleware\RunThemeActions->handle()
#20 /var/www/bookstack/app/Http/Middleware/CheckEmailConfirmed.php(47): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#21 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): BookStack\Http\Middleware\CheckEmailConfirmed->handle()
#22 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/VerifyCsrfToken.php(78): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#23 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Foundation\Http\Middleware\VerifyCsrfToken->handle()
#24 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/View/Middleware/ShareErrorsFromSession.php(49): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#25 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\View\Middleware\ShareErrorsFromSession->handle()
#26 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Session/Middleware/StartSession.php(121): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#27 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Session/Middleware/StartSession.php(64): Illuminate\Session\Middleware\StartSession->handleStatefulRequest()
#28 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Session\Middleware\StartSession->handle()
#29 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Cookie/Middleware/AddQueuedCookiesToResponse.php(37): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#30 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse->handle()
#31 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Cookie/Middleware/EncryptCookies.php(67): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#32 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Cookie\Middleware\EncryptCookies->handle()
#33 /var/www/bookstack/app/Http/Middleware/ApplyCspRules.php(33): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#34 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): BookStack\Http\Middleware\ApplyCspRules->handle()
#35 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(119): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#36 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Routing/Router.php(807): Illuminate\Pipeline\Pipeline->then()
#37 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Routing/Router.php(784): Illuminate\Routing\Router->runRouteWithinStack()
#38 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Routing/Router.php(748): Illuminate\Routing\Router->runRoute()
#39 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Routing/Router.php(737): Illuminate\Routing\Router->dispatchToRoute()
#40 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(200): Illuminate\Routing\Router->dispatch()
#41 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(144): Illuminate\Foundation\Http\Kernel->Illuminate\Foundation\Http\{closure}()
#42 /var/www/bookstack/app/Http/Middleware/PreventResponseCaching.php(21): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#43 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): BookStack\Http\Middleware\PreventResponseCaching->handle()
#44 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Http/Middleware/TrustProxies.php(39): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#45 /var/www/bookstack/app/Http/Middleware/TrustProxies.php(41): Illuminate\Http\Middleware\TrustProxies->handle()
#46 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): BookStack\Http\Middleware\TrustProxies->handle()
#47 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#48 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TrimStrings.php(40): Illuminate\Foundation\Http\Middleware\TransformsRequest->handle()
#49 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Foundation\Http\Middleware\TrimStrings->handle()
#50 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ValidatePostSize.php(27): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#51 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Foundation\Http\Middleware\ValidatePostSize->handle()
#52 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/PreventRequestsDuringMaintenance.php(99): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#53 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(183): Illuminate\Foundation\Http\Middleware\PreventRequestsDuringMaintenance->handle()
#54 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(119): Illuminate\Pipeline\Pipeline->Illuminate\Pipeline\{closure}()
#55 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(175): Illuminate\Pipeline\Pipeline->then()
#56 /var/www/bookstack/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(144): Illuminate\Foundation\Http\Kernel->sendRequestThroughRouter()
#57 /var/www/bookstack/public/index.php(53): Illuminate\Foundation\Http\Kernel->handle()
#58 {main}

@ssddanbrown ssddanbrown added this to the Next Feature Release milestone Feb 14, 2025
ssddanbrown added a commit that referenced this issue Feb 20, 2025
Changes default fallback for name to first DN part, otherwise the whole
DN, rather than leave as null which was causing a type error.

For #5443
@ssddanbrown
Copy link
Member

Thanks @dofeldsc for raising.

This was applicable for any scenario where the server was not providing a cn value in user responses.
I've now addressed this via 35b45a2, so that we fall back to the DN (first component and then full DN) for an actual name value.
These changes will be part of the next feature release.

@thereiam I couldn't find any specific issue in loading the LDAP_DISPLAY_NAME_ATTRIBUTE env option, and this error would have occurred regardless of that option (only matters that the cn wasn't being returned).
If you're sure though there's a specific issue regarding that option you could raise that as a separate issue.

@thereiam
Copy link

@ssddanbrown
Thank you for updating on this. I may have misinterpreted the error root cause. However I just tried your fix in the commit you provided and it fixes the issue too! Thanks a lot!

@ssddanbrown ssddanbrown changed the title LDAP LDAP: Error thrown when no CN provided by LDAP server Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants