fix(ldap): Escape search base correctly - [merged] #113

Closed famedly-bot closed 1 year ago

In GitLab by @agraven on Aug 8, 2022, 10:44

Merges agraven/ldap-escape-2 -> main

Should fix #23

In GitLab by @agraven on Aug 8, 2022, 10:44

requested review from @jdreichmann

In GitLab by @agraven on Aug 8, 2022, 10:44

This one needs extra close scrutiny, the logic for what needs to be escaped when is a little tricky

In GitLab by @agraven on Aug 8, 2022, 12:00

added 1 commit

e1c09e3e - fixup the escape

Compare with previous version

In GitLab by @jdreichmann on Aug 8, 2022, 16:37

For documentation: replacing /\\2C/gi with "\\\," works - this needs to be added and tagged

In GitLab by @agraven on Aug 8, 2022, 17:15

added 1 commit

952cbf93 - fix(ldap): Escape search base correctly

Compare with previous version

In GitLab by @agraven on Aug 8, 2022, 17:41

added 1 commit

d22a1dc6 - fix(ldap): Escape search base correctly

Compare with previous version

In GitLab by @agraven on Aug 9, 2022, 08:28

@jdreichmann Just to confirm, the current state is what you had in mind for the \2C substition, right? If so, this should be ready for final review

In GitLab by @jdreichmann on Aug 9, 2022, 09:42

Yes, this works:


Aug-9 07:41:55.340 [Webserver] debug: Creating new session
Aug-9 07:41:55.345 [StageHandler (login)] info: Got request
Aug-9 07:41:55.345 [StageHandler (login)] info: Requesting stage m.login.password...
Aug-9 07:41:55.357 [StageHandler (login)] info: Stage is valid
Aug-9 07:41:55.361 [PasswordProvider Ldap] info: Checking password for lkaiser...
Aug-9 07:41:55.362 [PasswordProvider Ldap] verbose: getLoginInfo: start for lkaiser
Aug-9 07:41:55.371 [PasswordProvider Ldap] verbose: Binding to LDAP using configured bindDN....
Aug-9 07:41:55.403 [PasswordProvider Ldap] verbose: ldap: search subtree=dc=dev-alpha,dc=famedly,dc=de for user=lkaiser using filter (&(|(objectClass=inetOrgPerson))(uid=lkaiser)(enabled=TRUE))
Aug-9 07:41:55.437 [PasswordProvider Ldap] verbose: ldap: found one user for lkaiser with dn=cn=Kaiser\2C Lars,ou=intern,ou=users,dc=dev-alpha,dc=famedly,dc=de
Aug-9 07:41:55.439 [PasswordProvider Ldap] verbose: ldap: found entry for user=lkaiser: {"dn":"cn=Kaiser\\2C Lars,ou=intern,ou=users,dc=dev-alpha,dc=famedly,dc=de","utf8":{"uid":"lkaiser"},"raw":{"uid":{"type":"Buffer","data":[108,107,97,105,115,101,114]}}}
Aug-9 07:41:55.442 [PasswordProvider Ldap] verbose: ldap: Binding as "cn=Kaiser\2C Lars,ou=intern,ou=users,dc=dev-alpha,dc=famedly,dc=de" for user=lkaiser
Aug-9 07:41:55.447 [StageHandler (login)] info: Handling GET endpoint...
Aug-9 07:41:55.459 [PasswordProvider Ldap] verbose: ldap: Bound successfully for user=lkaiser as cn=Kaiser\2C Lars,ou=intern,ou=users,dc=dev-alpha,dc=famedly,dc=de
Aug-9 07:41:55.467 [PasswordProvider Ldap] verbose: getLoginInfo: login for user=lkaiser succeeded with dn=cn=Kaiser\2C Lars,ou=intern,ou=users,dc=dev-alpha,dc=famedly,dc=de
Aug-9 07:41:55.469 [PasswordProvider Ldap] info: Successfully authenticated user
Aug-9 07:41:55.470 [StageHandler (login)] info: Stage got completed
Aug-9 07:41:55.472 [StageHandler (login)] info: Successfully identified, passing on request!
Aug-9 07:41:55.473 [Api] info: Received login request
Aug-9 07:41:55.473 [Api] verbose: Session seems valid, attempting login with matrix server...
Aug-9 07:41:55.474 [Api] verbose: Generating token for lkaiser...
Aug-9 07:41:55.529 [Api] info: Successfully logged in!
Aug-9 07:41:55.530 [Api] verbose: Checking if name should be updated
Aug-9 07:41:55.543 [Api] verbose: Name does not need to be updated
Aug-9 07:42:18.065 [StageHandler (login)] info: Handling GET endpoint...
Aug-9 07:42:18.189 [Webserver] debug: Creating new session
Aug-9 07:42:18.190 [StageHandler (login)] info: Got request
Aug-9 07:42:18.191 [StageHandler (login)] info: Requesting stage m.login.password...
Aug-9 07:42:18.192 [StageHandler (login)] info: Stage is valid
Aug-9 07:42:18.193 [StageHandler (login)] info: User didn't manage to complete this stage
Aug-9 07:42:18.196 [StageHandler (login)] info: Fetching parameters...
Aug-9 07:42:24.744 [Webserver] debug: Creating new session
Aug-9 07:42:24.744 [StageHandler (login)] info: Got request
Aug-9 07:42:24.744 [StageHandler (login)] info: Requesting stage m.login.password...
Aug-9 07:42:24.749 [StageHandler (login)] info: Stage is valid
Aug-9 07:42:24.754 [PasswordProvider Ldap] info: Checking password for jreichmann...
Aug-9 07:42:24.756 [PasswordProvider Ldap] verbose: getLoginInfo: start for jreichmann
Aug-9 07:42:24.758 [PasswordProvider Ldap] verbose: Binding to LDAP using configured bindDN....
Aug-9 07:42:24.768 [PasswordProvider Ldap] verbose: ldap: search subtree=dc=dev-alpha,dc=famedly,dc=de for user=jreichmann using filter (&(|(objectClass=inetOrgPerson))(uid=jreichmann)(enabled=TRUE))
Aug-9 07:42:24.789 [PasswordProvider Ldap] verbose: ldap: found one user for jreichmann with dn=cn=Reichmann\2C Johanna,ou=intern,ou=users,dc=dev-alpha,dc=famedly,dc=de
Aug-9 07:42:24.795 [PasswordProvider Ldap] verbose: ldap: found entry for user=jreichmann: {"dn":"cn=Reichmann\\2C Johanna,ou=intern,ou=users,dc=dev-alpha,dc=famedly,dc=de","utf8":{"uid":"jreichmann"},"raw":{"uid":{"type":"Buffer","data":[106,114,101,105,99,104,109,97,110,110]}}}
Aug-9 07:42:24.796 [PasswordProvider Ldap] verbose: ldap: Binding as "cn=Reichmann\2C Johanna,ou=intern,ou=users,dc=dev-alpha,dc=famedly,dc=de" for user=jreichmann
Aug-9 07:42:24.816 [PasswordProvider Ldap] verbose: ldap: Bound successfully for user=jreichmann as cn=Reichmann\2C Johanna,ou=intern,ou=users,dc=dev-alpha,dc=famedly,dc=de
Aug-9 07:42:24.829 [PasswordProvider Ldap] verbose: getLoginInfo: login for user=jreichmann succeeded with dn=cn=Reichmann\2C Johanna,ou=intern,ou=users,dc=dev-alpha,dc=famedly,dc=de
Aug-9 07:42:24.833 [PasswordProvider Ldap] info: Successfully authenticated user
Aug-9 07:42:24.834 [StageHandler (login)] info: Stage got completed
Aug-9 07:42:24.835 [StageHandler (login)] info: Successfully identified, passing on request!
Aug-9 07:42:24.836 [Api] info: Received login request
Aug-9 07:42:24.837 [Api] verbose: Session seems valid, attempting login with matrix server...
Aug-9 07:42:24.837 [Api] verbose: Generating token for jreichmann...
Aug-9 07:42:24.871 [Api] info: Successfully logged in!
Aug-9 07:42:24.872 [Api] verbose: Checking if name should be updated
Aug-9 07:42:24.883 [Api] verbose: Name does not need to be updated
Aug-9 07:42:25.538 [StageHandler (login)] info: Handling GET endpoint...

In GitLab by @jdreichmann on Aug 9, 2022, 09:52

Commented on src/passwordproviders/passwordprovider_ldap.ts line 341

Maybe documenting what those bytes are with inline comments would help

In GitLab by @jdreichmann on Aug 9, 2022, 09:52

Commented on src/passwordproviders/passwordprovider_ldap.ts line 317

Same here, short part about why a byte f.ex. is escaped that way

In GitLab by @agraven on Aug 9, 2022, 10:32

added 1 commit