2010-03-23 Code Review

<html><head><title></title></head><body>h1. Call-in Information

Call*712-432-0075 or Skype a voice call to freeconferencing
Enter the code*305801

Tickets to review

Review Notes for ticket 476

  • wording issue on liquibase-update-to-latest.xml:3117 — remove "must" from expectedResult

  • wording issue on liquibase-update-to-latest.xml:3135 — remove "must" and change to plural form ("it has" → "they have")

  • only reviewed the last updated patch, since Darius had reviewed everything else

Review Notes for ticket 1748

  • HibernateContextDAO: all String comparisons should use .equals()

  • HibernateContextDAO.authenticateUser() is unnecessary

    • should use UserService.getUserByUsername() for retrieving a user

    • need to use proxy privileges in order to get the user (see Context.addProxyPrivilege() and Context.removeProxyPrivilege())

    • change the method call in UserContext.authenticate()

  • HibernateContextDAO.authenticatePassword() should be split into two methods:

    • String getHashPassword(User u)

    • String getSalt(User u)

  • HibernateContextDAO.authenticateLockout() is not necessary, and is a security risk (being public):

    • use proxy privileges with UserService.saveUser() instead

    • change calls in HibernateContextDAO.authenticate()

  • UserContext: remove global property import from WebConstants; not used or required

  • make sure the archive:blank username changeset #12413 is incorporated

    • changes to ContextDAO → UserContext.authenticate()

    • changes to HibernateContextDAO.authenticate() → UserContext.authenticate()

    • new test in ContextDAOTest has to refer to UserContext.authenticate() instead of dao.authenticate()

Other to do items

</body></html>