[FEATURE] multiple ldap backend support #63
Comments
|
Hi, @V1pr I started developing this feature. Please feel free to test it and give me any feedback. The changes are in the branch multiple-ldap-backends. There are some use cases in the examples directory. |
thanks, that was quick :) Tried to test if, but was not able to put it in my setup (k8s). Any suggestions how to refer the branch? Or only localplugins will work (somehow)? |
Use the pre-release v0.1.9-beta. |
Thanks, this works! First of all, thanks for your work! The auth is working, but I found some small issues:
|
Fixed, I created a
For now, I'm just sorting the backends based on their Lines 155 to 157 in 37e8653
You should get a entry in log to The old parameters must be presented in the config to prevent old configurations from breaking. If the middleware detects old settings, they will be converted automatically.
This point is weird. You should get an error because both servers returned an error. The The Lines 218 to 236 in 37e8653 But you should be caught in the I will try to check with more details when I have some time. |
|
Hello, @V1pr Please, test again with the release v0.1.9-beta.3. |
All working, as it should. I've tested with 2 servers, all cases were ok (even if it's a TLS connect error). I couldn't reproduce the panic situation. As about my previous point 3): yes, I've seen the "dumped" ServerList. However it's a bit misleading, that the ServerList is just dumped, not prettified, however the old pretty paramslist are empty. I know this is just a "cosmetic issue", but still, it can be frustrating for other users. IMHO the TLSVersion should get a default value (tls.VersionTLS11), if not set to get rid of the WARNING. The only thing I would add, is to print out the server's URL, which did the auth: like: |
Glad to know. @fcinqmars can you review this feature? This will be a break change as we modify how LDAP backends are provided in the configuration. |
|
In my opinion, anything related to post-connection steps (bind, authorization, etc) should be configured globally for all servers in ServerList to ensure consistency in the authorization flow. With the current implementation, we could end up with a scenario where different servers are configured to allow different groups/users depending on which LDAP server is used for a same domain (replicas). I think supporting multiple backend servers makes sense as long as the servers are replicas of each other. As per @V1pr's initial message, it does look like that this is their use case (same root domain, same base DN) ldapAuth:
- name: ldap01
Attribute: uid
BaseDn: dc=example,dc=org
Enabled: "true"
LogLevel: DEBUG
Port: "389"
Url: ldap://ldap01.example.com
weight: 1
- name: ldap02
Attribute: uid
BaseDn: dc=example,dc=org
Enabled: "true"
LogLevel: DEBUG
Port: "389"
Url: ldap://ldap02.example.com
weight: 2With that in mind, the parameters below should probably be global to the plugin instead of being server-dependent.
On a less critical note, the TLS settings could also be left global. The plugin's LDAP client could be configured to apply the same settings such as:
The goal is to ensure the plugin behaves as consistently as possible, so we don't introduce security risks down the line. |
I agree that these parameters should be global to ensure consistency in the authorization flow. On the other hand, I think that all connection-related parameters should be per server since, from previous experiences, it's possible to have a production server with TLS configured but a replica without that. So the following parameters should be server-dependent in
Can we proceed with these definitions, @fcinqmars @V1pr ? |
Yes, this looks good and logical! Usually the servers run with the same config, but if anything happens - from an operational point of view - it's good that the connection security related settings can be changed per server. Just to confirm @fcinqmars assumption, we're using replicas for redundancy (that's why I've asked if same weighting means round-robin) |
Yeah, that's good for me too. |
Nice, I will update the code and update #65. |
Since I did not find any related info I'm opening this ticket. We're using multiple (redundant) LDAP servers, it would be nice if all were configurable, like:
The text was updated successfully, but these errors were encountered: