Hardening and test adjustments

lib/Db/OldPasswordMapper.php

@@ -71,11 +71,9 @@ public function getLatestPassword($uid) {
 	 * In addition, passwords from multiple users are expected
 	 * @param int $maxTimestamp timestamp marker, last passwords changed before
 	 * the timestamp will be selected
-	 * @return OldPassword[] the selected passwords
+	 * @return Generator to traverse the selected passwords
 	 */
 	public function getPasswordsAboutToExpire($maxTimestamp) {
-		$oldPasswords = [];
-
 		$query = "SELECT `f`.`id`, `f`.`uid`, `f`.`password`, `f`.`change_time` FROM (";
 		$query .= "SELECT `uid`, max(`change_time`) AS `maxtime` FROM `*PREFIX*user_password_history` GROUP BY `uid`";
 		$query .= ") AS `x` INNER JOIN `*PREFIX*user_password_history` AS `f` ON `f`.`uid` = `x`.`uid` AND `f`.`change_time` = `x`.`maxtime`";
@@ -93,9 +91,8 @@ public function getPasswordsAboutToExpire($maxTimestamp) {
 		}
 
 		while ($row = $stmt->fetch()) {
-			$oldPasswords[] = OldPassword::fromRow($row);
+			yield OldPassword::fromRow($row);
 		}
 		$stmt->closeCursor();
-		return $oldPasswords;
 	}
 }

lib/Jobs/PasswordExpirationNotifierJob.php

@@ -107,6 +107,15 @@ protected function run($arguments) {
 		}
 	}
 
+	/**
+	 * Send an "about to expire" notification using the password information
+	 * in $passInfo. The password should expire after $expirationTime (90 days
+	 * by default). This information will also be used in the notification
+	 * @param OldPassword $passInfo the password information used to send the
+	 * notification
+	 * @param int $expirationTime the time to expire the password in seconds
+	 * (for example, 90 days - in seconds)
+	 */
 	private function sendAboutToExpireNotification(OldPassword $passInfo, $expirationTime) {
 		if ($this->unConfigHandler->isSentAboutToExpireNotification($passInfo)) {
 			return;  // notification already sent
@@ -134,6 +143,15 @@ private function sendAboutToExpireNotification(OldPassword $passInfo, $expiratio
 		$this->unConfigHandler->markAboutToExpireNotificationSentFor($passInfo);
 	}
 
+	/**
+	 * Send an "expired" notification using the password information
+	 * in $passInfo. The password should expire after $expirationTime (90 days
+	 * by default). This information will also be used in the notification
+	 * @param OldPassword $passInfo the password information used to send the
+	 * notification
+	 * @param int $expirationTime the time to expire the password in seconds
+	 * (for example, 90 days - in seconds)
+	 */
 	private function sendPassExpiredNotification(OldPassword $passInfo, $expirationTime) {
 		if ($this->unConfigHandler->isSentExpiredNotification($passInfo)) {
 			return;  // notification already sent

lib/Notifier.php

@@ -24,7 +24,7 @@
 use OCP\Notification\IManager as INotificationManager;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\L10N\IFactory;
-use OC\L10N\L10N;
+use OCP\IL10N;
 
 class Notifier implements INotifier {
 	/** @var IFactory */
@@ -63,7 +63,7 @@ public function prepare(INotification $notification, $languageCode) {
 		}
 	}
 
-	private function formatAboutToExpire(INotification $notification, L10N $l) {
+	private function formatAboutToExpire(INotification $notification, IL10N $l) {
 		$params = $notification->getSubjectParameters();
 		$notification->setParsedSubject(
 			(string) $l->t('Password expiration notice', $params)
@@ -103,7 +103,7 @@ private function formatAboutToExpire(INotification $notification, L10N $l) {
 		return $notification;
 	}
 
-	private function formatExpired(INotification $notification, L10N $l) {
+	private function formatExpired(INotification $notification, IL10N $l) {
 		$params = $notification->getSubjectParameters();
 		$notification->setParsedSubject(
 			(string) $l->t('Your password has expired', $params)

lib/UserNotificationConfigHandler.php

@@ -55,7 +55,7 @@ public function getExpirationTime() {
 			'spv_user_password_expiration_value',
 			null
 		);
-		if ($expirationTime === null || !\is_numeric($expirationTime)) {
+		if ($expirationTime === null || !\is_numeric($expirationTime) || $expirationTime < 0) {
 			return null;  // passwords don't expire or have weird value
 		}
 		// the expiration time is currently stored in days, so we need to convert
@@ -65,7 +65,7 @@ public function getExpirationTime() {
 
 	/**
 	 * Return the number of seconds until a user should receive a notification
-	 * that his password is about to expire. This _should_ be less than the value
+	 * that their password is about to expire. This _should_ be less than the value
 	 * returned by the getExpirationTime function (you'll need to verify it outside)
 	 * It will return null if the value isn't set (or disabled) or it has a
 	 * non-parseable value
@@ -85,7 +85,7 @@ public function getExpirationTimeForNormalNotification() {
 			'password_policy',
 			'spv_user_password_expiration_notification_value',
 			self::DEFAULT_EXPIRATION_FOR_NORMAL_NOTIFICATION);
-		if ($expirationTime === null || !\is_numeric($expirationTime)) {
+		if ($expirationTime === null || !\is_numeric($expirationTime) || $expirationTime < 0) {
 			return null;  // passwords don't expire or have weird value
 		}
 		return \intval($expirationTime);

tests/Controller/PasswordControllerTest.php

@@ -135,7 +135,7 @@ public function testUpdatePasswordsMismatch() {
 		$redirect_url = 'redirect/target';
 		$this->c->expects($this->once())
 			->method('createPasswordTemplateResponse')
-			->with($redirect_url, 'New passwords are not the same.');
+			->with($redirect_url, 'Password confirmation does not match the password.');
 
 		$this->userSession
 			->expects($this->never())
@@ -170,7 +170,7 @@ public function testUpdateWrongPassword() {
 		$redirect_url = 'redirect/target';
 		$this->c->expects($this->once())
 			->method('createPasswordTemplateResponse')
-			->with($redirect_url, 'Incorrect current password supplied.');
+			->with($redirect_url, 'The current password is incorrect.');
 
 		$user
 			->expects($this->never())

tests/Db/OldPasswordMapperTest.php

@@ -122,6 +122,7 @@ public function testGetPasswordsAboutToExpireSomePassMarked() {
 
 
 		$passwordList = $this->mapper->getPasswordsAboutToExpire($baseTime + 44);
+		$passwordList = \iterator_to_array($passwordList);  // convert to array
 		// last password change before the timestamp
 		$this->assertCount(2, $passwordList);
 
@@ -144,6 +145,7 @@ public function testGetPasswordsAboutToExpireAllMarked() {
 
 
 		$passwordList = $this->mapper->getPasswordsAboutToExpire($baseTime + 130);
+		$passwordList = \iterator_to_array($passwordList);  // convert to array
 		// last password change before the timestamp
 		$this->assertCount(3, $passwordList);
 

tests/NotifierTest.php

@@ -99,7 +99,7 @@ public function testPrepareAboutToExpire() {
 
 		$notification->expects($this->once())
 			->method('setParsedSubject')
-			->with('Your password is about to expire!');
+			->with('Password expiration notice');
 
 		$notification->expects($this->once())
 			->method('setParsedMessage')
@@ -130,7 +130,7 @@ public function testPrepareAboutToExpireWithAction() {
 
 		$notification->expects($this->once())
 			->method('setParsedSubject')
-			->with('Your password is about to expire!');
+			->with('Password expiration notice');
 
 		$notification->expects($this->once())
 			->method('setParsedMessage')
@@ -160,7 +160,7 @@ public function testPrepareAboutToExpirePassDate() {
 
 		$notification->expects($this->once())
 			->method('setParsedSubject')
-			->with('Your password is about to expire!');
+			->with('Password expiration notice');
 
 		$notification->expects($this->once())
 			->method('setParsedMessage')
@@ -195,7 +195,7 @@ public function testPrepareExpired() {
 
 		$notification->expects($this->once())
 			->method('setParsedMessage')
-			->with('You have to change your password before you can access again');
+			->with('Please change your password to gain back access to your account');
 
 		$notification->expects($this->once())
 			->method('addParsedAction')

tests/Rules/PasswordHistoryTest.php

@@ -74,7 +74,7 @@ public function setUp() {
 	 * @dataProvider failDataProvider
 	 * @param string $password
 	 * @expectedException \OCA\PasswordPolicy\Rules\PolicyException
-	 * @expectedExceptionMessage The password must be different to your previous 2 passwords.
+	 * @expectedExceptionMessage The password must be different than your previous 2 passwords.
 	 */
 	public function testWithOldPassword($password) {
 		$this->r->verify($password, 2, 'testuser');