Index: /main/trunk/greenstone3/src/java/org/greenstone/gsdl3/service/Authentication.java
===================================================================
--- /main/trunk/greenstone3/src/java/org/greenstone/gsdl3/service/Authentication.java	(revision 32412)
+++ /main/trunk/greenstone3/src/java/org/greenstone/gsdl3/service/Authentication.java	(revision 32413)
@@ -66,4 +66,5 @@
 	protected static final int ERROR_NOT_AUTHORISED = -19;
   	protected static final int ERROR_MISSING_PARAMS = -20;
+  protected static final int ERROR_SOMETHING_WRONG = -21;
   
 	protected static final HashMap<Integer, String> _errorKeyMap;
@@ -91,4 +92,5 @@
 		errorKeyMap.put(ERROR_NOT_AUTHORISED, "auth.error.not_authorised");
 		errorKeyMap.put(ERROR_MISSING_PARAMS, "auth.error.missing_params"); // ???
+		errorKeyMap.put(ERROR_SOMETHING_WRONG, "auth.error.something_wrong");
 		_errorKeyMap = errorKeyMap;
 	}
@@ -521,5 +523,5 @@
 		if (param_list == null)
 		{
-			serviceNode.setAttribute(OPERATION, LOGIN);
+			serviceNode.setAttribute(OPERATION, BLANK);
 			logger.error("Authentication request has no param list");
 			return result; // Return the empty result
@@ -543,4 +545,7 @@
 		  // its an operation that requires the user to be logged on - direct them to login page
 			serviceNode.setAttribute(OPERATION, LOGIN);
+			if (_recaptchaOpList != null && _recaptchaOpList.contains(LOGIN)) {
+			  serviceNode.setAttribute("recaptcha_key", _recaptchaSiteKey);
+			}
 			GSXML.addError(result, getErrorTextString(ERROR_NOT_LOGGED_IN, lang));
 			return result;
@@ -551,4 +556,7 @@
 		  // actually, the user needs to be an admin user and they are not
 			serviceNode.setAttribute(OPERATION, LOGIN);
+			if (_recaptchaOpList != null && _recaptchaOpList.contains(LOGIN)) {
+			  serviceNode.setAttribute("recaptcha_key", _recaptchaSiteKey);
+			}
 			GSXML.addError(result, getErrorTextString(ERROR_ADMIN_NOT_LOGGED_IN, lang));
 			return result;
@@ -585,5 +593,18 @@
 			//Check the given user name
 			int error;
-			if ((error = checkUsername(newUsername)) != NO_ERROR)
+			if (checkUserExists(newUsername)) {
+			  error = ERROR_USER_ALREADY_EXISTS;
+			} else {
+			  error = checkUsername(newUsername);
+			}
+			if (error != NO_ERROR)
+			{
+			  serviceNode.setAttribute(OPERATION, ADD_USER);
+			  GSXML.addError(result, getErrorTextString(error, lang));
+			  return result;
+			}
+
+			//Check the given password
+			if ((error = checkPassword(newPassword)) != NO_ERROR)
 			{
 			  serviceNode.setAttribute(OPERATION, ADD_USER);
@@ -592,12 +613,4 @@
 			}
 
-			//Check the given password
-			if ((error = checkPassword(newPassword)) != NO_ERROR)
-			{
-			  serviceNode.setAttribute(OPERATION, ADD_USER);
-				GSXML.addError(result, getErrorTextString(error, lang));
-				return result;
-			}
-
 			newPassword = hashPassword(newPassword);
 
@@ -628,9 +641,13 @@
 			//Check the given details
 			int error;
-			if ((error = checkUsername(newUsername)) == NO_ERROR) {
+			if (checkUserExists(newUsername)) {
+			  error = ERROR_USER_ALREADY_EXISTS;
+			} else {
+			  error = checkUsername(newUsername);
+			}
+			if (error == NO_ERROR) {
 			  if ((error = checkPassword(newPassword)) == NO_ERROR) {
 			    newPassword = hashPassword(newPassword);
-			    if (_recaptchaSiteKey != null && _recaptchaSecretKey != null) {
-
+			    if (_recaptchaOpList != null && _recaptchaOpList.contains(REGISTER)) {
 			      String user_response = (String) paramMap.get("g-recaptcha-response");
 			      if ((error= verifyRecaptcha(_recaptchaSecretKey, user_response)) == NO_ERROR) {
@@ -652,151 +669,138 @@
 			return result;
 		}
-		
-		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
-			int error;
-			if ((error = checkUsername(newUsername)) != NO_ERROR)
-			{
-				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);
-			}
-
-			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);
-			}
+
+		// PERFORM_EDIT is caled when admin is running EditUser, and PERFORM_ACCOUNT_EDIT is called when a user is running AccountSettings to change their own details
+		if (op.equals(PERFORM_EDIT) || op.equals(PERFORM_ACCOUNT_EDIT)) {
+		  
+		  String parent_op = EDIT_USER;
+		  if (op.equals(PERFORM_ACCOUNT_EDIT)) {
+		    parent_op = ACCOUNT_SETTINGS;
+		  }
+		  String previousUsername = (String) paramMap.get(PREV_USERNAME);
+		  String newUsername = (String) paramMap.get(NEW_USERNAME);
+		  int error;
+		  // Has the user name been changed? Make sure it doesn't already exist and is a valid username
+		  if (previousUsername == null) {
+		    // we have ended up here by mistake (via s1.authpage which is no longer valid)
+		    serviceNode.setAttribute(OPERATION, BLANK);
+		    GSXML.addError(result, getErrorTextString(ERROR_SOMETHING_WRONG, lang));
+		    return result;
+		  }
+		    
+		  if (!previousUsername.equals(newUsername)) {
+
+		    error = NO_ERROR;
+		    if (checkUserExists(newUsername)) {
+		      error = ERROR_USER_ALREADY_EXISTS;
+		    } else {
+		      error = checkUsername(newUsername);
+		    }
+		    if (error != NO_ERROR) {
+		      addUserInformationToNode(previousUsername, serviceNode);
+		      serviceNode.setAttribute(OPERATION, parent_op);
+		      GSXML.addError(result, getErrorTextString(error, lang));
+		      return result;
+		    }
+		  }
+
+		  // password checking
+		  String newPassword;
+		  if (op.equals(PERFORM_EDIT)) {
+		    newPassword = (String) paramMap.get(PASSWORD);
+		  } else {
+		    newPassword = (String) paramMap.get(NEW_PASSWORD);
+		  }
+		  if (newPassword == null) {
+		    // we are not changing the password
+		    newPassword = retrieveDataForUser(previousUsername, PASSWORD);
+		  } else {
+		    // we need to check the old one
+		    if (op.equals(PERFORM_ACCOUNT_EDIT)) {
+		      // check that they entered their old password correctly
+		      String prevPassword = retrieveDataForUser(previousUsername, PASSWORD);
+		      String oldPassword = (String) paramMap.get(OLD_PASSWORD);
+		      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;
-		}
-		// 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;
-			}
-
-			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);
-			  GSXML.addError(result, getErrorTextString(error, lang));
-			  return result;
-			}
-
-			error = addUser(newUsername, newPassword, prevGroups, prevStatus, prevComment, newEmail);
-			if (error != NO_ERROR)
-			{
-			  serviceNode.setAttribute(OPERATION, ACCOUNT_SETTINGS);
-				GSXML.addError(result, getErrorTextString(error, lang));
-			}
-
-			addUserInformationToNode(newUsername, serviceNode);
-			serviceNode.setAttribute(OPERATION, ACCOUNT_SETTINGS);
-			GSXML.addError(result, getTextString("auth.success.account_settings", lang));
-			return result;
+		      }
+		    }
+		    // need to make sure the new password is a valid password
+		    if ((error = checkPassword(newPassword)) != NO_ERROR) {
+		      
+		      addUserInformationToNode(previousUsername, serviceNode);
+		      serviceNode.setAttribute(OPERATION, parent_op);
+		      GSXML.addError(result, getErrorTextString(error, lang));
+		      return result;
+		    }
+		    newPassword = hashPassword(newPassword);
+		  
+		  }
+
+		  // are we using recaptcha for AccountSettings or EditUser?
+		  if (_recaptchaOpList != null && _recaptchaOpList.contains(parent_op)) {
+		    String user_response = (String) paramMap.get("g-recaptcha-response");
+		    if ((error= verifyRecaptcha(_recaptchaSecretKey, user_response)) != NO_ERROR) {
+		      addUserInformationToNode(previousUsername, serviceNode);
+		      serviceNode.setAttribute(OPERATION, parent_op);
+		      serviceNode.setAttribute("recaptcha_key", _recaptchaSiteKey);
+		      GSXML.addError(result, getErrorTextString(error, lang));
+		      return result;
+		    }
+		  }
+		  
+		  groups = null;
+		  String status = null;
+		  String comment = null;
+		  String email = (String) paramMap.get(NEW_EMAIL);
+		  if (op.equals(PERFORM_EDIT)) {
+			groups = (String) paramMap.get(GROUPS);
+			status = (String) paramMap.get(STATUS);
+			comment = (String) paramMap.get(COMMENT);
+
+		  } else {
+		    groups = retrieveDataForUser(previousUsername, GROUPS);
+		    status = retrieveDataForUser(previousUsername, STATUS);
+		    comment = retrieveDataForUser(previousUsername, COMMENT);
+		  }
+		  
+
+		  error = removeUser(previousUsername);
+		  if (error != NO_ERROR) {
+		    addUserInformationToNode(previousUsername, serviceNode);
+		    serviceNode.setAttribute(OPERATION, parent_op);
+		    GSXML.addError(result, getErrorTextString(error, lang));
+		    return result;
+		  }
+		  
+		  error = addUser(newUsername, newPassword, groups, status, comment, email);
+		  if (error != NO_ERROR) {
+		    
+		    // oh dear. we have removed previous data, but were unable to add new data. The user is now gone :-(
+		    serviceNode.setAttribute(OPERATION, BLANK);
+		    GSXML.addError(result, getErrorTextString(error, lang));
+		  }
+		  else {
+		    if (op.equals(PERFORM_ACCOUNT_EDIT)) {
+		      addUserInformationToNode(newUsername, serviceNode);
+		      serviceNode.setAttribute(OPERATION, parent_op);
+		      if (_recaptchaOpList != null && _recaptchaOpList.contains(parent_op)) {
+			serviceNode.setAttribute("recaptcha_key", _recaptchaSiteKey);
+		      }
+		      GSXML.addError(result, getTextString("auth.success.account_settings", lang));
+
+		    } else {
+		      
+		      addUserInformationToNode(null, serviceNode);
+		      serviceNode.setAttribute(OPERATION, LIST_USERS);
+		      String [] args = {newUsername};
+		      GSXML.addError(result, getTextString("auth.success.edit_user", lang, args));
+		    }
+		    
+		  }
+		  return result;
 		}
 		if (op.equals(PERFORM_RETRIEVE_PASSWORD))
@@ -902,4 +906,6 @@
 			addUserInformationToNode(null, serviceNode);
 			serviceNode.setAttribute(OPERATION, LIST_USERS);
+			String[] args = {usernameToDelete};
+			GSXML.addError(result, getTextString("auth.success.delete_user", lang, args));
 			return result;
 		}
