search

datasektionen / hodis

User lookup and information management system
https://hodis.datasektionen.se
0 stars 0 forks source link

500 returned when no users are found because of ldap timeout #9

Open mattiasgrenfeldt opened 3 years ago

mattiasgrenfeldt commented 3 years ago

Maybe 404 should be returned instead. It would semantically make more sense. But still return 500 on other errors.

oskstr commented 3 years ago

If no users are found, surely [] should be returned? That is what happens when no users are found and it doesn't timeout. This issue is currently causing a runtime exception in datasektionen/zfinger.

I was gonna open an issue about this. I'm having a really hard time explaining what is causing the timeout in some cases.

I'm talking specifically about https://hodis.datasektionen.se/users/<query> btw.

I noticed that I got a timeout with ../users/cda . 

{
    "error": "LDAP search failed: Paged search failed: LDAP Result Code 3 \"Time Limit Exceeded\": "
}

So my first thoughts were either:

  1. No results
  2. Too many results

But I found 1 result with .../users/cdab so it shouldn't be 1.

But then I also tried .../users/(, since it looks like ".. LOWER(cn) LIKE ?" was part of the query and I think all cn contains (. And then I got 252905 results with no issues. I even looked through the results and found 62 matches for the cda query.

Edit: There is even an exact match to cda as a uid btw. .../users/cd also craches but .../users/c and .../users/da works fine.

addreas commented 3 years ago

So this was a long time ago, but I think the confusion might be caused by this: https://github.com/datasektionen/hodis/blob/e19e79539eccb156466f5711f84e8bf8cd974484/ldap/ldap.go#L50

There is both a check on the exact query string, and a threshold of 1000 results from the local DB that will prevent the LDAP search from happening. I think LDAP queries were painfully slow, and this was a hack to try to prevent the "Time Limit Exceeded" which would happen for most queries otherwise.

I guess 62 matches for cda doesn't trigger the 1000 DB result threshold, and since the LDAP query fails this line is never reached so the problem persists: https://github.com/datasektionen/hodis/blob/e19e79539eccb156466f5711f84e8bf8cd974484/ldap/ldap.go#L62 Perhaps that above the LDAP search would be an improvement?

There are probably better solutions, but I couldn't thing of any at the time. This was kind of a "good enough" solution when I couldn't come up with something more "correct". I know I considered handling the timeout by generating and running more queries that were more exact in an attempt to enumerate the LDAP database, but that felt a bit too ridiculous.