Hardening and test adjustments

appinfo/info.xml

@@ -12,7 +12,7 @@ The definition of certain password rules support administrators in the task of e
 Password history and expiration policies are supplements that allow IT to establish a level of password security that can comply with corporate guidelines of all sorts. The provided tools enable administrators to granularly choose their desired security level. At this point it is important to keep in mind that high levels of security might sacrifice usability and come at the expense of user experience. For this reason it is highly recommended to check [best practices](https://pages.nist.gov/800-63-3/sp800-63b.html) and decide carefully on the hurdles that are put upon users in order to maintain and optimize user adoption and satisfaction.
 
 Administrators find the configuration options in the 'Security' section of the ownCloud administration settings panel. The respective policies are designed for local user accounts, not for user accounts imported from LDAP or other user backends as these provide their own mechanisms. For more information and recommendations when deploying policies in an existing ownCloud, please consult the [ownCloud Documentation](https://doc.owncloud.com/server/latest/admin_manual/configuration/server/security/password-policy.html).</description>
-	<version>2.0.1</version>
+	<version>2.0.0</version>
 	<documentation>
 		<admin>https://doc.owncloud.com/server/10.0/admin_manual/configuration/server/security/password_policy.html</admin>
 	</documentation>

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');