implement verification for additional mails

- mails added by (sub)admins are automatically verified
- provisioning_api controller as verification endpoint
- IAccountProperty gets a locallyVerified property
- IPropertyCollection gets a method to fetch an IAccountProperty by value
  - an remove equivalent was already present
- AccountManager always initiates mail verification on update if necessary
- add core success template for arbitrary title and message

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
pull/28422/head
Arthur Schiwon 3 years ago
parent 19cc757531
commit aacaad2a3f
No known key found for this signature in database
GPG Key ID: 7424F1874854DF23
  1. 4
      apps/provisioning_api/appinfo/routes.php
  2. 1
      apps/provisioning_api/composer/composer/autoload_classmap.php
  3. 1
      apps/provisioning_api/composer/composer/autoload_static.php
  4. 4
      apps/provisioning_api/composer/composer/installed.php
  5. 14
      apps/provisioning_api/lib/Controller/UsersController.php
  6. 121
      apps/provisioning_api/lib/Controller/VerificationController.php
  7. 13
      core/templates/success.php
  8. 131
      lib/private/Accounts/AccountManager.php
  9. 21
      lib/private/Accounts/AccountProperty.php
  10. 9
      lib/private/Accounts/AccountPropertyCollection.php
  11. 4
      lib/private/Server.php
  12. 20
      lib/public/Accounts/IAccountProperty.php
  13. 9
      lib/public/Accounts/IAccountPropertyCollection.php
  14. 36
      tests/lib/Accounts/AccountManagerTest.php

@ -74,4 +74,8 @@ return [
['name' => 'AppConfig#setValue', 'url' => '/api/v1/config/apps/{app}/{key}', 'verb' => 'POST'],
['name' => 'AppConfig#deleteKey', 'url' => '/api/v1/config/apps/{app}/{key}', 'verb' => 'DELETE'],
],
'routes' => [
// Verification
['name' => 'Verification#verifyMail', 'url' => '/mailVerification/{key}/{token}/{userId}', 'verb' => 'GET'],
]
];

@ -14,6 +14,7 @@ return array(
'OCA\\Provisioning_API\\Controller\\AppsController' => $baseDir . '/../lib/Controller/AppsController.php',
'OCA\\Provisioning_API\\Controller\\GroupsController' => $baseDir . '/../lib/Controller/GroupsController.php',
'OCA\\Provisioning_API\\Controller\\UsersController' => $baseDir . '/../lib/Controller/UsersController.php',
'OCA\\Provisioning_API\\Controller\\VerificationController' => $baseDir . '/../lib/Controller/VerificationController.php',
'OCA\\Provisioning_API\\FederatedShareProviderFactory' => $baseDir . '/../lib/FederatedShareProviderFactory.php',
'OCA\\Provisioning_API\\Listener\\UserDeletedListener' => $baseDir . '/../lib/Listener/UserDeletedListener.php',
'OCA\\Provisioning_API\\Middleware\\Exceptions\\NotSubAdminException' => $baseDir . '/../lib/Middleware/Exceptions/NotSubAdminException.php',

@ -29,6 +29,7 @@ class ComposerStaticInitProvisioning_API
'OCA\\Provisioning_API\\Controller\\AppsController' => __DIR__ . '/..' . '/../lib/Controller/AppsController.php',
'OCA\\Provisioning_API\\Controller\\GroupsController' => __DIR__ . '/..' . '/../lib/Controller/GroupsController.php',
'OCA\\Provisioning_API\\Controller\\UsersController' => __DIR__ . '/..' . '/../lib/Controller/UsersController.php',
'OCA\\Provisioning_API\\Controller\\VerificationController' => __DIR__ . '/..' . '/../lib/Controller/VerificationController.php',
'OCA\\Provisioning_API\\FederatedShareProviderFactory' => __DIR__ . '/..' . '/../lib/FederatedShareProviderFactory.php',
'OCA\\Provisioning_API\\Listener\\UserDeletedListener' => __DIR__ . '/..' . '/../lib/Listener/UserDeletedListener.php',
'OCA\\Provisioning_API\\Middleware\\Exceptions\\NotSubAdminException' => __DIR__ . '/..' . '/../lib/Middleware/Exceptions/NotSubAdminException.php',

@ -5,7 +5,7 @@
'type' => 'library',
'install_path' => __DIR__ . '/../',
'aliases' => array(),
'reference' => 'fa56c13484afa1baf908b93ed5b6990c6a0e9ad6',
'reference' => '2e49000abb5acb09de041369a2239db23fa63ec7',
'name' => '__root__',
'dev' => false,
),
@ -16,7 +16,7 @@
'type' => 'library',
'install_path' => __DIR__ . '/../',
'aliases' => array(),
'reference' => 'fa56c13484afa1baf908b93ed5b6990c6a0e9ad6',
'reference' => '2e49000abb5acb09de041369a2239db23fa63ec7',
'dev_requirement' => false,
),
),

@ -621,6 +621,10 @@ class UsersController extends AUserData {
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}
$subAdminManager = $this->groupManager->getSubAdmin();
$isAdminOrSubadmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID())
|| $subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser);
$permittedFields = [];
if ($targetUser->getUID() === $currentLoggedInUser->getUID()) {
// Editing self (display, email)
@ -628,11 +632,8 @@ class UsersController extends AUserData {
$permittedFields[] = IAccountManager::COLLECTION_EMAIL . self::SCOPE_SUFFIX;
} else {
// Check if admin / subadmin
$subAdminManager = $this->groupManager->getSubAdmin();
if ($this->groupManager->isAdmin($currentLoggedInUser->getUID())
|| $subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) {
if ($isAdminOrSubadmin) {
// They have permissions over the user
$permittedFields[] = IAccountManager::COLLECTION_EMAIL;
} else {
// No rights
@ -652,6 +653,11 @@ class UsersController extends AUserData {
$mailCollection->removePropertyByValue($key);
if ($value !== '') {
$mailCollection->addPropertyWithDefaults($value);
$property = $mailCollection->getPropertyByValue($key);
if ($isAdminOrSubadmin && $property) {
// admin set mails are auto-verified
$property->setLocallyVerified(IAccountManager::VERIFIED);
}
}
$this->accountManager->updateAccount($userAccount);
break;

@ -0,0 +1,121 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2021 Arthur Schiwon <blizzz@arthur-schiwon.de>
*
* @author Arthur Schiwon <blizzz@arthur-schiwon.de>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
*/
namespace OCA\Provisioning_API\Controller;
use InvalidArgumentException;
use OC\Security\Crypto;
use OCP\Accounts\IAccountManager;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\IL10N;
use OCP\IRequest;
use OCP\IUserManager;
use OCP\IUserSession;
use OCP\Security\VerificationToken\InvalidTokenException;
use OCP\Security\VerificationToken\IVerificationToken;
class VerificationController extends Controller {
/** @var IVerificationToken */
private $verificationToken;
/** @var IUserManager */
private $userManager;
/** @var IL10N */
private $l10n;
/** @var IUserSession */
private $userSession;
/** @var IAccountManager */
private $accountManager;
/** @var Crypto */
private $crypto;
public function __construct(
string $appName,
IRequest $request,
IVerificationToken $verificationToken,
IUserManager $userManager,
IL10N $l10n,
IUserSession $userSession,
IAccountManager $accountManager,
Crypto $crypto
) {
parent::__construct($appName, $request);
$this->verificationToken = $verificationToken;
$this->userManager = $userManager;
$this->l10n = $l10n;
$this->userSession = $userSession;
$this->accountManager = $accountManager;
$this->crypto = $crypto;
}
/**
* @NoCSRFRequired
*/
public function verifyMail(string $token, string $userId, string $key) {
try {
if ($this->userSession->getUser()->getUID() !== $userId) {
throw new InvalidArgumentException('Logged in user is not mail address owner');
}
$email = $this->crypto->decrypt($key);
$ref = \substr(hash('sha256', $email), 0, 8);
$user = $this->userManager->get($userId);
$this->verificationToken->check($token, $user, 'verifyMail' . $ref, $email);
$userAccount = $this->accountManager->getAccount($user);
$emailProperty = $userAccount->getPropertyCollection(IAccountManager::COLLECTION_EMAIL)
->getPropertyByValue($email);
if ($emailProperty === null) {
throw new InvalidArgumentException($this->l10n->t('Email was already removed from account and cannot be confirmed anymore.'));
}
$emailProperty->setLocallyVerified(IAccountManager::VERIFIED);
$this->accountManager->updateAccount($userAccount);
} catch (InvalidTokenException $e) {
$error = $e->getCode() === InvalidTokenException::TOKEN_EXPIRED
? $this->l10n->t('Could not verify mail because the token is expired.')
: $this->l10n->t('Could not verify mail because the token is invalid.');
} catch (InvalidArgumentException $e) {
$error = $e->getMessage();
} catch (\Exception $e) {
$error = $this->l10n->t('An unexpected error occurred. Please consult your sysadmin.');
}
if (isset($error)) {
return new TemplateResponse(
'core', 'error', [
'errors' => [['error' => $error]]
], 'guest');
}
return new TemplateResponse(
'core', 'success', [
'title' => $this->l10n->t('Email confirmation successful'),
'message' => $this->l10n->t('Email confirmation successful'),
], 'guest');
}
}

@ -0,0 +1,13 @@
<?php
/** @var array $_ */
/** @var \OCP\IL10N $l */
/** @var \OCP\Defaults $theme */
?>
<div class="update">
<h2><?php p($_['title']) ?></h2>
<p><?php p($_['message']) ?></p>
<p><a class="button primary" href="<?php p(\OC::$server->get(\OCP\IURLGenerator::class)->linkTo('', 'index.php')) ?>">
<?php p($l->t('Go to %s', [$theme->getName()])); ?>
</a></p>
</div>

@ -32,6 +32,7 @@
*/
namespace OC\Accounts;
use Exception;
use InvalidArgumentException;
use libphonenumber\NumberParseException;
use libphonenumber\PhoneNumber;
@ -45,9 +46,17 @@ use OCP\Accounts\IAccountPropertyCollection;
use OCP\Accounts\PropertyDoesNotExistException;
use OCP\BackgroundJob\IJobList;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\Defaults;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\L10N\IFactory;
use OCP\Mail\IMailer;
use OCP\Security\ICrypto;
use OCP\Security\VerificationToken\IVerificationToken;
use OCP\Util;
use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\GenericEvent;
@ -88,17 +97,46 @@ class AccountManager implements IAccountManager {
/** @var LoggerInterface */
private $logger;
public function __construct(IDBConnection $connection,
IConfig $config,
EventDispatcherInterface $eventDispatcher,
IJobList $jobList,
LoggerInterface $logger) {
/** @var IVerificationToken */
private $verificationToken;
/** @var IMailer */
private $mailer;
/** @var Defaults */
private $defaults;
/** @var IL10N */
private $l10n;
/** @var IURLGenerator */
private $urlGenerator;
/** @var ICrypto */
private $crypto;
/** @var IFactory */
private $l10nfactory;
public function __construct(
IDBConnection $connection,
IConfig $config,
EventDispatcherInterface $eventDispatcher,
IJobList $jobList,
LoggerInterface $logger,
IVerificationToken $verificationToken,
IMailer $mailer,
Defaults $defaults,
IFactory $factory,
IURLGenerator $urlGenerator,
ICrypto $crypto
) {
$this->connection = $connection;
$this->config = $config;
$this->eventDispatcher = $eventDispatcher;
$this->jobList = $jobList;
$this->logger = $logger;
$this->verificationToken = $verificationToken;
$this->mailer = $mailer;
$this->defaults = $defaults;
$this->urlGenerator = $urlGenerator;
$this->crypto = $crypto;
// DIing IL10N results in a dependency loop
$this->l10nfactory = $factory;
}
/**
@ -337,7 +375,6 @@ class AccountManager implements IAccountManager {
/**
* check if we need to ask the server for email verification, if yes we create a cronjob
*
*/
protected function checkEmailVerification(IAccount $updatedAccount, array $oldData): void {
try {
@ -358,11 +395,73 @@ class AccountManager implements IAccountManager {
]
);
$property->setVerified(self::VERIFICATION_IN_PROGRESS);
}
}
protected function checkLocalEmailVerification(IAccount $updatedAccount, array $oldData): void {
$mailCollection = $updatedAccount->getPropertyCollection(self::COLLECTION_EMAIL);
foreach ($mailCollection->getProperties() as $property) {
if ($property->getLocallyVerified() !== self::NOT_VERIFIED) {
continue;
}
if ($this->sendEmailVerificationEmail($updatedAccount->getUser(), $property->getValue())) {
$property->setLocallyVerified(self::VERIFICATION_IN_PROGRESS);
}
}
}
protected function sendEmailVerificationEmail(IUser $user, string $email): bool {
$ref = \substr(hash('sha256', $email), 0, 8);
$key = $this->crypto->encrypt($email);
$token = $this->verificationToken->create($user, 'verifyMail' . $ref, $email);
$link = $this->urlGenerator->linkToRouteAbsolute('provisioning_api.Verification.verifyMail',
[
'userId' => $user->getUID(),
'token' => $token,
'key' => $key
]);
$emailTemplate = $this->mailer->createEMailTemplate('core.EmailVerification', [
'link' => $link,
]);
$property->setVerified(self::VERIFICATION_IN_PROGRESS);
if (!$this->l10n) {
$this->l10n = $this->l10nfactory->get('core');
}
$emailTemplate->setSubject($this->l10n->t('%s email verification', [$this->defaults->getName()]));
$emailTemplate->addHeader();
$emailTemplate->addHeading($this->l10n->t('Email verification'));
$emailTemplate->addBodyText(
htmlspecialchars($this->l10n->t('Click the following button to confirm your email.')),
$this->l10n->t('Click the following link to confirm your email.')
);
$emailTemplate->addBodyButton(
htmlspecialchars($this->l10n->t('Confirm your email')),
$link,
false
);
$emailTemplate->addFooter();
try {
$message = $this->mailer->createMessage();
$message->setTo([$email => $user->getDisplayName()]);
$message->setFrom([Util::getDefaultEmailAddress('verification-noreply') => $this->defaults->getName()]);
$message->useTemplate($emailTemplate);
$this->mailer->send($message);
} catch (Exception $e) {
// Log the exception and continue
$this->logger->info('Failed to send verification mail', [
'app' => 'core',
'exception' => $e
]);
return false;
}
return true;
}
/**
@ -406,7 +505,6 @@ class AccountManager implements IAccountManager {
}
}
/**
* add new user to accounts table
*
@ -435,6 +533,12 @@ class AccountManager implements IAccountManager {
foreach ($data as $dataRow) {
$propertyName = $dataRow['name'];
unset($dataRow['name']);
if (isset($dataRow['locallyVerified']) && $dataRow['locallyVerified'] === self::NOT_VERIFIED) {
// do not write default value, save DB space
unset($dataRow['locallyVerified']);
}
if (!$this->isCollection($propertyName)) {
$preparedData[$propertyName] = $dataRow;
continue;
@ -511,7 +615,6 @@ class AccountManager implements IAccountManager {
continue;
}
$query->setParameter('name', $property['name'])
->setParameter('value', $property['value'] ?? '');
$query->executeStatement();
@ -587,6 +690,7 @@ class AccountManager implements IAccountManager {
$data['verified'] ?? self::NOT_VERIFIED,
''
);
$p->setLocallyVerified($data['locallyVerified'] ?? self::NOT_VERIFIED);
$collection->addProperty($p);
return $collection;
@ -599,6 +703,10 @@ class AccountManager implements IAccountManager {
$account->setPropertyCollection($this->arrayDataToCollection($account, $accountData));
} else {
$account->setProperty($accountData['name'], $accountData['value'] ?? '', $accountData['scope'] ?? self::SCOPE_LOCAL, $accountData['verified'] ?? self::NOT_VERIFIED);
if (isset($accountData['locallyVerified'])) {
$property = $account->getProperty($accountData['name']);
$property->setLocallyVerified($accountData['locallyVerified']);
}
}
}
return $account;
@ -640,14 +748,17 @@ class AccountManager implements IAccountManager {
$oldData = $this->getUser($account->getUser(), false);
$this->updateVerificationStatus($account, $oldData);
$this->checkEmailVerification($account, $oldData);
$this->checkLocalEmailVerification($account, $oldData);
$data = [];
foreach ($account->getAllProperties() as $property) {
/** @var IAccountProperty $property */
$data[] = [
'name' => $property->getName(),
'value' => $property->getValue(),
'scope' => $property->getScope(),
'verified' => $property->getVerified(),
'locallyVerified' => $property->getLocallyVerified(),
];
}

@ -27,6 +27,7 @@ declare(strict_types=1);
*/
namespace OC\Accounts;
use InvalidArgumentException;
use OCP\Accounts\IAccountManager;
use OCP\Accounts\IAccountProperty;
@ -42,6 +43,8 @@ class AccountProperty implements IAccountProperty {
private $verified;
/** @var string */
private $verificationData;
/** @var string */
private $locallyVerified = IAccountManager::NOT_VERIFIED;
public function __construct(string $name, string $value, string $scope, string $verified, string $verificationData) {
$this->name = $name;
@ -90,7 +93,7 @@ class AccountProperty implements IAccountProperty {
IAccountManager::SCOPE_PRIVATE,
IAccountManager::SCOPE_PUBLISHED
])) {
throw new \InvalidArgumentException('Invalid scope');
throw new InvalidArgumentException('Invalid scope');
}
$this->scope = $newScope;
return $this;
@ -178,4 +181,20 @@ class AccountProperty implements IAccountProperty {
public function getVerificationData(): string {
return $this->verificationData;
}
public function setLocallyVerified(string $verified): IAccountProperty {
if (!in_array($verified, [
IAccountManager::NOT_VERIFIED,
IAccountManager::VERIFICATION_IN_PROGRESS,
IAccountManager::VERIFIED,
])) {
throw new InvalidArgumentException('Provided verification value is invalid');
}
$this->locallyVerified = $verified;
return $this;
}
public function getLocallyVerified(): string {
return $this->locallyVerified;
}
}

@ -84,6 +84,15 @@ class AccountPropertyCollection implements IAccountPropertyCollection {
return $this;
}
public function getPropertyByValue(string $value): ?IAccountProperty {
foreach ($this->properties as $i => $property) {
if ($property->getValue() === $value) {
return $property;
}
}
return null;
}
public function removePropertyByValue(string $value): IAccountPropertyCollection {
foreach ($this->properties as $i => $property) {
if ($property->getValue() === $value) {

@ -135,6 +135,7 @@ use OC\Security\CSRF\TokenStorage\SessionStorage;
use OC\Security\Hasher;
use OC\Security\SecureRandom;
use OC\Security\TrustedDomainHelper;
use OC\Security\VerificationToken\VerificationToken;
use OC\Session\CryptoWrapper;
use OC\Share20\ProviderFactory;
use OC\Share20\ShareHelper;
@ -224,6 +225,7 @@ use OCP\Security\ICredentialsManager;
use OCP\Security\ICrypto;
use OCP\Security\IHasher;
use OCP\Security\ISecureRandom;
use OCP\Security\VerificationToken\IVerificationToken;
use OCP\Share\IShareHelper;
use OCP\SystemTag\ISystemTagManager;
use OCP\SystemTag\ISystemTagObjectMapper;
@ -795,6 +797,8 @@ class Server extends ServerContainer implements IServerContainer {
/** @deprecated 19.0.0 */
$this->registerDeprecatedAlias('SecureRandom', \OCP\Security\ISecureRandom::class);
$this->registerAlias(IVerificationToken::class, VerificationToken::class);
$this->registerAlias(ICrypto::class, Crypto::class);
/** @deprecated 19.0.0 */
$this->registerDeprecatedAlias('Crypto', ICrypto::class);

@ -115,4 +115,24 @@ interface IAccountProperty extends \JsonSerializable {
* @since 22.0.0
*/
public function getVerificationData(): string;
/**
* Set the instance-based verification status of a property
*
* @since 23.0.0
*
* @param string $verified must be one of the verification constants of IAccountManager
* @return IAccountProperty
* @throws InvalidArgumentException
*/
public function setLocallyVerified(string $verified): IAccountProperty;
/**
* Get the instance-based verification status of a property
*
* @since 23.0.0
*
* @return string
*/
public function getLocallyVerified(): string;
}

@ -89,4 +89,13 @@ interface IAccountPropertyCollection extends JsonSerializable {
* @since 22.0.0
*/
public function removePropertyByValue(string $value): IAccountPropertyCollection;
/**
* retrieves a property identified by its value. null, if none was found.
*
* Returns only the first property if there are more with the same value.
*
* @since 23.0.0
*/
public function getPropertyByValue(string $value): ?IAccountProperty;
}

@ -25,9 +25,15 @@ use OC\Accounts\Account;
use OC\Accounts\AccountManager;
use OCP\Accounts\IAccountManager;
use OCP\BackgroundJob\IJobList;
use OCP\Defaults;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\L10N\IFactory;
use OCP\Mail\IMailer;
use OCP\Security\ICrypto;
use OCP\Security\VerificationToken\IVerificationToken;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@ -41,6 +47,18 @@ use Test\TestCase;
* @package Test\Accounts
*/
class AccountManagerTest extends TestCase {
/** @var IVerificationToken|MockObject */
protected $verificationToken;
/** @var IMailer|MockObject */
protected $mailer;
/** @var ICrypto|MockObject */
protected $crypto;
/** @var IURLGenerator|MockObject */
protected $urlGenerator;
/** @var Defaults|MockObject */
protected $defaults;
/** @var IFactory|MockObject */
protected $l10nFactory;
/** @var \OCP\IDBConnection */
private $connection;
@ -70,6 +88,12 @@ class AccountManagerTest extends TestCase {
$this->config = $this->createMock(IConfig::class);
$this->jobList = $this->createMock(IJobList::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->verificationToken = $this->createMock(IVerificationToken::class);
$this->mailer = $this->createMock(IMailer::class);
$this->defaults = $this->createMock(Defaults::class);
$this->l10nFactory = $this->createMock(IFactory::class);
$this->urlGenerator = $this->createMock(IURLGenerator::class);
$this->crypto = $this->createMock(ICrypto::class);
$this->accountManager = new AccountManager(
$this->connection,
@ -77,6 +101,12 @@ class AccountManagerTest extends TestCase {
$this->eventDispatcher,
$this->jobList,
$this->logger,
$this->verificationToken,
$this->mailer,
$this->defaults,
$this->l10nFactory,
$this->urlGenerator,
$this->crypto
);
}
@ -310,6 +340,12 @@ class AccountManagerTest extends TestCase {
$this->eventDispatcher,
$this->jobList,
$this->logger,
$this->verificationToken,
$this->mailer,
$this->defaults,
$this->l10nFactory,
$this->urlGenerator,
$this->crypto
])
->setMethods($mockedMethods)
->getMock();

Loading…
Cancel
Save