I was asked to review this code before we put it into our production environment. Here are the issues I found:
The code is not thread safe. Why? All the ldap calls are done asynchronously. Data structures are modified that are not locked. Here is an example: in ngx_http_auth_ldap_search after the call to ldap_search_ext is made the state is set ctx->c->state = STATE_SEARCHING;.
The problem is that this state is used in the ldap callback to identify what to do with the results. If this was all done on a single thread it would never work. The fact that it is multi threaded means it has a chance of not working.
Inefficient ldap searches. In our case valid users can be in one of several groups. With ldap you can do an or filter with |((...)(...)(...)). This code is hardcoded to only do AND lookups with one condition. We would have to setup our configuration with multiple "servers" in order to search each group, ultimately putting more load on the ldap servers than we should.
I was asked to review this code before we put it into our production environment. Here are the issues I found:
ngx_http_auth_ldap_searchafter the call toldap_search_extis made the state is setctx->c->state = STATE_SEARCHING;.The problem is that this state is used in the ldap callback to identify what to do with the results. If this was all done on a single thread it would never work. The fact that it is multi threaded means it has a chance of not working.