Bookstack: Strange behavior occurring when using custom LDAP id option

[ssddanbrown]

@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:

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_

[mflagler]

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|??

[ssddanbrown]

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.

[kanlas-net]

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

[mflagler]

@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.

[kanlas-net]

@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]

@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),
        ];
    }

[necouchman]

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 [email protected] already exists but with different credentials. 
{"exception":"[object] (BookStack\\Exceptions\\UserRegistrationException(code: 0): A user with the email [email protected] 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.

[mflagler]

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?

[ssddanbrown]

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

[joaomezzari]

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!

[mflagler]

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

[Windoze345]

Works for me too! :)

[ssddanbrown]

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