Fixed issues with confirmation e-mails

data/config.ini

@@ -118,6 +118,7 @@ changeUserPassword.own=registered
 changeUserPassword.all=admin
 changeUserEmail.own=registered
 changeUserEmail.all=admin
+changeUserEmailNoConfirm=admin
 changeUserAccessRank=admin
 changeUserName=moderator
 changeUserSettings.all=nobody

src/Access.php

@@ -72,7 +72,7 @@ class Access
 		if ($user === null)
 			$user = Auth::getCurrentUser();
 
-		if (!$user->emailConfirmed)
+		if (!$user->getConfirmedEmail())
 			return false;
 		return true;
 	}

src/Api/Jobs/ActivateUserEmailJob.php

@@ -9,9 +9,9 @@ class ActivateUserEmailJob extends AbstractJob
 		{
 			$user = UserModel::findByNameOrEmail($this->getArgument(self::USER_NAME));
 
-			if (empty($user->emailUnconfirmed))
+			if (empty($user->getUnconfirmedEmail()))
 			{
-				if (!empty($user->emailConfirmed))
+				if (!empty($user->getConfirmedEmail()))
 					throw new SimpleException('E-mail was already confirmed; activation skipped');
 				else
 					throw new SimpleException('This user has no e-mail specified; activation cannot proceed');
@@ -55,7 +55,7 @@ class ActivateUserEmailJob extends AbstractJob
 		$mail->subject = $regConfig->confirmationEmailSubject;
 		$mail->senderName = $regConfig->confirmationEmailSenderName;
 		$mail->senderEmail = $regConfig->confirmationEmailSenderEmail;
-		$mail->recipientEmail = $user->emailUnconfirmed;
+		$mail->recipientEmail = $user->getUnconfirmedEmail();
 
 		return Mailer::sendMailWithTokenLink(
 			$user,

src/Api/Jobs/AddUserJob.php

@@ -10,6 +10,15 @@ class AddUserJob extends AbstractJob
 		$user->staffConfirmed = $firstUser;
 		UserModel::forgeId($user);
 
+		if ($firstUser)
+		{
+			$user->setAccessRank(new AccessRank(AccessRank::Admin));
+		}
+		else
+		{
+			$user->setAccessRank(new AccessRank(AccessRank::Registered));
+		}
+
 		$arguments = $this->getArguments();
 		$arguments[EditUserJob::USER_ENTITY] = $user;
 
@@ -19,18 +28,9 @@ class AddUserJob extends AbstractJob
 		Api::run($job, $arguments);
 		Logger::setBuffer([]);
 
-		if ($firstUser)
-		{
-			$user->setAccessRank(new AccessRank(AccessRank::Admin));
-			$user->confirmEmail();
-		}
-		else
-		{
-			$user->setAccessRank(new AccessRank(AccessRank::Registered));
-		}
-
 		//save the user to db if everything went okay
 		UserModel::save($user);
+		EditUserEmailJob::observeSave($user);
 
 		Logger::log('{subject} just signed up', [
 			'subject' => TextHelper::reprUser($user)]);

src/Api/Jobs/EditUserEmailJob.php

@@ -17,25 +17,18 @@ class EditUserEmailJob extends AbstractUserJob
 		$user = $this->user;
 		$newEmail = UserModel::validateEmail($this->getArgument(self::NEW_EMAIL));
 
-		$oldEmail = $user->emailConfirmed;
+		$oldEmail = $user->getConfirmedEmail();
 		if ($oldEmail == $newEmail)
 			return $user;
 
-		$user->emailUnconfirmed = $newEmail;
-		$user->emailConfirmed = null;
-
-		if (Auth::getCurrentUser()->getId() == $user->getId())
-		{
-			if (!empty($newEmail))
-				ActivateUserEmailJob::sendEmail($user);
-		}
-		else
-		{
-			$user->confirmEmail();
-		}
+		$user->setUnconfirmedEmail($newEmail);
+		$user->setConfirmedEmail(null);
 
 		if ($this->getContext() == self::CONTEXT_NORMAL)
+		{
 			UserModel::save($user);
+			self::observeSave($user);
+		}
 
 		Logger::log('{user} changed {subject}\'s e-mail to {mail}', [
 			'user' => TextHelper::reprUser(Auth::getCurrentUser()),
@@ -45,6 +38,19 @@ class EditUserEmailJob extends AbstractUserJob
 		return $user;
 	}
 
+	public static function observeSave($user)
+	{
+		if (Access::check(new Privilege(Privilege::ChangeUserEmailNoConfirm), $user))
+		{
+			$user->confirmEmail();
+		}
+		else
+		{
+			if (!empty($user->getUnconfirmedEmail()))
+				ActivateUserEmailJob::sendEmail($user);
+		}
+	}
+
 	public function requiresPrivilege()
 	{
 		return new Privilege(

src/Api/Jobs/EditUserJob.php

@@ -56,7 +56,10 @@ class EditUserJob extends AbstractUserJob
 		}
 
 		if ($this->getContext() == self::CONTEXT_NORMAL)
+		{
 			UserModel::save($user);
+			EditUserEmailJob::observeSave($user);
+		}
 
 		Logger::flush();
 		return $user;

src/Api/Jobs/PasswordResetJob.php

@@ -10,10 +10,10 @@ class PasswordResetJob extends AbstractJob
 		{
 			$user = UserModel::findByNameOrEmail($this->getArgument(self::USER_NAME));
 
-			if (empty($user->emailConfirmed))
+			if (empty($user->getConfirmedEmail()))
 				throw new SimpleException('This user has no e-mail confirmed; password reset cannot proceed');
 
-			self::sendEmail($user);
+			UserModel::sendPasswordResetEmail($user);
 
 			return $user;
 		}
@@ -54,7 +54,7 @@ class PasswordResetJob extends AbstractJob
 		$mail->subject = $regConfig->passwordResetEmailSubject;
 		$mail->senderName = $regConfig->passwordResetEmailSenderName;
 		$mail->senderEmail = $regConfig->passwordResetEmailSenderEmail;
-		$mail->recipientEmail = $user->emailConfirmed;
+		$mail->recipientEmail = $user->getConfirmedEmail();
 
 		return Mailer::sendMailWithTokenLink(
 			$user,

src/Mailer.php

@@ -2,16 +2,35 @@
 class Mailer
 {
 	private static $mailCounter = 0;
+	private static $mock = false;
+
+	public static function init()
+	{
+		self::$mailCounter = 0;
+		self::$mock = false;
+	}
 
 	public static function getMailCounter()
 	{
 		return self::$mailCounter;
 	}
 
+	public static function mockSending()
+	{
+		self::$mock = true;
+	}
+
 	public static function sendMail(Mail $mail, array $tokens = [])
 	{
+		$host = isset($_SERVER['HTTP_HOST'])
+			? $_SERVER['HTTP_HOST']
+			: '';
+		$ip = isset($_SERVER['SERVER_ADDR'])
+			? $_SERVER['SERVER_ADDR']
+			: '';
+
 		if (!isset($tokens['host']))
-			$tokens['host'] = $_SERVER['HTTP_HOST'];
+			$tokens['host'] = $host;
 
 		if (!isset($tokens['nl']))
 			$tokens['nl'] = PHP_EOL;
@@ -25,7 +44,7 @@ class Mailer
 		if (empty($recipientEmail))
 			throw new SimpleException('Destination e-mail address was not found');
 
-		$messageId = $_SERVER['REQUEST_TIME'] . md5($_SERVER['REQUEST_TIME']) . '@' . $_SERVER['HTTP_HOST'];
+		$messageId = $_SERVER['REQUEST_TIME'] . md5($_SERVER['REQUEST_TIME']) . '@' . $host;
 
 		$headers = [];
 		$headers []= sprintf('MIME-Version: 1.0');
@@ -38,9 +57,11 @@ class Mailer
 		$headers []= sprintf('Subject: %s', $subject);
 		$headers []= sprintf('Content-Type: text/plain; charset=utf-8', $subject);
 		$headers []= sprintf('X-Mailer: PHP/%s', phpversion());
-		$headers []= sprintf('X-Originating-IP: %s', $_SERVER['SERVER_ADDR']);
+		$headers []= sprintf('X-Originating-IP: %s', $ip);
 		$encodedSubject = '=?UTF-8?B?' . base64_encode($subject) . '?=';
-		mail($recipientEmail, $encodedSubject, $body, implode("\r\n", $headers), '-f' . $senderEmail);
+
+		if (!self::$mock)
+			mail($recipientEmail, $encodedSubject, $body, implode("\r\n", $headers), '-f' . $senderEmail);
 
 		self::$mailCounter ++;
 
@@ -63,7 +84,8 @@ class Mailer
 		$token->expires = null;
 		TokenModel::save($token);
 
-		$tokens['link'] = \Chibi\Router::linkTo($linkDestination, ['token' => $token->token]);
+		if (!self::$mock)
+			$tokens['link'] = \Chibi\Router::linkTo($linkDestination, ['token' => $token->token]);
 
 		return self::sendMail($mail, $tokens);
 	}

src/Models/Entities/UserEntity.php

@@ -8,8 +8,8 @@ class UserEntity extends AbstractEntity implements IValidatable
 	protected $passSalt;
 	protected $passHash;
 	public $staffConfirmed;
-	public $emailUnconfirmed;
-	public $emailConfirmed;
+	protected $emailUnconfirmed;
+	protected $emailConfirmed;
 	public $joinDate;
 	public $lastLoginDate;
 	protected $accessRank;
@@ -42,7 +42,7 @@ class UserEntity extends AbstractEntity implements IValidatable
 		$otherUser = UserModel::findByName($userName, false);
 		if ($otherUser !== null and $otherUser->getId() != $this->getId())
 		{
-			if (!$otherUser->emailConfirmed
+			if (!$otherUser->getConfirmedEmail()
 				and isset($config->registration->needEmailForRegistering)
 				and $config->registration->needEmailForRegistering)
 			{
@@ -128,6 +128,26 @@ class UserEntity extends AbstractEntity implements IValidatable
 		$this->name = trim($name);
 	}
 
+	public function getUnconfirmedEmail()
+	{
+		return $this->emailUnconfirmed;
+	}
+
+	public function setUnconfirmedEmail($email)
+	{
+		$this->emailUnconfirmed = $email;
+	}
+
+	public function getConfirmedEmail()
+	{
+		return $this->emailConfirmed;
+	}
+
+	public function setConfirmedEmail($email)
+	{
+		$this->emailConfirmed = $email;
+	}
+
 	public function getPasswordHash()
 	{
 		return $this->passHash;
@@ -164,8 +184,8 @@ class UserEntity extends AbstractEntity implements IValidatable
 
 	public function getAvatarUrl($size = 32)
 	{
-		$subject = !empty($this->emailConfirmed)
-			? $this->emailConfirmed
+		$subject = !empty($this->getConfirmedEmail())
+			? $this->getConfirmedEmail()
 			: $this->passSalt . $this->getName();
 		$hash = md5(strtolower(trim($subject)));
 		$url = 'http://www.gravatar.com/avatar/' . $hash . '?s=' . $size . '&d=retro';
@@ -259,11 +279,11 @@ class UserEntity extends AbstractEntity implements IValidatable
 
 	public function confirmEmail()
 	{
-		if (!empty($this->emailConfirmed))
+		if (empty($this->getUnconfirmedEmail()))
 			return;
 
-		$this->emailConfirmed = $this->emailUnconfirmed;
-		$this->emailUnconfirmed = null;
+		$this->setConfirmedEmail($this->getUnconfirmedEmail());
+		$this->setUnconfirmedEmail(null);
 	}
 
 	public function hasFavorited($post)

src/Models/Enums/Privilege.php

@@ -36,6 +36,7 @@ class Privilege extends Enum
 	const ChangeUserPassword = 15;
 	const ChangeUserAccessRank = 16;
 	const ChangeUserEmail = 17;
+	const ChangeUserEmailNoConfirm = 46;
 	const ChangeUserName = 18;
 	const ChangeUserSettings = 28;
 	const DeleteUser = 19;

src/Models/UserModel.php

@@ -46,8 +46,8 @@ class UserModel extends AbstractCrudModel
 				'pass_salt' => $user->getPasswordSalt(),
 				'pass_hash' => $user->getPasswordHash(),
 				'staff_confirmed' => $user->staffConfirmed,
-				'email_unconfirmed' => $user->emailUnconfirmed,
-				'email_confirmed' => $user->emailConfirmed,
+				'email_unconfirmed' => $user->getUnconfirmedEmail(),
+				'email_confirmed' => $user->getConfirmedEmail(),
 				'join_date' => $user->joinDate,
 				'last_login_date' => $user->lastLoginDate,
 				'access_rank' => $user->getAccessRank()->toInteger(),

src/Views/user-view.phtml

@@ -58,9 +58,9 @@ Assets::addStylesheet('user-view.css');
 
 			<div class="key-value email">
 				<span class="key">E-mail:</span>
-				<span class="value" title="<?= $val = ($this->context->transport->user->emailUnconfirmed
-						? '(unconfirmed) ' . $this->context->transport->user->emailUnconfirmed
-						: $this->context->transport->user->emailConfirmed ?: 'none specified') ?>">
+				<span class="value" title="<?= $val = ($this->context->transport->user->getUnconfirmedEmail()
+						? '(unconfirmed) ' . $this->context->transport->user->getUnconfirmedEmail()
+						: $this->context->transport->user->getConfirmedEmail() ?: 'none specified') ?>">
 					<?= $val ?>
 				</span>
 				<br>(only you and staff can see this)

src/core.php

@@ -81,6 +81,7 @@ function prepareEnvironment($testEnvironment)
 	Auth::setCurrentUser(null);
 	Access::init();
 	Logger::init();
+	Mailer::init();
 
 	\Chibi\Database::connect(
 		$config->main->dbDriver,

tests/JobTests/AddUserJobTest.php

@@ -15,6 +15,7 @@ class AddUserJobTest extends AbstractTest
 				]);
 		});
 
+		//first user = admin
 		$this->assert->areEqual('dummy', $user1->getName());
 		$this->assert->areEquivalent(new AccessRank(AccessRank::Admin), $user1->getAccessRank());
 		$this->assert->isFalse(empty($user1->getPasswordSalt()));
@@ -30,6 +31,7 @@ class AddUserJobTest extends AbstractTest
 				]);
 		});
 
+		//any other user = non-admin
 		$this->assert->areEquivalent(new AccessRank(AccessRank::Registered), $user2->getAccessRank());
 	}
 
@@ -48,6 +50,28 @@ class AddUserJobTest extends AbstractTest
 		}, 'Password must have at least');
 	}
 
+	public function testSkippingMailingUponFailing() //yo dog
+	{
+		getConfig()->registration->needEmailForRegistering = true;
+		Mailer::mockSending();
+
+		$this->assert->areEqual(0, Mailer::getMailCounter());
+
+		$this->grantAccess('registerAccount');
+		$this->assert->throws(function()
+		{
+			Api::run(
+				new AddUserJob(),
+				[
+					EditUserNameJob::NEW_USER_NAME => 'dummy',
+					EditUserPasswordJob::NEW_PASSWORD => str_repeat('s', getConfig()->registration->passMinLength - 1),
+					EditUserEmailJob::NEW_EMAIL => 'godzilla@whitestar.gov',
+				]);
+		}, 'Password must have at least');
+
+		$this->assert->areEqual(0, Mailer::getMailCounter());
+	}
+
 	public function testVeryLongPassword()
 	{
 		$this->grantAccess('registerAccount');
@@ -116,4 +140,126 @@ class AddUserJobTest extends AbstractTest
 				]);
 		}, 'Insufficient privileges');
 	}
+
+	public function testEmailsMixedConfirmation()
+	{
+		getConfig()->registration->needEmailForRegistering = true;
+		Mailer::mockSending();
+		$this->assert->areEqual(0, Mailer::getMailCounter());
+
+		getConfig()->privileges->changeUserEmailNoConfirm = 'admin';
+		$this->grantAccess('registerAccount');
+
+		$user1 = $this->assert->doesNotThrow(function()
+		{
+			return Api::run(
+				new AddUserJob(),
+				[
+					EditUserNameJob::NEW_USER_NAME => 'dummy',
+					EditUserPasswordJob::NEW_PASSWORD => 'sekai',
+					EditUserEmailJob::NEW_EMAIL => 'godzilla@whitestar.gov',
+				]);
+		});
+
+		//first user = admin = has confirmed e-mail automatically
+		$this->assert->areEqual(null, $user1->getUnconfirmedEmail());
+		$this->assert->areEqual('godzilla@whitestar.gov', $user1->getConfirmedEmail());
+
+		$user2 = $this->assert->doesNotThrow(function()
+		{
+			return Api::run(
+				new AddUserJob(),
+				[
+					EditUserNameJob::NEW_USER_NAME => 'dummy2',
+					EditUserPasswordJob::NEW_PASSWORD => 'sekai',
+					EditUserEmailJob::NEW_EMAIL => 'godzilla2@whitestar.gov',
+				]);
+		});
+
+		//any other user = non-admin = has to confirmed e-mail manually
+		$this->assert->areEqual('godzilla2@whitestar.gov', $user2->getUnconfirmedEmail());
+		$this->assert->areEqual(null, $user2->getConfirmedEmail());
+
+		$this->assert->areEqual(1, Mailer::getMailCounter());
+	}
+
+	public function testEmailsEveryoneMustConfirm()
+	{
+		getConfig()->registration->needEmailForRegistering = true;
+		Mailer::mockSending();
+		$this->assert->areEqual(0, Mailer::getMailCounter());
+
+		getConfig()->privileges->changeUserEmailNoConfirm = 'nobody';
+		$this->grantAccess('registerAccount');
+
+		$user1 = $this->assert->doesNotThrow(function()
+		{
+			return Api::run(
+				new AddUserJob(),
+				[
+					EditUserNameJob::NEW_USER_NAME => 'dummy',
+					EditUserPasswordJob::NEW_PASSWORD => 'sekai',
+					EditUserEmailJob::NEW_EMAIL => 'godzilla@whitestar.gov',
+				]);
+		});
+
+		$this->assert->areEqual('godzilla@whitestar.gov', $user1->getUnconfirmedEmail());
+		$this->assert->areEqual(null, $user1->getConfirmedEmail());
+
+		$user2 = $this->assert->doesNotThrow(function()
+		{
+			return Api::run(
+				new AddUserJob(),
+				[
+					EditUserNameJob::NEW_USER_NAME => 'dummy2',
+					EditUserPasswordJob::NEW_PASSWORD => 'sekai',
+					EditUserEmailJob::NEW_EMAIL => 'godzilla2@whitestar.gov',
+				]);
+		});
+
+		$this->assert->areEqual('godzilla2@whitestar.gov', $user2->getUnconfirmedEmail());
+		$this->assert->areEqual(null, $user2->getConfirmedEmail());
+
+		$this->assert->areEqual(2, Mailer::getMailCounter());
+	}
+
+	public function testEmailsEveryoneSkipConfirm()
+	{
+		getConfig()->registration->needEmailForRegistering = true;
+		Mailer::mockSending();
+		$this->assert->areEqual(0, Mailer::getMailCounter());
+
+		getConfig()->privileges->changeUserEmailNoConfirm = 'anonymous';
+		$this->grantAccess('registerAccount');
+
+		$user1 = $this->assert->doesNotThrow(function()
+		{
+			return Api::run(
+				new AddUserJob(),
+				[
+					EditUserNameJob::NEW_USER_NAME => 'dummy',
+					EditUserPasswordJob::NEW_PASSWORD => 'sekai',
+					EditUserEmailJob::NEW_EMAIL => 'godzilla@whitestar.gov',
+				]);
+		});
+
+		$this->assert->areEqual(null, $user1->getUnconfirmedEmail());
+		$this->assert->areEqual('godzilla@whitestar.gov', $user1->getConfirmedEmail());
+
+		$user2 = $this->assert->doesNotThrow(function()
+		{
+			return Api::run(
+				new AddUserJob(),
+				[
+					EditUserNameJob::NEW_USER_NAME => 'dummy2',
+					EditUserPasswordJob::NEW_PASSWORD => 'sekai',
+					EditUserEmailJob::NEW_EMAIL => 'godzilla2@whitestar.gov',
+				]);
+		});
+
+		$this->assert->areEqual(null, $user2->getUnconfirmedEmail());
+		$this->assert->areEqual('godzilla2@whitestar.gov', $user2->getConfirmedEmail());
+
+		$this->assert->areEqual(0, Mailer::getMailCounter());
+	}
 }

tests/MiscTests/AuthTest.php

@@ -111,7 +111,7 @@ class AuthTest extends AbstractTest
 
 		$user = $this->prepareValidUser();
 		$user->staffConfirmed = false;
-		$user->emailUnconfirmed = 'test@example.com';
+		$user->setUnconfirmedEmail('test@example.com');
 		UserModel::save($user);
 
 		$this->assert->throws(function()
@@ -127,7 +127,7 @@ class AuthTest extends AbstractTest
 
 		$user = $this->prepareValidUser();
 		$user->staffConfirmed = false;
-		$user->emailConfirmed = 'test@example.com';
+		$user->setConfirmedEmail('test@example.com');
 		UserModel::save($user);
 
 		$this->assert->doesNotThrow(function()