Merge pull request #216 from nextcloud/bugfix/184/case-insensitive-email-domain-check

Enhance email handling

css/settings.css

@@ -0,0 +1,13 @@
+input,
+select {
+	width: 33%;
+	min-width: 250px;
+}
+
+h3 {
+	margin-top: 25px;
+}
+
+p {
+	margin-top: 15px;
+}

lib/Controller/RegisterController.php

@@ -17,6 +17,7 @@ declare(strict_types=1);
 namespace OCA\Registration\Controller;
 
 use Exception;
+use OCA\Registration\AppInfo\Application;
 use OCA\Registration\Db\Registration;
 use OCA\Registration\Service\LoginFlowService;
 use OCA\Registration\Service\MailService;
@@ -78,9 +79,26 @@ class RegisterController extends Controller {
 	 * @return TemplateResponse
 	 */
 	public function showEmailForm(string $email = '', string $message = ''): TemplateResponse {
+		$emailHint = '';
+		if ($this->config->getAppValue(Application::APP_ID, 'show_domains', 'no') === 'yes') {
+			if ($this->config->getAppValue(Application::APP_ID, 'domains_is_blocklist', 'no') === 'yes') {
+				$emailHint = $this->l10n->t(
+					'Registration is not allowed with the following domains:'
+				) . ' ' . implode(', ', explode(';',
+					$this->config->getAppValue(Application::APP_ID, 'allowed_domains', '')
+				));
+			} else {
+				$emailHint = $this->l10n->t(
+					'Registration is only allowed with the following domains:'
+				) . ' ' . implode(', ', explode(';',
+					$this->config->getAppValue(Application::APP_ID, 'allowed_domains', '')
+				));
+			}
+		}
+
 		$params = [
 			'email' => $email,
-			'message' => $message,
+			'message' => $message ?: $emailHint,
 		];
 		return new TemplateResponse('registration', 'form/email', $params, 'guest');
 	}
@@ -194,7 +212,7 @@ class RegisterController extends Controller {
 
 		return new TemplateResponse('registration', 'form/user', [
 			'email' => $registration->getEmail(),
-			'email_is_login' => $this->config->getAppValue('registration', 'email_is_login', '0') === '1',
+			'email_is_login' => $this->config->getAppValue('registration', 'email_is_login', 'no') === 'yes',
 			'username' => $username,
 			'message' => $message,
 		], 'guest');
@@ -218,7 +236,7 @@ class RegisterController extends Controller {
 			return $this->validateSecretAndTokenErrorPage();
 		}
 
-		if ($this->config->getAppValue('registration', 'email_is_login', '0') === '1') {
+		if ($this->config->getAppValue('registration', 'email_is_login', 'no') === 'yes') {
 			$username = $registration->getEmail();
 		}
 

lib/Controller/SettingsController.php

@@ -44,19 +44,29 @@ 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 bool $admin_approval_required newly registered users have to be validated by an admin
+	 * @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
+	 * @param bool|null $show_domains should the email list be shown to the user or not
 	 * @return DataResponse
 	 */
-	public function admin($registered_user_group, $allowed_domains, $admin_approval_required) {
+	public function admin(string $registered_user_group,
+						  string $allowed_domains,
+						  ?bool $admin_approval_required,
+						  ?bool $email_is_login,
+						  ?bool $domains_is_blocklist,
+						  ?bool $show_domains) {
 		// handle domains
-		if (($allowed_domains==='') || ($allowed_domains === null)) {
+		if (($allowed_domains === '') || ($allowed_domains === null)) {
 			$this->config->deleteAppValue($this->appName, 'allowed_domains');
 		} else {
 			$this->config->setAppValue($this->appName, 'allowed_domains', $allowed_domains);
 		}
 
-		// handle admin validation
+		$this->config->setAppValue($this->appName, 'admin_approval_required', $admin_approval_required ? 'yes' : 'no');
-		$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');
+		$this->config->setAppValue($this->appName, 'show_domains', $show_domains ? 'yes' : 'no');
 
 		// handle groups
 		$groups = $this->groupmanager->search('');

lib/Service/RegistrationService.php

@@ -34,6 +34,7 @@ use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Exceptions\PasswordlessTokenException;
 use OC\Authentication\Token\IProvider;
 use OC\Authentication\Token\IToken;
+use OCA\Registration\AppInfo\Application;
 use OCA\Registration\Db\Registration;
 use OCA\Registration\Db\RegistrationMapper;
 use OCP\AppFramework\Db\DoesNotExistException;
@@ -174,12 +175,41 @@ class RegistrationService {
 			);
 		}
 
-		if (!$this->checkAllowedDomains($email)) {
+		if ($this->config->getAppValue($this->appName, 'allowed_domains', '') === '') {
+			return;
+		}
+
+		$emailIsInDomainList = $this->checkAllowedDomains($email);
+		$blockDomains = $this->config->getAppValue(Application::APP_ID, 'domains_is_blocklist', 'no') === 'yes';
+		$showDomains = $this->config->getAppValue(Application::APP_ID, 'show_domains', 'no') === 'yes';
+
+		if (!$blockDomains && !$emailIsInDomainList) {
+			if ($showDomains) {
+				throw new RegistrationException(
+					$this->l10n->t(
+						'Registration is only allowed with the following domains:'
+					) . ' ' . implode(', ', explode(';',
+						$this->config->getAppValue(Application::APP_ID, 'allowed_domains', '')
+					))
+				);
+			}
 			throw new RegistrationException(
-				$this->l10n->t(
+				$this->l10n->t('Registration with this email domain is not allowed.')
-					'Registration is only allowed for the following domains: ' .
+			);
-					$this->config->getAppValue($this->appName, 'allowed_domains', '')
+		}
-				)
+
+		if ($blockDomains && $emailIsInDomainList) {
+			if ($showDomains) {
+				throw new RegistrationException(
+					$this->l10n->t(
+						'Registration is not allowed with the following domains:'
+					) . ' ' . implode(', ', explode(';',
+						$this->config->getAppValue(Application::APP_ID, 'allowed_domains', '')
+					))
+				);
+			}
+			throw new RegistrationException(
+				$this->l10n->t('Registration with this email domain is not allowed.')
 			);
 		}
 	}
@@ -217,17 +247,30 @@ class RegistrationService {
 	public function checkAllowedDomains(string $email): bool {
 		$allowedDomains = $this->config->getAppValue($this->appName, 'allowed_domains', '');
 		if ($allowedDomains !== '') {
-			$allowedDomains = explode(';', $allowedDomains);
+			[,$mailDomain] = explode('@', strtolower($email), 2);
-			$allowed = false;
+			$allowedDomains = explode(';', strtolower($allowedDomains));
+
 			foreach ($allowedDomains as $domain) {
-				[,$mailDomain] = explode('@', $email, 2);
 				// valid domain, everything's fine
-				if ($mailDomain === $domain) {
+
-					$allowed = true;
+				// Wildcards
-					break;
+				if (strpos($domain, '*') !== false) {
+					// *.example.com
+					// Make save for regex:
+					// \*\.example\.com
+					$regexDomain = preg_quote($domain, '\\');
+					// Replace "\*" with an actual regex wildcard and set start and end:
+					// /^.+\.example\.com$/
+					$regexDomain = '/^' . str_replace('\\*', '.+', $regexDomain) . '$/';
+
+					if (preg_match($regexDomain, $mailDomain)) {
+						return true;
+					}
+				} elseif ($mailDomain === $domain) {
+					return true;
 				}
 			}
-			return $allowed;
+			return false;
 		}
 		return true;
 	}

lib/Settings/RegistrationSettings.php

@@ -59,14 +59,19 @@ class RegistrationSettings implements ISettings {
 		// handle domains
 		$allowedDomains = $this->config->getAppValue($this->appName, 'allowed_domains', '');
 
-		// handle admin validation
+		$adminApprovalRequired = $this->config->getAppValue($this->appName, 'admin_approval_required', 'no');
-		$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');
+		$showDomains = $this->config->getAppValue($this->appName, 'show_domains', 'no');
 
 		return new TemplateResponse('registration', 'admin', [
 			'groups' => $groupIds,
 			'current' => $assignedGroups,
 			'allowed' => $allowedDomains,
-			'approval_required' => $adminApprovalRequired
+			'approval_required' => $adminApprovalRequired,
+			'email_is_login' => $emailIsLogin,
+			'domains_is_blocklist' => $domainsIsBlocklist,
+			'show_domains' => $showDomains,
 		], '');
 	}
 

templates/admin.php

@@ -2,37 +2,62 @@
 /** @var array $_ */
 /** @var \OCP\IL10N $l */
 script('registration', 'settings');
+style('registration', 'settings');
 ?>
 <form id="registration_settings_form" class="section">
 	<h2><?php p($l->t('Registration')); ?></h2><span id="registration_settings_msg" class="msg"></span>
+
+	<h3><?php p($l->t('Registered users default group')); ?></h3>
 	<p>
-	<label for="registered_user_group"><?php p($l->t('Default group that all registered users belong')); ?></label>
+		<label>
-	<select id="registered_user_group" name="registered_user_group">
+			<select id="registered_user_group" name="registered_user_group">
-		<option value="none" <?php echo $_['current'] === 'none' ? 'selected="selected"' : ''; ?>><?php p($l->t('None')); ?></option>
+				<option value="none" <?php echo $_['current'] === 'none' ? 'selected="selected"' : ''; ?>><?php p($l->t('None')); ?></option>
 <?php
 foreach ($_['groups'] as $group) {
 	$selected = $_['current'] === $group ? 'selected="selected"' : '';
 	echo '<option value="'.$group.'" '.$selected.'>'.$group.'</option>';
 }
 ?>
-	</select>
+			</select>
-	</p>
+		</label>
-	<p>
-	<label for="allowed_domains"><?php p($l->t('Allowed mail address domains for registration')); ?></label>
-	<input type="text" id="allowed_domains" name="allowed_domains" value=<?php p($_['allowed']);?>>
-	</p>
-	<p>
-	<em><?php p($l->t('Enter a semicolon-separated list of allowed domains. Example: nextcloud.com;example.com'));?></em>
 	</p>
 
-	<div style="margin-top: 10px;">
+	<h3><?php p($l->t('Allowed email domains')); ?></h3>
-		<p>
+	<p>
-			<input type="checkbox" id="admin_approval_required" class="checkbox" name="admin_approval_required" <?php if ($_['approval_required'] === "yes") {
+		<label>
-	echo " checked";
+			<input type="text" id="allowed_domains" name="allowed_domains" value="<?php p($_['allowed']);?>" placeholder="nextcloud.com;*.example.com">
+		</label>
+	</p>
+	<em><?php p($l->t('Enter a semicolon-separated list of allowed email domains, * for wildcard. Example: %s', ['nextcloud.com;*.example.com']));?></em>
+
+	<p>
+		<input type="checkbox" id="domains_is_blocklist" class="checkbox" name="domains_is_blocklist" <?php if ($_['domains_is_blocklist'] === 'yes') {
+	echo ' checked';
 } ?>>
-			<label for="admin_approval_required"><?php p($l->t('Require admin approval?')); ?></label>
+		<label for="domains_is_blocklist"><?php p($l->t('Block listed email domains instead of allowing them')); ?></label>
-		</p>
+	</p>
 
-		<em><?php p($l->t('Enabling "admin approval" will prevent registrations from mobile and desktop clients to complete as the credentials can not be verified by the client until the user was enabled.'));?></em>
+	<p>
-	</div>
+		<input type="checkbox" id="show_domains" class="checkbox" name="show_domains" <?php if ($_['show_domains'] === 'yes') {
+	echo ' checked';
+} ?>>
+		<label for="show_domains"><?php p($l->t('Show the allowed/blocked email domains to users')); ?></label>
+	</p>
+
+	<p>
+		<input type="checkbox" id="email_is_login" class="checkbox" name="email_is_login" <?php if ($_['email_is_login'] === 'yes') {
+	echo ' checked';
+} ?>>
+		<label for="email_is_login"><?php p($l->t('Force email as login name')); ?></label>
+	</p>
+
+	<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') {
+	echo ' checked';
+} ?>>
+		<label for="admin_approval_required"><?php p($l->t('Require admin approval')); ?></label>
+	</p>
+
+	<em><?php p($l->t('Enabling "admin approval" will prevent registrations from mobile and desktop clients to complete as the credentials can not be verified by the client until the user was enabled.'));?></em>
 </form>

tests/Integration/Service/RegistrationServiceTest.php

@@ -60,7 +60,6 @@ class RegistrationServiceTest extends TestCase {
 	private $tokenProvider;
 	/** @var ICrypto */
 	private $crypto;
-
 	/** @var RegistrationService */
 	private $service;
 
@@ -105,69 +104,75 @@ class RegistrationServiceTest extends TestCase {
 		);
 	}
 
-	public function testValidateNewEmail() {
+	public function dataValidateEmail(): array {
-		$email = 'aaaa@example.com';
+		return [
+			['aaaa@example.com', '', 'no'],
+			['aaaa@example.com', 'example.com', 'no'],
+			['aaaa@example.com', 'eXample.com', 'no'],
+			['aaaa@eXample.com', 'example.com', 'no'],
+			['aaaa@example.com', 'example.com;example.tld', 'no'],
+			['aaaa@example.com', 'example.tld;example.com', 'no'],
+			['aaaa@cloud.example.com', '*.example.com', 'no'],
+			['aaaa@cloud.example.com', 'cloud.example.*', 'no'],
 
-		$this->config->expects($this->once())
+			['aaaa@example.com', '', 'yes'],
-			->method('getAppValue')
+			['aaaa@example.com', 'nextcloud.com', 'yes'],
-			->with('registration', 'allowed_domains', '')
+			['aaaa@example.com', 'nextcloud.com;example.tld', 'yes'],
-			->willReturn('');
+		];
-
-		$this->service->validateEmail($email);
-	}
-
-	public function testValidateNewEmailWithinAllowedDomain() {
-		$email = 'aaaa@example.com';
-
-		$this->config->expects($this->atLeastOnce())
-			->method('getAppValue')
-			->with('registration', 'allowed_domains', '')
-			->willReturn('example.com');
-
-		$this->service->validateEmail($email);
 	}
 
 	/**
-	 * @depends testValidateNewEmailWithinAllowedDomain
+	 * @dataProvider dataValidateEmail
+	 * @param string $email
+	 * @param string $allowedDomains
+	 * @param string $blocked
+	 * @throws RegistrationException
 	 */
-	public function testValidateNewEmailNotWithinAllowedDomain() {
+	public function testValidateEmail(string $email, string $allowedDomains, string $blocked) {
-		$email2 = 'bbbb@gmail.com';
-
 		$this->config->expects($this->atLeastOnce())
 			->method('getAppValue')
-			->with('registration', 'allowed_domains', '')
+			->willReturnMap([
-			->willReturn('example.com');
+				['registration', 'allowed_domains', '', $allowedDomains],
-
+				['registration', 'domains_is_blocklist', 'no', $blocked],
-		$this->expectException(RegistrationException::class);
+				['registration', 'show_domains', 'no', 'no'],
-		$this->service->validateEmail($email2);
+			]);
-	}
-
-	public function testValidateNewEmailWithinMultipleAllowedDomain() {
-		$email = 'aaaa@example.com';
-		$email2 = 'bbbb@gmail.com';
-
-		$this->config->expects($this->atLeastOnce())
-			->method('getAppValue')
-			->with('registration', 'allowed_domains', '')
-			->willReturn('example.com;gmail.com');
 
 		$this->service->validateEmail($email);
-		$this->service->validateEmail($email2);
+	}
+
+	public function dataValidateEmailThrows(): array {
+		return [
+			['aaaa@example.com', 'nextcloud.com;example.tld', 'no'],
+			['aaaa@example.com', 'nextcloud.com', 'no'],
+
+			['aaaa@example.com', 'example.com', 'yes'],
+			['aaaa@example.com', 'eXample.com', 'yes'],
+			['aaaa@eXample.com', 'example.com', 'yes'],
+			['aaaa@example.com', 'example.com;example.tld', 'yes'],
+			['aaaa@example.com', 'example.tld;example.com', 'yes'],
+			['aaaa@cloud.example.com', '*.example.com', 'yes'],
+			['aaaa@cloud.example.com', 'cloud.example.*', 'yes'],
+		];
 	}
 
 	/**
-	 * @depends testValidateNewEmailWithinMultipleAllowedDomain
+	 * @dataProvider dataValidateEmailThrows
+	 * @param string $email
+	 * @param string $allowedDomains
+	 * @param string $blocked
+	 * @throws RegistrationException
 	 */
-	public function testValidateNewEmailNotWithinMultipleAllowedDomain() {
+	public function testValidateEmailThrows(string $email, string $allowedDomains, string $blocked) {
-		$email2 = 'cccc@yahoo.com';
-
 		$this->config->expects($this->atLeastOnce())
 			->method('getAppValue')
-			->with('registration', 'allowed_domains', '')
+			->willReturnMap([
-			->willReturn('example.com;gmail.com');
+				['registration', 'allowed_domains', '', $allowedDomains],
+				['registration', 'domains_is_blocklist', 'no', $blocked],
+				['registration', 'show_domains', 'no', 'no'],
+			]);
 
 		$this->expectException(RegistrationException::class);
-		$this->service->validateEmail($email2);
+		$this->service->validateEmail($email);
 	}
 
 	public function testCreatePendingReg() {