Repository: syncthing/syncthing (language: go)

Issue:
Omitting %s from LDAP bind DN sends corrupted bind DN string to LDAP server
### Syncthing Version
Syncthing version: v1.23.4, Linux (64-bit Intel/AMD Container)

### What Happened

Configuring LDAP auth with a bind DN which omits the `%s` placeholder (to restrict login to a specific user with LDAP) results in the following bind DN being sent to LDAP:

So configuring 

`cn=will,dc=home,dc=internal`

results in the following being sent to the LDAP server

`cn=will,dc=home,dc=internal%!(extra string=will)`

Which obviously fails.

### What Is Expected

If the %s is omitted, the the bind DN will be sent as-is to the LDAP server.

### Reproduction

- Configure a bind DN without a `%s` placeholder
- Enable LDAP auth
- Observe the logs on the LDAP auth attempt





Discussion / hints:

Fix patch:
diff --git a/lib/api/api_auth.go b/lib/api/api_auth.go
index e84ae645e49..ad3d86638f8 100644
--- a/lib/api/api_auth.go
+++ b/lib/api/api_auth.go
@@ -161,7 +161,7 @@ func authLDAP(username string, password string, cfg config.LDAPConfiguration) bo
 
 	defer connection.Close()
 
-	err = connection.Bind(fmt.Sprintf(cfg.BindDN, username), password)
+	err = connection.Bind(ldapTemplateBindDN(cfg.BindDN, username), password)
 	if err != nil {
 		l.Warnln("LDAP Bind:", err)
 		return false
@@ -199,6 +199,15 @@ func authLDAP(username string, password string, cfg config.LDAPConfiguration) bo
 	return true
 }
 
+func ldapTemplateBindDN(bindDN string, username string) string {
+	// Check if formatting directives are included in the ldapTemplateBindDN - if so add username.
+	// (%%s is a literal %s - unlikely for LDAP, but easy to handle here).
+	if strings.Count(bindDN, "%s") != strings.Count(bindDN, "%%s") {
+		bindDN = fmt.Sprintf(bindDN, username)
+	}
+	return bindDN
+}
+
 // Convert an ISO-8859-1 encoded byte string to UTF-8. Works by the
 // principle that ISO-8859-1 bytes are equivalent to unicode code points,
 // that a rune slice is a list of code points, and that stringifying a slice


Test patch:
diff --git a/lib/api/api_auth_test.go b/lib/api/api_auth_test.go
index 046b06a0771..be6a0c3a800 100644
--- a/lib/api/api_auth_test.go
+++ b/lib/api/api_auth_test.go
@@ -45,3 +45,23 @@ func TestStaticAuthPasswordFail(t *testing.T) {
 		t.Fatalf("should fail auth")
 	}
 }
+
+func TestAuthLDAPSendsCorrectBindDNWithTemplate(t *testing.T) {
+	t.Parallel()
+
+	templatedDn := ldapTemplateBindDN("cn=%s,dc=some,dc=example,dc=com", "username")
+	expectedDn := "cn=username,dc=some,dc=example,dc=com"
+	if expectedDn != templatedDn {
+		t.Fatalf("ldapTemplateBindDN should be %s != %s", expectedDn, templatedDn)
+	}
+}
+
+func TestAuthLDAPSendsCorrectBindDNWithNoTemplate(t *testing.T) {
+	t.Parallel()
+
+	templatedDn := ldapTemplateBindDN("cn=fixedusername,dc=some,dc=example,dc=com", "username")
+	expectedDn := "cn=fixedusername,dc=some,dc=example,dc=com"
+	if expectedDn != templatedDn {
+		t.Fatalf("ldapTemplateBindDN should be %s != %s", expectedDn, templatedDn)
+	}
+}