Validate email address and improve register dialog

7 files changed, 59 insertions, 68 deletions

diff --git a/lib/Controller/PreferencesController.php b/lib/Controller/PreferencesController.php
index f0ee44ab..26d6832f 100644
--- a/lib/Controller/PreferencesController.php
+++ b/lib/Controller/PreferencesController.php
@@ -25,13 +25,10 @@ namespace OCA\Polls\Controller;
 
 use OCP\AppFramework\Db\DoesNotExistException;
 
-
 use OCP\IRequest;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
-
-
 use OCA\Polls\Db\Preferences;
 use OCA\Polls\Db\PreferencesMapper;
 
@@ -39,11 +36,6 @@ class PreferencesController extends Controller {
 	private $userId;
 	private $preferencesMapper;
 
-	private $groupManager;
-	private $pollMapper;
-	private $anonymizer;
-	private $acl;
-
 	/**
 	 * PreferencesController constructor.
 	 * @param string $appName
@@ -62,13 +54,11 @@ class PreferencesController extends Controller {
 		$this->preferencesMapper = $preferencesMapper;
 	}
 
-
 	/**
 	 * get
 	 * Read all preferences
 	 * @NoAdminRequired
 	 * @NoCSRFRequired
-	 * @param integer $pollId
 	 * @return DataResponse
 	 */
 	public function get() {
@@ -81,12 +71,9 @@ class PreferencesController extends Controller {
 
 	/**
 	 * write
-	 * Write a new comment to the db and returns the new comment as array
+	 * Write wreferences
 	 * @NoAdminRequired
-	 * @NoCSRFRequired
-	 * @param int $pollId
-	 * @param string $userId
-	 * @param string $message
+	 * @param int $settings
 	 * @return DataResponse
 	 */
 	public function write($settings) {
@@ -110,26 +97,4 @@ class PreferencesController extends Controller {
 		return new DataResponse($preferences, Http::STATUS_OK);
 	}
 
-	// /**
-	//  * delete
-	//  * Delete Preferences
-	//  * @NoAdminRequired
-	//  * @param int $pollId
-	//  * @param string $message
-	//  * @return DataResponse
-	//  */
-	// public function delete($userId) {
-	// 	if (!\OC::$server->getUserSession()->isLoggedIn()) {
-	// 		return new DataResponse(null, Http::STATUS_UNAUTHORIZED);
-	// 	}
-	//
-	// 	try {
-	// 		$this->preferencesMapper->delete($userId);
-	// 	} catch (\Exception $e) {
-	// 		return new DataResponse($e, Http::STATUS_CONFLICT);
-	// 	}
-	//
-	// 	return new DataResponse(['deleted' => $userId], Http::STATUS_OK);
-	//
-	// }
 }
diff --git a/lib/Controller/ShareController.php b/lib/Controller/ShareController.php
index c3b0527e..a64f9f58 100644
--- a/lib/Controller/ShareController.php
+++ b/lib/Controller/ShareController.php
@@ -26,7 +26,7 @@ namespace OCA\Polls\Controller;
 use Exception;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCA\Polls\Exceptions\NotAuthorizedException;
-use OCA\Polls\Exceptions\InvalidUsername;
+use OCA\Polls\Exceptions\InvalidUsernameException;
 use OCA\Polls\Exceptions\InvalidShareType;
 
 
@@ -92,7 +92,7 @@ class ShareController extends Controller {
 	}
 
 	/**
-	 * Add share
+	 * Get share
 	 * @NoAdminRequired
 	 * @param int $pollId
 	 * @param int $pollId
@@ -112,8 +112,9 @@ class ShareController extends Controller {
 	}
 
 	/**
-	 * Add share
+	 * Set email address
 	 * @NoAdminRequired
+	 * @PublicPage
 	 * @param int $pollId
 	 * @param int $pollId
 	 * @param string $type
@@ -147,7 +148,7 @@ class ShareController extends Controller {
 			return new DataResponse($this->shareService->personal($token, $userName, $emailAddress), Http::STATUS_CREATED);
 		} catch (NotAuthorizedException $e) {
 			return new DataResponse(['error' => $e->getMessage()], $e->getStatus());
-		} catch (InvalidUsername $e) {
+		} catch (InvalidUsernameException $e) {
 			return new DataResponse(['error' => $userName . ' is not valid'], Http::STATUS_CONFLICT);
 		} catch (DoesNotExistException $e) {
 			// return forbidden in all not catched error cases
diff --git a/lib/Controller/SystemController.php b/lib/Controller/SystemController.php
index 1d2c6dfb..ea3a0be1 100644
--- a/lib/Controller/SystemController.php
+++ b/lib/Controller/SystemController.php
@@ -131,4 +131,14 @@ class SystemController extends Controller {
 	public function validatePublicUsername($pollId, $userName, $token) {
 		return new DataResponse(['result' => $this->systemService->validatePublicUsername($pollId, $userName, $token), 'name' => $userName], Http::STATUS_OK);
 	}
+
+	/**
+	 * Validate email address (simple validation)
+	 * @NoAdminRequired
+	 * @PublicPage
+	 * @return DataResponse
+	 */
+	public function validateEmailAddress($emailAddress) {
+		return new DataResponse(['result' => $this->systemService->validateEmailAddress($emailAddress), 'emailAddress' => $emailAddress], Http::STATUS_OK);
+	}
 }
diff --git a/lib/Exceptions/InvalidUsername.php b/lib/Exceptions/InvalidEmailAddress.php
index a33ea5b6..d622dac8 100644
--- a/lib/Exceptions/InvalidUsername.php
+++ b/lib/Exceptions/InvalidEmailAddress.php
@@ -25,12 +25,12 @@ namespace OCA\Polls\Exceptions;
 
 use OCP\AppFramework\Http;
 
-class InvalidUsername extends \Exception {
+class InvalidEmailAddress extends \Exception {
 	/**
-	 * InvalidUsername Constructor
+	 * InvalidEmailAddress Constructor
 	 * @param string $e exception message
 	 */
-	public function __construct($e = 'Invalid username') {
+	public function __construct($e = 'Invalid email address') {
 		parent::__construct($e);
 	}
 	public function getStatus() {
diff --git a/lib/Exceptions/UsernameInvalidException.php b/lib/Exceptions/InvalidUsernameException.php
index c1c3feec..11c4bed8 100644
--- a/lib/Exceptions/UsernameInvalidException.php
+++ b/lib/Exceptions/InvalidUsernameException.php
@@ -25,9 +25,9 @@ namespace OCA\Polls\Exceptions;
 
 use OCP\AppFramework\Http;
 
-class UsernameInvalidException extends \Exception {
+class InvalidUsernameException extends \Exception {
 	/**
-	 * UsernameInvalidException Constructor
+	 * InvalidUsernameException Constructor
 	 * @param string $e exception message
 	 */
 	public function __construct($e = 'Username not allowed') {
diff --git a/lib/Service/ShareService.php b/lib/Service/ShareService.php
index ae14935a..cba983d6 100644
--- a/lib/Service/ShareService.php
+++ b/lib/Service/ShareService.php
@@ -24,20 +24,19 @@
 namespace OCA\Polls\Service;
 
 use OCA\Polls\Exceptions\NotAuthorizedException;
-use OCA\Polls\Exceptions\InvalidUsername;
 use OCA\Polls\Exceptions\InvalidShareType;
 
 use OCP\Security\ISecureRandom;
 
-use OCA\Polls\Controller\SystemController;
+use OCA\Polls\Service\SystemService;
 use OCA\Polls\Db\ShareMapper;
 use OCA\Polls\Db\Share;
 use OCA\Polls\Model\Acl;
 
 class ShareService {
 
-	/** @var SystemController */
-	private $systemController;
+	/** @var SystemService */
+	private $systemService;
 
 	/** @var ShareMapper */
 	private $shareMapper;
@@ -53,20 +52,20 @@ class ShareService {
 
 	/**
 	 * ShareController constructor.
-	 * @param SystemController $systemController
+	 * @param SystemService $systemService
 	 * @param ShareMapper $shareMapper
 	 * @param Share $share
 	 * @param MailService $mailService
 	 * @param Acl $acl
 	 */
 	public function __construct(
-		SystemController $systemController,
+		SystemService $systemService,
 		ShareMapper $shareMapper,
 		Share $share,
 		MailService $mailService,
 		Acl $acl
 	) {
-		$this->systemController = $systemController;
+		$this->systemService = $systemService;
 		$this->shareMapper = $shareMapper;
 		$this->share = $share;
 		$this->mailService = $mailService;
@@ -142,12 +141,12 @@ class ShareService {
 	 * @param string $token
 	 * @param string $emailAddress
 	 * @return Share
-	 * @throws NotAuthorizedException
+	 * @throws InvalidShareType
 	 */
 	public function setEmailAddress($token, $emailAddress) {
 		$this->share = $this->shareMapper->findByToken($token);
 		if ($this->share->getType() === 'external') {
-			// TODO: Simple validate email address
+			$this->systemService->validateEmailAddress($emailAddress);
 			$this->share->setUserEmail($emailAddress);
 			// TODO: Send confirmation
 			return $this->shareMapper->update($this->share);
@@ -164,17 +163,14 @@ class ShareService {
 	 * @param string $userName
 	 * @return Share
 	 * @throws NotAuthorizedException
-	 * @throws InvalidUsername
 	 */
-	public function personal($token, $userName, $emailAddress) {
+	public function personal($token, $userName, $emailAddress = '') {
 		$this->share = $this->shareMapper->findByToken($token);
 
-		// Return of validatePublicUsername is a DataResponse
-		$checkUsername = $this->systemController->validatePublicUsername($this->share->getPollId(), $userName, $token);
+		$this->systemService->validatePublicUsername($this->share->getPollId(), $userName, $token);
 
-		// if status is not 200, return DataResponse from validatePublicUsername
-		if ($checkUsername->getStatus() !== 200) {
-			throw new InvalidUsername;
+		if ($emailAddress) {
+			$this->systemService->validateEmailAddress($emailAddress);
 		}
 
 		if ($this->share->getType() === 'public') {
diff --git a/lib/Service/SystemService.php b/lib/Service/SystemService.php
index 7ac14284..84355cb5 100644
--- a/lib/Service/SystemService.php
+++ b/lib/Service/SystemService.php
@@ -25,7 +25,8 @@ namespace OCA\Polls\Service;
 
 use OCA\Polls\Exceptions\NotAuthorizedException;
 use OCA\Polls\Exceptions\TooShortException;
-use OCA\Polls\Exceptions\UsernameInvalidException;
+use OCA\Polls\Exceptions\InvalidUsernameException;
+use OCA\Polls\Exceptions\InvalidEmailAddress;
 
 use OCP\IGroupManager;
 use OCP\IUserManager;
@@ -69,11 +70,11 @@ class SystemService {
 	/**
 	 * Validate string as email address
 	 * @NoAdminRequired
-	 * @param string $query
+	 * @param string $emailAddress
 	 * @return bool
 	 */
-	private function isValidEmail($email) {
-		return (!preg_match('/^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/', $email)) ? false : true;
+	private function isValidEmail($emailAddress) {
+		return (!preg_match('/^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/', $emailAddress)) ? false : true;
 	}
 
 
@@ -335,7 +336,25 @@ class SystemService {
 	 * @return Boolean
 	 * @throws NotAuthorizedException
 	 * @throws TooShortException
-	 * @throws UsernameInvalidException
+	 * @throws InvalidEmailAddress
+	 */
+	public function validateEmailAddress($emailAddress) {
+		if (!$this->isValidEmail($emailAddress)) {
+			throw new InvalidEmailAddress;
+		}
+		return true;
+	}
+
+
+	/**
+	 * Validate it the user name is reservrd
+	 * return false, if this username already exists as a user or as
+	 * a participant of the poll
+	 * @NoAdminRequired
+	 * @return Boolean
+	 * @throws NotAuthorizedException
+	 * @throws TooShortException
+	 * @throws InvalidUsernameException
 	 */
 	public function validatePublicUsername($pollId, $userName, $token) {
 
@@ -403,7 +422,7 @@ class SystemService {
 		// return forbidden, if list contains requested username
 		foreach ($list as $element) {
 			if (strtolower(trim($userName)) === strtolower(trim($element['id'])) || strtolower(trim($userName)) === strtolower(trim($element['displayName']))) {
-				throw new UsernameInvalidException;
+				throw new InvalidUsernameException;
 			}
 		}