Index: /main/trunk/greenstone3/src/java/org/greenstone/gsdl3/service/Authentication.java
===================================================================
--- /main/trunk/greenstone3/src/java/org/greenstone/gsdl3/service/Authentication.java	(revision 32389)
+++ /main/trunk/greenstone3/src/java/org/greenstone/gsdl3/service/Authentication.java	(revision 32390)
@@ -109,4 +109,5 @@
 		opList.add(PERFORM_ADD);
 		opList.add(PERFORM_EDIT);
+		opList.add(ADD_USER);
 		opList.add(EDIT_USER);
 		opList.add(PERFORM_DELETE_USER);
@@ -128,4 +129,6 @@
 		opList.add(PERFORM_ACCOUNT_EDIT);
 		opList.add(PERFORM_RESET_PASSWORD);
+		opList.add(PERFORM_CHANGE_PASSWORD);
+		opList.add(PERFORM_RETRIEVE_PASSWORD);
 		opList.addAll(_adminOpList);
 		_userOpList = opList;
@@ -145,7 +148,30 @@
 	protected static boolean _derbyWrapperDoneForcedShutdown = false;
 
+  // some XML strings
+  protected static final String RECAPTCHA_ELEM = "recaptcha";
+  protected static final String SITE_KEY = "site_key";
+   protected static final String SECRET_KEY = "secret_key";
+  protected static final String OPERATIONS = "operations";
+protected static final String OPERATION = "operation";
+  
+  protected static final String USERNAME = "username";
+  protected static final String PREV_USERNAME = "prevUsername";
+  protected static final String NEW_USERNAME = "newUsername";
+  protected static final String PASSWORD = "password";
+  protected static final String OLD_PASSWORD = "oldPassword";
+  protected static final String NEW_PASSWORD = "newPassword";
+
+  protected static final String GROUPS = "groups";
+  protected static final String ENABLED = "enabled";
+  protected static final String COMMENT = "comment";
+  protected static final String STATUS = "status";
+  protected static final String EMAIL = "email";
+  protected static final String NEW_EMAIL = "newEmail";
+  protected static final String ACCOUNT_STATUS = "accountstatus";
+  protected static final String EDIT_ENABLED = "editEnabled";
+  
 	protected String _recaptchaSiteKey = null;
 	protected String _recaptchaSecretKey = null;
-
+  protected static ArrayList<String> _recaptchaOpList = null;
 	/** constructor */
 	public Authentication()
@@ -204,24 +230,40 @@
 		DerbyWrapper.createDatabaseIfNeeded();
 
-		NodeList recaptchaElems = info.getElementsByTagName("recaptcha");
+		NodeList recaptchaElems = info.getElementsByTagName(RECAPTCHA_ELEM);
 		for (int i = 0; i < recaptchaElems.getLength(); i++)
 		{
 			Element currentElem = (Element) recaptchaElems.item(i);
-			if (currentElem.getAttribute(GSXML.NAME_ATT) != null && currentElem.getAttribute(GSXML.NAME_ATT).equals("site_key"))
-			{
-				if (currentElem.getAttribute(GSXML.VALUE_ATT) != null)
+			if (currentElem.getAttribute(GSXML.NAME_ATT).equals(SITE_KEY))
+			{
+			  if (!currentElem.getAttribute(GSXML.VALUE_ATT).equals(""))
 				{
 					_recaptchaSiteKey = currentElem.getAttribute(GSXML.VALUE_ATT);
 				}
 			}
-			else if (currentElem.getAttribute(GSXML.NAME_ATT) != null && currentElem.getAttribute(GSXML.NAME_ATT).equals("secret_key"))
-			{
-				if (currentElem.getAttribute(GSXML.VALUE_ATT) != null)
+			else if (currentElem.getAttribute(GSXML.NAME_ATT).equals(SECRET_KEY))
+			{
+			  if (!currentElem.getAttribute(GSXML.VALUE_ATT).equals(""))
 				{
 					_recaptchaSecretKey = currentElem.getAttribute(GSXML.VALUE_ATT);
 				}
 			}
-		}
-
+			else if (currentElem.getAttribute(GSXML.NAME_ATT).equals(OPERATIONS))
+			  {
+			    _recaptchaOpList = new ArrayList<String>();
+			    String value = currentElem.getAttribute(GSXML.VALUE_ATT);
+			    String[] ops = value.split(",");
+			    for (int j=0; j<ops.length; j++) {
+			      if (!ops[j].equals("")) {
+				_recaptchaOpList.add(ops[j]); /// value checking?
+			      }
+			    }
+			  }
+			
+		}
+		// check recaptcha
+		if (_recaptchaSecretKey == null || _recaptchaSecretKey.length() == 0 || _recaptchaSiteKey == null || _recaptchaSiteKey.length() == 0) {
+		  _recaptchaOpList = null;
+		}
+		  
 		return true;
 	}
@@ -301,6 +343,6 @@
 		HashMap<String, Serializable> params = GSXML.extractParams(paramList, true);
 
-		String username = (String) params.get("username");
-		String editMode = (String) params.get("enabled");
+		String username = (String) params.get(USERNAME);
+		String editMode = (String) params.get(ENABLED);
 
 		if (!editMode.toLowerCase().equals("true") && !editMode.toLowerCase().equals("false"))
@@ -332,6 +374,6 @@
 		
 		Element system = (Element) GSXML.getChildByTagName(request, GSXML.REQUEST_TYPE_SYSTEM);		
-		String username = system.hasAttribute("username") ? system.getAttribute("username") : "";
-		String password = system.hasAttribute("password") ? system.getAttribute("password") : "";
+		String username = system.hasAttribute(USERNAME) ? system.getAttribute(USERNAME) : "";
+		String password = system.hasAttribute(PASSWORD) ? system.getAttribute(PASSWORD) : "";
 		
 		
@@ -345,9 +387,9 @@
 		}		
 		else {		
-			String storedPassword = retrieveDataForUser(username, "password");
+			String storedPassword = retrieveDataForUser(username, PASSWORD);
 			if(storedPassword != null && (password.equals(storedPassword) || hashPassword(password).equals(storedPassword))) {
 				
 				// gliserver.pl used to return the groups when authentication succeeded
-				String groups = retrieveDataForUser(username, "groups"); //comma-separated list
+				String groups = retrieveDataForUser(username, GROUPS); //comma-separated list
 				
 				if(collection.equals("")) {
@@ -407,5 +449,5 @@
 		HashMap<String, Serializable> params = GSXML.extractParams(paramList, true);
 
-		String username = (String) params.get("username");
+		String username = (String) params.get(USERNAME);
 
 		if (username == null)
@@ -432,13 +474,13 @@
 		result.appendChild(userInfoList);
 
-		Element usernameField = GSXML.createParameter(result_doc, "username", userInfo.username);
-		Element passwordField = GSXML.createParameter(result_doc, "password", userInfo.password);
-		Element groupsField = GSXML.createParameter(result_doc, "groups", userInfo.groups);
-		Element accountStatusField = GSXML.createParameter(result_doc, "accountstatus", userInfo.accountstatus);
-		Element commentField = GSXML.createParameter(result_doc, "comment", userInfo.comment);
+		Element usernameField = GSXML.createParameter(result_doc, USERNAME, userInfo.username);
+		Element passwordField = GSXML.createParameter(result_doc, PASSWORD, userInfo.password);
+		Element groupsField = GSXML.createParameter(result_doc, GROUPS, userInfo.groups);
+		Element accountStatusField = GSXML.createParameter(result_doc, ACCOUNT_STATUS, userInfo.accountstatus);
+		Element commentField = GSXML.createParameter(result_doc, COMMENT, userInfo.comment);
 
 		if (editEnabled != null)
 		{
-			Element editEnabledElem = GSXML.createParameter(result_doc, "editEnabled", editEnabled);
+			Element editEnabledElem = GSXML.createParameter(result_doc, EDIT_ENABLED, editEnabled);
 			userInfoList.appendChild(editEnabledElem);
 		}
@@ -479,5 +521,5 @@
 		if (param_list == null)
 		{
-			serviceNode.setAttribute("operation", LOGIN);
+			serviceNode.setAttribute(OPERATION, LOGIN);
 			logger.error("Authentication request has no param list");
 			return result; // Return the empty result
@@ -485,5 +527,5 @@
 		HashMap<String, Serializable> paramMap = GSXML.extractParams(param_list, false);
 		String op = (String) paramMap.get("authpage");
-		serviceNode.setAttribute("operation", op);
+		serviceNode.setAttribute(OPERATION, op);
 
 		String username = null;
@@ -496,9 +538,9 @@
 			groups = userInformation.getAttribute(GSXML.GROUPS_ATT);
 		}
-
+		logger.error("username="+username+", groups = "+groups);
 		if ((userInformation == null || username == null) && _userOpList.contains(op))
 		{
 		  // its an operation that requires the user to be logged on - direct them to login page
-			serviceNode.setAttribute("operation", LOGIN);
+			serviceNode.setAttribute(OPERATION, LOGIN);
 			GSXML.addError(result, getErrorTextString(ERROR_NOT_LOGGED_IN, lang));
 			return result;
@@ -508,9 +550,13 @@
 		{
 		  // actually, the user needs to be an admin user and they are not
-			serviceNode.setAttribute("operation", LOGIN);
+			serviceNode.setAttribute(OPERATION, LOGIN);
 			GSXML.addError(result, getErrorTextString(ERROR_ADMIN_NOT_LOGGED_IN, lang));
 			return result;
 		}
 
+		if (_recaptchaOpList != null && _recaptchaOpList.contains(op)) {
+		  serviceNode.setAttribute("recaptcha_key", _recaptchaSiteKey);
+		}
+		
 		if (op.equals(LIST_USERS))
 		{
@@ -518,5 +564,5 @@
 		  if (error != NO_ERROR)
 		    {
-		      serviceNode.setAttribute("operation", BLANK);
+		      serviceNode.setAttribute(OPERATION, BLANK);
 		      GSXML.addError(result, getErrorTextString(error, lang));
 		    }
@@ -527,10 +573,10 @@
 		if (op.equals(PERFORM_ADD))
 		{
-			String newUsername = (String) paramMap.get("username");
-			String newPassword = (String) paramMap.get("password");
-			String newGroups = (String) paramMap.get("groups");
-			String newStatus = (String) paramMap.get("status");
-			String newComment = (String) paramMap.get("comment");
-			String newEmail = (String) paramMap.get("email");
+			String newUsername = (String) paramMap.get(USERNAME);
+			String newPassword = (String) paramMap.get(PASSWORD);
+			String newGroups = (String) paramMap.get(GROUPS);
+			String newStatus = (String) paramMap.get(STATUS);
+			String newComment = (String) paramMap.get(COMMENT);
+			String newEmail = (String) paramMap.get(EMAIL);
 
 			//Check the given user name
@@ -538,4 +584,5 @@
 			if ((error = checkUsername(newUsername)) != NO_ERROR)
 			{
+			  serviceNode.setAttribute(OPERATION, ADD_USER);
 				GSXML.addError(result, getErrorTextString(error, lang));
 				return result;
@@ -545,4 +592,5 @@
 			if ((error = checkPassword(newPassword)) != NO_ERROR)
 			{
+			  serviceNode.setAttribute(OPERATION, ADD_USER);
 				GSXML.addError(result, getErrorTextString(error, lang));
 				return result;
@@ -554,5 +602,5 @@
 			if (error != NO_ERROR)
 			{
-				serviceNode.setAttribute("operation", ADD_USER);
+				serviceNode.setAttribute(OPERATION, ADD_USER);
 				GSXML.addError(result, getErrorTextString(error, lang));
 			}
@@ -560,14 +608,55 @@
 			{
 				addUserInformationToNode(null, serviceNode);
-				serviceNode.setAttribute("operation", LIST_USERS);
+				serviceNode.setAttribute(OPERATION, LIST_USERS);
 			}
 			return result;
 		}
+
+		if (op.equals(REGISTER)) {
+		  // don't need any additional info
+		  return result;
+		}
+		if (op.equals(PERFORM_REGISTER))
+		{
+			String newUsername = (String) paramMap.get(USERNAME);
+			String newPassword = (String) paramMap.get(PASSWORD);
+			String newEmail = (String) paramMap.get(EMAIL);
+
+			//Check the given details
+			int error;
+			if ((error = checkUsername(newUsername)) == NO_ERROR) {
+			  if ((error = checkPassword(newPassword)) == NO_ERROR) {
+			    newPassword = hashPassword(newPassword);
+			    if (_recaptchaSiteKey != null && _recaptchaSecretKey != null) {
+
+			      String user_response = (String) paramMap.get("g-recaptcha-response");
+			      if ((error= verifyRecaptcha(_recaptchaSecretKey, user_response)) == NO_ERROR) {
+				error = addUser(newUsername, newPassword, "", "true", "", newEmail);
+			      }
+			    }
+			  }
+			}
+			
+			if (error != NO_ERROR)
+			{
+				serviceNode.setAttribute(OPERATION, REGISTER);
+				if (_recaptchaOpList != null && _recaptchaOpList.contains(REGISTER)) {
+				  serviceNode.setAttribute("recaptcha_key", _recaptchaSiteKey);
+				}
+				GSXML.addError(result, getErrorTextString(error, lang));
+			}
+			// otherwise everything hunky dory and we return result
+			return result;
+		}
 		
-		if (op.equals(PERFORM_REGISTER))
-		{
-			String newUsername = (String) paramMap.get("username");
-			String newPassword = (String) paramMap.get("password");
-			String newEmail = (String) paramMap.get("email");
+		if (op.equals(PERFORM_EDIT))
+		{
+			String previousUsername = (String) paramMap.get(PREV_USERNAME);
+			String newUsername = (String) paramMap.get(NEW_USERNAME);
+			String newPassword = (String) paramMap.get(PASSWORD);
+			String newGroups = (String) paramMap.get(GROUPS);
+			String newStatus = (String) paramMap.get(STATUS);
+			String newComment = (String) paramMap.get(COMMENT);
+			String newEmail = (String) paramMap.get(NEW_EMAIL);
 
 			//Check the given user name
@@ -579,43 +668,97 @@
 			}
 
-			//Check the given password
-			if ((error = checkPassword(newPassword)) != NO_ERROR)
-			{
+			if (newPassword == null)
+			{
+				newPassword = retrieveDataForUser(previousUsername, PASSWORD);
+			}
+			else
+			{
+				//Check the given password
+				if ((error = checkPassword(newPassword)) != NO_ERROR)
+				{
+					GSXML.addError(result, getErrorTextString(error, lang));
+					return result;
+				}
+
+				newPassword = hashPassword(newPassword);
+			}
+
+			error = removeUser(previousUsername);
+			if (error != NO_ERROR)
+			{
+				if (error == ERROR_USERNAME_NOT_SPECIFIED)
+				{
+					addUserInformationToNode(null, serviceNode);
+					serviceNode.setAttribute(OPERATION, LIST_USERS);
+				}
+				else
+				{
+					serviceNode.setAttribute(OPERATION, EDIT_USER);
+					GSXML.addError(result, getErrorTextString(error, lang));
+				}
+				return result;
+			}
+
+			error = addUser(newUsername, newPassword, newGroups, newStatus, newComment, newEmail);
+			if (error != NO_ERROR)
+			{
+				serviceNode.setAttribute(OPERATION, EDIT_USER);
 				GSXML.addError(result, getErrorTextString(error, lang));
+			}
+			else
+			{
+				addUserInformationToNode(null, serviceNode);
+				serviceNode.setAttribute(OPERATION, LIST_USERS);
+			}
+			return result;
+		}
+		// this operation is done by a user when editing their own details. Should not return userNode info.
+		if (op.equals(PERFORM_ACCOUNT_EDIT))
+		{
+			String previousUsername = (String) paramMap.get(PREV_USERNAME);
+			String newUsername = (String) paramMap.get(NEW_USERNAME);
+			String oldPassword = (String) paramMap.get(OLD_PASSWORD);
+			String newPassword = (String) paramMap.get(NEW_PASSWORD);
+			String newEmail = (String) paramMap.get(NEW_EMAIL);
+
+			//Make sure the user name does not already exist
+			if (!previousUsername.equals(newUsername) && checkUserExists(newUsername))
+			{
+				addUserInformationToNode(previousUsername, serviceNode);
+				serviceNode.setAttribute(OPERATION, ACCOUNT_SETTINGS);
+				GSXML.addError(result, getErrorTextString(ERROR_USER_ALREADY_EXISTS, lang));
 				return result;
 			}
 
-			newPassword = hashPassword(newPassword);
-
-			// check the recaptcha
-			if (_recaptchaSiteKey != null && _recaptchaSecretKey.length() > 0) {
-
-			  String user_response = (String) paramMap.get("g-recaptcha-response");
-			  int recaptcha_error = verifyRecaptcha(_recaptchaSecretKey, user_response);
-			  if (recaptcha_error != NO_ERROR) {
-			    serviceNode.setAttribute("operation", REGISTER);
-			    GSXML.addError(result, getErrorTextString(recaptcha_error, lang));
-			    return result;
-			  }
-			}
-
-			error = addUser(newUsername, newPassword, "", "true", "", newEmail);
-			if (error != NO_ERROR)
-			{
-				serviceNode.setAttribute("operation", REGISTER);
-				GSXML.addError(result, getErrorTextString(error, lang));
-			}
-			return result;
-		}
-		
-		else if (op.equals(PERFORM_EDIT))
-		{
-			String previousUsername = (String) paramMap.get("prevUsername");
-			String newUsername = (String) paramMap.get("newUsername");
-			String newPassword = (String) paramMap.get("password");
-			String newGroups = (String) paramMap.get("groups");
-			String newStatus = (String) paramMap.get("status");
-			String newComment = (String) paramMap.get("comment");
-			String newEmail = (String) paramMap.get("newEmail");
+			String prevPassword = retrieveDataForUser(previousUsername, PASSWORD);
+
+			if (newPassword != null)
+			{
+				oldPassword = hashPassword(oldPassword);
+
+				if (oldPassword == null || !oldPassword.equals(prevPassword))
+				{
+					addUserInformationToNode(previousUsername, serviceNode);
+					serviceNode.setAttribute(OPERATION, ACCOUNT_SETTINGS);
+					GSXML.addError(result, getErrorTextString(ERROR_INCORRECT_PASSWORD, lang), "INCORRECT_PASSWORD");
+					return result;
+				}
+
+				//Check the given password
+				int error;
+				if ((error = checkPassword(newPassword)) != NO_ERROR)
+				{
+				  addUserInformationToNode(previousUsername, serviceNode);
+				  serviceNode.setAttribute(OPERATION, ACCOUNT_SETTINGS);
+					GSXML.addError(result, getErrorTextString(error, lang));
+					return result;
+				}
+
+				newPassword = hashPassword(newPassword);
+			}
+			else
+			{
+				newPassword = prevPassword;
+			}
 
 			//Check the given user name
@@ -623,120 +766,19 @@
 			if ((error = checkUsername(newUsername)) != NO_ERROR)
 			{
+			  addUserInformationToNode(previousUsername, serviceNode);
+				  serviceNode.setAttribute(OPERATION, ACCOUNT_SETTINGS);
 				GSXML.addError(result, getErrorTextString(error, lang));
 				return result;
 			}
 
-			if (newPassword == null)
-			{
-				newPassword = retrieveDataForUser(previousUsername, "password");
-			}
-			else
-			{
-				//Check the given password
-				if ((error = checkPassword(newPassword)) != NO_ERROR)
-				{
-					GSXML.addError(result, getErrorTextString(error, lang));
-					return result;
-				}
-
-				newPassword = hashPassword(newPassword);
-			}
+			String prevGroups = retrieveDataForUser(previousUsername, GROUPS);
+			String prevStatus = retrieveDataForUser(previousUsername, STATUS);
+			String prevComment = retrieveDataForUser(previousUsername, COMMENT);
 
 			error = removeUser(previousUsername);
 			if (error != NO_ERROR)
 			{
-				if (error == ERROR_USERNAME_NOT_SPECIFIED)
-				{
-					addUserInformationToNode(null, serviceNode);
-					serviceNode.setAttribute("operation", LIST_USERS);
-				}
-				else
-				{
-					serviceNode.setAttribute("operation", EDIT_USER);
-					GSXML.addError(result, getErrorTextString(error, lang));
-				}
-				return result;
-			}
-
-			error = addUser(newUsername, newPassword, newGroups, newStatus, newComment, newEmail);
-			if (error != NO_ERROR)
-			{
-				serviceNode.setAttribute("operation", EDIT_USER);
-				GSXML.addError(result, getErrorTextString(error, lang));
-			}
-			else
-			{
-				addUserInformationToNode(null, serviceNode);
-				serviceNode.setAttribute("operation", LIST_USERS);
-			}
-		}
-		// this operation is done by a user when editing their own details. Should not return userNode info.
-		else if (op.equals(PERFORM_ACCOUNT_EDIT))
-		{
-			String previousUsername = (String) paramMap.get("prevUsername");
-			String newUsername = (String) paramMap.get("newUsername");
-			String oldPassword = (String) paramMap.get("oldPassword");
-			String newPassword = (String) paramMap.get("newPassword");
-			String newEmail = (String) paramMap.get("newEmail");
-
-			//Make sure the user name does not already exist
-			if (!previousUsername.equals(newUsername) && checkUserExists(newUsername))
-			{
-				addUserInformationToNode(previousUsername, serviceNode);
-				serviceNode.setAttribute("operation", ACCOUNT_SETTINGS);
-				GSXML.addError(result, getErrorTextString(ERROR_USER_ALREADY_EXISTS, lang));
-				return result;
-			}
-
-			String prevPassword = retrieveDataForUser(previousUsername, "password");
-
-			if (newPassword != null)
-			{
-				oldPassword = hashPassword(oldPassword);
-
-				if (oldPassword == null || !oldPassword.equals(prevPassword))
-				{
-					addUserInformationToNode(previousUsername, serviceNode);
-					serviceNode.setAttribute("operation", ACCOUNT_SETTINGS);
-					GSXML.addError(result, getErrorTextString(ERROR_INCORRECT_PASSWORD, lang), "INCORRECT_PASSWORD");
-					return result;
-				}
-
-				//Check the given password
-				int error;
-				if ((error = checkPassword(newPassword)) != NO_ERROR)
-				{
-				  addUserInformationToNode(previousUsername, serviceNode);
-				  serviceNode.setAttribute("operation", ACCOUNT_SETTINGS);
-					GSXML.addError(result, getErrorTextString(error, lang));
-					return result;
-				}
-
-				newPassword = hashPassword(newPassword);
-			}
-			else
-			{
-				newPassword = prevPassword;
-			}
-
-			//Check the given user name
-			int error;
-			if ((error = checkUsername(newUsername)) != NO_ERROR)
-			{
 			  addUserInformationToNode(previousUsername, serviceNode);
-				  serviceNode.setAttribute("operation", ACCOUNT_SETTINGS);
-				GSXML.addError(result, getErrorTextString(error, lang));
-				return result;
-			}
-
-			String prevGroups = retrieveDataForUser(previousUsername, "groups");
-			String prevStatus = retrieveDataForUser(previousUsername, "status");
-			String prevComment = retrieveDataForUser(previousUsername, "comment");
-
-			error = removeUser(previousUsername);
-			if (error != NO_ERROR)
-			{
-			  addUserInformationToNode(previousUsername, serviceNode);
-			  serviceNode.setAttribute("operation", ACCOUNT_SETTINGS);
+			  serviceNode.setAttribute(OPERATION, ACCOUNT_SETTINGS);
 			  GSXML.addError(result, getErrorTextString(error, lang));
 			  return result;
@@ -746,23 +788,23 @@
 			if (error != NO_ERROR)
 			{
-			  serviceNode.setAttribute("operation", ACCOUNT_SETTINGS);
+			  serviceNode.setAttribute(OPERATION, ACCOUNT_SETTINGS);
 				GSXML.addError(result, getErrorTextString(error, lang));
 			}
 
 			addUserInformationToNode(newUsername, serviceNode);
-			serviceNode.setAttribute("operation", ACCOUNT_SETTINGS);
+			serviceNode.setAttribute(OPERATION, ACCOUNT_SETTINGS);
 			GSXML.addError(result, getTextString("auth.success.account_settings", lang));
-			
-		}
-		else if (op.equals(PERFORM_RETRIEVE_PASSWORD))
-		{
-
-		}
-		else if (op.equals(PERFORM_CHANGE_PASSWORD))
-		{
-			serviceNode.setAttribute("operation", PERFORM_CHANGE_PASSWORD);
-			String user_name = (String) paramMap.get("username");
-			String oldPassword = (String) paramMap.get("oldPassword");
-			String newPassword = (String) paramMap.get("newPassword");
+			return result;
+		}
+		if (op.equals(PERFORM_RETRIEVE_PASSWORD))
+		{
+		  return result;
+		}
+		if (op.equals(PERFORM_CHANGE_PASSWORD))
+		{
+			serviceNode.setAttribute(OPERATION, PERFORM_CHANGE_PASSWORD);
+			String user_name = (String) paramMap.get(USERNAME);
+			String oldPassword = (String) paramMap.get(OLD_PASSWORD);
+			String newPassword = (String) paramMap.get(NEW_PASSWORD);
 			if (user_name == null || oldPassword == null || newPassword == null)
 			{
@@ -771,5 +813,5 @@
 			}
 
-			String prevPassword = retrieveDataForUser(user_name, "password");
+			String prevPassword = retrieveDataForUser(user_name, PASSWORD);
 			if (!hashPassword(oldPassword).equals(prevPassword))
 			{
@@ -788,5 +830,5 @@
 
 			DerbyWrapper derbyWrapper = openDatabase();
-			String chpa_groups = retrieveDataForUser(user_name, "groups");
+			String chpa_groups = retrieveDataForUser(user_name, GROUPS);
 			String chpa_comment = "password_changed_by_user";
 			String info = derbyWrapper.modifyUserInfo(user_name, hashPassword(newPassword), chpa_groups, null, chpa_comment, null);
@@ -797,8 +839,9 @@
 				return result;
 			}
-		}
-		else if (op.equals(EDIT_USER))
-		{
-			String editUsername = (String) paramMap.get("username");
+			return result;
+		}
+		if (op.equals(EDIT_USER))
+		{
+			String editUsername = (String) paramMap.get(USERNAME);
 			int error = addUserInformationToNode(editUsername, serviceNode);
 			if (error != NO_ERROR)
@@ -806,12 +849,13 @@
 			  GSXML.addError(result, getErrorTextString(error, lang));
 			}
-		}
-		else if (op.equals(ACCOUNT_SETTINGS))
-		{
-			String editUsername = (String) paramMap.get("username");
+			return result;
+		}
+		if (op.equals(ACCOUNT_SETTINGS))
+		{
+			String editUsername = (String) paramMap.get(USERNAME);
 
 			if (editUsername == null)
 			{
-				serviceNode.setAttribute("operation", "");
+				serviceNode.setAttribute(OPERATION, "");
 				GSXML.addError(result, getErrorTextString(ERROR_USERNAME_NOT_SPECIFIED, lang));
 				return result;
@@ -820,5 +864,5 @@
 			if (!editUsername.equals(username))
 			{
-				serviceNode.setAttribute("operation", LOGIN);
+				serviceNode.setAttribute(OPERATION, LOGIN);
 				GSXML.addError(result, getErrorTextString(ERROR_NOT_AUTHORISED, lang));
 				return result;
@@ -829,24 +873,23 @@
 			  GSXML.addError(result, getErrorTextString(error, lang));
 			}
-		}
-		else if (op.equals(PERFORM_RESET_PASSWORD))
-		{
-			String passwordResetUser = (String) paramMap.get("username");
+			return result;
+		}
+		if (op.equals(PERFORM_RESET_PASSWORD))
+		{
+			String passwordResetUser = (String) paramMap.get(USERNAME);
 
 			String newPassword = UUID.randomUUID().toString();
 			newPassword = newPassword.substring(0, newPassword.indexOf("-"));
 
-			String email = retrieveDataForUser(passwordResetUser, "email");
+			String email = retrieveDataForUser(passwordResetUser, EMAIL);
 			String from = "admin@greenstone.org";
 			String host = request.getAttribute("remoteAddress");
 
 			//TODO: FINISH THIS
-		}
-		else if (op.equals(REGISTER))
-		{
-		}
-		else if (op.equals(PERFORM_DELETE_USER))
-		{
-			String usernameToDelete = (String) paramMap.get("username");
+			return result;
+		}
+		if (op.equals(PERFORM_DELETE_USER))
+		{
+			String usernameToDelete = (String) paramMap.get(USERNAME);
 			int error = removeUser(usernameToDelete);
 			if (error != NO_ERROR)
@@ -855,8 +898,9 @@
 			}
 			addUserInformationToNode(null, serviceNode);
-			serviceNode.setAttribute("operation", LIST_USERS);
-		}
-
-		return result;
+			serviceNode.setAttribute(OPERATION, LIST_USERS);
+			return result;
+		}
+
+		return result; // or should we return null, as we haven't recognised the operation??
 	}
 
@@ -1114,25 +1158,25 @@
 			for (int i = 0; i < result.getSize(); i++)
 			{
-				if (dataType.equals("password"))
+				if (dataType.equals(PASSWORD))
 				{
 					data = ((UserTermInfo) userInfo.get(i)).password;
 					break;
 				}
-				else if (dataType.equals("groups"))
+				else if (dataType.equals(GROUPS))
 				{
 					data = ((UserTermInfo) userInfo.get(i)).groups;
 					break;
 				}
-				else if (dataType.equals("status"))
+				else if (dataType.equals(STATUS))
 				{
 					data = ((UserTermInfo) userInfo.get(i)).accountstatus;
 					break;
 				}
-				else if (dataType.equals("comment"))
+				else if (dataType.equals(COMMENT))
 				{
 					data = ((UserTermInfo) userInfo.get(i)).comment;
 					break;
 				}
-				else if (dataType.equals("email"))
+				else if (dataType.equals(EMAIL))
 				{
 					data = ((UserTermInfo) userInfo.get(i)).email;
@@ -1163,9 +1207,9 @@
 			String comment = ((UserTermInfo) userInfo.get(i)).comment;
 			String email = ((UserTermInfo) userInfo.get(i)).email;
-			user_node.setAttribute("username", username);
-			user_node.setAttribute("groups", groups);
-			user_node.setAttribute("status", accountstatus);
-			user_node.setAttribute("comment", comment);
-			user_node.setAttribute("email", email);
+			user_node.setAttribute(USERNAME, username);
+			user_node.setAttribute(GROUPS, groups);
+			user_node.setAttribute(STATUS, accountstatus);
+			user_node.setAttribute(COMMENT, comment);
+			user_node.setAttribute(EMAIL, email);
 
 			user_list_node.appendChild(user_node);
