search

kamax-matrix / mxisd

Federated Matrix Identity Server
GNU Affero General Public License v3.0
220 stars 112 forks source link

LDAP Lookup - Support list of LDAP servers #7

Closed ReqX closed 6 years ago

ReqX commented 7 years ago

Hey, as discussed earlier, I'd like you to consider to have the ldap: section in the config to be a list just as the "password_providers:" section in matrix-synapse-ldap3 is currently (probably by coincidence).

This will enable us to a) query more than one ldap (domain) for a potential hit (in the order of your entry to the list item) or b) query more than one e.g. Active Directory/Samba controller (in the order of your entry). Of course for this you could just use the ad domain like domain.local and DNS will return (round robin) one controller, but it does not help if that server would be done (HA).

Currently there is no further handling in matrix-synapse-ldap3, it will stop with the first correct(!) hit as it's just a simple for each in the password_auth_providers[1] module. While this seems not optimal I think this behavior is acceptable on an auth-provider as the chance that two dirs have the same e.g. email and matching password/bind is probably nil and wouldnt change the effect that the user get's authenticated properly. I do not know if there would be a better way or even a need to differently in mxisd? Best result would be to e.g. on invite to new room members show all hits as a list of |email | |uid| in supporting clients. Just use Riot and search for e.g. "test" on invite new room members while on matrix.org to see that at least Riot supports that.

Let me know if I need to be more specific. Thx, cu Mike

[1] https://github.com/matrix-org/synapse/blob/f5a4001bb116c468cc5e8e0ae04a1c570e2cb171/synapse/config/password_auth_providers.py#L35 ,

maxidorius commented 7 years ago

The IS spec does not allow several mappings per 3PID, only one.
As we need to remain consistent with the LDAP auth of synapse, so the first hit would win, going in the order from the config file.

ReqX commented 7 years ago

Hi Max, sorry for confusion. It's not about several mappings but potential several hits on various ldap dirs and the possibility to choose which one e.g. to invite. I think it should be a very, very rare case (read: never?), I can only think of 2 ldap dirs using the same email domain for whatever reason, but even then I think you're right and it makes no sense, an email is just as unique as mappings. I think I got ahead of myself.

maxidorius commented 7 years ago

Having several mapping is fine, and I have no problem with it, but the Matrix spec only allows one mapping per lookup, and the lookup is performed on the server side, not the client side.
Basically, even if mxisd would return several mapping, you can only see one given the current Matrix protocols.

maxidorius commented 6 years ago

I've create #57 which is a generic issue coverage of this issue, as this could be applied to any potential identity store.