diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/Authenticator.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/Authenticator.java index 65f17d7eb..38683164d 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/Authenticator.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/Authenticator.java @@ -62,23 +62,26 @@ public abstract class Authenticator { public abstract User getUserByUsername(String username); /** - * Return a list of URIs of the people that this user is allowed to edit. + * Get a list of URIs of the people that this user is allowed to edit. */ public abstract List asWhomMayThisUserEdit(User user); /** * Record a new password for the user. */ - public abstract void recordNewPassword(User user, + public abstract void recordNewPassword(String username, String newClearTextPassword); /** - * Record that the user has logged in. + *
+	 * Record that the user has logged in, with all of the housekeeping that 
+	 * goes with it:
+	 * - updating the user record
+	 * - setting login status and timeout limit in the session
+	 * - record the user in the session map
+	 * - notify other users of the model
+	 * 
*/ - public abstract void recordSuccessfulLogin(User user); + public abstract void recordUserIsLoggedIn(String username); - /** - * Set the login status in the session. - */ - public abstract void setLoggedIn(User user); } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java index 8ed97663b..196be4da3 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java @@ -66,24 +66,30 @@ public class BasicAuthenticator extends Authenticator { } @Override - public void recordNewPassword(User user, String newClearTextPassword) { + public void recordNewPassword(String username, String newClearTextPassword) { + User user = getUserByUsername(username); + if (user == null) { + log.error("Trying to change password on non-existent user: " + + username); + return; + } user.setOldPassword(user.getMd5password()); user.setMd5password(Authenticate.applyMd5Encoding(newClearTextPassword)); getUserDao(request).updateUser(user); } @Override - public void recordSuccessfulLogin(User user) { - user.setLoginCount(user.getLoginCount() + 1); - if (user.getFirstTime() == null) { // first login - user.setFirstTime(new Date()); + public void recordUserIsLoggedIn(String username) { + User user = getUserByUsername(username); + if (user == null) { + log.error("Trying to change password on non-existent user: " + + username); + return; } - getUserDao(request).updateUser(user); - } - @Override - public void setLoggedIn(User user) { HttpSession session = request.getSession(); + + recordLoginOnUserRecord(user); createLoginFormBean(user, session); createLoginStatusBean(user, session); setSessionTimeoutLimit(session); @@ -91,6 +97,17 @@ public class BasicAuthenticator extends Authenticator { notifyOtherUsers(user, session); } + /** + * Update the user record to record the login. + */ + private void recordLoginOnUserRecord(User user) { + user.setLoginCount(user.getLoginCount() + 1); + if (user.getFirstTime() == null) { // first login + user.setFirstTime(new Date()); + } + getUserDao(request).updateUser(user); + } + /** * Put the login bean into the session. * diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Authenticate.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Authenticate.java index 946388020..1d9281622 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Authenticate.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Authenticate.java @@ -261,6 +261,7 @@ public class Authenticate extends FreemarkerHttpServlet { /** * They are already logged in. There's nothing to do; no transition. */ + @SuppressWarnings("unused") private void processInputLoggedIn(HttpServletRequest request) { } @@ -298,16 +299,7 @@ public class Authenticate extends FreemarkerHttpServlet { private void transitionToLoggedIn(HttpServletRequest request, String username) { log.debug("Completed login: " + username); - - // Record the login on the user record (start with a fresh copy). - // TODO All this should be a single call to Authenticator. - User user = getAuthenticator(request).getUserByUsername(username); - getAuthenticator(request).recordSuccessfulLogin(user); - - // Record that a new user has logged in to this session. - getAuthenticator(request).setLoggedIn(user); - - // Remove the login process info from the session. + getAuthenticator(request).recordUserIsLoggedIn(username); LoginProcessBean.removeBean(request); } @@ -318,12 +310,9 @@ public class Authenticate extends FreemarkerHttpServlet { private void transitionToLoggedIn(HttpServletRequest request, String username, String newPassword) { log.debug("Completed login: " + username + ", password changed."); - - // TODO these should be a single call to Authenticator. - User user = getAuthenticator(request).getUserByUsername(username); - getAuthenticator(request).recordNewPassword(user, newPassword); - - transitionToLoggedIn(request, username); + getAuthenticator(request).recordNewPassword(username, newPassword); + getAuthenticator(request).recordUserIsLoggedIn(username); + LoginProcessBean.removeBean(request); } /** diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/authenticate/AuthenticatorStub.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/authenticate/AuthenticatorStub.java index 254244d0a..bca786164 100644 --- a/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/authenticate/AuthenticatorStub.java +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/authenticate/AuthenticatorStub.java @@ -4,7 +4,6 @@ package edu.cornell.mannlib.vitro.webapp.controller.authenticate; import java.lang.reflect.Field; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -72,7 +71,6 @@ public class AuthenticatorStub extends Authenticator { private final Map usersByName = new HashMap(); private final Map> editingPermissions = new HashMap>(); private final List recordedLogins = new ArrayList(); - private final List loginSessions = new ArrayList(); private final Map newPasswords = new HashMap(); private HttpServletRequest request; @@ -100,10 +98,6 @@ public class AuthenticatorStub extends Authenticator { return newPasswords; } - public Collection getLoginSessions() { - return loginSessions; - } - // ---------------------------------------------------------------------- // Stub methods // ---------------------------------------------------------------------- @@ -129,8 +123,8 @@ public class AuthenticatorStub extends Authenticator { } @Override - public void recordNewPassword(User user, String newClearTextPassword) { - newPasswords.put(user.getUsername(), newClearTextPassword); + public void recordNewPassword(String username, String newClearTextPassword) { + newPasswords.put(username, newClearTextPassword); } @Override @@ -144,17 +138,13 @@ public class AuthenticatorStub extends Authenticator { } @Override - public void recordSuccessfulLogin(User user) { - recordedLogins.add(user.getUsername()); - } + public void recordUserIsLoggedIn(String username) { + recordedLogins.add(username); - @Override - public void setLoggedIn(User user) { - LoginStatusBean lsb = new LoginStatusBean(user.getURI(), - user.getUsername(), parseUserSecurityLevel(user.getRoleURI())); + User user = getUserByUsername(username); + LoginStatusBean lsb = new LoginStatusBean(user.getURI(), username, + parseUserSecurityLevel(user.getRoleURI())); LoginStatusBean.setBean(request.getSession(), lsb); - - loginSessions.add(user.getUsername()); } private static final String ROLE_NAMESPACE = "role:/"; diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java index b77c42487..8e42f700b 100644 --- a/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java @@ -400,11 +400,7 @@ public class AuthenticateTest extends AbstractTestClass { Set actualRecorded = new HashSet( authenticator.getRecordedLoginUsernames()); - assertEquals("login recorded on user", expected, actualRecorded); - - Set actualSessions = new HashSet( - authenticator.getLoginSessions()); - assertEquals("login sessions", expected, actualSessions); + assertEquals("recorded logins", expected, actualRecorded); } /** Boilerplate login process for the rediret tests. */