From d9361edc06b63061c86d66e24cfba3f4bba4a1f4 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 29 Sep 2021 11:29:59 +0200 Subject: [PATCH] Reduce some code duplication in LDAP related drivers This moves the LDAP Bind operation into utils/ldap to reduce some code duplication accross the auth, user- and group-providers. --- changelog/unreleased/ldap-util.md | 3 +++ pkg/auth/manager/ldap/ldap.go | 9 --------- pkg/group/manager/ldap/ldap.go | 26 -------------------------- pkg/user/manager/ldap/ldap.go | 26 -------------------------- pkg/utils/ldap.go | 24 +++++++++++++++++++----- 5 files changed, 22 insertions(+), 66 deletions(-) create mode 100644 changelog/unreleased/ldap-util.md diff --git a/changelog/unreleased/ldap-util.md b/changelog/unreleased/ldap-util.md new file mode 100644 index 0000000000..545652846a --- /dev/null +++ b/changelog/unreleased/ldap-util.md @@ -0,0 +1,3 @@ +Enhancement: Reduce code duplication in LDAP related drivers + +https://github.com/cs3org/reva/pull/2115 diff --git a/pkg/auth/manager/ldap/ldap.go b/pkg/auth/manager/ldap/ldap.go index d956b26d9d..4d135eeb6c 100644 --- a/pkg/auth/manager/ldap/ldap.go +++ b/pkg/auth/manager/ldap/ldap.go @@ -53,8 +53,6 @@ type config struct { BaseDN string `mapstructure:"base_dn"` UserFilter string `mapstructure:"userfilter"` LoginFilter string `mapstructure:"loginfilter"` - BindUsername string `mapstructure:"bind_username"` - BindPassword string `mapstructure:"bind_password"` Idp string `mapstructure:"idp"` GatewaySvc string `mapstructure:"gatewaysvc"` Schema attributes `mapstructure:"schema"` @@ -141,13 +139,6 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string) } defer l.Close() - // First bind with a read only user - err = l.Bind(am.c.BindUsername, am.c.BindPassword) - if err != nil { - log.Error().Err(err).Msg("bind with system user failed") - return nil, nil, err - } - // Search for the given clientID searchRequest := ldap.NewSearchRequest( am.c.BaseDN, diff --git a/pkg/group/manager/ldap/ldap.go b/pkg/group/manager/ldap/ldap.go index fd50b86d52..d388e835e9 100644 --- a/pkg/group/manager/ldap/ldap.go +++ b/pkg/group/manager/ldap/ldap.go @@ -56,8 +56,6 @@ type config struct { MemberFilter string `mapstructure:"memberfilter"` AttributeFilter string `mapstructure:"attributefilter"` FindFilter string `mapstructure:"findfilter"` - BindUsername string `mapstructure:"bind_username"` - BindPassword string `mapstructure:"bind_password"` Idp string `mapstructure:"idp"` Schema attributes `mapstructure:"schema"` Nobody int64 `mapstructure:"nobody"` @@ -139,12 +137,6 @@ func (m *manager) GetGroup(ctx context.Context, gid *grouppb.GroupId) (*grouppb. } defer l.Close() - // First bind with a read only user - err = l.Bind(m.c.BindUsername, m.c.BindPassword) - if err != nil { - return nil, err - } - // Search for the given clientID searchRequest := ldap.NewSearchRequest( m.c.BaseDN, @@ -216,12 +208,6 @@ func (m *manager) GetGroupByClaim(ctx context.Context, claim, value string) (*gr } defer l.Close() - // First bind with a read only user - err = l.Bind(m.c.BindUsername, m.c.BindPassword) - if err != nil { - return nil, err - } - // Search for the given clientID searchRequest := ldap.NewSearchRequest( m.c.BaseDN, @@ -274,12 +260,6 @@ func (m *manager) FindGroups(ctx context.Context, query string) ([]*grouppb.Grou } defer l.Close() - // First bind with a read only user - err = l.Bind(m.c.BindUsername, m.c.BindPassword) - if err != nil { - return nil, err - } - // Search for the given clientID searchRequest := ldap.NewSearchRequest( m.c.BaseDN, @@ -326,12 +306,6 @@ func (m *manager) GetMembers(ctx context.Context, gid *grouppb.GroupId) ([]*user } defer l.Close() - // First bind with a read only user - err = l.Bind(m.c.BindUsername, m.c.BindPassword) - if err != nil { - return nil, err - } - // Search for the given clientID searchRequest := ldap.NewSearchRequest( m.c.BaseDN, diff --git a/pkg/user/manager/ldap/ldap.go b/pkg/user/manager/ldap/ldap.go index cfe4a6b08b..dbaf00b09e 100644 --- a/pkg/user/manager/ldap/ldap.go +++ b/pkg/user/manager/ldap/ldap.go @@ -55,8 +55,6 @@ type config struct { AttributeFilter string `mapstructure:"attributefilter"` FindFilter string `mapstructure:"findfilter"` GroupFilter string `mapstructure:"groupfilter"` - BindUsername string `mapstructure:"bind_username"` - BindPassword string `mapstructure:"bind_password"` Idp string `mapstructure:"idp"` Schema attributes `mapstructure:"schema"` Nobody int64 `mapstructure:"nobody"` @@ -151,12 +149,6 @@ func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User } defer l.Close() - // First bind with a read only user - err = l.Bind(m.c.BindUsername, m.c.BindPassword) - if err != nil { - return nil, err - } - // Search for the given clientID searchRequest := ldap.NewSearchRequest( m.c.BaseDN, @@ -239,12 +231,6 @@ func (m *manager) GetUserByClaim(ctx context.Context, claim, value string) (*use } defer l.Close() - // First bind with a read only user - err = l.Bind(m.c.BindUsername, m.c.BindPassword) - if err != nil { - return nil, err - } - // Search for the given clientID searchRequest := ldap.NewSearchRequest( m.c.BaseDN, @@ -311,12 +297,6 @@ func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User, } defer l.Close() - // First bind with a read only user - err = l.Bind(m.c.BindUsername, m.c.BindPassword) - if err != nil { - return nil, err - } - // Search for the given clientID searchRequest := ldap.NewSearchRequest( m.c.BaseDN, @@ -381,12 +361,6 @@ func (m *manager) GetUserGroups(ctx context.Context, uid *userpb.UserId) ([]stri } defer l.Close() - // First bind with a read only user - err = l.Bind(m.c.BindUsername, m.c.BindPassword) - if err != nil { - return []string{}, err - } - // Search for the given clientID searchRequest := ldap.NewSearchRequest( m.c.BaseDN, diff --git a/pkg/utils/ldap.go b/pkg/utils/ldap.go index 7487b2c2c3..415bc4a796 100644 --- a/pkg/utils/ldap.go +++ b/pkg/utils/ldap.go @@ -31,10 +31,12 @@ import ( // LDAPConn holds the basic parameter for setting up an // LDAP connection. type LDAPConn struct { - Hostname string `mapstructure:"hostname"` - Port int `mapstructure:"port"` - Insecure bool `mapstructure:"insecure"` - CACert string `mapstructure:"cacert"` + Hostname string `mapstructure:"hostname"` + Port int `mapstructure:"port"` + Insecure bool `mapstructure:"insecure"` + CACert string `mapstructure:"cacert"` + BindUsername string `mapstructure:"bind_username"` + BindPassword string `mapstructure:"bind_password"` } // GetLDAPConnection initializes an LDAPS connection and allows @@ -52,5 +54,17 @@ func GetLDAPConnection(c *LDAPConn) (*ldap.Conn, error) { return nil, errors.Wrapf(err, "Error reading LDAP CA Cert '%s.'", c.CACert) } } - return ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", c.Hostname, c.Port), tlsconfig) + l, err := ldap.DialTLS("tcp", fmt.Sprintf("%s:%d", c.Hostname, c.Port), tlsconfig) + if err != nil { + return nil, err + } + + if c.BindUsername != "" && c.BindPassword != "" { + err = l.Bind(c.BindUsername, c.BindPassword) + if err != nil { + l.Close() + return nil, err + } + } + return l, nil }