diff options
46 files changed, 1422 insertions, 448 deletions
diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php index 4a422fa9628..8002f963798 100644 --- a/apps/dav/lib/Connector/Sabre/Principal.php +++ b/apps/dav/lib/Connector/Sabre/Principal.php @@ -300,16 +300,13 @@ class Principal implements BackendInterface { if (!$allowEnumeration) { if ($allowEnumerationFullMatch) { $users = $this->userManager->getByEmail($value); - $users = \array_filter($users, static function (IUser $user) use ($value) { - return $user->getEMailAddress() === $value; - }); } else { $users = []; } } else { $users = $this->userManager->getByEmail($value); $users = \array_filter($users, function (IUser $user) use ($currentUser, $value, $limitEnumerationPhone, $limitEnumerationGroup, $allowEnumerationFullMatch, $currentUserGroups) { - if ($allowEnumerationFullMatch && $user->getEMailAddress() === $value) { + if ($allowEnumerationFullMatch && $user->getSystemEMailAddress() === $value) { return true; } @@ -516,7 +513,7 @@ class Principal implements BackendInterface { '{http://nextcloud.com/ns}language' => $this->languageFactory->getUserLanguage($user), ]; - $email = $user->getEMailAddress(); + $email = $user->getSystemEMailAddress(); if (!empty($email)) { $principal['{http://sabredav.org/ns}email-address'] = $email; } diff --git a/apps/dav/lib/DAV/GroupPrincipalBackend.php b/apps/dav/lib/DAV/GroupPrincipalBackend.php index 34ac26d7d97..6317fc59cc2 100644 --- a/apps/dav/lib/DAV/GroupPrincipalBackend.php +++ b/apps/dav/lib/DAV/GroupPrincipalBackend.php @@ -331,7 +331,7 @@ class GroupPrincipalBackend implements BackendInterface { '{urn:ietf:params:xml:ns:caldav}calendar-user-type' => 'INDIVIDUAL', ]; - $email = $user->getEMailAddress(); + $email = $user->getSystemEMailAddress(); if (!empty($email)) { $principal['{http://sabredav.org/ns}email-address'] = $email; } diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php index ba65edf2dd1..d7c074c9e3b 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php @@ -120,7 +120,7 @@ class PrincipalTest extends TestCase { ->willReturn('Dr. Foo-Bar'); $fooUser ->expects($this->once()) - ->method('getEMailAddress') + ->method('getSystemEMailAddress') ->willReturn(''); $barUser = $this->createMock(User::class); $barUser @@ -129,7 +129,7 @@ class PrincipalTest extends TestCase { ->willReturn('bar'); $barUser ->expects($this->once()) - ->method('getEMailAddress') + ->method('getSystemEMailAddress') ->willReturn('bar@nextcloud.com'); $this->userManager ->expects($this->once()) @@ -205,7 +205,7 @@ class PrincipalTest extends TestCase { $fooUser = $this->createMock(User::class); $fooUser ->expects($this->once()) - ->method('getEMailAddress') + ->method('getSystemEMailAddress') ->willReturn('foo@nextcloud.com'); $fooUser ->expects($this->once()) @@ -605,15 +605,15 @@ class PrincipalTest extends TestCase { $user2 = $this->createMock(IUser::class); $user2->method('getUID')->willReturn('user2'); $user2->method('getDisplayName')->willReturn('User 2'); - $user2->method('getEMailAddress')->willReturn('user2@foo.bar'); + $user2->method('getSystemEMailAddress')->willReturn('user2@foo.bar'); $user3 = $this->createMock(IUser::class); $user3->method('getUID')->willReturn('user3'); $user2->method('getDisplayName')->willReturn('User 22'); - $user2->method('getEMailAddress')->willReturn('user2@foo.bar123'); + $user2->method('getSystemEMailAddress')->willReturn('user2@foo.bar123'); $user4 = $this->createMock(IUser::class); $user4->method('getUID')->willReturn('user4'); $user2->method('getDisplayName')->willReturn('User 222'); - $user2->method('getEMailAddress')->willReturn('user2@foo.bar456'); + $user2->method('getSystemEMailAddress')->willReturn('user2@foo.bar456'); $this->userManager->expects($this->at(0)) ->method('searchDisplayName') @@ -665,20 +665,20 @@ class PrincipalTest extends TestCase { $user2 = $this->createMock(IUser::class); $user2->method('getUID')->willReturn('user2'); $user2->method('getDisplayName')->willReturn('User 2'); - $user2->method('getEMailAddress')->willReturn('user2@foo.bar'); + $user2->method('getSystemEMailAddress')->willReturn('user2@foo.bar'); $user3 = $this->createMock(IUser::class); $user3->method('getUID')->willReturn('user3'); $user2->method('getDisplayName')->willReturn('User 22'); - $user2->method('getEMailAddress')->willReturn('user2@foo.bar123'); + $user2->method('getSystemEMailAddress')->willReturn('user2@foo.bar123'); $user4 = $this->createMock(IUser::class); $user4->method('getUID')->willReturn('user4'); $user2->method('getDisplayName')->willReturn('User 222'); - $user2->method('getEMailAddress')->willReturn('user2@foo.bar456'); + $user2->method('getSystemEMailAddress')->willReturn('user2@foo.bar456'); - $this->userManager->expects($this->at(0)) + $this->userManager->expects($this->once()) ->method('getByEmail') ->with('user2@foo.bar') - ->willReturn([$user2, $user3, $user4]); + ->willReturn([$user2]); $this->assertEquals(['principals/users/user2'], $this->connector->searchPrincipals('principals/users', ['{http://sabredav.org/ns}email-address' => 'user2@foo.bar'])); @@ -726,15 +726,15 @@ class PrincipalTest extends TestCase { $user2 = $this->createMock(IUser::class); $user2->method('getUID')->willReturn('user2'); $user2->method('getDisplayName')->willReturn('User 2'); - $user2->method('getEMailAddress')->willReturn('user2@foo.bar'); + $user2->method('getSystemEMailAddress')->willReturn('user2@foo.bar'); $user3 = $this->createMock(IUser::class); $user3->method('getUID')->willReturn('user3'); $user3->method('getDisplayName')->willReturn('User 22'); - $user3->method('getEMailAddress')->willReturn('user2@foo.bar123'); + $user3->method('getSystemEMailAddress')->willReturn('user2@foo.bar123'); $user4 = $this->createMock(IUser::class); $user4->method('getUID')->willReturn('user4'); $user4->method('getDisplayName')->willReturn('User 222'); - $user4->method('getEMailAddress')->willReturn('user2@foo.bar456'); + $user4->method('getSystemEMailAddress')->willReturn('user2@foo.bar456'); $this->userSession->expects($this->at(0)) @@ -787,15 +787,15 @@ class PrincipalTest extends TestCase { $user2 = $this->createMock(IUser::class); $user2->method('getUID')->willReturn('user2'); $user2->method('getDisplayName')->willReturn('User 2'); - $user2->method('getEMailAddress')->willReturn('user2@foo.bar'); + $user2->method('getSystemEMailAddress')->willReturn('user2@foo.bar'); $user3 = $this->createMock(IUser::class); $user3->method('getUID')->willReturn('user3'); $user3->method('getDisplayName')->willReturn('User 22'); - $user3->method('getEMailAddress')->willReturn('user2@foo.bar123'); + $user3->method('getSystemEMailAddress')->willReturn('user2@foo.bar123'); $user4 = $this->createMock(IUser::class); $user4->method('getUID')->willReturn('user4'); $user4->method('getDisplayName')->willReturn('User 222'); - $user4->method('getEMailAddress')->willReturn('user2@foo.bar456'); + $user4->method('getSystemEMailAddress')->willReturn('user2@foo.bar456'); $this->userSession->expects($this->at(0)) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 668bbb7cca8..c9853f1e12c 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -238,7 +238,7 @@ class ShareAPIController extends OCSController { $result['share_with'] = $share->getSharedWith(); $result['share_with_displayname'] = $sharedWith !== null ? $sharedWith->getDisplayName() : $share->getSharedWith(); $result['share_with_displayname_unique'] = $sharedWith !== null ? ( - $sharedWith->getEMailAddress() !== '' ? $sharedWith->getEMailAddress() : $sharedWith->getUID() + !empty($sharedWith->getSystemEMailAddress()) ? $sharedWith->getSystemEMailAddress() : $sharedWith->getUID() ) : $share->getSharedWith(); $result['status'] = []; diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 411496f7a27..86a7d479899 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -47,6 +47,7 @@ use OCP\Files\Mount\IMountPoint; use OCP\Files\NotFoundException; use OCP\Files\Storage; use OCP\IConfig; +use OCP\IGroup; use OCP\IGroupManager; use OCP\IL10N; use OCP\IPreview; @@ -785,7 +786,7 @@ class ShareAPIControllerTest extends TestCase { $user = $this->getMockBuilder(IUser::class)->getMock(); $user->method('getUID')->willReturn('userId'); $user->method('getDisplayName')->willReturn('userDisplay'); - $user->method('getEMailAddress')->willReturn('userId@example.com'); + $user->method('getSystemEMailAddress')->willReturn('userId@example.com'); $group = $this->getMockBuilder('OCP\IGroup')->getMock(); $group->method('getGID')->willReturn('groupId'); @@ -3586,7 +3587,7 @@ class ShareAPIControllerTest extends TestCase { $initiator->method('getDisplayName')->willReturn('initiatorDN'); $recipient = $this->getMockBuilder(IUser::class)->getMock(); $recipient->method('getDisplayName')->willReturn('recipientDN'); - $recipient->method('getEmailAddress')->willReturn('recipient'); + $recipient->method('getSystemEMailAddress')->willReturn('recipient'); $result = []; @@ -4387,7 +4388,7 @@ class ShareAPIControllerTest extends TestCase { public function testFormatShare(array $expects, \OCP\Share\IShare $share, array $users, $exception) { $this->userManager->method('get')->willReturnMap($users); - $recipientGroup = $this->createMock('\OCP\IGroup'); + $recipientGroup = $this->createMock(IGroup::class); $recipientGroup->method('getDisplayName')->willReturn('recipientGroupDisplayName'); $this->groupManager->method('get')->willReturnMap([ ['recipientGroup', $recipientGroup], @@ -4397,7 +4398,6 @@ class ShareAPIControllerTest extends TestCase { ->with('files_sharing.sharecontroller.showShare', ['token' => 'myToken']) ->willReturn('myLink'); - $this->rootFolder->method('getUserFolder') ->with($this->currentUser) ->willReturnSelf(); diff --git a/apps/provisioning_api/appinfo/routes.php b/apps/provisioning_api/appinfo/routes.php index 2f981e0c924..54d550260b8 100644 --- a/apps/provisioning_api/appinfo/routes.php +++ b/apps/provisioning_api/appinfo/routes.php @@ -74,4 +74,9 @@ 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#showVerifyMail', 'url' => '/mailVerification/{key}/{token}/{userId}', 'verb' => 'GET'], + ['name' => 'Verification#verifyMail', 'url' => '/mailVerification/{key}/{token}/{userId}', 'verb' => 'POST'], + ] ]; diff --git a/apps/provisioning_api/composer/composer/autoload_classmap.php b/apps/provisioning_api/composer/composer/autoload_classmap.php index 22927806e65..447f92afc8d 100644 --- a/apps/provisioning_api/composer/composer/autoload_classmap.php +++ b/apps/provisioning_api/composer/composer/autoload_classmap.php @@ -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', diff --git a/apps/provisioning_api/composer/composer/autoload_static.php b/apps/provisioning_api/composer/composer/autoload_static.php index f5a4b73f4f8..6dbf6b45c79 100644 --- a/apps/provisioning_api/composer/composer/autoload_static.php +++ b/apps/provisioning_api/composer/composer/autoload_static.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', diff --git a/apps/provisioning_api/composer/composer/installed.php b/apps/provisioning_api/composer/composer/installed.php index b99ca67ef3a..561b3105cde 100644 --- a/apps/provisioning_api/composer/composer/installed.php +++ b/apps/provisioning_api/composer/composer/installed.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, ), ), diff --git a/apps/provisioning_api/lib/Controller/AUserData.php b/apps/provisioning_api/lib/Controller/AUserData.php index e358d282061..5bb62f2b7dc 100644 --- a/apps/provisioning_api/lib/Controller/AUserData.php +++ b/apps/provisioning_api/lib/Controller/AUserData.php @@ -54,6 +54,13 @@ use OCP\User\Backend\ISetPasswordBackend; abstract class AUserData extends OCSController { public const SCOPE_SUFFIX = 'Scope'; + public const USER_FIELD_DISPLAYNAME = 'display'; + public const USER_FIELD_LANGUAGE = 'language'; + public const USER_FIELD_LOCALE = 'locale'; + public const USER_FIELD_PASSWORD = 'password'; + public const USER_FIELD_QUOTA = 'quota'; + public const USER_FIELD_NOTIFICATION_EMAIL = 'notify_email'; + /** @var IUserManager */ protected $userManager; /** @var IConfig */ @@ -139,14 +146,14 @@ abstract class AUserData extends OCSController { $data['lastLogin'] = $targetUserObject->getLastLogin() * 1000; $data['backend'] = $targetUserObject->getBackendClassName(); $data['subadmin'] = $this->getUserSubAdminGroupsData($targetUserObject->getUID()); - $data['quota'] = $this->fillStorageInfo($targetUserObject->getUID()); + $data[self::USER_FIELD_QUOTA] = $this->fillStorageInfo($targetUserObject->getUID()); try { if ($includeScopes) { $data[IAccountManager::PROPERTY_AVATAR . self::SCOPE_SUFFIX] = $userAccount->getProperty(IAccountManager::PROPERTY_AVATAR)->getScope(); } - $data[IAccountManager::PROPERTY_EMAIL] = $targetUserObject->getEMailAddress(); + $data[IAccountManager::PROPERTY_EMAIL] = $targetUserObject->getSystemEMailAddress(); if ($includeScopes) { $data[IAccountManager::PROPERTY_EMAIL . self::SCOPE_SUFFIX] = $userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getScope(); } @@ -187,8 +194,9 @@ abstract class AUserData extends OCSController { } $data['groups'] = $gids; - $data['language'] = $this->l10nFactory->getUserLanguage($targetUserObject); - $data['locale'] = $this->config->getUserValue($targetUserObject->getUID(), 'core', 'locale'); + $data[self::USER_FIELD_LANGUAGE] = $this->l10nFactory->getUserLanguage($targetUserObject); + $data[self::USER_FIELD_LOCALE] = $this->config->getUserValue($targetUserObject->getUID(), 'core', 'locale'); + $data[self::USER_FIELD_NOTIFICATION_EMAIL] = $targetUserObject->getPrimaryEMailAddress(); $backend = $targetUserObject->getBackend(); $data['backendCapabilities'] = [ @@ -238,7 +246,7 @@ abstract class AUserData extends OCSController { 'used' => $storage['used'], 'total' => $storage['total'], 'relative' => $storage['relative'], - 'quota' => $storage['quota'], + self::USER_FIELD_QUOTA => $storage['quota'], ]; } catch (NotFoundException $ex) { // User fs is not setup yet @@ -251,7 +259,7 @@ abstract class AUserData extends OCSController { $quota = OC_Helper::computerFileSize($quota); } $data = [ - 'quota' => $quota !== false ? $quota : 'none', + self::USER_FIELD_QUOTA => $quota !== false ? $quota : 'none', 'used' => 0 ]; } diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index a0eda5848ec..dd8397a8a89 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -42,6 +42,7 @@ declare(strict_types=1); */ namespace OCA\Provisioning_API\Controller; +use InvalidArgumentException; use libphonenumber\NumberParseException; use libphonenumber\PhoneNumber; use libphonenumber\PhoneNumberFormat; @@ -418,15 +419,15 @@ class UsersController extends AUserData { } if ($displayName !== '') { - $this->editUser($userid, 'display', $displayName); + $this->editUser($userid, self::USER_FIELD_DISPLAYNAME, $displayName); } if ($quota !== '') { - $this->editUser($userid, 'quota', $quota); + $this->editUser($userid, self::USER_FIELD_QUOTA, $quota); } if ($language !== '') { - $this->editUser($userid, 'language', $language); + $this->editUser($userid, self::USER_FIELD_LANGUAGE, $language); } // Send new user mail only if a mail is set @@ -466,7 +467,7 @@ class UsersController extends AUserData { ] ); throw $e; - } catch (\InvalidArgumentException $e) { + } catch (InvalidArgumentException $e) { $this->logger->error('Failed addUser attempt with invalid argument exeption.', [ 'app' => 'ocs_api', @@ -621,6 +622,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 +633,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 +654,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; @@ -670,7 +677,7 @@ class UsersController extends AUserData { try { $targetProperty->setScope($value); $this->accountManager->updateAccount($userAccount); - } catch (\InvalidArgumentException $e) { + } catch (InvalidArgumentException $e) { throw new OCSException('', 102); } } else { @@ -711,7 +718,7 @@ class UsersController extends AUserData { if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) { if ($targetUser->getBackend() instanceof ISetDisplayNameBackend || $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)) { - $permittedFields[] = 'display'; + $permittedFields[] = self::USER_FIELD_DISPLAYNAME; $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; } $permittedFields[] = IAccountManager::PROPERTY_EMAIL; @@ -722,15 +729,16 @@ class UsersController extends AUserData { $permittedFields[] = IAccountManager::COLLECTION_EMAIL; - $permittedFields[] = 'password'; + $permittedFields[] = self::USER_FIELD_PASSWORD; + $permittedFields[] = self::USER_FIELD_NOTIFICATION_EMAIL; if ($this->config->getSystemValue('force_language', false) === false || $this->groupManager->isAdmin($currentLoggedInUser->getUID())) { - $permittedFields[] = 'language'; + $permittedFields[] = self::USER_FIELD_LANGUAGE; } if ($this->config->getSystemValue('force_locale', false) === false || $this->groupManager->isAdmin($currentLoggedInUser->getUID())) { - $permittedFields[] = 'locale'; + $permittedFields[] = self::USER_FIELD_LOCALE; } $permittedFields[] = IAccountManager::PROPERTY_PHONE; @@ -746,7 +754,7 @@ class UsersController extends AUserData { // If admin they can edit their own quota if ($this->groupManager->isAdmin($currentLoggedInUser->getUID())) { - $permittedFields[] = 'quota'; + $permittedFields[] = self::USER_FIELD_QUOTA; } } else { // Check if admin / subadmin @@ -756,19 +764,20 @@ class UsersController extends AUserData { // They have permissions over the user if ($targetUser->getBackend() instanceof ISetDisplayNameBackend || $targetUser->getBackend()->implementsActions(Backend::SET_DISPLAYNAME)) { - $permittedFields[] = 'display'; + $permittedFields[] = self::USER_FIELD_DISPLAYNAME; $permittedFields[] = IAccountManager::PROPERTY_DISPLAYNAME; } $permittedFields[] = IAccountManager::PROPERTY_EMAIL; $permittedFields[] = IAccountManager::COLLECTION_EMAIL; - $permittedFields[] = 'password'; - $permittedFields[] = 'language'; - $permittedFields[] = 'locale'; + $permittedFields[] = self::USER_FIELD_PASSWORD; + $permittedFields[] = self::USER_FIELD_LANGUAGE; + $permittedFields[] = self::USER_FIELD_LOCALE; $permittedFields[] = IAccountManager::PROPERTY_PHONE; $permittedFields[] = IAccountManager::PROPERTY_ADDRESS; $permittedFields[] = IAccountManager::PROPERTY_WEBSITE; $permittedFields[] = IAccountManager::PROPERTY_TWITTER; - $permittedFields[] = 'quota'; + $permittedFields[] = self::USER_FIELD_QUOTA; + $permittedFields[] = self::USER_FIELD_NOTIFICATION_EMAIL; } else { // No rights throw new OCSException('', OCSController::RESPOND_NOT_FOUND); @@ -780,11 +789,11 @@ class UsersController extends AUserData { } // Process the edit switch ($key) { - case 'display': + case self::USER_FIELD_DISPLAYNAME: case IAccountManager::PROPERTY_DISPLAYNAME: $targetUser->setDisplayName($value); break; - case 'quota': + case self::USER_FIELD_QUOTA: $quota = $value; if ($quota !== 'none' && $quota !== 'default') { if (is_numeric($quota)) { @@ -814,7 +823,7 @@ class UsersController extends AUserData { } $targetUser->setQuota($quota); break; - case 'password': + case self::USER_FIELD_PASSWORD: try { if (!$targetUser->canChangePassword()) { throw new OCSException('Setting the password is not supported by the users backend', 103); @@ -824,19 +833,39 @@ class UsersController extends AUserData { throw new OCSException($e->getMessage(), 103); } break; - case 'language': + case self::USER_FIELD_LANGUAGE: $languagesCodes = $this->l10nFactory->findAvailableLanguages(); if (!in_array($value, $languagesCodes, true) && $value !== 'en') { throw new OCSException('Invalid language', 102); } $this->config->setUserValue($targetUser->getUID(), 'core', 'lang', $value); break; - case 'locale': + case self::USER_FIELD_LOCALE: if (!$this->l10nFactory->localeExists($value)) { throw new OCSException('Invalid locale', 102); } $this->config->setUserValue($targetUser->getUID(), 'core', 'locale', $value); break; + case self::USER_FIELD_NOTIFICATION_EMAIL: + $success = false; + if ($value === '' || filter_var($value, FILTER_VALIDATE_EMAIL)) { + try { + $targetUser->setPrimaryEMailAddress($value); + $success = true; + } catch (InvalidArgumentException $e) { + $this->logger->info( + 'Cannot set primary email, because provided address is not verified', + [ + 'app' => 'provisioning_api', + 'exception' => $e, + ] + ); + } + } + if (!$success) { + throw new OCSException('', 102); + } + break; case IAccountManager::PROPERTY_EMAIL: if (filter_var($value, FILTER_VALIDATE_EMAIL) || $value === '') { $targetUser->setEMailAddress($value); @@ -845,7 +874,7 @@ class UsersController extends AUserData { } break; case IAccountManager::COLLECTION_EMAIL: - if (filter_var($value, FILTER_VALIDATE_EMAIL) && $value !== $targetUser->getEMailAddress()) { + if (filter_var($value, FILTER_VALIDATE_EMAIL) && $value !== $targetUser->getSystemEMailAddress()) { $userAccount = $this->accountManager->getAccount($targetUser); $mailCollection = $userAccount->getPropertyCollection(IAccountManager::COLLECTION_EMAIL); foreach ($mailCollection->getProperties() as $property) { @@ -872,7 +901,7 @@ class UsersController extends AUserData { if ($userProperty->getName() === IAccountManager::PROPERTY_PHONE) { $this->knownUserService->deleteByContactUserId($targetUser->getUID()); } - } catch (\InvalidArgumentException $e) { + } catch (InvalidArgumentException $e) { throw new OCSException('Invalid ' . $e->getMessage(), 102); } } @@ -895,7 +924,7 @@ class UsersController extends AUserData { try { $userProperty->setScope($value); $this->accountManager->updateAccount($userAccount); - } catch (\InvalidArgumentException $e) { + } catch (InvalidArgumentException $e) { throw new OCSException('Invalid ' . $e->getMessage(), 102); } } diff --git a/apps/provisioning_api/lib/Controller/VerificationController.php b/apps/provisioning_api/lib/Controller/VerificationController.php new file mode 100644 index 00000000000..c4ddd1e644d --- /dev/null +++ b/apps/provisioning_api/lib/Controller/VerificationController.php @@ -0,0 +1,143 @@ +<?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 + * @NoAdminRequired + * @NoSubAdminRequired + */ + public function showVerifyMail(string $token, string $userId, string $key) { + if ($this->userSession->getUser()->getUID() !== $userId) { + // not a public page, hence getUser() must return an IUser + throw new InvalidArgumentException('Logged in user is not mail address owner'); + } + $email = $this->crypto->decrypt($key); + + return new TemplateResponse( + 'core', 'confirmation', [ + 'title' => $this->l10n->t('Email confirmation'), + 'message' => $this->l10n->t('To enable the email address %s please click the button below.', [$email]), + 'action' => $this->l10n->t('Confirm'), + ], TemplateResponse::RENDER_AS_GUEST); + } + + /** + * @NoAdminRequired + * @NoSubAdminRequired + */ + 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); + $this->verificationToken->delete($token, $user, 'verifyMail' . $ref); + } 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]] + ], TemplateResponse::RENDER_AS_GUEST); + } + + return new TemplateResponse( + 'core', 'success', [ + 'title' => $this->l10n->t('Email confirmation successful'), + 'message' => $this->l10n->t('Email confirmation successful'), + ], TemplateResponse::RENDER_AS_GUEST); + } +} diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index cc638c89a63..7ae5d0c245f 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -952,7 +952,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $targetUser->expects($this->once()) - ->method('getEMailAddress') + ->method('getSystemEMailAddress') ->willReturn('demo@nextcloud.com'); $this->userSession ->expects($this->once()) @@ -1067,6 +1067,7 @@ class UsersControllerTest extends TestCase { 'setPassword' => true, ], 'additional_mail' => [], + 'notify_email' => null, ]; $this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID'])); } @@ -1083,9 +1084,9 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $targetUser - ->expects($this->once()) - ->method('getEMailAddress') - ->willReturn('demo@nextcloud.com'); + ->expects($this->once()) + ->method('getSystemEMailAddress') + ->willReturn('demo@nextcloud.com'); $this->userSession ->expects($this->once()) ->method('getUser') @@ -1195,6 +1196,7 @@ class UsersControllerTest extends TestCase { 'setPassword' => true, ], 'additional_mail' => [], + 'notify_email' => null, ]; $this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID'])); } @@ -1306,7 +1308,7 @@ class UsersControllerTest extends TestCase { ->willReturn('Subadmin User'); $targetUser ->expects($this->once()) - ->method('getEMailAddress') + ->method('getSystemEMailAddress') ->willReturn('subadmin@nextcloud.com'); $targetUser ->method('getUID') @@ -1361,6 +1363,7 @@ class UsersControllerTest extends TestCase { 'setPassword' => false, ], 'additional_mail' => [], + 'notify_email' => null, ]; $this->assertEquals($expected, $this->invokePrivate($this->api, 'getUserData', ['UID'])); } diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index f78fa7dd9b8..6be93d6a3a0 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -482,7 +482,7 @@ class UsersController extends Controller { } } - $oldEmailAddress = $userAccount->getUser()->getEMailAddress(); + $oldEmailAddress = $userAccount->getUser()->getSystemEMailAddress(); $oldEmailAddress = strtolower((string)$oldEmailAddress); if ($oldEmailAddress !== $userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getValue()) { // this is the only permission a backend provides and is also used @@ -490,7 +490,7 @@ class UsersController extends Controller { if (!$userAccount->getUser()->canChangeDisplayName()) { throw new ForbiddenException($this->l10n->t('Unable to change email address')); } - $userAccount->getUser()->setEMailAddress($userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getValue()); + $userAccount->getUser()->setSystemEMailAddress($userAccount->getProperty(IAccountManager::PROPERTY_EMAIL)->getValue()); } try { diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index 7faca378cdf..797fa1621fa 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -621,16 +621,15 @@ class UsersControllerTest extends \Test\TestCase { $user = $this->createMock(IUser::class); $user->method('getDisplayName')->willReturn($oldDisplayName); - $user->method('getEMailAddress')->willReturn($oldEmailAddress); + $user->method('getSystemEMailAddress')->willReturn($oldEmailAddress); $user->method('canChangeDisplayName')->willReturn(true); if ($data[IAccountManager::PROPERTY_EMAIL]['value'] === $oldEmailAddress || ($oldEmailAddress === null && $data[IAccountManager::PROPERTY_EMAIL]['value'] === '')) { - $user->expects($this->never())->method('setEMailAddress'); + $user->expects($this->never())->method('setSystemEMailAddress'); } else { - $user->expects($this->once())->method('setEMailAddress') - ->with($data[IAccountManager::PROPERTY_EMAIL]['value']) - ->willReturn(true); + $user->expects($this->once())->method('setSystemEMailAddress') + ->with($data[IAccountManager::PROPERTY_EMAIL]['value']); } if ($data[IAccountManager::PROPERTY_DISPLAYNAME]['value'] === $oldDisplayName || diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index 7d57fcbb275..b09fbd18327 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -448,7 +448,7 @@ class User { if ($email !== '') { $user = $this->userManager->get($this->uid); if (!is_null($user)) { - $currentEmail = (string)$user->getEMailAddress(); + $currentEmail = (string)$user->getSystemEMailAddress(); if ($currentEmail !== $email) { $user->setEMailAddress($email); } diff --git a/core/Command/User/Info.php b/core/Command/User/Info.php index 377b722d293..abc8938ed54 100644 --- a/core/Command/User/Info.php +++ b/core/Command/User/Info.php @@ -76,7 +76,7 @@ class Info extends Base { $data = [ 'user_id' => $user->getUID(), 'display_name' => $user->getDisplayName(), - 'email' => $user->getEMailAddress() ? $user->getEMailAddress() : '', + 'email' => (string)$user->getSystemEMailAddress(), 'cloud_id' => $user->getCloudId(), 'enabled' => $user->isEnabled(), 'groups' => $groups, diff --git a/core/Command/User/ListCommand.php b/core/Command/User/ListCommand.php index 3a2708d4da2..20422afbf9d 100644 --- a/core/Command/User/ListCommand.php +++ b/core/Command/User/ListCommand.php @@ -104,7 +104,7 @@ class ListCommand extends Base { return [ 'user_id' => $user->getUID(), 'display_name' => $user->getDisplayName(), - 'email' => $user->getEMailAddress() ? $user->getEMailAddress() : '', + 'email' => (string)$user->getSystemEMailAddress(), 'cloud_id' => $user->getCloudId(), 'enabled' => $user->isEnabled(), 'groups' => $groups, diff --git a/core/Controller/LostController.php b/core/Controller/LostController.php index bed96eeec83..cee3837ac5a 100644 --- a/core/Controller/LostController.php +++ b/core/Controller/LostController.php @@ -40,7 +40,6 @@ use OC\Core\Exception\ResetPasswordException; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; -use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IManager; @@ -54,8 +53,8 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\Mail\IMailer; -use OCP\Security\ICrypto; -use OCP\Security\ISecureRandom; +use OCP\Security\VerificationToken\InvalidTokenException; +use OCP\Security\VerificationToken\IVerificationToken; use function array_filter; use function count; use function reset; @@ -82,67 +81,46 @@ class LostController extends Controller { protected $encryptionManager; /** @var IConfig */ protected $config; - /** @var ISecureRandom */ - protected $secureRandom; /** @var IMailer */ protected $mailer; - /** @var ITimeFactory */ - protected $timeFactory; - /** @var ICrypto */ - protected $crypto; /** @var ILogger */ private $logger; /** @var Manager */ private $twoFactorManager; /** @var IInitialStateService */ private $initialStateService; - - /** - * @param string $appName - * @param IRequest $request - * @param IURLGenerator $urlGenerator - * @param IUserManager $userManager - * @param Defaults $defaults - * @param IL10N $l10n - * @param IConfig $config - * @param ISecureRandom $secureRandom - * @param string $defaultMailAddress - * @param IManager $encryptionManager - * @param IMailer $mailer - * @param ITimeFactory $timeFactory - * @param ICrypto $crypto - */ - public function __construct($appName, - IRequest $request, - IURLGenerator $urlGenerator, - IUserManager $userManager, - Defaults $defaults, - IL10N $l10n, - IConfig $config, - ISecureRandom $secureRandom, - $defaultMailAddress, - IManager $encryptionManager, - IMailer $mailer, - ITimeFactory $timeFactory, - ICrypto $crypto, - ILogger $logger, - Manager $twoFactorManager, - IInitialStateService $initialStateService) { + /** @var IVerificationToken */ + private $verificationToken; + + public function __construct( + $appName, + IRequest $request, + IURLGenerator $urlGenerator, + IUserManager $userManager, + Defaults $defaults, + IL10N $l10n, + IConfig $config, + $defaultMailAddress, + IManager $encryptionManager, + IMailer $mailer, + ILogger $logger, + Manager $twoFactorManager, + IInitialStateService $initialStateService, + IVerificationToken $verificationToken + ) { parent::__construct($appName, $request); $this->urlGenerator = $urlGenerator; $this->userManager = $userManager; $this->defaults = $defaults; $this->l10n = $l10n; - $this->secureRandom = $secureRandom; $this->from = $defaultMailAddress; $this->encryptionManager = $encryptionManager; $this->config = $config; $this->mailer = $mailer; - $this->timeFactory = $timeFactory; - $this->crypto = $crypto; $this->logger = $logger; $this->twoFactorManager = $twoFactorManager; $this->initialStateService = $initialStateService; + $this->verificationToken = $verificationToken; } /** @@ -192,36 +170,14 @@ class LostController extends Controller { * @param string $userId * @throws \Exception */ - protected function checkPasswordResetToken($token, $userId) { - $user = $this->userManager->get($userId); - if ($user === null || !$user->isEnabled()) { - throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid')); - } - - $encryptedToken = $this->config->getUserValue($userId, 'core', 'lostpassword', null); - if ($encryptedToken === null) { - throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid')); - } - + protected function checkPasswordResetToken(string $token, string $userId): void { try { - $mailAddress = !is_null($user->getEMailAddress()) ? $user->getEMailAddress() : ''; - $decryptedToken = $this->crypto->decrypt($encryptedToken, $mailAddress.$this->config->getSystemValue('secret')); - } catch (\Exception $e) { - throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid')); - } - - $splittedToken = explode(':', $decryptedToken); - if (count($splittedToken) !== 2) { - throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid')); - } - - if ($splittedToken[0] < ($this->timeFactory->getTime() - 60 * 60 * 24 * 7) || - $user->getLastLogin() > $splittedToken[0]) { - throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is expired')); - } - - if (!hash_equals($splittedToken[1], $token)) { - throw new \Exception($this->l10n->t('Couldn\'t reset password because the token is invalid')); + $this->verificationToken->check($token, $this->userManager->get($userId), 'lostpassword', '', true); + } catch (InvalidTokenException $e) { + $error = $e->getCode() === InvalidTokenException::TOKEN_EXPIRED + ? $this->l10n->t('Could not reset password because the token is expired') + : $this->l10n->t('Could not reset password because the token is invalid'); + throw new \Exception($error, (int)$e->getCode(), $e); } } @@ -343,15 +299,7 @@ class LostController extends Controller { // secret being the users' email address appended with the system secret. // This makes the token automatically invalidate once the user changes // their email address. - $token = $this->secureRandom->generate( - 21, - ISecureRandom::CHAR_DIGITS. - ISecureRandom::CHAR_LOWER. - ISecureRandom::CHAR_UPPER - ); - $tokenValue = $this->timeFactory->getTime() .':'. $token; - $encryptedValue = $this->crypto->encrypt($tokenValue, $email . $this->config->getSystemValue('secret')); - $this->config->setUserValue($user->getUID(), 'core', 'lostpassword', $encryptedValue); + $token = $this->verificationToken->create($user, 'lostpassword', $email); $link = $this->urlGenerator->linkToRouteAbsolute('core.lost.resetform', ['userId' => $user->getUID(), 'token' => $token]); diff --git a/core/templates/confirmation.php b/core/templates/confirmation.php new file mode 100644 index 00000000000..26014cd1e79 --- /dev/null +++ b/core/templates/confirmation.php @@ -0,0 +1,20 @@ +<?php +/** @var array $_ */ +/** @var \OCP\IL10N $l */ +/** @var \OCP\Defaults $theme */ + +?> + +<div class="update"> + <form method="POST" action="<?php print_unescaped($_['targetUrl']);?>"> + <h2><?php p($_['title']) ?></h2> + <p><?php p($_['message']) ?></p> + <div class="buttons"> + <input type="submit" class="primary" value="<?php p($_['action']); ?>"> + </div> + <?php foreach ($_['parameters'] as $name => $value) {?> + <input type="hidden" name="<?php p($name); ?>" value="<?php p($value); ?>"> + <?php } ?> + <input type="hidden" name="requesttoken" value="<?php p($_['requesttoken']) ?>"> + </form> +</div> diff --git a/core/templates/success.php b/core/templates/success.php new file mode 100644 index 00000000000..5ce8ff4f045 --- /dev/null +++ b/core/templates/success.php @@ -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> diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index ea1473f27dc..3f1f0d3b336 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -488,6 +488,8 @@ return array( 'OCP\\Security\\ICrypto' => $baseDir . '/lib/public/Security/ICrypto.php', 'OCP\\Security\\IHasher' => $baseDir . '/lib/public/Security/IHasher.php', 'OCP\\Security\\ISecureRandom' => $baseDir . '/lib/public/Security/ISecureRandom.php', + 'OCP\\Security\\VerificationToken\\IVerificationToken' => $baseDir . '/lib/public/Security/VerificationToken/IVerificationToken.php', + 'OCP\\Security\\VerificationToken\\InvalidTokenException' => $baseDir . '/lib/public/Security/VerificationToken/InvalidTokenException.php', 'OCP\\Session\\Exceptions\\SessionNotAvailableException' => $baseDir . '/lib/public/Session/Exceptions/SessionNotAvailableException.php', 'OCP\\Settings\\IIconSection' => $baseDir . '/lib/public/Settings/IIconSection.php', 'OCP\\Settings\\IManager' => $baseDir . '/lib/public/Settings/IManager.php', @@ -1371,6 +1373,8 @@ return array( 'OC\\Security\\RateLimiting\\Limiter' => $baseDir . '/lib/private/Security/RateLimiting/Limiter.php', 'OC\\Security\\SecureRandom' => $baseDir . '/lib/private/Security/SecureRandom.php', 'OC\\Security\\TrustedDomainHelper' => $baseDir . '/lib/private/Security/TrustedDomainHelper.php', + 'OC\\Security\\VerificationToken\\CleanUpJob' => $baseDir . '/lib/private/Security/VerificationToken/CleanUpJob.php', + 'OC\\Security\\VerificationToken\\VerificationToken' => $baseDir . '/lib/private/Security/VerificationToken/VerificationToken.php', 'OC\\Server' => $baseDir . '/lib/private/Server.php', 'OC\\ServerContainer' => $baseDir . '/lib/private/ServerContainer.php', 'OC\\ServerNotAvailableException' => $baseDir . '/lib/private/ServerNotAvailableException.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 93208f6ff15..a5b624e6e6c 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -517,6 +517,8 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OCP\\Security\\ICrypto' => __DIR__ . '/../../..' . '/lib/public/Security/ICrypto.php', 'OCP\\Security\\IHasher' => __DIR__ . '/../../..' . '/lib/public/Security/IHasher.php', 'OCP\\Security\\ISecureRandom' => __DIR__ . '/../../..' . '/lib/public/Security/ISecureRandom.php', + 'OCP\\Security\\VerificationToken\\IVerificationToken' => __DIR__ . '/../../..' . '/lib/public/Security/VerificationToken/IVerificationToken.php', + 'OCP\\Security\\VerificationToken\\InvalidTokenException' => __DIR__ . '/../../..' . '/lib/public/Security/VerificationToken/InvalidTokenException.php', 'OCP\\Session\\Exceptions\\SessionNotAvailableException' => __DIR__ . '/../../..' . '/lib/public/Session/Exceptions/SessionNotAvailableException.php', 'OCP\\Settings\\IIconSection' => __DIR__ . '/../../..' . '/lib/public/Settings/IIconSection.php', 'OCP\\Settings\\IManager' => __DIR__ . '/../../..' . '/lib/public/Settings/IManager.php', @@ -1400,6 +1402,8 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Security\\RateLimiting\\Limiter' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Limiter.php', 'OC\\Security\\SecureRandom' => __DIR__ . '/../../..' . '/lib/private/Security/SecureRandom.php', 'OC\\Security\\TrustedDomainHelper' => __DIR__ . '/../../..' . '/lib/private/Security/TrustedDomainHelper.php', + 'OC\\Security\\VerificationToken\\CleanUpJob' => __DIR__ . '/../../..' . '/lib/private/Security/VerificationToken/CleanUpJob.php', + 'OC\\Security\\VerificationToken\\VerificationToken' => __DIR__ . '/../../..' . '/lib/private/Security/VerificationToken/VerificationToken.php', 'OC\\Server' => __DIR__ . '/../../..' . '/lib/private/Server.php', 'OC\\ServerContainer' => __DIR__ . '/../../..' . '/lib/private/ServerContainer.php', 'OC\\ServerNotAvailableException' => __DIR__ . '/../../..' . '/lib/private/ServerNotAvailableException.php', diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index 9fc5accfa08..a3f971df6a1 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -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(), ]; } diff --git a/lib/private/Accounts/AccountProperty.php b/lib/private/Accounts/AccountProperty.php index 1a21baf9698..0e6356e9e92 100644 --- a/lib/private/Accounts/AccountProperty.php +++ b/lib/private/Accounts/AccountProperty.php @@ -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; + } } diff --git a/lib/private/Accounts/AccountPropertyCollection.php b/lib/private/Accounts/AccountPropertyCollection.php index eb92536a6a0..3aed76d8746 100644 --- a/lib/private/Accounts/AccountPropertyCollection.php +++ b/lib/private/Accounts/AccountPropertyCollection.php @@ -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) { diff --git a/lib/private/Collaboration/Collaborators/UserPlugin.php b/lib/private/Collaboration/Collaborators/UserPlugin.php index e3e4b37f383..9ed94082f0d 100644 --- a/lib/private/Collaboration/Collaborators/UserPlugin.php +++ b/lib/private/Collaboration/Collaborators/UserPlugin.php @@ -157,7 +157,7 @@ class UserPlugin implements ISearchPlugin { $userStatuses = $this->userStatusManager->getUserStatuses(array_keys($users)); foreach ($users as $uid => $user) { $userDisplayName = $user->getDisplayName(); - $userEmail = $user->getEMailAddress(); + $userEmail = $user->getSystemEMailAddress(); $uid = (string) $uid; $status = []; @@ -244,7 +244,7 @@ class UserPlugin implements ISearchPlugin { if ($addUser) { $status = []; $uid = $user->getUID(); - $userEmail = $user->getEMailAddress(); + $userEmail = $user->getSystemEMailAddress(); if (array_key_exists($user->getUID(), $userStatuses)) { $userStatus = $userStatuses[$user->getUID()]; $status = [ diff --git a/lib/private/Mail/EMailTemplate.php b/lib/private/Mail/EMailTemplate.php index efe1a6eef1d..a83f7787829 100644 --- a/lib/private/Mail/EMailTemplate.php +++ b/lib/private/Mail/EMailTemplate.php @@ -568,7 +568,7 @@ EOF; * * @param string $text Text of button; Note: When $plainText falls back to this, HTML is automatically escaped in the HTML email * @param string $url URL of button - * @param string $plainText Text of button in plain text version + * @param string|false $plainText Text of button in plain text version * if empty the $text is used, if false none will be used * * @since 12.0.0 diff --git a/lib/private/Security/VerificationToken/CleanUpJob.php b/lib/private/Security/VerificationToken/CleanUpJob.php new file mode 100644 index 00000000000..331172898ec --- /dev/null +++ b/lib/private/Security/VerificationToken/CleanUpJob.php @@ -0,0 +1,90 @@ +<?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 OC\Security\VerificationToken; + +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IConfig; +use OCP\ILogger; +use OCP\IUserManager; +use OCP\Security\VerificationToken\InvalidTokenException; +use OCP\Security\VerificationToken\IVerificationToken; + +class CleanUpJob extends \OCP\BackgroundJob\Job { + + /** @var int */ + protected $runNotBefore; + /** @var string */ + protected $userId; + /** @var string */ + protected $subject; + /** @var string */ + protected $pwdPrefix; + /** @var IConfig */ + private $config; + /** @var IVerificationToken */ + private $verificationToken; + /** @var IUserManager */ + private $userManager; + + public function __construct(ITimeFactory $time, IConfig $config, IVerificationToken $verificationToken, IUserManager $userManager) { + parent::__construct($time); + $this->config = $config; + $this->verificationToken = $verificationToken; + $this->userManager = $userManager; + } + + public function setArgument($argument) { + parent::setArgument($argument); + $args = \json_decode($argument); + $this->userId = (string)$args['userId']; + $this->subject = (string)$args['subject']; + $this->pwdPrefix = (string)$args['pp']; + $this->runNotBefore = (int)$args['notBefore']; + } + + protected function run($argument) { + try { + $user = $this->userManager->get($this->userId); + if ($user === null) { + return; + } + $this->verificationToken->check('irrelevant', $user, $this->subject, $this->pwdPrefix); + } catch (InvalidTokenException $e) { + if ($e->getCode() === InvalidTokenException::TOKEN_EXPIRED) { + // make sure to only remove expired tokens + $this->config->deleteUserValue($this->userId, 'core', $this->subject); + } + } + } + + public function execute($jobList, ILogger $logger = null) { + if ($this->time->getTime() >= $this->runNotBefore) { + $jobList->remove($this, $this->argument); + parent::execute($jobList, $logger); + } + } +} diff --git a/lib/private/Security/VerificationToken/VerificationToken.php b/lib/private/Security/VerificationToken/VerificationToken.php new file mode 100644 index 00000000000..c85e0e7b5a1 --- /dev/null +++ b/lib/private/Security/VerificationToken/VerificationToken.php @@ -0,0 +1,129 @@ +<?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 OC\Security\VerificationToken; + +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJobList; +use OCP\IConfig; +use OCP\IUser; +use OCP\Security\ICrypto; +use OCP\Security\ISecureRandom; +use OCP\Security\VerificationToken\InvalidTokenException; +use OCP\Security\VerificationToken\IVerificationToken; +use function json_encode; + +class VerificationToken implements IVerificationToken { + protected const TOKEN_LIFETIME = 60 * 60 * 24 * 7; + + /** @var IConfig */ + private $config; + /** @var ICrypto */ + private $crypto; + /** @var ITimeFactory */ + private $timeFactory; + /** @var ISecureRandom */ + private $secureRandom; + /** @var IJobList */ + private $jobList; + + public function __construct( + IConfig $config, + ICrypto $crypto, + ITimeFactory $timeFactory, + ISecureRandom $secureRandom, + IJobList $jobList + ) { + $this->config = $config; + $this->crypto = $crypto; + $this->timeFactory = $timeFactory; + $this->secureRandom = $secureRandom; + $this->jobList = $jobList; + } + + /** + * @throws InvalidTokenException + */ + protected function throwInvalidTokenException(int $code): void { + throw new InvalidTokenException($code); + } + + public function check(string $token, ?IUser $user, string $subject, string $passwordPrefix = '', bool $expiresWithLogin = false): void { + if ($user === null || !$user->isEnabled()) { + $this->throwInvalidTokenException(InvalidTokenException::USER_UNKNOWN); + } + + $encryptedToken = $this->config->getUserValue($user->getUID(), 'core', $subject, null); + if ($encryptedToken === null) { + $this->throwInvalidTokenException(InvalidTokenException::TOKEN_NOT_FOUND); + } + + try { + $decryptedToken = $this->crypto->decrypt($encryptedToken, $passwordPrefix.$this->config->getSystemValue('secret')); + } catch (\Exception $e) { + $this->throwInvalidTokenException(InvalidTokenException::TOKEN_DECRYPTION_ERROR); + } + + $splitToken = explode(':', $decryptedToken ?? ''); + if (count($splitToken) !== 2) { + $this->throwInvalidTokenException(InvalidTokenException::TOKEN_INVALID_FORMAT); + } + + if ($splitToken[0] < ($this->timeFactory->getTime() - self::TOKEN_LIFETIME) + || ($expiresWithLogin && $user->getLastLogin() > $splitToken[0])) { + $this->throwInvalidTokenException(InvalidTokenException::TOKEN_EXPIRED); + } + + if (!hash_equals($splitToken[1], $token)) { + $this->throwInvalidTokenException(InvalidTokenException::TOKEN_MISMATCH); + } + } + + public function create(IUser $user, string $subject, string $passwordPrefix = ''): string { + $token = $this->secureRandom->generate( + 21, + ISecureRandom::CHAR_DIGITS. + ISecureRandom::CHAR_LOWER. + ISecureRandom::CHAR_UPPER + ); + $tokenValue = $this->timeFactory->getTime() .':'. $token; + $encryptedValue = $this->crypto->encrypt($tokenValue, $passwordPrefix . $this->config->getSystemValue('secret')); + $this->config->setUserValue($user->getUID(), 'core', $subject, $encryptedValue); + $jobArgs = json_encode([ + 'userId' => $user->getUID(), + 'subject' => $subject, + 'pp' => $passwordPrefix, + 'notBefore' => $this->timeFactory->getTime() + self::TOKEN_LIFETIME * 2, // multiply to provide a grace period + ]); + $this->jobList->add(CleanUpJob::class, $jobArgs); + + return $token; + } + + public function delete(string $token, IUser $user, string $subject): void { + $this->config->deleteUserValue($user->getUID(), 'core', $subject); + } +} diff --git a/lib/private/Server.php b/lib/private/Server.php index 0320eda2b91..6b6a1402a04 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -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); diff --git a/lib/private/Setup.php b/lib/private/Setup.php index a4873e63aa9..c24d417f8cf 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -439,7 +439,7 @@ class Setup { // Set email for admin if (!empty($options['adminemail'])) { - $config->setUserValue($user->getUID(), 'settings', 'email', $options['adminemail']); + $user->setSystemEMailAddress($options['adminemail']); } } diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 1827be61a7a..3e30861f2a4 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -700,6 +700,7 @@ class Manager extends PublicEmitter implements IUserManager { * @since 9.1.0 */ public function getByEmail($email) { + // looking for 'email' only (and not primary_mail) is intentional $userIds = $this->config->getUsersForUserValueCaseInsensitive('settings', 'email', $email); $users = array_map(function ($uid) { diff --git a/lib/private/User/User.php b/lib/private/User/User.php index f17824f51b9..5fa1272f95c 100644 --- a/lib/private/User/User.php +++ b/lib/private/User/User.php @@ -34,10 +34,12 @@ */ namespace OC\User; +use InvalidArgumentException; use OC\Accounts\AccountManager; use OC\Avatar\AvatarManager; use OC\Hooks\Emitter; use OC_Helper; +use OCP\Accounts\IAccountManager; use OCP\EventDispatcher\IEventDispatcher; use OCP\Group\Events\BeforeUserRemovedEvent; use OCP\Group\Events\UserRemovedEvent; @@ -55,6 +57,8 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\EventDispatcher\GenericEvent; class User implements IUser { + /** @var IAccountManager */ + protected $accountManager; /** @var string */ private $uid; @@ -165,25 +169,62 @@ class User implements IUser { } /** - * set the email address of the user - * - * @param string|null $mailAddress - * @return void - * @since 9.0.0 + * @inheritDoc */ public function setEMailAddress($mailAddress) { - $oldMailAddress = $this->getEMailAddress(); + $this->setSystemEMailAddress($mailAddress); + } + + /** + * @inheritDoc + */ + public function setSystemEMailAddress(string $mailAddress): void { + $oldMailAddress = $this->getSystemEMailAddress(); + + if ($mailAddress === '') { + $this->config->deleteUserValue($this->uid, 'settings', 'email'); + } else { + $this->config->setUserValue($this->uid, 'settings', 'email', $mailAddress); + } + + $primaryAddress = $this->getPrimaryEMailAddress(); + if ($primaryAddress === $mailAddress) { + // on match no dedicated primary settings is necessary + $this->setPrimaryEMailAddress(''); + } + if ($oldMailAddress !== $mailAddress) { - if ($mailAddress === '') { - $this->config->deleteUserValue($this->uid, 'settings', 'email'); - } else { - $this->config->setUserValue($this->uid, 'settings', 'email', $mailAddress); - } $this->triggerChange('eMailAddress', $mailAddress, $oldMailAddress); } } /** + * @inheritDoc + */ + public function setPrimaryEMailAddress(string $mailAddress): void { + if ($mailAddress === '') { + $this->config->deleteUserValue($this->uid, 'settings', 'primary_email'); + return; + } + + $this->ensureAccountManager(); + $account = $this->accountManager->getAccount($this); + $property = $account->getPropertyCollection(IAccountManager::COLLECTION_EMAIL) + ->getPropertyByValue($mailAddress); + + if ($property === null || $property->getLocallyVerified() !== IAccountManager::VERIFIED) { + throw new InvalidArgumentException('Only verified emails can be set as primary'); + } + $this->config->setUserValue($this->uid, 'settings', 'primary_email', $mailAddress); + } + + private function ensureAccountManager() { + if (!$this->accountManager instanceof IAccountManager) { + $this->accountManager = \OC::$server->get(IAccountManager::class); + } + } + + /** * returns the timestamp of the user's last login or 0 if the user did never * login * @@ -390,10 +431,24 @@ class User implements IUser { * @since 9.0.0 */ public function getEMailAddress() { + return $this->getPrimaryEMailAddress() ?? $this->getSystemEMailAddress(); + } + + /** + * @inheritDoc + */ + public function getSystemEMailAddress(): ?string { return $this->config->getUserValue($this->uid, 'settings', 'email', null); } /** + * @inheritDoc + */ + public function getPrimaryEMailAddress(): ?string { + return $this->config->getUserValue($this->uid, 'settings', 'primary_email', null); + } + + /** * get the users' quota * * @return string diff --git a/lib/public/Accounts/IAccountProperty.php b/lib/public/Accounts/IAccountProperty.php index 20505f299dd..94866fc4807 100644 --- a/lib/public/Accounts/IAccountProperty.php +++ b/lib/public/Accounts/IAccountProperty.php @@ -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; } diff --git a/lib/public/Accounts/IAccountPropertyCollection.php b/lib/public/Accounts/IAccountPropertyCollection.php index 779fb1299b4..0d4c416cbaa 100644 --- a/lib/public/Accounts/IAccountPropertyCollection.php +++ b/lib/public/Accounts/IAccountPropertyCollection.php @@ -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; } diff --git a/lib/public/IUser.php b/lib/public/IUser.php index 7e75704ed5b..1a1d1e44d8a 100644 --- a/lib/public/IUser.php +++ b/lib/public/IUser.php @@ -27,6 +27,8 @@ */ namespace OCP; +use InvalidArgumentException; + /** * Interface IUser * @@ -157,7 +159,7 @@ interface IUser { public function setEnabled(bool $enabled = true); /** - * get the users email address + * get the user's email address * * @return string|null * @since 9.0.0 @@ -165,6 +167,35 @@ interface IUser { public function getEMailAddress(); /** + * get the user's system email address + * + * The system mail address may be read only and may be set from different + * sources like LDAP, SAML or simply the admin. + * + * Use this getter only when the system address is needed. For picking the + * proper address to e.g. send a mail to, use getEMailAddress(). + * + * @return string|null + * @since 23.0.0 + */ + public function getSystemEMailAddress(): ?string; + + /** + * get the user's preferred email address + * + * The primary mail address may be set be the user to specify a different + * email address where mails by Nextcloud are sent to. It is not necessarily + * set. + * + * Use this getter only when the primary address is needed. For picking the + * proper address to e.g. send a mail to, use getEMailAddress(). + * + * @return string|null + * @since 23.0.0 + */ + public function getPrimaryEMailAddress(): ?string; + + /** * get the avatar image if it exists * * @param int $size @@ -184,13 +215,43 @@ interface IUser { /** * set the email address of the user * + * It is an alias to setSystemEMailAddress() + * * @param string|null $mailAddress * @return void * @since 9.0.0 + * @deprecated 23.0.0 use setSystemEMailAddress() or setPrimaryEMailAddress() */ public function setEMailAddress($mailAddress); /** + * Set the system email address of the user + * + * This is supposed to be used when the email is set from different sources + * (i.e. other user backends, admin). + * + * @since 23.0.0 + */ + public function setSystemEMailAddress(string $mailAddress): void; + + /** + * Set the primary email address of the user. + * + * This method should be typically called when the user is changing their + * own primary address and is not allowed to change their system email. + * + * The mail address provided here must be already registered as an + * additional mail in the user account and also be verified locally. Also + * an empty string is allowed to delete this preference. + * + * @throws InvalidArgumentException when the provided email address does not + * satisfy constraints. + * + * @since 23.0.0 + */ + public function setPrimaryEMailAddress(string $mailAddress): void; + + /** * get the users' quota in human readable form. If a specific quota is not * set for the user, the default value is returned. If a default setting * was not set otherwise, it is return as 'none', i.e. quota is not limited. diff --git a/lib/public/IUserManager.php b/lib/public/IUserManager.php index c6cad6f0549..e5c220af40c 100644 --- a/lib/public/IUserManager.php +++ b/lib/public/IUserManager.php @@ -196,6 +196,8 @@ interface IUserManager { public function callForSeenUsers(\Closure $callback); /** + * returns all users having the provided email set as system email address + * * @param string $email * @return IUser[] * @since 9.1.0 diff --git a/lib/public/Mail/IEMailTemplate.php b/lib/public/Mail/IEMailTemplate.php index 39d6661b624..2d776549814 100644 --- a/lib/public/Mail/IEMailTemplate.php +++ b/lib/public/Mail/IEMailTemplate.php @@ -130,7 +130,7 @@ interface IEMailTemplate { * * @param string $text Text of button; Note: When $plainText falls back to this, HTML is automatically escaped in the HTML email * @param string $url URL of button - * @param string $plainText Text of button in plain text version + * @param string|false $plainText Text of button in plain text version * if empty the $text is used, if false none will be used * * @since 12.0.0 diff --git a/lib/public/Security/VerificationToken/IVerificationToken.php b/lib/public/Security/VerificationToken/IVerificationToken.php new file mode 100644 index 00000000000..e1d9203ec3b --- /dev/null +++ b/lib/public/Security/VerificationToken/IVerificationToken.php @@ -0,0 +1,62 @@ +<?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 OCP\Security\VerificationToken; + +use OCP\IUser; + +/** + * @since 23.0.0 + */ +interface IVerificationToken { + + /** + * Checks whether the a provided tokent matches a stored token and its + * constraints. An InvalidTokenException is thrown on issues, otherwise + * the check is successful. + * + * null can be passed as $user, but mind that this is for conveniently + * passing the return of IUserManager::getUser() to this method. When + * $user is null, InvalidTokenException is thrown for all the issued + * tokens are user related. + * + * @throws InvalidTokenException + * @since 23.0.0 + */ + public function check(string $token, ?IUser $user, string $subject, string $passwordPrefix = '', bool $expiresWithLogin = false): void; + + /** + * @since 23.0.0 + */ + public function create(IUser $user, string $subject, string $passwordPrefix = ''): string; + + /** + * Deletes the token identified by the provided parameters + * + * @since 23.0.0 + */ + public function delete(string $token, IUser $user, string $subject): void; +} diff --git a/lib/public/Security/VerificationToken/InvalidTokenException.php b/lib/public/Security/VerificationToken/InvalidTokenException.php new file mode 100644 index 00000000000..5c8144c5c74 --- /dev/null +++ b/lib/public/Security/VerificationToken/InvalidTokenException.php @@ -0,0 +1,74 @@ +<?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 OCP\Security\VerificationToken; + +/** @since 23.0.0 */ +class InvalidTokenException extends \Exception { + + /** + * @since 23.0.0 + */ + public function __construct(int $code) { + parent::__construct('', $code); + } + + /** + * @var int + * @since 23.0.0 + */ + public const USER_UNKNOWN = 1; + + /** + * @var int + * @since 23.0.0 + */ + public const TOKEN_NOT_FOUND = 2; + + /** + * @var int + * @since 23.0.0 + */ + public const TOKEN_DECRYPTION_ERROR = 3; + + /** + * @var int + * @since 23.0.0 + */ + public const TOKEN_INVALID_FORMAT = 4; + + /** + * @var int + * @since 23.0.0 + */ + public const TOKEN_EXPIRED = 5; + + /** + * @var int + * @since 23.0.0 + */ + public const TOKEN_MISMATCH = 6; +} diff --git a/tests/Core/Controller/LostControllerTest.php b/tests/Core/Controller/LostControllerTest.php index fd4e27d47f1..a9dd4c1797b 100644 --- a/tests/Core/Controller/LostControllerTest.php +++ b/tests/Core/Controller/LostControllerTest.php @@ -26,7 +26,6 @@ use OC\Core\Controller\LostController; use OC\Mail\Message; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; -use OCP\AppFramework\Utility\ITimeFactory; use OCP\Defaults; use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IManager; @@ -40,8 +39,8 @@ use OCP\IUser; use OCP\IUserManager; use OCP\Mail\IEMailTemplate; use OCP\Mail\IMailer; -use OCP\Security\ICrypto; -use OCP\Security\ISecureRandom; +use OCP\Security\VerificationToken\InvalidTokenException; +use OCP\Security\VerificationToken\IVerificationToken; /** * Class LostControllerTest @@ -66,22 +65,18 @@ class LostControllerTest extends \Test\TestCase { private $config; /** @var IMailer | \PHPUnit\Framework\MockObject\MockObject */ private $mailer; - /** @var ISecureRandom | \PHPUnit\Framework\MockObject\MockObject */ - private $secureRandom; /** @var IManager|\PHPUnit\Framework\MockObject\MockObject */ private $encryptionManager; - /** @var ITimeFactory | \PHPUnit\Framework\MockObject\MockObject */ - private $timeFactory; /** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */ private $request; - /** @var ICrypto|\PHPUnit\Framework\MockObject\MockObject */ - private $crypto; /** @var ILogger|\PHPUnit\Framework\MockObject\MockObject */ private $logger; /** @var Manager|\PHPUnit\Framework\MockObject\MockObject */ private $twofactorManager; /** @var IInitialStateService|\PHPUnit\Framework\MockObject\MockObject */ private $initialStateService; + /** @var IVerificationToken|\PHPUnit\Framework\MockObject\MockObject */ + private $verificationToken; protected function setUp(): void { parent::setUp(); @@ -123,10 +118,6 @@ class LostControllerTest extends \Test\TestCase { ->disableOriginalConstructor()->getMock(); $this->mailer = $this->getMockBuilder('\OCP\Mail\IMailer') ->disableOriginalConstructor()->getMock(); - $this->secureRandom = $this->getMockBuilder('\OCP\Security\ISecureRandom') - ->disableOriginalConstructor()->getMock(); - $this->timeFactory = $this->getMockBuilder('\OCP\AppFramework\Utility\ITimeFactory') - ->disableOriginalConstructor()->getMock(); $this->request = $this->getMockBuilder(IRequest::class) ->disableOriginalConstructor()->getMock(); $this->encryptionManager = $this->getMockBuilder(IManager::class) @@ -134,10 +125,10 @@ class LostControllerTest extends \Test\TestCase { $this->encryptionManager->expects($this->any()) ->method('isEnabled') ->willReturn(true); - $this->crypto = $this->createMock(ICrypto::class); $this->logger = $this->createMock(ILogger::class); $this->twofactorManager = $this->createMock(Manager::class); $this->initialStateService = $this->createMock(IInitialStateService::class); + $this->verificationToken = $this->createMock(IVerificationToken::class); $this->lostController = new LostController( 'Core', $this->request, @@ -146,89 +137,31 @@ class LostControllerTest extends \Test\TestCase { $this->defaults, $this->l10n, $this->config, - $this->secureRandom, 'lostpassword-noreply@localhost', $this->encryptionManager, $this->mailer, - $this->timeFactory, - $this->crypto, $this->logger, $this->twofactorManager, - $this->initialStateService + $this->initialStateService, + $this->verificationToken ); } - public function testResetFormWithNotExistingUser() { - $this->userManager->method('get') - ->with('NotExistingUser') - ->willReturn(null); - - $expectedResponse = new TemplateResponse( - 'core', - 'error', - [ - 'errors' => [ - ['error' => 'Couldn\'t reset password because the token is invalid'], - ] - ], - 'guest' - ); - $this->assertEquals($expectedResponse, $this->lostController->resetform('MySecretToken', 'NotExistingUser')); - } - - public function testResetFormInvalidTokenMatch() { - $this->config->method('getUserValue') - ->with('ValidTokenUser', 'core', 'lostpassword', null) - ->willReturn('encryptedToken'); - $this->existingUser->method('getLastLogin') - ->willReturn(12344); + public function testResetFormTokenError() { $this->userManager->method('get') ->with('ValidTokenUser') ->willReturn($this->existingUser); - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedToken'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); + $this->verificationToken->expects($this->once()) + ->method('check') + ->with('12345:MySecretToken', $this->existingUser, 'lostpassword') + ->willThrowException(new InvalidTokenException(InvalidTokenException::TOKEN_DECRYPTION_ERROR)); $response = $this->lostController->resetform('12345:MySecretToken', 'ValidTokenUser'); $expectedResponse = new TemplateResponse('core', 'error', [ 'errors' => [ - ['error' => 'Couldn\'t reset password because the token is invalid'], - ] - ], - 'guest'); - $this->assertEquals($expectedResponse, $response); - } - - - public function testResetFormExpiredToken() { - $this->userManager->method('get') - ->with('ValidTokenUser') - ->willReturn($this->existingUser); - $this->config - ->expects($this->once()) - ->method('getUserValue') - ->with('ValidTokenUser', 'core', 'lostpassword', null) - ->willReturn('encryptedToken'); - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedToken'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(999999); - - $response = $this->lostController->resetform('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser'); - $expectedResponse = new TemplateResponse('core', - 'error', - [ - 'errors' => [ - ['error' => 'Couldn\'t reset password because the token is expired'], + ['error' => 'Could not reset password because the token is invalid'], ] ], 'guest'); @@ -236,39 +169,14 @@ class LostControllerTest extends \Test\TestCase { } public function testResetFormValidToken() { - $this->existingUser->method('getLastLogin') - ->willReturn(12344); $this->userManager->method('get') ->with('ValidTokenUser') ->willReturn($this->existingUser); - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(12348); + $this->verificationToken->expects($this->once()) + ->method('check') + ->with('MySecretToken', $this->existingUser, 'lostpassword'); - $this->config->method('getUserValue') - ->with('ValidTokenUser', 'core', 'lostpassword', null) - ->willReturn('encryptedToken'); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedToken'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); - $this->urlGenerator - ->expects($this->once()) - ->method('linkToRouteAbsolute') - ->with('core.lost.setPassword', ['userId' => 'ValidTokenUser', 'token' => 'TheOnlyAndOnlyOneTokenToResetThePassword']) - ->willReturn('https://example.tld/index.php/lostpassword/'); - - $this->initialStateService->expects($this->at(0)) - ->method('provideInitialState') - ->with('core', 'resetPasswordUser', 'ValidTokenUser'); - $this->initialStateService->expects($this->at(1)) - ->method('provideInitialState') - ->with('core', 'resetPasswordTarget', 'https://example.tld/index.php/lostpassword/'); - - $response = $this->lostController->resetform('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser'); + $response = $this->lostController->resetform('MySecretToken', 'ValidTokenUser'); $expectedResponse = new TemplateResponse('core', 'login', [], @@ -319,24 +227,14 @@ class LostControllerTest extends \Test\TestCase { } public function testEmailSuccessful() { - $this->secureRandom - ->expects($this->once()) - ->method('generate') - ->with('21') - ->willReturn('ThisIsMaybeANotSoSecretToken!'); $this->userManager ->expects($this->any()) ->method('get') ->with('ExistingUser') ->willReturn($this->existingUser); - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(12348); - $this->config - ->expects($this->once()) - ->method('setUserValue') - ->with('ExistingUser', 'core', 'lostpassword', 'encryptedToken'); + $this->verificationToken->expects($this->once()) + ->method('create') + ->willReturn('ThisIsMaybeANotSoSecretToken!'); $this->urlGenerator ->expects($this->once()) ->method('linkToRouteAbsolute') @@ -379,12 +277,6 @@ class LostControllerTest extends \Test\TestCase { ->method('send') ->with($message); - $this->crypto->method('encrypt') - ->with( - $this->equalTo('12348:ThisIsMaybeANotSoSecretToken!'), - $this->equalTo('test@example.comSECRET') - )->willReturn('encryptedToken'); - $response = $this->lostController->email('ExistingUser'); $expectedResponse = new JSONResponse(['status' => 'success']); $expectedResponse->throttle(); @@ -392,11 +284,6 @@ class LostControllerTest extends \Test\TestCase { } public function testEmailWithMailSuccessful() { - $this->secureRandom - ->expects($this->once()) - ->method('generate') - ->with('21') - ->willReturn('ThisIsMaybeANotSoSecretToken!'); $this->userManager ->expects($this->any()) ->method('get') @@ -407,14 +294,9 @@ class LostControllerTest extends \Test\TestCase { ->method('getByEmail') ->with('test@example.com') ->willReturn([$this->existingUser]); - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(12348); - $this->config - ->expects($this->once()) - ->method('setUserValue') - ->with('ExistingUser', 'core', 'lostpassword', 'encryptedToken'); + $this->verificationToken->expects($this->once()) + ->method('create') + ->willReturn('ThisIsMaybeANotSoSecretToken!'); $this->urlGenerator ->expects($this->once()) ->method('linkToRouteAbsolute') @@ -457,12 +339,6 @@ class LostControllerTest extends \Test\TestCase { ->method('send') ->with($message); - $this->crypto->method('encrypt') - ->with( - $this->equalTo('12348:ThisIsMaybeANotSoSecretToken!'), - $this->equalTo('test@example.comSECRET') - )->willReturn('encryptedToken'); - $response = $this->lostController->email('test@example.com'); $expectedResponse = new JSONResponse(['status' => 'success']); $expectedResponse->throttle(); @@ -470,24 +346,14 @@ class LostControllerTest extends \Test\TestCase { } public function testEmailCantSendException() { - $this->secureRandom - ->expects($this->once()) - ->method('generate') - ->with('21') - ->willReturn('ThisIsMaybeANotSoSecretToken!'); $this->userManager ->expects($this->any()) ->method('get') ->with('ExistingUser') ->willReturn($this->existingUser); - $this->config - ->expects($this->once()) - ->method('setUserValue') - ->with('ExistingUser', 'core', 'lostpassword', 'encryptedToken'); - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(12348); + $this->verificationToken->expects($this->once()) + ->method('create') + ->willReturn('ThisIsMaybeANotSoSecretToken!'); $this->urlGenerator ->expects($this->once()) ->method('linkToRouteAbsolute') @@ -530,12 +396,6 @@ class LostControllerTest extends \Test\TestCase { ->with($message) ->will($this->throwException(new \Exception())); - $this->crypto->method('encrypt') - ->with( - $this->equalTo('12348:ThisIsMaybeANotSoSecretToken!'), - $this->equalTo('test@example.comSECRET') - )->willReturn('encryptedToken'); - $this->logger->expects($this->exactly(1)) ->method('logException'); @@ -560,14 +420,6 @@ class LostControllerTest extends \Test\TestCase { ->willReturn($this->existingUser); $this->config->expects($this->never()) ->method('deleteUserValue'); - $this->timeFactory->method('getTime') - ->willReturn(12348); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedData'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true); $expectedResponse = ['status' => 'error', 'msg' => '']; @@ -590,14 +442,6 @@ class LostControllerTest extends \Test\TestCase { $this->config->expects($this->once()) ->method('deleteUserValue') ->with('ValidTokenUser', 'core', 'lostpassword'); - $this->timeFactory->method('getTime') - ->willReturn(12348); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedData'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true); $expectedResponse = ['user' => 'ValidTokenUser', 'status' => 'success']; @@ -611,19 +455,14 @@ class LostControllerTest extends \Test\TestCase { $this->userManager->method('get') ->with('ValidTokenUser') ->willReturn($this->existingUser); - $this->timeFactory->method('getTime') - ->willReturn(617146); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedData'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); + $this->verificationToken->expects($this->atLeastOnce()) + ->method('check') + ->willThrowException(new InvalidTokenException(InvalidTokenException::TOKEN_EXPIRED)); $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true); $expectedResponse = [ 'status' => 'error', - 'msg' => 'Couldn\'t reset password because the token is expired', + 'msg' => 'Could not reset password because the token is expired', ]; $this->assertSame($expectedResponse, $response); } @@ -636,45 +475,14 @@ class LostControllerTest extends \Test\TestCase { ->method('get') ->with('ValidTokenUser') ->willReturn($this->existingUser); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('invalidEncryptedData'), - $this->equalTo('test@example.comSECRET') - )->willReturn('TheOnlyAndOnlyOneTokenToResetThePassword'); + $this->verificationToken->expects($this->atLeastOnce()) + ->method('check') + ->willThrowException(new InvalidTokenException(InvalidTokenException::TOKEN_INVALID_FORMAT)); $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true); $expectedResponse = [ 'status' => 'error', - 'msg' => 'Couldn\'t reset password because the token is invalid', - ]; - $this->assertSame($expectedResponse, $response); - } - - public function testSetPasswordExpiredTokenDueToLogin() { - $this->config->method('getUserValue') - ->with('ValidTokenUser', 'core', 'lostpassword', null) - ->willReturn('encryptedData'); - $this->existingUser->method('getLastLogin') - ->willReturn(12346); - $this->userManager - ->method('get') - ->with('ValidTokenUser') - ->willReturn($this->existingUser); - $this->timeFactory - ->method('getTime') - ->willReturn(12345); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedData'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); - - $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', true); - $expectedResponse = [ - 'status' => 'error', - 'msg' => 'Couldn\'t reset password because the token is expired', + 'msg' => 'Could not reset password because the token is invalid', ]; $this->assertSame($expectedResponse, $response); } @@ -686,33 +494,14 @@ class LostControllerTest extends \Test\TestCase { $this->userManager->method('get') ->with('ValidTokenUser') ->willReturn($this->existingUser); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('aValidtoken'), - $this->equalTo('test@example.comSECRET') - )->willThrowException(new \Exception()); - - $response = $this->lostController->setPassword('', 'ValidTokenUser', 'NewPassword', true); - $expectedResponse = [ - 'status' => 'error', - 'msg' => 'Couldn\'t reset password because the token is invalid' - ]; - $this->assertSame($expectedResponse, $response); - } - - public function testIsSetPasswordTokenNullFailing() { - $this->config->method('getUserValue') - ->with('ValidTokenUser', 'core', 'lostpassword', null) - ->willReturn(null); - $this->userManager->method('get') - ->with('ValidTokenUser') - ->willReturn($this->existingUser); + $this->verificationToken->expects($this->atLeastOnce()) + ->method('check') + ->willThrowException(new InvalidTokenException(InvalidTokenException::TOKEN_MISMATCH)); $response = $this->lostController->setPassword('', 'ValidTokenUser', 'NewPassword', true); $expectedResponse = [ 'status' => 'error', - 'msg' => 'Couldn\'t reset password because the token is invalid' + 'msg' => 'Could not reset password because the token is invalid' ]; $this->assertSame($expectedResponse, $response); } @@ -732,10 +521,14 @@ class LostControllerTest extends \Test\TestCase { ->with('DisabledUser') ->willReturn($user); + $this->verificationToken->expects($this->atLeastOnce()) + ->method('check') + ->willThrowException(new InvalidTokenException(InvalidTokenException::USER_UNKNOWN)); + $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'DisabledUser', 'NewPassword', true); $expectedResponse = [ 'status' => 'error', - 'msg' => 'Couldn\'t reset password because the token is invalid' + 'msg' => 'Could not reset password because the token is invalid' ]; $this->assertSame($expectedResponse, $response); } @@ -798,14 +591,6 @@ class LostControllerTest extends \Test\TestCase { $this->config->expects($this->once()) ->method('deleteUserValue') ->with('ValidTokenUser', 'core', 'lostpassword'); - $this->timeFactory->method('getTime') - ->willReturn(12348); - - $this->crypto->method('decrypt') - ->with( - $this->equalTo('encryptedData'), - $this->equalTo('test@example.comSECRET') - )->willReturn('12345:TheOnlyAndOnlyOneTokenToResetThePassword'); $response = $this->lostController->setPassword('TheOnlyAndOnlyOneTokenToResetThePassword', 'ValidTokenUser', 'NewPassword', false); $expectedResponse = ['user' => 'ValidTokenUser', 'status' => 'success']; diff --git a/tests/lib/Accounts/AccountManagerTest.php b/tests/lib/Accounts/AccountManagerTest.php index 8ed0e29d7ce..bf79d233131 100644 --- a/tests/lib/Accounts/AccountManagerTest.php +++ b/tests/lib/Accounts/AccountManagerTest.php @@ -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(); diff --git a/tests/lib/AllConfigTest.php b/tests/lib/AllConfigTest.php index 9daa7c9be9c..b0b0b7eff8b 100644 --- a/tests/lib/AllConfigTest.php +++ b/tests/lib/AllConfigTest.php @@ -15,6 +15,8 @@ namespace Test; * * @package Test */ + +use OC\SystemConfig; use OCP\IDBConnection; class AllConfigTest extends \Test\TestCase { @@ -145,7 +147,7 @@ class AllConfigTest extends \Test\TestCase { $config->setUserValue('userSetBool', 'appSetBool', 'keySetBool', $value); } - + public function testSetUserValueWithPreConditionFailure() { $this->expectException(\OCP\PreConditionNotMetException::class); @@ -437,4 +439,22 @@ class AllConfigTest extends \Test\TestCase { // cleanup $this->connection->executeUpdate('DELETE FROM `*PREFIX*preferences`'); } + + public function testGetUsersForUserValueCaseInsensitive() { + // mock the check for the database to run the correct SQL statements for each database type + $systemConfig = $this->createMock(SystemConfig::class); + $systemConfig->expects($this->once()) + ->method('getValue') + ->with($this->equalTo('dbtype'), $this->equalTo('sqlite')) + ->willReturn(\OC::$server->getConfig()->getSystemValue('dbtype', 'sqlite')); + $config = $this->getConfig($systemConfig); + + $config->setUserValue('user1', 'myApp', 'myKey', 'test123'); + $config->setUserValue('user2', 'myApp', 'myKey', 'TEST123'); + $config->setUserValue('user3', 'myApp', 'myKey', 'test12345'); + + $users = $config->getUsersForUserValueCaseInsensitive('myApp', 'myKey', 'test123'); + $this->assertSame(2, count($users)); + $this->assertSame(['user1', 'user2'], $users); + } } diff --git a/tests/lib/Security/VerificationToken/VerificationTokenTest.php b/tests/lib/Security/VerificationToken/VerificationTokenTest.php new file mode 100644 index 00000000000..4d90e304ab7 --- /dev/null +++ b/tests/lib/Security/VerificationToken/VerificationTokenTest.php @@ -0,0 +1,309 @@ +<?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 Test\Security\VerificationToken; + +use OC\Security\VerificationToken\VerificationToken; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJobList; +use OCP\IConfig; +use OCP\IUser; +use OCP\Security\ICrypto; +use OCP\Security\ISecureRandom; +use OCP\Security\VerificationToken\InvalidTokenException; +use Test\TestCase; + +class VerificationTokenTest extends TestCase { + /** @var VerificationToken */ + protected $token; + /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ + protected $config; + /** @var ISecureRandom|\PHPUnit\Framework\MockObject\MockObject */ + protected $secureRandom; + /** @var ICrypto|\PHPUnit\Framework\MockObject\MockObject */ + protected $crypto; + /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ + protected $timeFactory; + + protected function setUp(): void { + parent::setUp(); + + $this->config = $this->createMock(IConfig::class); + $this->crypto = $this->createMock(ICrypto::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); + $this->secureRandom = $this->createMock(ISecureRandom::class); + $this->jobList = $this->createMock(IJobList::class); + + $this->token = new VerificationToken( + $this->config, + $this->crypto, + $this->timeFactory, + $this->secureRandom, + $this->jobList + ); + } + + public function testTokenUserUnknown() { + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::USER_UNKNOWN); + $this->token->check('encryptedToken', null, 'fingerprintToken', 'foobar'); + } + + public function testTokenUserUnknown2() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(false); + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::USER_UNKNOWN); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar'); + } + + public function testTokenNotFound() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + + // implicit: IConfig::getUserValue returns null by default + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::TOKEN_NOT_FOUND); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar'); + } + + public function testTokenDecryptionError() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with('alice', 'core', 'fingerprintToken', null) + ->willReturn('encryptedToken'); + $this->config->expects($this->any()) + ->method('getSystemValue') + ->with('secret') + ->willReturn('357111317'); + + $this->crypto->method('decrypt') + ->with('encryptedToken', 'foobar' . '357111317') + ->willThrowException(new \Exception('decryption failed')); + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::TOKEN_DECRYPTION_ERROR); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar'); + } + + public function testTokenInvalidFormat() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with('alice', 'core', 'fingerprintToken', null) + ->willReturn('encryptedToken'); + $this->config->expects($this->any()) + ->method('getSystemValue') + ->with('secret') + ->willReturn('357111317'); + + $this->crypto->method('decrypt') + ->with('encryptedToken', 'foobar' . '357111317') + ->willReturn('decrypted^nonsense'); + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::TOKEN_INVALID_FORMAT); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar'); + } + + public function testTokenExpired() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + $user->expects($this->any()) + ->method('getLastLogin') + ->willReturn(604803); + + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with('alice', 'core', 'fingerprintToken', null) + ->willReturn('encryptedToken'); + $this->config->expects($this->any()) + ->method('getSystemValue') + ->with('secret') + ->willReturn('357111317'); + + $this->crypto->method('decrypt') + ->with('encryptedToken', 'foobar' . '357111317') + ->willReturn('604800:mY70K3n'); + + $this->timeFactory->expects($this->any()) + ->method('getTime') + ->willReturn(604800 * 3); + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::TOKEN_EXPIRED); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar'); + } + + public function testTokenExpiredByLogin() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + $user->expects($this->any()) + ->method('getLastLogin') + ->willReturn(604803); + + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with('alice', 'core', 'fingerprintToken', null) + ->willReturn('encryptedToken'); + $this->config->expects($this->any()) + ->method('getSystemValue') + ->with('secret') + ->willReturn('357111317'); + + $this->crypto->method('decrypt') + ->with('encryptedToken', 'foobar' . '357111317') + ->willReturn('604800:mY70K3n'); + + $this->timeFactory->expects($this->any()) + ->method('getTime') + ->willReturn(604801); + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::TOKEN_EXPIRED); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar', true); + } + + public function testTokenMismatch() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + $user->expects($this->any()) + ->method('getLastLogin') + ->willReturn(604703); + + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with('alice', 'core', 'fingerprintToken', null) + ->willReturn('encryptedToken'); + $this->config->expects($this->any()) + ->method('getSystemValue') + ->with('secret') + ->willReturn('357111317'); + + $this->crypto->method('decrypt') + ->with('encryptedToken', 'foobar' . '357111317') + ->willReturn('604802:mY70K3n'); + + $this->timeFactory->expects($this->any()) + ->method('getTime') + ->willReturn(604801); + + $this->expectException(InvalidTokenException::class); + $this->expectExceptionCode(InvalidTokenException::TOKEN_MISMATCH); + $this->token->check('encryptedToken', $user, 'fingerprintToken', 'foobar'); + } + + public function testTokenSuccess() { + $user = $this->createMock(IUser::class); + $user->expects($this->atLeastOnce()) + ->method('isEnabled') + ->willReturn(true); + $user->expects($this->atLeastOnce()) + ->method('getUID') + ->willReturn('alice'); + $user->expects($this->any()) + ->method('getLastLogin') + ->willReturn(604703); + + $this->config->expects($this->atLeastOnce()) + ->method('getUserValue') + ->with('alice', 'core', 'fingerprintToken', null) + ->willReturn('encryptedToken'); + $this->config->expects($this->any()) + ->method('getSystemValue') + ->with('secret') + ->willReturn('357111317'); + + $this->crypto->method('decrypt') + ->with('encryptedToken', 'foobar' . '357111317') + ->willReturn('604802:barfoo'); + + $this->timeFactory->expects($this->any()) + ->method('getTime') + ->willReturn(604801); + + $this->token->check('barfoo', $user, 'fingerprintToken', 'foobar'); + } + + public function testCreate() { + $user = $this->createMock(IUser::class); + $user->expects($this->any()) + ->method('getUID') + ->willReturn('alice'); + + $this->secureRandom->expects($this->atLeastOnce()) + ->method('generate') + ->willReturn('barfoo'); + $this->crypto->expects($this->atLeastOnce()) + ->method('encrypt') + ->willReturn('encryptedToken'); + $this->config->expects($this->atLeastOnce()) + ->method('setUserValue') + ->with('alice', 'core', 'fingerprintToken', 'encryptedToken'); + + $vToken = $this->token->create($user, 'fingerprintToken', 'foobar'); + $this->assertSame('barfoo', $vToken); + } +} diff --git a/tests/lib/User/UserTest.php b/tests/lib/User/UserTest.php index 2366bf45321..ad8b01555ea 100644 --- a/tests/lib/User/UserTest.php +++ b/tests/lib/User/UserTest.php @@ -676,11 +676,14 @@ class UserTest extends TestCase { $emitter->expects($this->never()) ->method('emit'); + $this->dispatcher->expects($this->never()) + ->method('dispatch'); + $config = $this->createMock(IConfig::class); $config->expects($this->any()) ->method('getUserValue') ->willReturn('foo@bar.com'); - $config->expects($this->never()) + $config->expects($this->any()) ->method('setUserValue'); $user = new User('foo', $backend, $this->dispatcher, $emitter, $config); |