open-dsi
/
nextcloud-server
mirror of https://github.com/nextcloud/server
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

adjust verification state updater method

Browse Source 
- also fixes scope of internal methods

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
pull/27474/head
Arthur Schiwon 4 years ago
parent af3fdbea7b
commit 3d6ffd3da2
No known key found for this signature in database
GPG Key ID: 7424F1874854DF23
2 changed files with 38 additions and 80 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 101
      lib/private/Accounts/AccountManager.php
  2. 15
      tests/lib/Accounts/AccountManagerTest.php

101
lib/private/Accounts/AccountManager.php
Unescape View File

@ -223,24 +223,12 @@ class AccountManager implements IAccountManager {
}
}
/**
* update user record
*
* @param IUser $user
* @param array $data
* @param bool $throwOnData Set to true if you can inform the user about invalid data
* @return array The potentially modified data (e.g. phone numbers are converted to E.164 format)
* @throws InvalidArgumentException Message is the property that was invalid
*/
public function updateUser(IUser $user, array $data, bool $throwOnData = false): array {
$userData = $this->getUser($user);
protected function updateUser(IUser $user, array $data, bool $throwOnData = false): array {
$userData = $this->getUser($user, false);
$updated = true;
if (empty($userData)) {
$this->insertNewUser($user, $data);
} elseif ($userData !== $data) {
if ($userData !== $data) {
$data = $this->checkEmailVerification($userData, $data, $user);
$data = $this->updateVerifyStatus($userData, $data);
$this->updateExistingUser($user, $data);
} else {
// nothing needs to be done if new and old data set are the same
@ -287,10 +275,8 @@ class AccountManager implements IAccountManager {
/**
* get stored data from a given user
*
* @deprecated use getAccount instead to make sure migrated properties work correctly
*/
public function getUser(IUser $user, bool $insertIfNotExists = true): array {
protected function getUser(IUser $user, bool $insertIfNotExists = true): array {
$uid = $user->getUID();
$query = $this->connection->getQueryBuilder();
$query->select('data')
@ -362,6 +348,7 @@ class AccountManager implements IAccountManager {
*/
protected function checkEmailVerification($oldData, $newData, IUser $user): array {
if ($oldData[self::PROPERTY_EMAIL]['value'] !== $newData[self::PROPERTY_EMAIL]['value']) {
$this->jobList->add(VerifyUserData::class,
[
'verificationCode' => '',
@ -373,6 +360,7 @@ class AccountManager implements IAccountManager {
]
);
$newData[self::PROPERTY_EMAIL]['verified'] = self::VERIFICATION_IN_PROGRESS;
}
return $newData;
@ -404,59 +392,31 @@ class AccountManager implements IAccountManager {
return $userData;
}
/**
* reset verification status if personal data changed
*
* @param array $oldData
* @param array $newData
* @return array
*/
protected function updateVerifyStatus(array $oldData, array $newData): array {
// which account was already verified successfully?
$twitterVerified = isset($oldData[self::PROPERTY_TWITTER]['verified']) && $oldData[self::PROPERTY_TWITTER]['verified'] === self::VERIFIED;
$websiteVerified = isset($oldData[self::PROPERTY_WEBSITE]['verified']) && $oldData[self::PROPERTY_WEBSITE]['verified'] === self::VERIFIED;
$emailVerified = isset($oldData[self::PROPERTY_EMAIL]['verified']) && $oldData[self::PROPERTY_EMAIL]['verified'] === self::VERIFIED;
// keep old verification status if we don't have a new one
if (!isset($newData[self::PROPERTY_TWITTER]['verified'])) {
// keep old verification status if value didn't changed and an old value exists
$keepOldStatus = $newData[self::PROPERTY_TWITTER]['value'] === $oldData[self::PROPERTY_TWITTER]['value'] && isset($oldData[self::PROPERTY_TWITTER]['verified']);
$newData[self::PROPERTY_TWITTER]['verified'] = $keepOldStatus ? $oldData[self::PROPERTY_TWITTER]['verified'] : self::NOT_VERIFIED;
}
if (!isset($newData[self::PROPERTY_WEBSITE]['verified'])) {
// keep old verification status if value didn't changed and an old value exists
$keepOldStatus = $newData[self::PROPERTY_WEBSITE]['value'] === $oldData[self::PROPERTY_WEBSITE]['value'] && isset($oldData[self::PROPERTY_WEBSITE]['verified']);
$newData[self::PROPERTY_WEBSITE]['verified'] = $keepOldStatus ? $oldData[self::PROPERTY_WEBSITE]['verified'] : self::NOT_VERIFIED;
}
if (!isset($newData[self::PROPERTY_EMAIL]['verified'])) {
// keep old verification status if value didn't changed and an old value exists
$keepOldStatus = $newData[self::PROPERTY_EMAIL]['value'] === $oldData[self::PROPERTY_EMAIL]['value'] && isset($oldData[self::PROPERTY_EMAIL]['verified']);
$newData[self::PROPERTY_EMAIL]['verified'] = $keepOldStatus ? $oldData[self::PROPERTY_EMAIL]['verified'] : self::VERIFICATION_IN_PROGRESS;
}
// reset verification status if a value from a previously verified data was changed
if ($twitterVerified &&
$oldData[self::PROPERTY_TWITTER]['value'] !== $newData[self::PROPERTY_TWITTER]['value']
) {
$newData[self::PROPERTY_TWITTER]['verified'] = self::NOT_VERIFIED;
}
if ($websiteVerified &&
$oldData[self::PROPERTY_WEBSITE]['value'] !== $newData[self::PROPERTY_WEBSITE]['value']
) {
$newData[self::PROPERTY_WEBSITE]['verified'] = self::NOT_VERIFIED;
}
protected function updateVerificationStatus(IAccount $updatedAccount, array $oldData): void {
static $propertiesVerifiableByLookupServer = [
self::PROPERTY_TWITTER,
self::PROPERTY_WEBSITE,
self::PROPERTY_EMAIL,
];
if ($emailVerified &&
$oldData[self::PROPERTY_EMAIL]['value'] !== $newData[self::PROPERTY_EMAIL]['value']
) {
$newData[self::PROPERTY_EMAIL]['verified'] = self::NOT_VERIFIED;
foreach ($propertiesVerifiableByLookupServer as $propertyName) {
try {
$property = $updatedAccount->getProperty($propertyName);
} catch (PropertyDoesNotExistException $e) {
continue;
}
$wasVerified = isset($oldData[$propertyName])
&& isset($oldData[$propertyName]['verified'])
&& $oldData[$propertyName]['verified'] === self::VERIFIED;
if ((!isset($oldData[$propertyName])
|| !isset($oldData[$propertyName]['value'])
|| $property->getValue() !== $oldData[$propertyName]['value'])
&& ($property->getVerified() !== self::NOT_VERIFIED
|| $wasVerified)
) {
$property->setVerified(self::NOT_VERIFIED);
}
}
return $newData;
}
protected function dataArrayToJson(array $accountData): string {
@ -679,6 +639,9 @@ class AccountManager implements IAccountManager {
$this->testPropertyScope($property, $allowedScopes, true);
}
$this->updateVerificationStatus($account, $this->getUser($account->getUser(), false));
$this->updateUser($account->getUser(), $data, true);
}
}

15
tests/lib/Accounts/AccountManagerTest.php
Unescape View File

@ -246,7 +246,7 @@ class AccountManagerTest extends TestCase {
],
];
foreach ($users as $userInfo) {
$this->accountManager->updateUser($userInfo['user'], $userInfo['data'], false);
$this->invokePrivate($this->accountManager, 'updateUser', [$userInfo['user'], $userInfo['data'], false]);
}
}
@ -278,7 +278,7 @@ class AccountManagerTest extends TestCase {
* @param bool $updateExisting
*/
public function testUpdateUser($newData, $oldData, $insertNew, $updateExisting) {
$accountManager = $this->getInstance(['getUser', 'insertNewUser', 'updateExistingUser', 'updateVerifyStatus', 'checkEmailVerification']);
$accountManager = $this->getInstance(['getUser', 'insertNewUser', 'updateExistingUser', 'checkEmailVerification']);
/** @var IUser $user */
$user = $this->createMock(IUser::class);
@ -288,8 +288,6 @@ class AccountManagerTest extends TestCase {
if ($updateExisting) {
$accountManager->expects($this->once())->method('checkEmailVerification')
->with($oldData, $newData, $user)->willReturn($newData);
$accountManager->expects($this->once())->method('updateVerifyStatus')
->with($oldData, $newData)->willReturn($newData);
$accountManager->expects($this->once())->method('updateExistingUser')
->with($user, $newData);
$accountManager->expects($this->never())->method('insertNewUser');
@ -303,7 +301,6 @@ class AccountManagerTest extends TestCase {
if (!$insertNew && !$updateExisting) {
$accountManager->expects($this->never())->method('updateExistingUser');
$accountManager->expects($this->never())->method('checkEmailVerification');
$accountManager->expects($this->never())->method('updateVerifyStatus');
$accountManager->expects($this->never())->method('insertNewUser');
$this->eventDispatcher->expects($this->never())->method('dispatch');
} else {
@ -319,13 +316,13 @@ class AccountManagerTest extends TestCase {
);
}
$accountManager->updateUser($user, $newData);
$this->invokePrivate($accountManager, 'updateUser', [$user, $newData]);
}
public function dataTrueFalse() {
return [
#$newData | $oldData | $insertNew | $updateExisting
[['myProperty' => ['value' => 'newData']], ['myProperty' => ['value' => 'oldData']], false, true],
[['myProperty' => ['value' => 'newData']], [], true, false],
[['myProperty' => ['value' => 'oldData']], ['myProperty' => ['value' => 'oldData']], false, false]
];
}
@ -356,9 +353,7 @@ class AccountManagerTest extends TestCase {
}
$this->addDummyValuesToTable($setUser, $setData);
$this->assertEquals($expectedData,
$accountManager->getUser($askUser)
);
$this->assertEquals($expectedData, $this->invokePrivate($accountManager, 'getUser', [$askUser]));
}
public function dataTestGetUser() {

Write Preview
Loading…
Cancel
Save