Missing LDAP group sync error checking #810

Closed
opened 2026-02-04 22:21:23 +03:00 by OVERLORD · 1 comment
Owner

Originally created by @MarkOnDuty on GitHub (Sep 11, 2018).

Describe the bug
If a BookStack installation is configured with the default role matching something that already exists in LDAP (ie. Editor is the default role and an Editor group exists in LDAP), users are added to that role twice on login, which causes an exception.

Steps To Reproduce
Steps to reproduce the behavior:

  1. Install BookStack, including using LDAP for authentication and group sync
  2. Set the default role to 'Editor'
  3. Create an 'Editor' role in LDAP and add a user to it
  4. Try to log in as that user

Expected behavior
Roles in LDAP should not conflict with the default role. Before creating a new entry for the user with the default role, attachDefaultRole() should check whether such an entry already exists.

Configuration:

  • Bookstack 0.23.2
  • PHP 7.2.7
  • Apache

Kudos
BookStack is really useful and its development has moved at a furious pace. Looking though the code, for the first time today, I am really impressed. If there were a way to donate cash to support it's development, I would. In the meantime, I'll help where I can, starting with this bug report.

Originally created by @MarkOnDuty on GitHub (Sep 11, 2018). **Describe the bug** If a BookStack installation is configured with the default role matching something that already exists in LDAP (ie. Editor is the default role and an Editor group exists in LDAP), users are added to that role twice on login, which causes an exception. **Steps To Reproduce** Steps to reproduce the behavior: 1. Install BookStack, including using LDAP for authentication and group sync 2. Set the default role to 'Editor' 3. Create an 'Editor' role in LDAP and add a user to it 4. Try to log in as that user **Expected behavior** Roles in LDAP should not conflict with the default role. Before creating a new entry for the user with the default role, `attachDefaultRole()` should check whether such an entry already exists. **Configuration:** - Bookstack 0.23.2 - PHP 7.2.7 - Apache **Kudos** BookStack is really useful and its development has moved at a furious pace. Looking though the code, for the first time today, I am really impressed. If there were a way to donate cash to support it's development, I would. In the meantime, I'll help where I can, starting with this bug report.
OVERLORD added the 🐛 Bug label 2026-02-04 22:21:23 +03:00
Author
Owner

@ssddanbrown commented on GitHub (Sep 16, 2018):

Thanks @MarkOnDuty for spotting this. I had thought attaching in Laravel would essentially "upsert" but I was mistaken. Have marked to be fixed for the next release.

Thanks for your kind words about the Project and the code, Is good to hear, Especially as I worry about how messy and complex I've made things over time 😅 .

@ssddanbrown commented on GitHub (Sep 16, 2018): Thanks @MarkOnDuty for spotting this. I had thought `attach`ing in Laravel would essentially "upsert" but I was mistaken. Have marked to be fixed for the next release. Thanks for your kind words about the Project and the code, Is good to hear, Especially as I worry about how messy and complex I've made things over time :sweat_smile: .
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/BookStack#810