Skip to content

Commit

Permalink
fix: concurrent calls not authorized and LDAP timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
kpetremann committed Aug 21, 2023
1 parent 3238d03 commit e3f59db
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 21 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ build:
go build -ldflags="-X 'github.com/criteo/data-aggregation-api/internal/version.version=$(VER)' \
-X 'github.com/criteo/data-aggregation-api/internal/version.buildUser=$(USER)' \
-X 'github.com/criteo/data-aggregation-api/internal/version.buildTime=$(BUILDDATE)'" \
-o .build/data_aggregation_api ./cmd/data_aggregation_api
-o .build/data-aggregation-api ./cmd/data-aggregation-api

run:
go run -ldflags="-X 'github.com/criteo/data-aggregation-api/internal/version.version=$(VER)' \
-X 'github.com/criteo/data-aggregation-api/internal/version.buildUser=$(USER)' \
-X 'github.com/criteo/data-aggregation-api/internal/version.buildTime=$(BUILDDATE)'" \
./cmd/data_aggregation_api
./cmd/data-aggregation-api

update_openconfig:
./update_openconfig.sh
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ require (
github.com/prometheus/client_golang v1.16.0
github.com/rs/zerolog v1.30.0
github.com/spf13/viper v1.16.0
golang.org/x/sync v0.3.0
)

require (
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,8 @@ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.3.0 h1:ftCYgMx6zT/asHUrPw8BLLscYtGznsLAnjq5RH9P66E=
golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand Down
14 changes: 13 additions & 1 deletion internal/api/auth/basic_auth.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package auth

import (
"context"
"crypto/tls"
"errors"
"fmt"
Expand All @@ -19,6 +20,8 @@ const (

type authMode string

const maxLDAPWorkers = 1

const (
noAuth authMode = "None"
ldapMode authMode = "LDAP"
Expand All @@ -29,7 +32,7 @@ type BasicAuth struct {
mode authMode
}

func NewBasicAuth(cfg config.AuthConfig) (BasicAuth, error) {
func NewBasicAuth(ctx context.Context, cfg config.AuthConfig) (BasicAuth, error) {
b := BasicAuth{mode: noAuth}

if cfg.LDAP == nil {
Expand All @@ -45,6 +48,15 @@ func NewBasicAuth(cfg config.AuthConfig) (BasicAuth, error) {
}
b.mode = ldapMode

// trying a connection to LDAP to check the configuration
conn, err := ldap.connect()
defer conn.Close()

Check failure on line 53 in internal/api/auth/basic_auth.go

View workflow job for this annotation

GitHub Actions / Lint and test

Error return value of `conn.Close` is not checked (errcheck)
ldap.StartWorkers(ctx, maxLDAPWorkers)

if err != nil {
return b, fmt.Errorf("first connection to LDAP failed: %w", err)
}

return b, nil
}

Expand Down
117 changes: 100 additions & 17 deletions internal/api/auth/ldap.go
Original file line number Diff line number Diff line change
@@ -1,41 +1,125 @@
package auth

import (
"context"
"crypto/tls"
"fmt"
"time"

"github.com/rs/zerolog/log"

"github.com/go-ldap/ldap/v3"
)

type authRequest struct {

Check failure on line 14 in internal/api/auth/ldap.go

View workflow job for this annotation

GitHub Actions / Lint and test

fieldalignment: struct with 40 pointer bytes could be 32 (govet)
username string
password string
authResp chan bool
}

type LDAPAuth struct {

Check failure on line 20 in internal/api/auth/ldap.go

View workflow job for this annotation

GitHub Actions / Lint and test

fieldalignment: struct with 80 pointer bytes could be 72 (govet)
ldapClient *ldap.Conn
bindDN string
password string
baseDN string
tlsConfig *tls.Config
ldapURL string
bindDN string
password string
baseDN string
reqCh chan authRequest
}

func NewLDAPAuth(ldapURL string, bindDN string, password string, baseDN string, tlsConfig *tls.Config) *LDAPAuth {
conn, err := ldap.DialURL(ldapURL, ldap.DialWithTLSConfig(tlsConfig))
if err != nil {
log.Error().Err(err).Str("ldapURL", ldapURL).Msg("failed to connect to the LDAP server")
return nil
}
return &LDAPAuth{
ldapClient: conn,
bindDN: bindDN,
password: password,
baseDN: baseDN,
tlsConfig: tlsConfig,
ldapURL: ldapURL,
bindDN: bindDN,
password: password,
baseDN: baseDN,
reqCh: make(chan authRequest),
}
}

func (l *LDAPAuth) StartWorkers(ctx context.Context, maxWorker int) {
for i := 0; i < maxWorker; i++ {
go l.spawnWorker(ctx)
}
}

func (l *LDAPAuth) AuthenticateUser(username string, password string) bool {
if err := l.ldapClient.Bind(l.bindDN, l.password); err != nil {
req := authRequest{
username: username,
password: password,
authResp: make(chan bool),
}
l.reqCh <- req
return <-req.authResp
}

func (l *LDAPAuth) spawnWorker(ctx context.Context) {
var conn *ldap.Conn
var err error
tick := time.NewTicker(time.Minute)

for {
select {
case req := <-l.reqCh:
// (re)connect if needed
if conn == nil || conn.IsClosing() {
log.Debug().Msg("LDAP connection is closed, reconnecting")
conn, err = l.connect()
if err != nil {
log.Error().Err(err).Msg("worker LDAP reconnection failed")
req.authResp <- false
return
}
}

// bind with the user credentials
req.authResp <- l.authenticate(conn, req.username, req.password)

tick.Reset(time.Minute)

case <-tick.C:
// close connection if no request has been made for a minute
log.Debug().Msg("timer reached, closing connection")
if conn != nil {
if err := conn.Close(); err != nil {
log.Error().Err(err).Msg("unable to close the LDAP connection")
}
}
return

case <-ctx.Done():
// gracefully close connection if context is done
log.Debug().Msg("context is closed, closing connection")
if conn != nil {
_ = conn.Close()
}
return
}
}
}

func (l *LDAPAuth) connect() (*ldap.Conn, error) {
conn, err := ldap.DialURL(l.ldapURL, ldap.DialWithTLSConfig(l.tlsConfig))
if err != nil {
return nil, fmt.Errorf("failed to dial to the LDAP server: %w", err)
}

return conn, nil
}

func (l *LDAPAuth) authenticate(conn *ldap.Conn, username string, password string) bool {
if err := conn.Bind(l.bindDN, l.password); err != nil {
log.Error().Err(err).Str("bindDN", l.bindDN).Msg("failed to bind to LDAP")

// detect TCP connection closed or any network errors
if ldap.IsErrorWithCode(err, ldap.ErrorNetwork) {
if err := conn.Close(); err != nil {
log.Error().Err(err).Msg("connection was closed by the server but failed to close on client side")
}
}
return false
}
search, err := l.ldapClient.Search(ldap.NewSearchRequest(
search, err := conn.Search(ldap.NewSearchRequest(
l.baseDN,
ldap.ScopeWholeSubtree,
ldap.NeverDerefAliases,
Expand All @@ -56,10 +140,9 @@ func (l *LDAPAuth) AuthenticateUser(username string, password string) bool {
log.Error().Str(userKey, username).Msg("no result or more than 1 result found for user")
return false
}
if err := l.ldapClient.Bind(search.Entries[0].DN, password); err != nil {
if err := conn.Bind(search.Entries[0].DN, password); err != nil {
log.Error().Err(err).Str(userKey, username).Msg("failed to bind with user")
return false
}
log.Debug().Str(userKey, username).Msg("succesfully authenticated user")
return true
}
2 changes: 1 addition & 1 deletion internal/api/router/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (m *Manager) ListenAndServe(ctx context.Context, address string, port int)
log.Warn().Msg("Shutdown.")
}()

withAuth, err := auth.NewBasicAuth(config.Cfg.Authentication)
withAuth, err := auth.NewBasicAuth(ctx, config.Cfg.Authentication)
if err != nil {
return err
}
Expand Down

0 comments on commit e3f59db

Please sign in to comment.