Merge branch 'pxlfrk/master'

* pxlfrk/master:
  Fix and add new unit tests
  ✨ add user instructions and regex option

css/style.css

@@ -33,6 +33,10 @@ input[type="submit"] {
 	text-decoration: underline;
 }
 
+.error {
+	margin-bottom: 15px;
+}
+
 .groupofone {
 	position: relative;
 }

lib/Controller/RegisterController.php

@@ -223,11 +223,14 @@ class RegisterController extends Controller {
 			return $this->validateSecretAndTokenErrorPage();
 		}
 		
+		$additional_hint = $this->config->getAppValue('registration', 'additional_hint');
+
 		return new TemplateResponse('registration', 'form/user', [
 			'email' => $registration->getEmail(),
 			'email_is_login' => $this->config->getAppValue('registration', 'email_is_login', 'no') === 'yes',
 			'username' => $username,
 			'message' => $message,
+			'additional_hint' => $additional_hint,
 		], 'guest');
 	}
 

lib/Controller/SettingsController.php

@@ -44,6 +44,9 @@ class SettingsController extends Controller {
 	 *
 	 * @param string $registered_user_group all newly registered user will be put in this group
 	 * @param string $allowed_domains Registrations are only allowed for E-Mailadresses with these domains
+	 * @param string $additional_hint show Text at user-creation form
+	 * @param string $email_verification_hint if filled embed Text in Verification mail send to user
+	 * @param string $username_policy_regex optional regex to check usernames against a pattern
 	 * @param bool|null $admin_approval_required newly registered users have to be validated by an admin
 	 * @param bool|null $email_is_login email address is forced as user id
 	 * @param bool|null $domains_is_blocklist is the domain list an allow or block list
@@ -52,6 +55,9 @@ class SettingsController extends Controller {
 	 */
 	public function admin(string $registered_user_group,
 						  string $allowed_domains,
+						  string $additional_hint,
+						  string $email_verification_hint,
+						  string $username_policy_regex,
 						  ?bool $admin_approval_required,
 						  ?bool $email_is_login,
 						  ?bool $domains_is_blocklist,
@@ -64,6 +70,34 @@ class SettingsController extends Controller {
 			$this->config->setAppValue($this->appName, 'allowed_domains', $allowed_domains);
 		}
 
+		// handle hints
+		if (($additional_hint === '') || ($additional_hint === null)) {
+			$this->config->deleteAppValue($this->appName, 'additional_hint');
+		} else {
+			$this->config->setAppValue($this->appName, 'additional_hint', $additional_hint);
+		}
+
+		if (($email_verification_hint === '') || ($email_verification_hint === null)) {
+			$this->config->deleteAppValue($this->appName, 'email_verification_hint');
+		} else {
+			$this->config->setAppValue($this->appName, 'email_verification_hint', $email_verification_hint);
+		}
+
+		//handle regex
+		if (($username_policy_regex === '') || ($username_policy_regex === null)) {
+			$this->config->deleteAppValue($this->appName, 'username_policy_regex');
+		} elseif ((@preg_match($username_policy_regex, null) === false)) {
+			// validate regex
+			return new DataResponse([
+				'data' => [
+					'message' => $this->l10n->t('Invalid username policy regex'),
+				],
+				'status' => 'error',
+			], Http::STATUS_BAD_REQUEST);
+		} else {
+			$this->config->setAppValue($this->appName, 'username_policy_regex', $username_policy_regex);
+		}
+
 		$this->config->setAppValue($this->appName, 'admin_approval_required', $admin_approval_required ? 'yes' : 'no');
 		$this->config->setAppValue($this->appName, 'email_is_login', $email_is_login ? 'yes' : 'no');
 		$this->config->setAppValue($this->appName, 'domains_is_blocklist', $domains_is_blocklist ? 'yes' : 'no');

lib/Service/MailService.php

@@ -36,6 +36,7 @@ use OCP\ILogger;
 use OCP\IURLGenerator;
 use OCP\Mail\IMailer;
 use OCP\Util;
+use OCP\IConfig;
 
 class MailService {
 
@@ -51,15 +52,19 @@ class MailService {
 	private $groupManager;
 	/** @var ILogger */
 	private $logger;
+	/** @var IConfig */
+	private $config;
 
 	public function __construct(IURLGenerator $urlGenerator,
 								IMailer $mailer,
 								Defaults $defaults,
 								IL10N $l10n,
 								IGroupManager $groupManager,
+								IConfig $config,
 								ILogger $logger) {
 		$this->urlGenerator = $urlGenerator;
 		$this->mailer = $mailer;
+		$this->config = $config;
 		$this->defaults = $defaults;
 		$this->l10n = $l10n;
 		$this->groupManager = $groupManager;
@@ -103,6 +108,12 @@ class MailService {
 			$body
 		);
 		
+		// if the parameter is set through the settings panel add to body text
+		$email_verification_hint = $this->config->getAppValue('registration', 'email_verification_hint');
+		if (!empty($email_verification_hint)) {
+			$template->addBodyText($email_verification_hint);
+		};
+
 		$template->addBodyText(
 			$this->l10n->t('Verification code: %s', $registration->getToken())
 		);

lib/Service/RegistrationService.php

@@ -233,6 +233,11 @@ class RegistrationService {
 			throw new RegistrationException($this->l10n->t('Please provide a valid user name.'));
 		}
 
+		$regex = $this->config->getAppValue($this->appName, 'username_policy_regex', '');
+		if ($regex && preg_match($regex, $username) === 0) {
+			throw new RegistrationException($this->l10n->t('Please provide a valid user name.'));
+		}
+
 		if ($this->registrationMapper->usernameIsPending($username) || $this->userManager->get($username) !== null) {
 			throw new RegistrationException($this->l10n->t('The username you have chosen already exists.'));
 		}

lib/Settings/RegistrationSettings.php

@@ -56,9 +56,14 @@ class RegistrationSettings implements ISettings {
 		}
 		$assignedGroups = $this->config->getAppValue($this->appName, 'registered_user_group', 'none');
 
+		// handle additional hint
+		$additional_hint = $this->config->getAppValue($this->appName, 'additional_hint', '');
+		$email_verification_hint = $this->config->getAppValue($this->appName, 'email_verification_hint', '');
+
 		// handle domains
 		$allowedDomains = $this->config->getAppValue($this->appName, 'allowed_domains', '');
 
+		$username_policy_regex = $this->config->getAppValue($this->appName, 'username_policy_regex', '');
 		$adminApprovalRequired = $this->config->getAppValue($this->appName, 'admin_approval_required', 'no');
 		$emailIsLogin = $this->config->getAppValue($this->appName, 'email_is_login', 'no');
 		$domainsIsBlocklist = $this->config->getAppValue($this->appName, 'domains_is_blocklist', 'no');
@@ -68,6 +73,9 @@ class RegistrationSettings implements ISettings {
 		return new TemplateResponse('registration', 'admin', [
 			'groups' => $groupIds,
 			'current' => $assignedGroups,
+			'additional_hint' => $additional_hint,
+			'email_verification_hint' => $email_verification_hint,
+			'username_policy_regex' => $username_policy_regex,
 			'allowed' => $allowedDomains,
 			'approval_required' => $adminApprovalRequired,
 			'email_is_login' => $emailIsLogin,

templates/admin.php

@@ -59,6 +59,30 @@ foreach ($_['groups'] as $group) {
 		<label for="email_is_login"><?php p($l->t('Force email as login name')); ?></label>
 	</p>
 
+	<h3><?php p($l->t('Username policy')); ?></h3>
+	<p>
+		<label>
+			<input type="text" id="username_policy_regex" name="username_policy_regex" value="<?php p($_['username_policy_regex']);?>" placeholder="E.g.: /^[a-z-]+\.[a-z-]+$/">
+		</label>
+	</p>
+	<em><?php p($l->t('If configured usernames will be validated through the regular expression. If the validation fails the user is prompted with a generic error. Make sure your regex is working correctly.'));?></em>
+
+	<h3><?php p($l->t('User instructions')); ?></h3>
+	<em><?php p($l->t('Caution: The user instructions will not be translated and will therefore be displayed as configured below for all users regardless of their actual language.'));?></em>
+	<p>
+		<label>
+			<input type="text" id="additional_hint" name="additional_hint" value="<?php p($_['additional_hint']);?>" placeholder="Please create your username following the scheme 'firstname.lastname'.">
+		</label>
+	</p>
+	<em><?php p($l->t('Add additional user instructions (e.g. for choosing their usernames). If configured the text is displayed in the account creation step of the registration process.'));?></em>
+
+	<p>
+		<label>
+			<input type="text" id="email_verification_hint" name="email_verification_hint" value="<?php p($_['email_verification_hint']);?>" placeholder="Please create your username following the scheme 'firstname.lastname'.">
+		</label>
+	</p>
+	<em><?php p($l->t('Add additional user instructions (e.g. for choosing their usernames). If configured the text is embedded in the the verification-Email.'));?></em>
+
 	<h3><?php p($l->t('Admin approval')); ?></h3>
 	<p>
 		<input type="checkbox" id="admin_approval_required" class="checkbox" name="admin_approval_required" <?php if ($_['approval_required'] === 'yes') {

templates/form/user.php

@@ -15,6 +15,14 @@ script('registration', 'form');
 			<li><?php p($l->t('Welcome, you can create your account below.'));?></li>
 		</ul>
 		<?php } ?>
+
+		<?php if (!empty($_['additional_hint'])): ?>
+			<ul class="msg">
+				<li><?php p($_['additional_hint']); ?></li>
+			</ul>		
+		<?php endif; ?>
+
+
 		<p class="grouptop">
 			<input type="email" name="email" id="email" value="<?php p($_['email']); ?>" disabled />
 			<label for="email" class="infield"><?php p($_['email']); ?></label>

tests/Unit/Controller/RegisterControllerTest.php

@@ -457,6 +457,7 @@ class RegisterControllerTest extends TestCase {
 			'email_is_login' => false,
 			'username' => $username,
 			'message' => $message,
+			'additional_hint' => null,
 		], $response->getParams());
 	}
 

tests/Unit/Service/RegistrationServiceTest.php

@@ -67,6 +67,11 @@ class RegistrationServiceTest extends TestCase {
 		parent::setUp();
 		$this->mailService = $this->createMock(MailService::class);
 		$this->l10n = $this->createMock(IL10N::class);
+		$this->l10n->expects($this->any())
+			->method('t')
+			->willReturnCallback(function ($text, $parameters = []) {
+				return vsprintf($text, $parameters);
+			});
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		#$this->userManager = $this->createMock(IUserManager::class);
 		$this->userManager = \OC::$server->getUserManager();
@@ -233,7 +238,8 @@ class RegistrationServiceTest extends TestCase {
 		$reg->setEmailConfirmed(true);
 
 		$this->expectException(RegistrationException::class);
-		$resulting_user = $this->service->createAccount($reg, 'alice1', 'asdf');
+		$this->expectExceptionMessage('The username you have chosen already exists.');
+		$this->service->createAccount($reg, 'alice1', 'asdf');
 	}
 
 	/*
@@ -256,13 +262,39 @@ class RegistrationServiceTest extends TestCase {
 		$reg->setEmailConfirmed(true);
 
 		$this->expectException(RegistrationException::class);
-		$resulting_user = $this->service->createAccount($reg);
+		$this->expectExceptionMessage('The username you have chosen already exists.');
+		$this->service->createAccount($reg);
+	}
+
+	/**
+	 * @depends testDuplicateUsernameApi
+	 */
+	public function testUsernameDoesntMatchPattern() {
+
+
+		$this->config->expects($this->atLeastOnce())
+			->method('getAppValue')
+			->willReturnMap([
+				['registration', 'username_policy_regex', '', '/^[a-z]\.[a-z]+$/'],
+			]);
+
+		$reg = new Registration();
+		$reg->setEmail("pppp@example.com");
+		$reg->setUsername("alice23");
+		$reg->setDisplayname("Alice");
+		$reg->setPassword("asdf");
+		$reg->setEmailConfirmed(true);
+
+		$this->expectException(RegistrationException::class);
+		$this->expectExceptionMessage('Please provide a valid user name.');
+		$this->service->createAccount($reg);
 	}
 
 	public function settingsCallback1($app, $key, $default) {
 		$map = [
 			'registered_user_group' => 'none',
-			'admin_approval_required' => 'no'
+			'admin_approval_required' => 'no',
+			'username_policy_regex' => '',
 		];
 
 		return $map[$key];