Improve login and registration pages (#2794)

* Keep the user on login page on failure

* Show an error if username already exists

* Check the password format in the backend

* Return a better message if username is invalid

* Add a title to the login page

* wip: Improve look of login and register pages

* Set a capital M in username help message

On the registration page, username tip started with a minuscule, while
the password tip started with a capital.

* Change message if username is taken

app/Controllers/authController.php

@@ -109,6 +109,7 @@ class FreshRSS_auth_Controller extends Minz_ActionController {
 	public function formLoginAction() {
 		invalidateHttpCache();
 
+		Minz_View::prependTitle(_t('gen.auth.login') . ' · ');
 		Minz_View::appendScript(Minz_Url::display('/scripts/bcrypt.min.js?' . @filemtime(PUBLIC_PATH . '/scripts/bcrypt.min.js')));
 
 		$conf = Minz_Configuration::get('system');
@@ -122,7 +123,10 @@ class FreshRSS_auth_Controller extends Minz_ActionController {
 
 			$conf = get_user_configuration($username);
 			if ($conf == null) {
-				Minz_Error::error(403, array(_t('feedback.auth.login.invalid')), false);
+				Minz_Request::bad(
+					_t('feedback.auth.login.invalid'),
+					array('c' => 'auth', 'a' => 'login')
+				);
 				return;
 			}
 
@@ -151,7 +155,10 @@ class FreshRSS_auth_Controller extends Minz_ActionController {
 				                  ' user=' . $username .
 				                  ', nonce=' . $nonce .
 				                  ', c=' . $challenge);
-				Minz_Error::error(403, array(_t('feedback.auth.login.invalid')), false);
+				Minz_Request::bad(
+					_t('feedback.auth.login.invalid'),
+					array('c' => 'auth', 'a' => 'login')
+				);
 			}
 		} elseif (FreshRSS_Context::$system_conf->unsafe_autologin_enabled) {
 			$username = Minz_Request::param('u', '');
@@ -182,7 +189,10 @@ class FreshRSS_auth_Controller extends Minz_ActionController {
 				                   array('c' => 'index', 'a' => 'index'));
 			} else {
 				Minz_Log::warning('Unsafe password mismatch for user ' . $username);
-				Minz_Error::error(403, array(_t('feedback.auth.login.invalid')), false);
+				Minz_Request::bad(
+					_t('feedback.auth.login.invalid'),
+					array('c' => 'auth', 'a' => 'login')
+				);
 			}
 		}
 	}

app/Controllers/userController.php

@@ -284,6 +284,27 @@ class FreshRSS_user_Controller extends Minz_ActionController {
 			$email = Minz_Request::param('new_user_email', '');
 			$passwordPlain = Minz_Request::param('new_user_passwordPlain', '', true);
 
+			if (!self::checkUsername($new_user_name)) {
+				Minz_Request::bad(
+					_t('user.username.invalid'),
+					array('c' => 'auth', 'a' => 'register')
+				);
+			}
+
+			if (FreshRSS_UserDAO::exists($new_user_name)) {
+				Minz_Request::bad(
+					_t('user.username.taken', $new_user_name),
+					array('c' => 'auth', 'a' => 'register')
+				);
+			}
+
+			if (!FreshRSS_password_Util::check($passwordPlain)) {
+				Minz_Request::bad(
+					_t('user.password.invalid'),
+					array('c' => 'auth', 'a' => 'register')
+				);
+			}
+
 			$tos_enabled = file_exists(join_path(DATA_PATH, 'tos.html'));
 			$accept_tos = Minz_Request::param('accept_tos', false);
 

app/Utils/passwordUtil.php

@@ -24,4 +24,17 @@ class FreshRSS_password_Util {
 
 		return $passwordHash == '' ? '' : $passwordHash;
 	}
+
+	/**
+	 * Verify the given password is valid.
+	 *
+	 * A valid password is a string of at least 7 characters.
+	 *
+	 * @param string $password
+	 *
+	 * @return boolean True if the password is valid, false otherwise
+	 */
+	public static function check($password) {
+		return strlen($password) >= 7;
+	}
 }

app/i18n/cz/gen.php

@@ -43,7 +43,7 @@ return array(
 		'reset' => 'Reset přihlášení',
 		'username' => array(
 			'admin' => 'Název administrátorského účtu',
-			'format' => '<small>maximálně 16 alfanumerických znaků</small>',
+			'format' => '<small>Maximálně 16 alfanumerických znaků</small>',
 			'_' => 'Uživatel',
 		),
 	),

app/i18n/cz/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Welcome %s,',	// TODO - Translation
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'You must accept the Terms of Service to be able to register.',	// TODO - Translation
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/i18n/de/gen.php

@@ -43,7 +43,7 @@ return array(
 		'reset' => 'Zurücksetzen der Authentifizierung',
 		'username' => array(
 			'admin' => 'Administrator-Nutzername',
-			'format' => '<small>maximal 16 alphanumerische Zeichen</small>',
+			'format' => '<small>Maximal 16 alphanumerische Zeichen</small>',
 			'_' => 'Nutzername',
 		),
 	),

app/i18n/de/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Willkommen, %s,',
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'Sie müssen die Nutzungsbedingungen akzeptieren um sich zu registrieren.',
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/i18n/en/gen.php

@@ -43,7 +43,7 @@ return array(
 		'reset' => 'Authentication reset',
 		'username' => array(
 			'admin' => 'Administrator username',
-			'format' => '<small>maximum 16 alphanumeric characters</small>',
+			'format' => '<small>Maximum 16 alphanumeric characters</small>',
 			'_' => 'Username',
 		),
 	),

app/i18n/en/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Welcome %s,',
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'You must accept the Terms of Service to be able to register.',
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',
+		'taken' => 'The username %s is taken.',
+	),
 );

app/i18n/es/gen.php

@@ -43,7 +43,7 @@ return array(
 		'reset' => 'Reinicar identificación',
 		'username' => array(
 			'admin' => 'Nombre de usuario del Administrador',
-			'format' => '<small>máximo 16 caracteres alfanuméricos</small>',
+			'format' => '<small>Máximo 16 caracteres alfanuméricos</small>',
 			'_' => 'Nombre de usuario',
 		),
 	),

app/i18n/es/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Welcome %s,',	// TODO - Translation
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'You must accept the Terms of Service to be able to register.',	// TODO - Translation
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/i18n/fr/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Bienvenue %s,',
 		),
 	),
+	'password' => array(
+		'invalid' => 'Le mot de passe est invalide.',
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'Vous devez accepter les conditions générales d’utilisation pour pouvoir vous inscrire.',
 		),
 	),
+	'username' => array(
+		'invalid' => 'Le nom d’utilisateur est invalide.',
+		'taken' => 'Le nom d’utilisateur %s est pris.',
+	),
 );

app/i18n/he/gen.php

@@ -43,7 +43,7 @@ return array(
 		'reset' => 'איפוס אימות',
 		'username' => array(
 			'admin' => 'שם משתמש של המנהל',
-			'format' => '<small>maximum 16 alphanumeric characters</small>',	// TODO - Translation
+			'format' => '<small>Maximum 16 alphanumeric characters</small>',	// TODO - Translation
 			'_' => 'שם משתמש',
 		),
 	),

app/i18n/he/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Welcome %s,',	// TODO - Translation
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'You must accept the Terms of Service to be able to register.',	// TODO - Translation
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/i18n/it/gen.php

@@ -43,7 +43,7 @@ return array(
 		'reset' => 'Reset autenticazione',
 		'username' => array(
 			'admin' => 'Username amministratore',
-			'format' => '<small>massimo 16 caratteri alfanumerici</small>',
+			'format' => '<small>Massimo 16 caratteri alfanumerici</small>',
 			'_' => 'Username',	// TODO - Translation
 		),
 	),

app/i18n/it/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Welcome %s,',	// TODO - Translation
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'You must accept the Terms of Service to be able to register.',	// TODO - Translation
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/i18n/kr/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Welcome %s,',	// TODO - Translation
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'You must accept the Terms of Service to be able to register.',	// TODO - Translation
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/i18n/nl/gen.php

@@ -43,7 +43,7 @@ return array(
 		'reset' => 'Authenticatie reset',
 		'username' => array(
 			'admin' => 'Beheerdersgebruikersnaam',
-			'format' => '<small>maximaal 16 alfanumerieke tekens</small>',
+			'format' => '<small>Maximaal 16 alfanumerieke tekens</small>',
 			'_' => 'Gebruikersnaam',
 		),
 	),

app/i18n/nl/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Welkom %s,',
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'De gebruiksvoorwaarden moeten worden geaccepteerd om te kunnen registeren.',
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/i18n/oc/gen.php

@@ -43,7 +43,7 @@ return array(
 		'reset' => 'Reïnicializacion de l’autentificacion',
 		'username' => array(
 			'admin' => 'Nom d’utilizaire administrator',
-			'format' => '<small>16 caractèrs alfanumerics maximum)</small>',
+			'format' => '<small>16 caractèrs alfanumerics maximum</small>',
 			'_' => 'Nom d’utilizaire',
 		),
 	),

app/i18n/oc/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'La benvenguda %s,',
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'Vos cal acceptar las condicions d’utilizacion per poder vos inscriure.',
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/i18n/pt-br/gen.php

@@ -43,7 +43,7 @@ return array(
 		'reset' => 'Reset autenticação',
 		'username' => array(
 			'admin' => 'Usuário administrador',
-			'format' => '<small>máximo 16 caracteres alphanumericos</small>',
+			'format' => '<small>Máximo 16 caracteres alphanumericos</small>',
 			'_' => 'Usuário',
 		),
 	),

app/i18n/pt-br/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Welcome %s,',	// TODO - Translation
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'You must accept the Terms of Service to be able to register.',	// TODO - Translation
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/i18n/ru/gen.php

@@ -43,7 +43,7 @@ return array(
 		'reset' => 'Authentication reset',	// TODO - Translation
 		'username' => array(
 			'admin' => 'Administrator username',	// TODO - Translation
-			'format' => '<small>maximum 16 alphanumeric characters</small>',	// TODO - Translation
+			'format' => '<small>Maximum 16 alphanumeric characters</small>',	// TODO - Translation
 			'_' => 'Username',	// TODO - Translation
 		),
 	),

app/i18n/ru/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Welcome %s,',	// TODO - Translation
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'You must accept the Terms of Service to be able to register.',	// TODO - Translation
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/i18n/sk/gen.php

@@ -43,7 +43,7 @@ return array(
 		'reset' => 'Reset prihlásenia',
 		'username' => array(
 			'admin' => 'Administrátorské používateľské meno',
-			'format' => '<small>maximálne 16 alfanumerických znakov</small>',
+			'format' => '<small>Maximálne 16 alfanumerických znakov</small>',
 			'_' => 'Používateľské meno',
 		),
 	),

app/i18n/sk/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Welcome %s,',	// TODO - Translation
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'You must accept the Terms of Service to be able to register.',	// TODO - Translation
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/i18n/tr/gen.php

@@ -43,7 +43,7 @@ return array(
 		'reset' => 'Kimlik doğrulama sıfırla',
 		'username' => array(
 			'admin' => 'Yönetici kullanıcı adı',
-			'format' => '<small>en fazla 16 alfanümerik karakter</small>',
+			'format' => '<small>En fazla 16 alfanümerik karakter</small>',
 			'_' => 'Kullancı adı',
 		),
 	),

app/i18n/tr/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => 'Welcome %s,',	// TODO - Translation
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => 'You must accept the Terms of Service to be able to register.',	// TODO - Translation
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/i18n/zh-cn/user.php

@@ -29,9 +29,16 @@ return array(
 			'welcome' => '欢迎来到 %s,',
 		),
 	),
+	'password' => array(
+		'invalid' => 'The password is invalid.',	// TODO - Translation
+	),
 	'tos' => array(
 		'feedback' => array(
 			'invalid' => '您必须接受服务条款才能注册',
 		),
 	),
+	'username' => array(
+		'invalid' => 'The username is invalid.',	// TODO - Translation
+		'taken' => 'The username %s is taken.',	// TODO - Translation
+	),
 );

app/views/auth/formLogin.phtml

@@ -7,25 +7,30 @@
 
 	<form id="crypto-form" method="post" action="<?= _url('auth', 'login') ?>">
 		<input type="hidden" name="_csrf" value="<?= FreshRSS_Auth::csrfToken() ?>" />
-		<div>
+
+		<div class="form-group">
 			<label for="username"><?= _t('gen.auth.username') ?></label>
 			<input type="text" id="username" name="username" autocomplete="username" size="16" required="required" pattern="<?= FreshRSS_user_Controller::USERNAME_PATTERN ?>" autofocus="autofocus" />
 		</div>
-		<div>
+
+		<div class="form-group">
 			<label for="passwordPlain"><?= _t('gen.auth.password') ?></label>
-				<input type="password" id="passwordPlain" required="required" />
-				<input type="hidden" id="challenge" name="challenge" /><br />
-				<noscript><strong><?= _t('gen.js.should_be_activated') ?></strong></noscript>
+			<input type="password" id="passwordPlain" required="required" />
+			<input type="hidden" id="challenge" name="challenge" />
+			<noscript><strong><?= _t('gen.js.should_be_activated') ?></strong></noscript>
 		</div>
-		<div>
+
+		<div class="form-group">
 			<label class="checkbox" for="keep_logged_in">
 				<input type="checkbox" name="keep_logged_in" id="keep_logged_in" value="1" />
 				<?= _t('gen.auth.keep_logged_in', $this->cookie_days) ?>
 			</label>
-			<br />
 		</div>
-		<div>
-			<button id="loginButton" type="submit" class="btn btn-important"><?= _t('gen.auth.login') ?></button>
+
+		<div class="form-group form-group-actions">
+			<button id="loginButton" type="submit" class="btn btn-important">
+				<?= _t('gen.auth.login') ?>
+			</button>
 		</div>
 	</form>
 

app/views/auth/register.phtml

@@ -3,22 +3,23 @@
 
 	<form method="post" action="<?= _url('user', 'create') ?>">
 		<input type="hidden" name="_csrf" value="<?= FreshRSS_Auth::csrfToken() ?>" />
-		<div>
-			<label class="group-name" for="new_user_name"><?= _t('gen.auth.username'), '<br />', _i('help'), ' ', _t('gen.auth.username.format') ?></label>
+
+		<div class="form-group">
+			<label for="new_user_name"><?= _t('gen.auth.username'), '<br />', _i('help'), ' ', _t('gen.auth.username.format') ?></label>
 			<input id="new_user_name" name="new_user_name" type="text" size="16" required="required" autocomplete="off" pattern="<?= FreshRSS_user_Controller::USERNAME_PATTERN ?>" />
 		</div>
 
 		<?php if ($this->show_email_field) { ?>
-			<div>
-				<label class="group-name" for="new_user_email">
+			<div class="form-group">
+				<label for="new_user_email">
 					<?= _t('gen.auth.email') ?>
 				</label>
 				<input id="new_user_email" name="new_user_email" type="email" required />
 			</div>
 		<?php } ?>
 
-		<div>
-			<label class="group-name" for="new_user_passwordPlain"><?= _t('gen.auth.password'), '<br />', _i('help'), ' ', _t('gen.auth.password.format') ?></label>
+		<div class="form-group">
+			<label for="new_user_passwordPlain"><?= _t('gen.auth.password'), '<br />', _i('help'), ' ', _t('gen.auth.password.format') ?></label>
 			<div class="stick">
 				<input type="password" id="new_user_passwordPlain" name="new_user_passwordPlain" required="required" autocomplete="new-password" pattern=".{7,}" />
 				<a class="btn toggle-password" data-toggle="new_user_passwordPlain"><?= _i('key') ?></a>
@@ -27,7 +28,7 @@
 		</div>
 
 		<?php if ($this->show_tos_checkbox) { ?>
-			<div>
+			<div class="form-group">
 				<label class="checkbox" for="accept-tos">
 					<input type="checkbox" name="accept_tos" id="accept-tos" value="1" required />
 					<?= _t('gen.auth.accept_tos', _url('index', 'tos')) ?>
@@ -35,7 +36,7 @@
 			</div>
 		<?php } ?>
 
-		<div>
+		<div class="form-group form-group-actions">
 			<?php
 				$redirect_url = urlencode(Minz_Url::display(
 					array('c' => 'index', 'a' => 'index'),
@@ -43,8 +44,10 @@
 				));
 			?>
 			<input type="hidden" name="r" value="<?= $redirect_url ?>" />
+
 			<button type="submit" class="btn btn-important"><?= _t('gen.action.create') ?></button>
-			<a class="btn" href="<?= _url('index', 'index') ?>"><?= _t('gen.action.cancel') ?></a>
+
+			<a href="<?= _url('index', 'index') ?>"><?= _t('gen.action.cancel') ?></a>
 		</div>
 	</form>
 

p/themes/Origine/origine.css

@@ -693,23 +693,50 @@ a.btn {
 
 /*=== Prompt (centered) */
 .prompt {
+	max-width: 20rem;
+	margin-left: auto;
+	margin-right: auto;
+	padding-left: .5rem;
+	padding-right: .5rem;
+
 	text-align: center;
 }
 
-.prompt label {
+.prompt form {
+	margin-top: 2rem;
+	margin-bottom: 3rem;
+
 	text-align: left;
 }
 
-.prompt form {
-	margin: 10px auto 20px auto;
-	width: 200px;
+.prompt .form-group {
+	margin-bottom: 1rem;
+}
+
+.prompt .form-group::after {
+	display: none;
 }
 
+.prompt .form-group.form-group-actions {
+	display: flex;
+	margin-top: 2rem;
+
+	align-items: center;
+	justify-content: space-between;
+}
+
+.prompt .stick,
 .prompt input {
-	margin: 5px auto;
 	width: 100%;
 }
 
+.prompt .btn.btn-important {
+	padding-left: 1.5rem;
+	padding-right: 1.5rem;
+
+	font-size: 1.1rem;
+}
+
 .prompt p {
 	margin: 20px 0;
 }

tests/app/Utils/passwordUtilTest.php

@@ -0,0 +1,27 @@
+<?php
+
+class FreshRSS_password_UtilTest extends PHPUnit\Framework\TestCase {
+	public function testCheck() {
+		$password = '1234567';
+
+		$ok = FreshRSS_password_Util::check($password);
+
+		$this->assertTrue($ok);
+	}
+
+	public function testCheckReturnsFalseIfEmpty() {
+		$password = '';
+
+		$ok = FreshRSS_password_Util::check($password);
+
+		$this->assertFalse($ok);
+	}
+
+	public function testCheckReturnsFalseIfLessThan7Characters() {
+		$password = '123456';
+
+		$ok = FreshRSS_password_Util::check($password);
+
+		$this->assertFalse($ok);
+	}
+}