Menu

#238 HTTP session authenticates for every table cell

Major
closed
MRBS (195)
5
2012-08-08
2012-04-05
dwpoon
No

MRBS 1.4.8. When generating a table view (e.g. day or week view), the table is generated by calling cell_html() for every cell. The cell_html() function calls getUserName(), which, despite its name, actually verifies the password by calling authValidateUser().

If using HTTP sessions and LDAP authentication, generating a day view for an area with 10 rooms and 30 time slots takes over 25 seconds. The effect of ~ 300 LDAP connects and binds on performance is brutal. Ideally, there should only be one connect and bind to LDAP.

A quick hack is to add a static $cachedUserName to getUserName() in session_http.inc. If authValidUser($user,getAuthPassword()) succeeds, then set $cachedUserName=$user. It's not architecturally elegant, but it does bring the response time down to ~ 400 ms when the right password is given. A more appropriate solution would be to lift the invariant out of the table-cell loop. Another solution would be to separate authValidateUser() from getUserName() — the cell just wants to know whether the user should be presented with the UI elements to initiate editing of an entry, which is not in itself an act that needs to be verified. There is still the edit screen and the submission of that form.

Discussion

  • dwpoon

    dwpoon - 2012-04-05

    Quick hack

     
  • Campbell Morrison

    Thanks for spotting this. I'll have a closer look at this later, but my initial reaction is that there may well be other places where getUserName() is called multiple times on the assumption that there's a negligible performance penalty, so it would be better to do something with getUserName() rather than cell_html().

    Campbell

     
  • Campbell Morrison

    Fixed in Rev 2281

    Campbell

     
  • John Beranek

    John Beranek - 2012-08-01
    • assigned_to: Campbell Morrison
     
  • Campbell Morrison

    • status: open --> closed
     
  • Campbell Morrison

    The fix will appear in MRBS 1.4.9