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

Allow BindDN to exclude any username formatting #8900

Merged
merged 1 commit into from
May 10, 2023

Conversation

wrouesnel
Copy link
Contributor

Purpose

Fix the error string being injected when the template variable for username is omitted in the LDAP Bind DN string (fixes #8899).

Testing

Regression tests have been added to check the new behavior for the original use case and the new one.

Documentation

No documentation change should be required, although the language for LDAP configuration could be changed to note that %s is optional. This would be in-line with the more useful use LDAP auth, which is locking a single syncthing instance to a single user account which manages it's password via LDAP.

This allows a syncthing instance to be locked to exactly 1 user without
needing search capability on the LDAP instance.
Copy link
Member

@acolomb acolomb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix, looks all good though I haven't tested it.

@calmh calmh merged commit b2fb2ef into syncthing:main May 10, 2023
calmh added a commit to calmh/syncthing that referenced this pull request Jun 4, 2023
* main:
  gui: Remove HTML support in tooltips
  cmd/syncthing: Use correct binary when restarting monitor (syncthing#8919)
  build(deps): bump github.com/minio/sha256-simd from 1.0.0 to 1.0.1 (syncthing#8916)
  build: Increase concurrency, add "basic checks" gatekeeper
  gui, man, authors: Update docs, translations, and contributors
  gui, man, authors: Update docs, translations, and contributors
  lib/model: Set platform data for new folders w/ ignorePerms (ref syncthing#8883) (syncthing#8907)
  gui, man, authors: Update docs, translations, and contributors
  lib: replace `Readdir(-1)` with `os.ReadDir` (syncthing#8901)
  lib/api: Allow BindDN to exclude any username formatting (fixes syncthing#8899) (syncthing#8900)
  build: Attempt cross compilation for ~all targets, allow it to fail
  lib/fs: Fix root path handling for Windows (fixes syncthing#8778)
  all: Grand test refactor (fixes syncthing#8779, fixes syncthing#8799)
  all: Correct various typos (syncthing#8870)
@calmh calmh added this to the v1.23.6 milestone Jun 14, 2023
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label May 9, 2024
@syncthing syncthing locked and limited conversation to collaborators May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Omitting %s from LDAP bind DN sends corrupted bind DN string to LDAP server
4 participants