From cbf5acca45388a44e22410abcd25a19d86fa68ee Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 21 Apr 2017 12:09:42 +0200 Subject: [PATCH] check verification proof and update account table Signed-off-by: Bjoern Schiessle --- settings/BackgroundJobs/VerifyUserData.php | 275 ++++++++++++++++++ settings/Controller/UsersController.php | 62 +++- .../Controller/UsersControllerTest.php | 106 ++++++- 3 files changed, 433 insertions(+), 10 deletions(-) create mode 100644 settings/BackgroundJobs/VerifyUserData.php diff --git a/settings/BackgroundJobs/VerifyUserData.php b/settings/BackgroundJobs/VerifyUserData.php new file mode 100644 index 00000000000..f19f622c97a --- /dev/null +++ b/settings/BackgroundJobs/VerifyUserData.php @@ -0,0 +1,275 @@ + + * + * @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 . + * + */ + + +namespace OC\Settings\BackgroundJobs; + + +use OC\Accounts\AccountManager; +use OC\BackgroundJob\Job; +use OC\BackgroundJob\JobList; +use OCP\AppFramework\Http; +use OCP\Http\Client\IClientService; +use OCP\IConfig; +use OCP\ILogger; +use OCP\IUserManager; + +class VerifyUserData extends Job { + + /** @var bool */ + private $retainJob = true; + + /** @var int max number of attempts to send the request */ + private $maxTry = 24; + + /** @var int how much time should be between two tries (1 hour) */ + private $interval = 3600; + + /** @var AccountManager */ + private $accountManager; + + /** @var IUserManager */ + private $userManager; + + /** @var IClientService */ + private $httpClientService; + + /** @var ILogger */ + private $logger; + + /** @var string */ + private $lookupServerUrl; + + /** + * VerifyUserData constructor. + * + * @param AccountManager|null $accountManager + * @param IUserManager|null $userManager + * @param IClientService|null $clientService + * @param IConfig|null $config + */ + public function __construct(AccountManager $accountManager = null, + IUserManager $userManager = null, + IClientService $clientService = null, + ILogger $logger = null, + IConfig $config = null + ) { + $this->accountManager = $accountManager !== null ? $accountManager : \OC::$server->query(AccountManager::class); + $this->userManager = $userManager !== null ? $userManager : \OC::$server->getUserManager(); + $this->httpClientService = $clientService !== null ? $clientService : \OC::$server->getHTTPClientService(); + $this->logger = $logger !== null ? $logger : \OC::$server->getLogger(); + + if ($config !== null) { + $this->lookupServerUrl = $config->getSystemValue('lookup_server', 'https://lookup.nextcloud.com'); + } else { + $this->lookupServerUrl = \OC::$server->getConfig()->getSystemValue('lookup_server', 'https://lookup.nextcloud.com'); + } + } + + /** + * run the job, then remove it from the jobList + * + * @param JobList $jobList + * @param ILogger $logger + */ + public function execute($jobList, ILogger $logger = null) { + + if ($this->shouldRun($this->argument)) { + parent::execute($jobList, $logger); + $jobList->remove($this, $this->argument); + if ($this->retainJob) { + $this->reAddJob($jobList, $this->argument); + } + } + + } + + protected function run($argument) { + + $try = (int)$argument['try'] + 1; + + switch($argument['type']) { + case AccountManager::PROPERTY_WEBSITE: + $result = $this->verifyWebsite($argument); + break; + case AccountManager::PROPERTY_TWITTER: + case AccountManager::PROPERTY_EMAIL: + $result = $this->verifyViaLookupServer($argument, $argument['type']); + break; + default: + // no valid type given, no need to retry + $this->logger->error($argument['type'] . ' is no valid type for user account data.'); + $result = true; + } + + if ($result === true || $try > $this->maxTry) { + $this->retainJob = false; + } + } + + /** + * verify web page + * + * @param array $argument + * @return bool true if we could check the verification code, otherwise false + */ + protected function verifyWebsite(array $argument) { + + $result = false; + + $url = rtrim($argument['data'], '/') . '/' . 'CloudIdVerificationCode.txt'; + + $client = $this->httpClientService->newClient(); + try { + $response = $client->get($url); + } catch (\Exception $e) { + return false; + } + + if ($response->getStatusCode() === Http::STATUS_OK) { + $result = true; + $publishedCode = $response->getBody(); + $user = $this->userManager->get($argument['uid']); + // we don't check a valid user -> give up + if ($user === null) { + $this->logger->error($argument['uid'] . ' doesn\'t exist, can\'t verify user data.'); + return $result; + } + $userData = $this->accountManager->getUser($user); + + if ($publishedCode === $argument['verificationCode']) { + + $userData[AccountManager::PROPERTY_WEBSITE]['verified'] === AccountManager::VERIFIED; + } else { + $userData[AccountManager::PROPERTY_WEBSITE]['verified'] === AccountManager::NOT_VERIFIED; + } + + $this->accountManager->updateUser($user, $userData); + } + + return $result; + } + + /** + * verify email address + * + * @param array $argument + * @param string $dataType + * @return bool true if we could check the verification code, otherwise false + */ + protected function verifyViaLookupServer(array $argument, $dataType) { + + $user = $this->userManager->get($argument['uid']); + + // we don't check a valid user -> give up + if ($user === null) { + $this->logger->error($argument['uid'] . ' doesn\'t exist, can\'t verify user data.'); + return true; + } + + $localUserData = $this->accountManager->getUser($user); + $cloudId = $user->getCloudId(); + + // ask lookup-server for user data + $lookupServerData = $this->queryLookupServer($cloudId); + + // for some reasons we couldn't read any data from the lookup server, try again later + if (empty($lookupServerData)) { + return false; + } + + // lookup server has verification data for wrong user data (e.g. email address), try again later + if ($lookupServerData[$dataType]['value'] !== $argument['data']) { + return false; + } + + // lookup server hasn't verified the email address so far, try again later + if ($lookupServerData[$dataType]['verified'] === AccountManager::VERIFICATION_IN_PROGRESS) { + return false; + } + + $localUserData[$dataType]['verified'] === $lookupServerData[$dataType]['verified']; + $this->accountManager->updateUser($user, $localUserData); + + return true; + } + + /** + * @param string $cloudId + * @return array + */ + protected function queryLookupServer($cloudId) { + try { + $client = $this->clientService->newClient(); + $response = $client->get( + $this->lookupServerUrl . '/users?search=' . urlencode($cloudId), + [ + 'timeout' => 10, + 'connect_timeout' => 3, + ] + ); + + $body = json_decode($response->getBody(), true); + + foreach ($body as $lookup) { + if ($lookup['federationId'] === $cloudId) { + return $lookup; + } + } + + } catch (\Exception $e) { + // do nothing, we will just re-try later + } + + return []; + } + + /** + * re-add background job with new arguments + * + * @param IJobList $jobList + * @param array $argument + */ + protected function reAddJob(IJobList $jobList, array $argument) { + $jobList->add('OC\Settings\BackgroundJobs\VerifyUserData', + [ + 'verificationCode' => $argument['verificationCode'], + 'data' => $argument['data'], + 'type' => $argument['type'], + 'uid' => $argument['uid'], + 'try' => (int)$argument['try'] + 1, + 'lastRun' => time() + ] + ); + } + + /** + * test if it is time for the next run + * + * @param array $argument + * @return bool + */ + protected function shouldRun(array $argument) { + $lastRun = (int)$argument['lastRun']; + return ((time() - $lastRun) > $this->interval); + } + +} diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php index e80c4956bcb..41433ea8363 100644 --- a/settings/Controller/UsersController.php +++ b/settings/Controller/UsersController.php @@ -38,6 +38,8 @@ use OC\Security\IdentityProof\Manager; use OCP\App\IAppManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJobList; use OCP\IConfig; use OCP\IGroupManager; use OCP\IL10N; @@ -49,6 +51,7 @@ use OCP\IUserManager; use OCP\IUserSession; use OCP\Mail\IMailer; use OCP\IAvatarManager; +use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; /** @@ -89,7 +92,8 @@ class UsersController extends Controller { private $crypto; /** @var Manager */ private $keyManager; - + /** @var IJobList */ + private $jobList; /** * @param string $appName @@ -111,6 +115,7 @@ class UsersController extends Controller { * @param ITimeFactory $timeFactory * @param ICrypto $crypto * @param Manager $keyManager + * @param IJobList $jobList */ public function __construct($appName, IRequest $request, @@ -130,7 +135,8 @@ class UsersController extends Controller { NewUserMailHelper $newUserMailHelper, ITimeFactory $timeFactory, ICrypto $crypto, - Manager $keyManager) { + Manager $keyManager, + IJobList $jobList) { parent::__construct($appName, $request); $this->userManager = $userManager; $this->groupManager = $groupManager; @@ -147,6 +153,7 @@ class UsersController extends Controller { $this->timeFactory = $timeFactory; $this->crypto = $crypto; $this->keyManager = $keyManager; + $this->jobList = $jobList; // check for encryption state - TODO see formatUserForIndex $this->isEncryptionAppEnabled = $appManager->isEnabledForUser('encryption'); @@ -524,22 +531,24 @@ class UsersController extends Controller { $accountData = $this->accountManager->getUser($user); $cloudId = $user->getCloudId(); $message = "Use my Federated Cloud ID to share with me: " . $cloudId; - $privateKey = $this->keyManager->getKey($user)->getPrivate(); - openssl_sign(json_encode($message), $signature, $privateKey, OPENSSL_ALGO_SHA512); - $signatureBase64 = base64_encode($signature); + $signature = $this->signMessage($user, $message); - $code = $message . ' ' . $signatureBase64; - $codeMd5 = $message . ' ' . md5($signatureBase64); + $code = $message . ' ' . $signature; + $codeMd5 = $message . ' ' . md5($signature); switch ($account) { case 'verify-twitter': $accountData[AccountManager::PROPERTY_TWITTER]['verified'] = AccountManager::VERIFICATION_IN_PROGRESS; $msg = $this->l10n->t('In order to verify your Twitter account post following tweet on Twitter:'); $code = $codeMd5; + $type = AccountManager::PROPERTY_TWITTER; + $data = $accountData[AccountManager::PROPERTY_TWITTER]['value']; break; case 'verify-website': $accountData[AccountManager::PROPERTY_WEBSITE]['verified'] = AccountManager::VERIFICATION_IN_PROGRESS; - $msg = $this->l10n->t('In order to verify your Website store following content in your webroot at \'CloudIdVerificationCode.txt\':'); + $msg = $this->l10n->t('In order to verify your Website store following content in your web-root at \'CloudIdVerificationCode.txt\':'); + $type = AccountManager::PROPERTY_WEBSITE; + $data = $accountData[AccountManager::PROPERTY_WEBSITE]['value']; break; default: return new DataResponse([], Http::STATUS_BAD_REQUEST); @@ -547,9 +556,46 @@ class UsersController extends Controller { $this->accountManager->updateUser($user, $accountData); + + $this->jobList->add('OC\Settings\BackgroundJobs\VerifyUserData', + [ + 'verificationCode' => $code, + 'data' => $data, + 'type' => $type, + 'uid' => $user->getUID(), + 'try' => 0, + 'lastRun' => $this->getCurrentTime() + ] + ); + return new DataResponse(['msg' => $msg, 'code' => $code]); } + /** + * get current timestamp + * + * @return int + */ + protected function getCurrentTime() { + return time(); + } + + /** + * sign message with users private key + * + * @param IUser $user + * @param string $message + * + * @return string base64 encoded signature + */ + protected function signMessage(IUser $user, $message) { + $privateKey = $this->keyManager->getKey($user)->getPrivate(); + openssl_sign(json_encode($message), $signature, $privateKey, OPENSSL_ALGO_SHA512); + $signatureBase64 = base64_encode($signature); + + return $signatureBase64; + } + /** * @NoAdminRequired * @NoSubadminRequired diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php index d659d812b0d..918f3ae694d 100644 --- a/tests/Settings/Controller/UsersControllerTest.php +++ b/tests/Settings/Controller/UsersControllerTest.php @@ -18,6 +18,7 @@ use OCP\App\IAppManager; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Utility\ITimeFactory; +use OCP\BackgroundJob\IJobList; use OCP\IAvatar; use OCP\IAvatarManager; use OCP\IConfig; @@ -74,6 +75,10 @@ class UsersControllerTest extends \Test\TestCase { private $newUserMailHelper; /** @var ICrypto | \PHPUnit_Framework_MockObject_MockObject */ private $crypto; + /** @var IJobList | \PHPUnit_Framework_MockObject_MockObject */ + private $jobList; + /** @var \OC\Security\IdentityProof\Manager |\PHPUnit_Framework_MockObject_MockObject */ + private $securityManager; protected function setUp() { parent::setUp(); @@ -92,6 +97,10 @@ class UsersControllerTest extends \Test\TestCase { $this->timeFactory = $this->createMock(ITimeFactory::class); $this->crypto = $this->createMock(ICrypto::class); $this->newUserMailHelper = $this->createMock(NewUserMailHelper::class); + $this->timeFactory = $this->getMock(ITimeFactory::class); + $this->crypto = $this->getMock(ICrypto::class); + $this->securityManager = $this->getMockBuilder(\OC\Security\IdentityProof\Manager::class)->disableOriginalConstructor()->getMock(); + $this->jobList = $this->createMock(IJobList::class); $this->l = $this->createMock(IL10N::class); $this->l->method('t') ->will($this->returnCallback(function ($text, $parameters = []) { @@ -136,7 +145,12 @@ class UsersControllerTest extends \Test\TestCase { $this->avatarManager, $this->accountManager, $this->secureRandom, - $this->newUserMailHelper + $this->newUserMailHelper, + $this->timeFactory, + $this->crypto, + $this->securityManager, + $this->jobList + ); } else { return $this->getMockBuilder(UsersController::class) @@ -157,7 +171,11 @@ class UsersControllerTest extends \Test\TestCase { $this->avatarManager, $this->accountManager, $this->secureRandom, - $this->newUserMailHelper + $this->newUserMailHelper, + $this->timeFactory, + $this->crypto, + $this->securityManager, + $this->jobList ] )->setMethods($mockedMethods)->getMock(); } @@ -2267,4 +2285,88 @@ class UsersControllerTest extends \Test\TestCase { $response = $controller->create('foo', '', array(), 'abc@example.org'); $this->assertEquals($expectedResponse, $response); } + + /** + * @param string $account + * @param string $type + * @param array $dataBefore + * @param array $expectedData + * + * @dataProvider dataTestGetVerificationCode + */ + public function testGetVerificationCode($account, $type, $dataBefore, $expectedData) { + + $message = 'Use my Federated Cloud ID to share with me: user@nextcloud.com'; + $signature = 'theSignature'; + + $code = $message . ' ' . $signature; + if($type === AccountManager::PROPERTY_TWITTER) { + $code = $message . ' ' . md5($signature); + } + + $controller = $this->getController(false, ['signMessage', 'getCurrentTime']); + + $user = $this->getMock(IUser::class); + $this->userSession->expects($this->once())->method('getUser')->willReturn($user); + $this->accountManager->expects($this->once())->method('getUser')->with($user)->willReturn($dataBefore); + $user->expects($this->any())->method('getCloudId')->willReturn('user@nextcloud.com'); + $user->expects($this->any())->method('getUID')->willReturn('uid'); + $controller->expects($this->once())->method('signMessage')->with($user, $message)->willReturn($signature); + $controller->expects($this->once())->method('getCurrentTime')->willReturn(1234567); + + $this->accountManager->expects($this->once())->method('updateUser')->with($user, $expectedData); + $this->jobList->expects($this->once())->method('add') + ->with('OC\Settings\BackgroundJobs\VerifyUserData', + [ + 'verificationCode' => $code, + 'data' => $dataBefore[$type]['value'], + 'type' => $type, + 'uid' => 'uid', + 'try' => 0, + 'lastRun' => 1234567 + ]); + + + $result = $controller->getVerificationCode($account); + + $data = $result->getData(); + $this->assertSame(Http::STATUS_OK, $result->getStatus()); + $this->assertSame($code, $data['code']); + } + + public function dataTestGetVerificationCode() { + + $accountDataBefore = [ + AccountManager::PROPERTY_WEBSITE => ['value' => 'https://nextcloud.com', 'verified' => AccountManager::NOT_VERIFIED], + AccountManager::PROPERTY_TWITTER => ['value' => '@nextclouders', 'verified' => AccountManager::NOT_VERIFIED], + ]; + + $accountDataAfterWebsite = [ + AccountManager::PROPERTY_WEBSITE => ['value' => 'https://nextcloud.com', 'verified' => AccountManager::VERIFICATION_IN_PROGRESS], + AccountManager::PROPERTY_TWITTER => ['value' => '@nextclouders', 'verified' => AccountManager::NOT_VERIFIED], + ]; + + $accountDataAfterTwitter = [ + AccountManager::PROPERTY_WEBSITE => ['value' => 'https://nextcloud.com', 'verified' => AccountManager::NOT_VERIFIED], + AccountManager::PROPERTY_TWITTER => ['value' => '@nextclouders', 'verified' => AccountManager::VERIFICATION_IN_PROGRESS], + ]; + + return [ + ['verify-twitter', AccountManager::PROPERTY_TWITTER, $accountDataBefore, $accountDataAfterTwitter], + ['verify-website', AccountManager::PROPERTY_WEBSITE, $accountDataBefore, $accountDataAfterWebsite], + ]; + } + + /** + * test get verification code in case no valid user was given + */ + public function testGetVerificationCodeInvalidUser() { + + $controller = $this->getController(); + $this->userSession->expects($this->once())->method('getUser')->willReturn(null); + $result = $controller->getVerificationCode('account'); + + $this->assertSame(Http::STATUS_BAD_REQUEST ,$result->getStatus()); + + } }