search

vivo-project / VIVO

VIVO is an extensible semantic web application for research discovery and showcasing scholarly work
http://vivoweb.org
BSD 3-Clause "New" or "Revised" License
205 stars 129 forks source link

AdminLoginController throws NPE if nonexistent account is requested #3888

Open brianjlowe opened 1 year ago

brianjlowe commented 1 year ago

Describe the bug AdminLoginController -- a special login controller not linked from the UI for bypassing SSO -- tries to retrieve a user account object and then attempts to use the object before checking whether null was returned. A client started receiving emails with the NPE when something attempted to post to this controller with invalid credentials.

To Reproduce Steps to reproduce the behavior:

  1. Go to /admin/login
  2. Enter invalid email and password\
  3. Observe error in logs:
  4. java.lang.NullPointerException at edu.cornell.mannlib.vitro.webapp.controller.authenticate.BasicAuthenticator.md5HashIsNull(BasicAuthenticator.java:103) at edu.cornell.mannlib.vitro.webapp.controller.authenticate.AdminLoginController$Core.newPasswordRequired(AdminLoginController.java:144) at edu.cornell.mannlib.vitro.webapp.controller.authenticate.AdminLoginController$Core.process(AdminLoginController.java:113) at edu.cornell.mannlib.vitro.webapp.controller.authenticate.AdminLoginController.processRequest(AdminLoginController.java:66) at edu.cornell.mannlib.vitro.webapp.controller.freemarker.FreemarkerHttpServlet.doGet(FreemarkerHttpServlet.java:107) at edu.cornell.mannlib.vitro.webapp.controller.freemarker.FreemarkerHttpServlet.doPost(FreemarkerHttpServlet.java:197) at javax.servlet.http.HttpServlet.service(HttpServlet.java:652) at javax.servlet.http.HttpServlet.service(HttpServlet.java:733) at edu.cornell.mannlib.vitro.webapp.controller.VitroHttpServlet.service(VitroHttpServlet.java:71) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166) at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)

Expected behavior Incorrect username/password message should be displayed.

Additional context The call to .getAccountForInternalAuth() on line 97 returns null if account doesn't exist. The call to md5HashIsNull() on line 144 passes the null pointer to the BasicAuthenticator, which then throws the NPE.

brianjlowe commented 1 year ago

At first glance it looks like this may have been introduced back when the password hashing was changed to Argon2. In this particular controller, the check for an un-upgraded account with obsolete MD5 hash may be occurring in the wrong place.