kamax-matrix / mxisd

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

LDAP users from AD & case sensitivity #151

Closed tgurr closed 5 years ago

tgurr commented 5 years ago

We've recently discovered a problem. When searching for users e.g. to invite them to a room or a direct chat the results are displayed in upper case, e.g. @TESTUSER123:domain.local and an invite to any room or direct chat will fail/not reach the matrix user.

If the user previously logged in to matrix so a matrix id is already present the search result displays both, the one from AD @TESTUSER123:domain.local and the matrix one @testuser123:domain.local so users wonder why there are two results and the more urgent problem they often invite the wrong one.

Is there any option for mxisd to force the output/return of the user to be lowercase?

maxidorius commented 5 years ago

Given that mxisd uses the same attribute for all User Id computations, could you clarify how the users are created on the Matrix side? It doesn't make sense that in one case it would be lower case but in the other it would be upper case.

tgurr commented 5 years ago

I guess they're created automatically when logging in via a client like Riot the first time, at least I don't create any users manually.

Just having setup mxisd

ldap:
  enabled: true
  connection:
    host: 'dc.domain.local'
    port: 389
    bindDn: 'CN=matrixldapuser,OU=services,DC=domain,DC=local'
    bindPassword: 'xxx'
    baseDNs:
      - 'OU=users,DC=domain,DC=local'

and synapse like:

password_providers:
  - module: "rest_auth_provider.RestAuthProvider"
    config:
      endpoint: "http://localhost:8090"
      policy:
        registration:
          profile:
            name: true
        login:
          profile:
            name: true

Please let me know if I miss any useful information. My guess is that in matrix everything is lowercase, but the LDAP search from the identity server (mxisd) returns the sAMAccountName 1:1 like it is stored in AD instead of forcing the output to be lowercase. Logging in to Riot for example does only work when providing the login in lowercase and the matrix user also gets created fine in lowercase, it's just the search results comming from LDAP/AD that seem to be off.

maxidorius commented 5 years ago

You did not miss anything, it would seem like the REST password provider does not block upper case usernames, even tho it should.

Matrix IDs must be lower case as per the spec, and both synapse and the REST password provider have checks to ensure this kind of thing doesn't happen.

In terms of your AD and mxisd, you must use an attribute that has the username in lower case. mxisd doesn't change case on purpose, and will not. There are far-reaching issues if case is messed with.

I'll open an issue on the password provider repo to double check that the case check is happening correctly, but only you can ensure the LDAP attribute you choose as id in mxisd has lower case values.

tgurr commented 5 years ago

The matrix IDs do seem to get created in lowercase just fine and also the login works fine, it's just the search results messing things up. The problem is that we don't have any LDAP attribute which already is in lower case so we need to have a way to force lowercasing the username/sAMAccountName comming out of LDAP, either in the password provider or in mxisd. A (working) check would probably result in things not working for us anymore if I understood your answer correctly?

maxidorius commented 5 years ago

That's correct, if we were to change anything in mxisd for the v1.x releases, it would only to strip out results which are not strictly correct Matrix IDs/localparts, but that would also break many setups. Hence why it was never changed (this always was a very though trough choice).

This is something we want to tackle in more depth with the v2.x release. mxisd exists for more than 2 years now and many things changed in the ecosystem during this time, including related software like synapse, Riot and the spec in general.

At this time, I do not have a good solution for you I'm afraid. The best I can suggest is to find a way to have an attribute that gives lowercase results. Given our efforts with The Grid protocol, we can't plan anything regarding v2 releases.

tgurr commented 5 years ago

Would it be feasible for the time being to add an option somewhere in mxisd/matrix-synapse-rest-password-provider that can be set in mxisd.yaml which defaults to the way it is right now to not introduce any breakage but which would allow to just force lowercasing the incomming attribute from LDAP?

maxidorius commented 5 years ago

Give it a try by replacing the top key ldap: by netiq: in mxisd.yaml, restart, and see what happens? This might break awefuly, but it might work in your specific case.

tgurr commented 5 years ago

For a quick test it appears to work, thank you very much! I'll see if it'll cause any breakage in the comming days.