NIHVIVO-1207 Simplify the Authenticator interface.

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<String> 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.
+	 * <pre>
+	 * 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
+	 * </pre>
 	 */
-	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);
 }

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) {
+	public void recordUserIsLoggedIn(String username) {
-		user.setLoginCount(user.getLoginCount() + 1);
+		User user = getUserByUsername(username);
-		if (user.getFirstTime() == null) { // first login
+		if (user == null) {
-			user.setFirstTime(new Date());
+			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.
 	 * 

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);
-
+		getAuthenticator(request).recordUserIsLoggedIn(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.
 		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.");
-
+		getAuthenticator(request).recordNewPassword(username, newPassword);
-		// TODO these should be a single call to Authenticator.
+		getAuthenticator(request).recordUserIsLoggedIn(username);
-		User user = getAuthenticator(request).getUserByUsername(username);
+		LoginProcessBean.removeBean(request);
-		getAuthenticator(request).recordNewPassword(user, newPassword);
-
-		transitionToLoggedIn(request, username);
 	}
 
 	/**

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<String, User> usersByName = new HashMap<String, User>();
 	private final Map<String, List<String>> editingPermissions = new HashMap<String, List<String>>();
 	private final List<String> recordedLogins = new ArrayList<String>();
-	private final List<String> loginSessions = new ArrayList<String>();
 	private final Map<String, String> newPasswords = new HashMap<String, String>();
 
 	private HttpServletRequest request;
@@ -100,10 +98,6 @@ public class AuthenticatorStub extends Authenticator {
 		return newPasswords;
 	}
 
-	public Collection<? extends String> getLoginSessions() {
-		return loginSessions;
-	}
-
 	// ----------------------------------------------------------------------
 	// Stub methods
 	// ----------------------------------------------------------------------
@@ -129,8 +123,8 @@ public class AuthenticatorStub extends Authenticator {
 	}
 
 	@Override
-	public void recordNewPassword(User user, String newClearTextPassword) {
+	public void recordNewPassword(String username, String newClearTextPassword) {
-		newPasswords.put(user.getUsername(), newClearTextPassword);
+		newPasswords.put(username, newClearTextPassword);
 	}
 
 	@Override
@@ -144,17 +138,13 @@ public class AuthenticatorStub extends Authenticator {
 	}
 
 	@Override
-	public void recordSuccessfulLogin(User user) {
+	public void recordUserIsLoggedIn(String username) {
-		recordedLogins.add(user.getUsername());
+		recordedLogins.add(username);
-	}
 
-	@Override
+		User user = getUserByUsername(username);
-	public void setLoggedIn(User user) {
+		LoginStatusBean lsb = new LoginStatusBean(user.getURI(), username,
-		LoginStatusBean lsb = new LoginStatusBean(user.getURI(),
+				parseUserSecurityLevel(user.getRoleURI()));
-				user.getUsername(), parseUserSecurityLevel(user.getRoleURI()));
 		LoginStatusBean.setBean(request.getSession(), lsb);
-
-		loginSessions.add(user.getUsername());
 	}
 
 	private static final String ROLE_NAMESPACE = "role:/";

webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java

@@ -400,11 +400,7 @@ public class AuthenticateTest extends AbstractTestClass {
 
 		Set<String> actualRecorded = new HashSet<String>(
 				authenticator.getRecordedLoginUsernames());
-		assertEquals("login recorded on user", expected, actualRecorded);
+		assertEquals("recorded logins", expected, actualRecorded);
-
-		Set<String> actualSessions = new HashSet<String>(
-				authenticator.getLoginSessions());
-		assertEquals("login sessions", expected, actualSessions);
 	}
 
 	/** Boilerplate login process for the rediret tests. */