fix: Fix exposing password hashes in user edit page

When an administrator edits a user entry, the user's password hash is
present on the edit page. This is unnecessary. But it exposes the hash
to an administrator who could choose to try to brute-force the hash and
use the password on other logins of that user.

This is an issue for administrative users who have no access to the
actual database on disk but access to the user edit web page.

10 files changed, 34 insertions, 24 deletions

diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties
index 221878e1..40f84538 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties
@@ -248,7 +248,7 @@ gb.teamMustSpecifyRepository = A team must specify at least one repository.
 gb.teamCreated = New team ''{0}'' successfully created.
 gb.pleaseSetUsername = Please enter a username!
 gb.usernameUnavailable = Username ''{0}'' is unavailable.
-gb.combinedMd5Rename = Gitblit is configured for combined-md5 password hashing. You must enter a new password on account rename.
+gb.combinedMd5Rename = This user is configured for combined-md5 password hashing. You must enter a new password on account rename.
 gb.userCreated = New user ''{0}'' successfully created.
 gb.couldNotFindFederationRegistration = Could not find federation registration!
 gb.failedToFindGravatarProfile = Failed to find Gravatar profile for {0}
diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp_de.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp_de.properties
index 6c08bd60..8990b823 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp_de.properties
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp_de.properties
@@ -248,7 +248,7 @@ gb.teamMustSpecifyRepository = Ein Team muss mindestens einem Repository zugewie
 gb.teamCreated = Neues Team ''{0}'' erfolgreich angelegt.
 gb.pleaseSetUsername = Bitte geben Sie einen Benutzernamen an!
 gb.usernameUnavailable = Benutzername ''{0}'' ist nicht verf\u00fcgbar.
-gb.combinedMd5Rename = Gitblit ist f\u00fcr kombiniertes MD5-Passwort-Hashing konfiguriert. Sie m\u00fcssen beim Umbenennen des Kontos ein neues Passwort angeben.
+gb.combinedMd5Rename = Dieser Benutzer ist f\u00fcr kombiniertes MD5-Passwort-Hashing konfiguriert. Sie m\u00fcssen beim Umbenennen des Kontos ein neues Passwort angeben.
 gb.userCreated = Neuer Benutzer ''{0}'' erfolgreich angelegt.
 gb.couldNotFindFederationRegistration = Konnte Verbindungsregistrierung (Federation) nicht finden!
 gb.failedToFindGravatarProfile = Das Gravatar Profil f\u00fcr {0} konnte nicht gefunden werden
diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp_es.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp_es.properties
index 2865aa91..83690fc2 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp_es.properties
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp_es.properties
@@ -248,7 +248,7 @@ gb.teamMustSpecifyRepository = Debe especificar al menos un repositorio para el
 gb.teamCreated = Nuevo Equipo ''{0}'' creado satisfactoriamente.

 gb.pleaseSetUsername = \u00A1Por favor, introduce un usuario!

 gb.usernameUnavailable = El usuario ''{0}'' no est\u00E1 disponible.

-gb.combinedMd5Rename = GitBlit est\u00E1 configurado para Hashes combinados md5. Debes introducir una nueva contrase\u00F1a para renombrar la cuenta.

+gb.combinedMd5Rename = El usuario est\u00E1 configurado para Hashes combinados md5. Debes introducir una nueva contrase\u00F1a para renombrar la cuenta.

 gb.userCreated = Nuevo usuario ''{0}'' creado satisfactoriamente.

 gb.couldNotFindFederationRegistration = \u00A1No se pudo encontrar el registro de federaci\u00F3n!

 gb.failedToFindGravatarProfile = Fallo al buscar el perfil Gravatar de {0}

diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp_fr.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp_fr.properties
index f02748c0..2a5f2aa2 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp_fr.properties
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp_fr.properties
@@ -248,7 +248,7 @@ gb.teamMustSpecifyRepository = Une \u00e9quipe doit d\u00e9finir au moins un d\u
 gb.teamCreated = La nouvelle \u00e9quipe ''{0}'' cr\u00e9\u00e9 avec succ\u00e8s.
 gb.pleaseSetUsername = Entrez un identifiant SVP !
 gb.usernameUnavailable = L'identifiant ''{0}'' est indisponible.
-gb.combinedMd5Rename = Gitblit est configur\u00e9 pour des mots de passe hash\u00e9s combined-md5. Vous devez entrer un nouveau mot de passe pour ce compte.
+gb.combinedMd5Rename = L'identifiant est configur\u00e9 pour des mots de passe hash\u00e9s combined-md5. Vous devez entrer un nouveau mot de passe pour ce compte.
 gb.userCreated = Le nouveau utilisateur ''{0}'' est cr\u00e9\u00e9 avec succ\u00e8s.
 gb.couldNotFindFederationRegistration = N'arrive pas \u00e0 joindre l'enregistrement de la f\u00e9d\u00e9ration !
 gb.failedToFindGravatarProfile = N'arrive pas trouver un profil Gravatar pour {0}
diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp_it.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp_it.properties
index e0c406fe..9c08c377 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp_it.properties
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp_it.properties
@@ -248,7 +248,7 @@ gb.teamMustSpecifyRepository = Un gruppo deve specificare almeno un repository.
 gb.teamCreated = Nuovo gruppo ''{0}'' creato con successo.
 gb.pleaseSetUsername = Nome utente non specificato!
 gb.usernameUnavailable = Il nome utente ''{0}'' non è disponibile.
-gb.combinedMd5Rename = Gitblit è configurato per effettuare un hashing delle password di tipo combinato-md5. E' quindi necessario specificare una nuova password quando si rinomina un utenza.
+gb.combinedMd5Rename = Il nome utente è configurato per effettuare un hashing delle password di tipo combinato-md5. E' quindi necessario specificare una nuova password quando si rinomina un utenza.
 gb.userCreated = Nuovo utente ''{0}'' creato con successo.
 gb.couldNotFindFederationRegistration = Impossibile trovare la registrazione di federazione!
 gb.failedToFindGravatarProfile = Profilo Gravatar per {0} non reperito!
diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp_nl.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp_nl.properties
index a869e96b..e05c1940 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp_nl.properties
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp_nl.properties
@@ -248,7 +248,7 @@ gb.teamMustSpecifyRepository = Een team moet minimaal één repositorie specific
 gb.teamCreated = Nieuw team ''{0}'' successvol aangemaakt.
 gb.pleaseSetUsername = Vul aub een gebruikersnaam in!
 gb.usernameUnavailable = Gebruikersnaam ''{0}'' is niet beschikbaar.
-gb.combinedMd5Rename = Gitblit is geconfigureerd voor combined-md5 wachtwoord hashing. U moet een nieuw wachtwoord opgeven bij het hernoemen van een account.
+gb.combinedMd5Rename = Gebruikersnaam is geconfigureerd voor combined-md5 wachtwoord hashing. U moet een nieuw wachtwoord opgeven bij het hernoemen van een account.
 gb.userCreated = Nieuwe gebruiker ''{0}'' succesvol aangemaakt.
 gb.couldNotFindFederationRegistration = Kon de federatie registratie niet vinden!
 gb.failedToFindGravatarProfile = Kon het Gravatar profiel voor {0} niet vinden
diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp_no.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp_no.properties
index 96522ec6..1efc6363 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp_no.properties
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp_no.properties
@@ -248,7 +248,7 @@ gb.teamMustSpecifyRepository = Et team m\u00e5 ha minst et repository.
 gb.teamCreated = Team ''{0}'' opprettet.
 gb.pleaseSetUsername = Vennlist angi et brukernavn!
 gb.usernameUnavailable = Brukernavnet ''{0}'' er ikke tilgjengelig.
-gb.combinedMd5Rename = Gitblit er satt opp med combined-md5 passord hashing. Du m\u00e5 angi et nytt passord n\u00e5r du gir en konto et nytt navn.
+gb.combinedMd5Rename = Brukernavnet er satt opp med combined-md5 passord hashing. Du m\u00e5 angi et nytt passord n\u00e5r du gir en konto et nytt navn.
 gb.userCreated = Ny bruker ''{0}'' opprettet.
 gb.couldNotFindFederationRegistration = Kunne ikke finne federeringsoppf\u00F8ringen!
 gb.failedToFindGravatarProfile = Fant ikke gravatar-profilen for {0}
diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp_pl.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp_pl.properties
index a4753e72..a2e107fd 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp_pl.properties
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp_pl.properties
@@ -246,8 +246,8 @@ gb.teamNameUnavailable = Nazwa zespo\u0142u ''{0}'' jest niedost\u0119pna.
 gb.teamMustSpecifyRepository = Zesp\u00F3\u0142 musi posiada\u0107 conajmniej jedno repozytorium.
 gb.teamCreated = Zesp\u00F3\u0142 ''{0}'' zosta\u0142 utworzony.
 gb.pleaseSetUsername = Wpisz nazw\u0119 u\u017Cytkownika!
-gb.usernameUnavailable = Nazwa u\u017Cytkownika''{0}'' jest niedost\u0119pna.
-gb.combinedMd5Rename = Gitblit jest skonfigurowany na po\u0142\u0105czone haszowanie hase\u0142 md5. Musisz wpisa\u0107 nowe has\u0142o przy zmianie nazwy konta.
+gb.usernameUnavailable = Nazwa u\u017Cytkownika ''{0}'' jest niedost\u0119pna.
+gb.combinedMd5Rename = Nazwa u\u017Cytkownika jest skonfigurowany na po\u0142\u0105czone haszowanie hase\u0142 md5. Musisz wpisa\u0107 nowe has\u0142o przy zmianie nazwy konta.
 gb.userCreated = U\u017Cytkownik ''{0}'' zosta\u0142 utworzony.
 gb.couldNotFindFederationRegistration = Nie mo\u017Cna znale\u017A\u0107 rejestracji federacji!
 gb.failedToFindGravatarProfile = B\u0142\u0105d podczas dopasowania profilu Gravatar dla {0}
diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp_pt_BR.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp_pt_BR.properties
index 26b6838d..b8473d2c 100644
--- a/src/main/java/com/gitblit/wicket/GitBlitWebApp_pt_BR.properties
+++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp_pt_BR.properties
@@ -247,7 +247,7 @@ gb.teamMustSpecifyRepository = Uma equipe deve especificar pelo menos um reposit
 gb.teamCreated = Nova equipe ''{0}'' criada com sucesso.

 gb.pleaseSetUsername = Por favor entre com um username!

 gb.usernameUnavailable = Username ''{0}'' est\u00e1 indispon\u00edvel.

-gb.combinedMd5Rename = Gitblit est\u00e1 configurado para usar um hash combinado-md5. Voc\u00ea deve inserir um novo password ao renamear a conta.

+gb.combinedMd5Rename = Username est\u00e1 configurado para usar um hash combinado-md5. Voc\u00ea deve inserir um novo password ao renamear a conta.

 gb.userCreated = Novo usu\u00e1rio ''{0}'' criado com sucesso.

 gb.couldNotFindFederationRegistration = N\u00e3o foi poss\u00edvel localizar o registro da federa\u00e7\u00e3o!

 gb.failedToFindGravatarProfile = Falha ao localizar um perfil Gravatar para {0}

diff --git a/src/main/java/com/gitblit/wicket/pages/EditUserPage.java b/src/main/java/com/gitblit/wicket/pages/EditUserPage.java
index c6014e8f..add83dce 100644
--- a/src/main/java/com/gitblit/wicket/pages/EditUserPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/EditUserPage.java
@@ -93,8 +93,11 @@ public class EditUserPage extends RootSubPage {
 			super.setupPage(getString("gb.edit"), userModel.username);

 		}

 

-		final Model<String> confirmPassword = new Model<String>(

-				StringUtils.isEmpty(userModel.password) ? "" : userModel.password);

+		final Model<String> confirmPassword = new Model<String>("");

+

+		// Saving current password of user and clearing the one in the model so that it doesn't show up in the page.

+		final String oldPassword = userModel.password;

+		userModel.password = "";

 		CompoundPropertyModel<UserModel> model = new CompoundPropertyModel<UserModel>(userModel);

 

 		// build list of projects including all repositories wildcards

@@ -149,13 +152,15 @@ public class EditUserPage extends RootSubPage {
 				boolean rename = !StringUtils.isEmpty(oldName)

 						&& !oldName.equalsIgnoreCase(username);

 				if (app().authentication().supportsCredentialChanges(userModel)) {

-					if (!userModel.password.equals(confirmPassword.getObject())) {

-						error(getString("gb.passwordsDoNotMatch"));

-						return;

-					}

-					String password = userModel.password;

-					if (!PasswordHash.isHashedEntry(password)) {

-						// This is a plain text password.

+

+					if (!StringUtils.isEmpty(userModel.password)) {

+						// The password was changed

+						String password = userModel.password;

+						if (!password.equals(confirmPassword.getObject())) {

+							error(getString("gb.passwordsDoNotMatch"));

+							return;

+						}

+

 						// Check length.

 						int minLength = app().settings().getInteger(Keys.realm.minPasswordLength, 5);

 						if (minLength < 4) {

@@ -170,16 +175,19 @@ public class EditUserPage extends RootSubPage {
 						// change the cookie

 						userModel.cookie = userModel.createCookie();

 

-						// Optionally store the password MD5 digest.

+						// Optionally store the password hash digest.

 						String type = app().settings().getString(Keys.realm.passwordStorage, PasswordHash.getDefaultType().name());

 						PasswordHash pwdh = PasswordHash.instanceOf(type);

 						if (pwdh != null) { // Hash the password

 							userModel.password = pwdh.toHashedEntry(password, username);

 						}

-					} else if (rename

-							&& password.toUpperCase().startsWith(PasswordHash.Type.CMD5.name())) {

-						error(getString("gb.combinedMd5Rename"));

-						return;

+					} else {

+						if (rename && oldPassword.toUpperCase().startsWith(PasswordHash.Type.CMD5.name())) {

+							error(getString("gb.combinedMd5Rename"));

+							return;

+						}

+						// Set back saved password so that it is kept in the DB.

+						userModel.password = oldPassword;

 					}

 				}

 

@@ -251,10 +259,12 @@ public class EditUserPage extends RootSubPage {
 		form.add(new TextField<String>("username").setEnabled(editCredentials));

 		NonTrimmedPasswordTextField passwordField = new NonTrimmedPasswordTextField("password");

 		passwordField.setResetPassword(false);

+		passwordField.setRequired(false);

 		form.add(passwordField.setEnabled(editCredentials));

 		NonTrimmedPasswordTextField confirmPasswordField = new NonTrimmedPasswordTextField("confirmPassword",

 				confirmPassword);

 		confirmPasswordField.setResetPassword(false);

+		confirmPasswordField.setRequired(false);

 		form.add(confirmPasswordField.setEnabled(editCredentials));

 		form.add(new TextField<String>("displayName").setEnabled(editDisplayName));

 		form.add(new TextField<String>("emailAddress").setEnabled(editEmailAddress));