This is a silly question - is it possible to register "BryanDavis" as a uid? (probably not, because it has to be lower case?)

The main reason I'm asking is that I just want to ensure there's no possible collisions if we do fallbacks in the /user/ route. If collisions are possible, we can just introduce a new /cn/<name> route that redirects to /user/<name>.

In T376769#10212996, @Legoktm wrote:

This is a silly question - is it possible to register "BryanDavis" as a uid? (probably not, because it has to be lower case?)

Yes, someone could register a developer account with the uid bryandavis. Because LDAP lookups are case insensitive by default it would match a search for "BryanDavis" too. This would however be a different account than my uid=bd808 account which owns the cn BryanDavis.

The main reason I'm asking is that I just want to ensure there's no possible collisions if we do fallbacks in the /user/ route. If collisions are possible, we can just introduce a new /cn/<name> route that redirects to /user/<name>.

There are very much collisions possible (and actually existing) if the current route changed to be a cn lookup instead of a uid lookup:

• https://ldap.toolforge.org/user/bd808: uid=bd808, cn=BryanDavis
• https://ldap.toolforge.org/user/bender: uid=bender, cn=Bd808

Deployed https://gitlab.wikimedia.org/toolforge-repos/ldap/-/commit/99594edfa46508acc8d3f286472e6f2dbd8c08e9

$ curl -I 'https://ldap.toolforge.org/cn/BryanDavis'
HTTP/2 303 
server: nginx/1.22.1
date: Thu, 10 Oct 2024 01:01:39 GMT
content-length: 0
location: /user/bd808
...

legoktm opened https://gitlab.wikimedia.org/repos/phabricator/extensions/-/merge_requests/41

Fix ldap.toolforge.org link to use new /cn/ route

This still isn't quite right. Taking matmarex as an example, Phabricator links to https://ldap.toolforge.org/user/Bartosz%20Dziewo%C5%84ski but both that and https://ldap.toolforge.org/cn/Bartosz%20Dziewo%C5%84ski are 404s.

Hmm, they’re not even 404s, but 500s for me. Both the space and the ‘ń’ seem to cause that.

Yeah, they're 500s for me too. I saw an error and made a bad assumption.

brennen merged https://gitlab.wikimedia.org/repos/phabricator/extensions/-/merge_requests/41

Fix ldap.toolforge.org link to use new /cn/ route

In T376769#10223261, @matmarex wrote:

Hmm, they’re not even 404s, but 500s for me. Both the space and the ‘ń’ seem to cause that.

That's caused by https://gitlab.wikimedia.org/toolforge-repos/ldap/-/blob/0149bb4cec8878071fcab2931260e57ad43709d7/src/lookup.rs#L126, which I mostly just (incorrectly) copied from the same user route. I'll spend a bit of time figuring out actual LDAP query escaping instead of using a too-restrictive regex.

In T376769#10230611, @Legoktm wrote:

I'll spend a bit of time figuring out actual LDAP query escaping instead of using a too-restrictive regex.

https://stackoverflow.com/a/39805523/8171 gives one possible answer. The Python LDAP library I use most often has a escape_filter_chars function that can be used for escaping. https://docs.rs/ldap3/latest/ldap3/fn.ldap_escape.html looks like an equivalent in the Rust library you are using.

Thanks, just deployed: https://gitlab.wikimedia.org/toolforge-repos/ldap/-/commit/7b5baa2b2f25f2ba2e88070bff8319fb20a062f4

So now:

$ curl -I 'https://ldap.toolforge.org/cn/Bartosz%20Dziewo%C5%84ski'
HTTP/2 303 
server: nginx/1.22.1
date: Wed, 16 Oct 2024 23:08:50 GMT
content-length: 0
location: /user/matmarex
...

(I need to create a proper test suite for this tool now...)

In T376769#10255342, @Legoktm wrote:

The Phabricator part of this was deployed, so we should be all set here!

Thanks for the prompt attention @Legoktm!