mirror of
https://github.com/go-vikunja/vikunja.git
synced 2026-04-29 19:10:51 -05:00
feat(auth): allow LDAP authentication with anonymous bind (#2226)
As discussed on Matrix, Vikunja currently prevents users from using LDAP authentication if the server allows anonymous binds (common in local environments like YunoHost). The application would previously trigger a `log.Fatal` if `AuthLdapBindDN` or `AuthLdapBindPassword` were left empty in the configuration. #### **How this fixes the problem:** * **Validation:** Removed the strict requirement for Bind credentials in `InitializeLDAPConnection`. * **Connection Logic:** Updated `ConnectAndBindToLDAPDirectory` to attempt an `UnauthenticatedBind` from the `go-ldap` library when no credentials are provided. * **Safety:** If a Bind DN is provided, the behavior remains unchanged (authenticated bind). #### **Testing:** * Tested manually on a **YunoHost** instance by replacing the binary. * Confirmed that Vikunja now successfully starts and authenticates users via the local LDAP (localhost) without requiring a service account. * Added a basic unit test in `pkg/modules/auth/ldap/ldap_test.go` to ensure the initialization logic doesn't crash with empty credentials. *Note: This is my first contribution to a Go project (assisted by an LLM for syntax). Feedback on code style is more than welcome!*
This commit is contained in:
@@ -51,12 +51,6 @@ func InitializeLDAPConnection() {
|
||||
if config.AuthLdapBaseDN.GetString() == "" {
|
||||
log.Fatal("LDAP base DN is not configured")
|
||||
}
|
||||
if config.AuthLdapBindDN.GetString() == "" {
|
||||
log.Fatal("LDAP bind DN is not configured")
|
||||
}
|
||||
if config.AuthLdapBindPassword.GetString() == "" {
|
||||
log.Fatal("LDAP bind password is not configured")
|
||||
}
|
||||
if config.AuthLdapUserFilter.GetString() == "" {
|
||||
log.Fatal("LDAP user filter is not configured")
|
||||
}
|
||||
@@ -99,10 +93,17 @@ func ConnectAndBindToLDAPDirectory() (l *ldap.Conn, err error) {
|
||||
return nil, fmt.Errorf("could not connect to LDAP server: %w", err)
|
||||
}
|
||||
|
||||
err = l.Bind(
|
||||
config.AuthLdapBindDN.GetString(),
|
||||
config.AuthLdapBindPassword.GetString(),
|
||||
)
|
||||
bindDN := config.AuthLdapBindDN.GetString()
|
||||
bindPassword := config.AuthLdapBindPassword.GetString()
|
||||
|
||||
if bindDN != "" && bindPassword != "" {
|
||||
// Standard authentication
|
||||
err = l.Bind(bindDN, bindPassword)
|
||||
} else {
|
||||
// Anonymous bind attempt (depending on the server, this call is explicit or automatic)
|
||||
log.Info("No LDAP bind DN or password configured, attempting anonymous bind")
|
||||
err = l.UnauthenticatedBind("")
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
@@ -117,6 +117,37 @@ func TestLdapLogin(t *testing.T) {
|
||||
"avatar_provider": "ldap",
|
||||
}, false)
|
||||
})
|
||||
|
||||
t.Run("should bind anonymously", func(t *testing.T) {
|
||||
// Backup original config
|
||||
origBindDN := config.AuthLdapBindDN.GetString()
|
||||
origBindPW := config.AuthLdapBindPassword.GetString()
|
||||
defer func() {
|
||||
config.AuthLdapBindDN.Set(origBindDN)
|
||||
config.AuthLdapBindPassword.Set(origBindPW)
|
||||
}()
|
||||
|
||||
// Set empty bind credentials
|
||||
config.AuthLdapBindDN.Set("")
|
||||
config.AuthLdapBindPassword.Set("")
|
||||
|
||||
db.LoadAndAssertFixtures(t)
|
||||
s := db.NewSession()
|
||||
defer s.Close()
|
||||
|
||||
// Attempt to authenticate
|
||||
// Note: This test might fail if the test LDAP server doesn't support anonymous bind,
|
||||
// but it verifies the code path executes
|
||||
user, err := AuthenticateUserInLDAP(s, "professor", "professor", false, "")
|
||||
|
||||
// We mainly want to ensure we don't panic or error out due to missing config
|
||||
if err != nil {
|
||||
// If it fails, it should be an LDAP error, not a "configuration missing" error
|
||||
require.NotContains(t, err.Error(), "configured")
|
||||
} else {
|
||||
assert.Equal(t, "professor", user.Username)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestEscapeLDAPFilterValue(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user