Strange behavior occurring when using custom LDAP id option #1516

Closed
opened 2026-02-05 01:07:02 +03:00 by OVERLORD · 13 comments
Owner

Originally created by @ssddanbrown on GitHub (Feb 5, 2020).

Originally assigned to: @ssddanbrown on GitHub.

@ssddanbrown It seems that it's not possible to login again after the first one. I checked and the external authentication ID is very strange, actually:

image

Obviously, this is not the objectGUID that this user have in AD.

Originally posted by @joaomezzari in https://github.com/BookStackApp/BookStack/issues/592#issuecomment-582371753

Originally created by @ssddanbrown on GitHub (Feb 5, 2020). Originally assigned to: @ssddanbrown on GitHub. @ssddanbrown It seems that it's not possible to login again after the first one. I checked and the external authentication ID is very strange, actually: ![image](https://user-images.githubusercontent.com/53047818/73839258-4c8b2080-47f4-11ea-82c8-404d1220ff47.png) Obviously, this is not the objectGUID that this user have in AD. _Originally posted by @joaomezzari in https://github.com/BookStackApp/BookStack/issues/592#issuecomment-582371753_
OVERLORD added the 📖 Docs Update🚪 Authentication🔍 Pending Validation labels 2026-02-05 01:07:02 +03:00
Author
Owner

@mflagler commented on GitHub (Feb 10, 2020):

I'm having the same issue. I tried using a couple different formats for the objectGUID such as the Hex value as well as the GUID string and neither worked, so I created a temp user in AD and registered that user in Bookstack. Initial login worked, but subsequent logins did not work. It seems that it's not storing the value correctly. In the database and on the display for that user for their External Authentication ID I have this value: ?K?E??"?S|??

@mflagler commented on GitHub (Feb 10, 2020): I'm having the same issue. I tried using a couple different formats for the objectGUID such as the Hex value as well as the GUID string and neither worked, so I created a temp user in AD and registered that user in Bookstack. Initial login worked, but subsequent logins did not work. It seems that it's not storing the value correctly. In the database and on the display for that user for their External Authentication ID I have this value: ?K?E??"?S|??
Author
Owner

@ssddanbrown commented on GitHub (Feb 10, 2020):

Thanks for reporting @mflagler.

This is strange, I didn't think we handled this value any differently than other values we get from LDAP.
In the next bugfix, I'll sneak in an option to dump retrieved details upon fetch so we have an easier way to debug this, since I don't really have visibility of the raw values coming back from AD.

@ssddanbrown commented on GitHub (Feb 10, 2020): Thanks for reporting @mflagler. This is strange, I didn't think we handled this value any differently than other values we get from LDAP. In the next bugfix, I'll sneak in an option to dump retrieved details upon fetch so we have an easier way to debug this, since I don't really have visibility of the raw values coming back from AD.
Author
Owner

@kanlas-net commented on GitHub (Feb 13, 2020):

@ssddanbrown I used GUID instead of objectGUID an it worked!

@kanlas-net commented on GitHub (Feb 13, 2020): @ssddanbrown I used **GUID** instead of **objectGUID** an it worked!
Author
Owner

@mflagler commented on GitHub (Feb 13, 2020):

@kanlas-net I just tried it and while it does work to register users and login, it's still just using the old distinguished name and not the unique (and non-changing) objectGUID. I also tried ObjectSid which gave similar results to using objectGUID with ��������??Vz*???, �� as the external authentication ID.

@mflagler commented on GitHub (Feb 13, 2020): @kanlas-net I just tried it and while it does work to register users and login, it's still just using the old distinguished name and not the unique (and non-changing) objectGUID. I also tried ObjectSid which gave similar results to using objectGUID with ��������??Vz*???, �� as the external authentication ID.
Author
Owner

@kanlas-net commented on GitHub (Feb 13, 2020):

@mflagler as mentioned here this is because guid is stored as hexadecimal byte arrays, so it needs some conversion to become a text

@kanlas-net commented on GitHub (Feb 13, 2020): @mflagler as mentioned [here](https://social.technet.microsoft.com/Forums/ie/en-US/b4e65253-cac7-4a9a-be58-5e55becbe1b9/active-directory-objectguid-format-conversion?forum=winservergen) this is because guid is stored as hexadecimal byte arrays, so it needs some conversion to become a text
Author
Owner

@kanlas-net commented on GitHub (Feb 13, 2020):

@mflagler, @ssddanbrown seems like I have fixed it.
FIle: ./app/Auth/Access/LdapService.php

    public function getUserDetails(string $userName): ?array
    {
        $idAttr = $this->config['id_attribute'];
        $emailAttr = $this->config['email_attribute'];
        $displayNameAttr = $this->config['display_name_attribute'];

        $user = $this->getUserWithAttributes($userName, ['cn', 'dn', $idAttr, $emailAttr, $displayNameAttr]);

        if ($user === null) {
            return null;
        }

        $userCn = $this->getUserResponseProperty($user, 'cn', null);
        return [
-           'uid'   => $this->getUserResponseProperty($user, $idAttr, $user['dn']),
+           'uid'   => bin2hex($this->getUserResponseProperty($user, $idAttr, $user['dn'])),
            'name'  => $this->getUserResponseProperty($user, $displayNameAttr, $userCn),
            'dn'    => $user['dn'],
            'email' => $this->getUserResponseProperty($user, $emailAttr, null),
        ];
    }

screenshot

@kanlas-net commented on GitHub (Feb 13, 2020): @mflagler, @ssddanbrown seems like I have fixed it. FIle: _./app/Auth/Access/LdapService.php_ ```diff public function getUserDetails(string $userName): ?array { $idAttr = $this->config['id_attribute']; $emailAttr = $this->config['email_attribute']; $displayNameAttr = $this->config['display_name_attribute']; $user = $this->getUserWithAttributes($userName, ['cn', 'dn', $idAttr, $emailAttr, $displayNameAttr]); if ($user === null) { return null; } $userCn = $this->getUserResponseProperty($user, 'cn', null); return [ - 'uid' => $this->getUserResponseProperty($user, $idAttr, $user['dn']), + 'uid' => bin2hex($this->getUserResponseProperty($user, $idAttr, $user['dn'])), 'name' => $this->getUserResponseProperty($user, $displayNameAttr, $userCn), 'dn' => $user['dn'], 'email' => $this->getUserResponseProperty($user, $emailAttr, null), ]; } ``` ![screenshot](https://user-images.githubusercontent.com/16520060/74465243-16aa0400-4ea6-11ea-8a76-a074060cf255.png)
Author
Owner

@necouchman commented on GitHub (Feb 14, 2020):

I was seeing this issue, too, with the odd behavior that I could log in with my AD account once, but the next time I logged it would just take me back to the login page, and I'd get the following error in the storage/logs/laravel.log file:

[2020-02-14 02:05:23] production.ERROR: A user with the email nick@example.com already exists but with different credentials. 
{"exception":"[object] (BookStack\\Exceptions\\UserRegistrationException(code: 0): A user with the email nick@example.com already exists but with different credentials. at /var/www/bookstack/app/Auth/Access/RegistrationService.php:60)

The patch provided by @kanlas-net seems to have resolved the issue.

@necouchman commented on GitHub (Feb 14, 2020): I was seeing this issue, too, with the odd behavior that I could log in with my AD account once, but the next time I logged it would just take me back to the login page, and I'd get the following error in the `storage/logs/laravel.log` file: ``` [2020-02-14 02:05:23] production.ERROR: A user with the email nick@example.com already exists but with different credentials. {"exception":"[object] (BookStack\\Exceptions\\UserRegistrationException(code: 0): A user with the email nick@example.com already exists but with different credentials. at /var/www/bookstack/app/Auth/Access/RegistrationService.php:60) ``` The patch provided by @kanlas-net seems to have resolved the issue.
Author
Owner

@mflagler commented on GitHub (Feb 15, 2020):

I was looking around and want to test it, but don't have a dev server to test it on right now, but would it be better to use a suggested function similar to what is shown here in the comments: https://www.php.net/manual/en/function.mssql-guid-string.php#119391

This is removed in PHP 7, but the top note on the link suggests a different function to change the formatting to a standard GUID string like is viewable in Attribute Editor in this format: 4a9209bd-9252-4914-01a3-24c283062394

Also, would the fix given above by @kanlas-net break the existing storage of a DN instead of objectGUID, so it would need to be conditioned to only perform the function if the result is returned in binary format or if we're using objectGUID or ObjectSid?

@mflagler commented on GitHub (Feb 15, 2020): I was looking around and want to test it, but don't have a dev server to test it on right now, but would it be better to use a suggested function similar to what is shown here in the comments: https://www.php.net/manual/en/function.mssql-guid-string.php#119391 This is removed in PHP 7, but the top note on the link suggests a different function to change the formatting to a standard GUID string like is viewable in Attribute Editor in this format: 4a9209bd-9252-4914-01a3-24c283062394 Also, would the fix given above by @kanlas-net break the existing storage of a DN instead of objectGUID, so it would need to be conditioned to only perform the function if the result is returned in binary format or if we're using objectGUID or ObjectSid?
Author
Owner

@ssddanbrown commented on GitHub (Feb 16, 2020):

Thank you very much @kanlas-net for figuring out what was going on here.

I've just deployed v0.28.1 and v0.28.2. In these I have added the ability to prefix any LDAP attribute options with BIN; to mark them as binary to be decoded to hex on fetch. Therefore you should be able to use the following in your .env:

LDAP_ID_ATTRIBUTE=BIN;objectGUID

If you have many any edits so far to the app/Auth/Access/LdapService.php you might want to revert and clear your changes before updating to avoid issues when running the update commands.

I have also added a LDAP_DUMP_USER_DETAILS=true option which will dump user details as JSON upon fetch to help with debugging where needed.

I don't have AD myself so have not been able to fully test the changes made. Please let me know the above attribute option works. If so I'll go ahead and update the docs to reflect these changes otherwise I'll remain hesitant for now.


@joaomezzari FYI

@ssddanbrown commented on GitHub (Feb 16, 2020): Thank you very much @kanlas-net for figuring out what was going on here. I've just deployed [v0.28.1](https://github.com/BookStackApp/BookStack/releases/tag/v0.28.1) and [v0.28.2](https://github.com/BookStackApp/BookStack/releases/tag/v0.28.2). In these I have added the ability to prefix any LDAP attribute options with `BIN;` to mark them as binary to be decoded to hex on fetch. Therefore you should be able to use the following in your .env: ```bash LDAP_ID_ATTRIBUTE=BIN;objectGUID ``` If you have many any edits so far to the `app/Auth/Access/LdapService.php` you might want to revert and clear your changes before updating to avoid issues when running the update commands. I have also added a `LDAP_DUMP_USER_DETAILS=true` option which will dump user details as JSON upon fetch to help with debugging where needed. I don't have AD myself so have not been able to fully test the changes made. Please let me know the above attribute option works. If so I'll go ahead and update the docs to reflect these changes otherwise I'll remain hesitant for now. --- @joaomezzari FYI
Author
Owner

@joaomezzari commented on GitHub (Feb 17, 2020):

Strange, I just tested with the new patch but it still doesn't work. I used the binary value and hex for the user ID, no success.

EDIT: My mistake. It works!

@joaomezzari commented on GitHub (Feb 17, 2020): Strange, I just tested with the new patch but it still doesn't work. I used the binary value and hex for the user ID, no success. EDIT: My mistake. It works!
Author
Owner

@mflagler commented on GitHub (Feb 17, 2020):

Works perfectly! Took a little bit of work to convert my users over, but it's working great! Thank you!

@mflagler commented on GitHub (Feb 17, 2020): Works perfectly! Took a little bit of work to convert my users over, but it's working great! Thank you!
Author
Owner

@Windoze345 commented on GitHub (Feb 17, 2020):

Works for me too! :)

@Windoze345 commented on GitHub (Feb 17, 2020): Works for me too! :)
Author
Owner

@ssddanbrown commented on GitHub (Feb 18, 2020):

Thanks everyone for confirming. Docs have now been updated to reflect these changes so I'll close this off.

@ssddanbrown commented on GitHub (Feb 18, 2020): Thanks everyone for confirming. Docs have now been updated to reflect these changes so I'll close this off.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/BookStack#1516