Add LDAP mode for basic authentication #871
Conversation
samanhappy
changed the title
samanhappy
changed the title
Codecov ReportBase: 76.07% // Head: 76.03% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #871 +/- ##
==========================================
- Coverage 76.07% 76.03% -0.05%
==========================================
Files 110 110
Lines 12698 12741 +43
==========================================
+ Hits 9660 9687 +27
- Misses 2492 2508 +16
Partials 546 546
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
pkg/filters/validator/basicauth.go
Outdated
| defer luc.client.Close() | ||
| if err := luc.client.Connect(); err != nil { | ||
| logger.Warnf("failed to connect LDAP server %v", err) | ||
| return false | ||
| } | ||
|
|
||
| userdn := fmt.Sprintf("%s=%s,%s", luc.spec.UID, username, luc.spec.BaseDN) | ||
| if err := luc.client.Conn.Bind(userdn, password); err != nil { | ||
| logger.Warnf("failed to bind LDAP user %v", err) | ||
| return false | ||
| } | ||
|
|
||
| return true |
There was a problem hiding this comment.
could this function be called concurrently?
There was a problem hiding this comment.
this quesion is very considerate, the client.Connect() will create a connection if not exists and then store it which can be used by other function like Conn.Bind, but the code defer luc.client.Close() will break it and could case concurrency problem, so I remove it.
There was a problem hiding this comment.
do we sure that luc.client.Conn.Bind is goroutine safe.
There was a problem hiding this comment.
it should be safe according to https://github.com/go-ldap/ldap/blob/master/ldap_test.go#L198, and I also did a test which shows safe.
* add LDAP mode for basic authentication * hold connection to avoid concurrenty problem
No description provided.