From 378f4e78fc72af3e8906bef18704e4434a06bfd0 Mon Sep 17 00:00:00 2001 From: Achim <39946364+pxlfrk@users.noreply.github.com> Date: Thu, 10 Dec 2020 10:49:17 +0100 Subject: [PATCH 1/2] =?UTF-8?q?=E2=9C=A8=20add=20user=20instructions=20and?= =?UTF-8?q?=20regex=20option?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Achim Blanarsch Signed-off-by: Achim <39946364+pxlfrk@users.noreply.github.com> --- css/style.css | 4 ++++ lib/Controller/RegisterController.php | 3 +++ lib/Controller/SettingsController.php | 34 +++++++++++++++++++++++++++ lib/Service/MailService.php | 11 +++++++++ lib/Service/RegistrationService.php | 7 ++++++ lib/Settings/RegistrationSettings.php | 8 +++++++ templates/admin.php | 24 +++++++++++++++++++ templates/form/user.php | 8 +++++++ 8 files changed, 99 insertions(+) diff --git a/css/style.css b/css/style.css index 7171a26..d4526b2 100644 --- a/css/style.css +++ b/css/style.css @@ -33,6 +33,10 @@ input[type="submit"] { text-decoration: underline; } +.error { + margin-bottom: 15px; +} + .groupofone { position: relative; } diff --git a/lib/Controller/RegisterController.php b/lib/Controller/RegisterController.php index 3f83d4a..a3a7438 100644 --- a/lib/Controller/RegisterController.php +++ b/lib/Controller/RegisterController.php @@ -222,12 +222,15 @@ class RegisterController extends Controller { } catch (RegistrationException $e) { 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'); } diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index d939fd9..204e17d 100644 --- a/lib/Controller/SettingsController.php +++ b/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'); diff --git a/lib/Service/MailService.php b/lib/Service/MailService.php index 0a8b688..c90cbf3 100644 --- a/lib/Service/MailService.php +++ b/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; @@ -102,6 +107,12 @@ class MailService { htmlspecialchars($body . ' ' . $this->l10n->t('Click the button below to continue.')), $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()) diff --git a/lib/Service/RegistrationService.php b/lib/Service/RegistrationService.php index 65bde42..025e9ed 100644 --- a/lib/Service/RegistrationService.php +++ b/lib/Service/RegistrationService.php @@ -233,6 +233,13 @@ 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 === '')) { + if (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.')); } diff --git a/lib/Settings/RegistrationSettings.php b/lib/Settings/RegistrationSettings.php index b783dd7..5414213 100644 --- a/lib/Settings/RegistrationSettings.php +++ b/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, diff --git a/templates/admin.php b/templates/admin.php index a04270d..11d7c79 100644 --- a/templates/admin.php +++ b/templates/admin.php @@ -59,6 +59,30 @@ foreach ($_['groups'] as $group) {

+

t('Username policy')); ?>

+

+ +

+ 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.'));?> + +

t('User instructions')); ?>

+ 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.'));?> +

+ +

+ 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.'));?> + +

+ +

+ t('Add additional user instructions (e.g. for choosing their usernames). If configured the text is embedded in the the verification-Email.'));?> +

t('Admin approval')); ?>

t('Welcome, you can create your account below.'));?> + + +

+ + +

From fbb0b22155352eb81f0d8b001b3659d7b61351fa Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 14 Dec 2020 11:49:05 +0100 Subject: [PATCH 2/2] Fix and add new unit tests Signed-off-by: Joas Schilling --- lib/Service/RegistrationService.php | 6 +-- .../Controller/RegisterControllerTest.php | 1 + .../Unit/Service/RegistrationServiceTest.php | 38 +++++++++++++++++-- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/lib/Service/RegistrationService.php b/lib/Service/RegistrationService.php index 025e9ed..6d9ff14 100644 --- a/lib/Service/RegistrationService.php +++ b/lib/Service/RegistrationService.php @@ -234,10 +234,8 @@ class RegistrationService { } $regex = $this->config->getAppValue($this->appName, 'username_policy_regex', ''); - if (!($regex === '')) { - if (preg_match($regex, $username) === 0) { - throw new RegistrationException($this->l10n->t('Please provide a valid user name.')); - } + 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) { diff --git a/tests/Unit/Controller/RegisterControllerTest.php b/tests/Unit/Controller/RegisterControllerTest.php index 25ea874..5b9324c 100644 --- a/tests/Unit/Controller/RegisterControllerTest.php +++ b/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()); } diff --git a/tests/Unit/Service/RegistrationServiceTest.php b/tests/Unit/Service/RegistrationServiceTest.php index 7994d07..7a819cc 100644 --- a/tests/Unit/Service/RegistrationServiceTest.php +++ b/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];