Index: lams_central/src/java/org/lamsfoundation/lams/web/ShibLoginServlet.java
===================================================================
RCS file: /usr/local/cvsroot/lams_central/src/java/org/lamsfoundation/lams/web/Attic/ShibLoginServlet.java,v
diff -u -r1.1.2.5 -r1.1.2.6
--- lams_central/src/java/org/lamsfoundation/lams/web/ShibLoginServlet.java	12 Jul 2007 02:53:11 -0000	1.1.2.5
+++ lams_central/src/java/org/lamsfoundation/lams/web/ShibLoginServlet.java	12 Jul 2007 06:25:36 -0000	1.1.2.6
@@ -21,7 +21,6 @@
 package org.lamsfoundation.lams.web;
 
 import java.io.IOException;
-import java.net.URL;
 import java.util.Date;
 import java.util.Enumeration;
 
@@ -83,46 +82,54 @@
 		String identityProvider = request.getHeader("Shib-Identity-Provider");
 		
 		if (username != null && username.trim().length()>0) {
-			User user = service.getUserByLogin(username);
+			// prefix new usernames with their origin server's fedId so as not to mix up with local usernames
+			String prefixedUsername = null;
+			try {
+				prefixedUsername = getUsername(username, identityProvider);
+			} catch (FederationException e) {
+				flagError(request, response);
+			}
+			if (prefixedUsername == null) {
+				flagError(request, response);
+			}
+			
+			User user = service.getUserByLogin(prefixedUsername);
 			if (user != null) {
 				// user is authenticated by Shibboleth IdP
-				response.sendRedirect("j_security_check?j_username="+username+"&j_password="+user.getPassword());
+				response.sendRedirect("j_security_check?j_username="+prefixedUsername+"&j_password="+user.getPassword());
 			} else {
 				// create user account for new shib user;
-				// prefix usernames with their origin server's fedId so as not to mix up with local usernames
-				String newUsername = null;
 				try {
-					newUsername = getUsername(username, identityProvider);
-				} catch (FederationException e) {
-					request.getSession().setAttribute("shibLoginError", "true");
-					response.sendRedirect("/lams/");
-				}
-				log.info("Creating new user: "+newUsername);
-				user = new User();
-				user.setLogin(newUsername);
-				user.setPassword("dummy");
-				if (firstname != null && firstname.trim().length()>0) {
-					user.setFirstName(firstname);
-				} else {
-					user.setFirstName(username);
-				}
-				if (lastname != null && lastname.trim().length()>0) {
-					user.setLastName(lastname);
-				} else {
-					user.setLastName(username);
-				}
-				user.setEmail(username);
-				user.setAuthenticationMethod((AuthenticationMethod)
-						service.findById(AuthenticationMethod.class, AuthenticationMethod.DB));
-				user.setChangePassword(false);
-				user.setDisabledFlag(false);
-				user.setLocale(service.getDefaultLocale());
-				user.setFlashTheme(service.getDefaultFlashTheme());
-				user.setHtmlTheme(service.getDefaultHtmlTheme());
-				user.setCreateDate(new Date());
-				service.save(user);
+					log.info("Creating new user: "+prefixedUsername);
+					user = new User();
+					user.setLogin(prefixedUsername);
+					user.setPassword("dummy");
+					if (firstname != null && firstname.trim().length()>0) {
+						user.setFirstName(firstname);
+					} else {
+						user.setFirstName(username);
+					}
+					if (lastname != null && lastname.trim().length()>0) {
+						user.setLastName(lastname);
+					} else {
+						user.setLastName(username);
+					}
+					user.setEmail(username);
+					user.setAuthenticationMethod((AuthenticationMethod)
+							service.findById(AuthenticationMethod.class, AuthenticationMethod.DB));
+					user.setChangePassword(false);
+					user.setDisabledFlag(false);
+					user.setLocale(service.getDefaultLocale());
+					user.setFlashTheme(service.getDefaultFlashTheme());
+					user.setHtmlTheme(service.getDefaultHtmlTheme());
+					user.setCreateDate(new Date());
+					service.save(user);
 				
-				response.sendRedirect("j_security_check?j_username="+username+"&j_password=dummy");
+					response.sendRedirect("j_security_check?j_username="+username+"&j_password=dummy");
+				} catch (Exception e) {
+					log.error("Couldn't save new user with username: "+prefixedUsername);
+					flagError(request, response);
+				}
 			}
 		} else {
 			// somehow lost shib username attribute, send back to login page
@@ -131,6 +138,11 @@
 		}
 	}
 	
+	private void flagError(HttpServletRequest request, HttpServletResponse response) throws IOException {
+		request.getSession().setAttribute("shibLoginError", "true");
+		response.sendRedirect("/lams/");
+	}
+	
 	private String getHost(String url) {
 		if (url == null) {
 			log.error("Couldn't get host from url.");
@@ -145,14 +157,21 @@
 		}
 	}
 	
+	// produces a local version of shibboleth user's username; it is prefixed by the fedId of their origin server
 	private String getUsername(String username, String providerId) throws FederationException {
 		try {
 			String host = getHost(providerId);
 			FederationServer fedServer = fedService.getFedServerByURLHost(host);
-			return fedServer.getFedId()+username;
-		} catch (FederationException e) {
+			if (fedServer != null) {
+				log.debug("Shibboleth user "+username+" appears to come from fedServer with fedId "+fedServer.getFedId());
+				return fedServer.getFedId()+"_"+username;
+			}
+		} catch (Exception e) {
 			log.error("Couldn't create prefixed username: "+e);
 		}
+		// return nothing rather than original username so there is no chance of accidental login
+		// to another person's local account
 		return null;
 	}
-}
+
+}
\ No newline at end of file