Merge pull request #5270 from nextcloud/stable12-5223
[stable12] Don't allow the user to set fields they can't see
This commit is contained in:
Björn Schießle
GitHub
commit
f29d53f509
|
|
@ -32,6 +32,7 @@ namespace OCA\Provisioning_API\Controller;
|
|||
use OC\Accounts\AccountManager;
|
||||
use OC\Settings\Mailer\NewUserMailHelper;
|
||||
use OC_Helper;
|
||||
use OCP\App\IAppManager;
|
||||
use OCP\AppFramework\Http\DataResponse;
|
||||
use OCP\AppFramework\OCS\OCSException;
|
||||
use OCP\AppFramework\OCS\OCSForbiddenException;
|
||||
|
|
@ -52,6 +53,8 @@ class UsersController extends OCSController {
|
|||
private $userManager;
|
||||
/** @var IConfig */
|
||||
private $config;
|
||||
/** @var IAppManager */
|
||||
private $appManager;
|
||||
/** @var IGroupManager|\OC\Group\Manager */ // FIXME Requires a method that is not on the interface
|
||||
private $groupManager;
|
||||
/** @var IUserSession */
|
||||
|
|
@ -70,6 +73,7 @@ class UsersController extends OCSController {
|
|||
* @param IRequest $request
|
||||
* @param IUserManager $userManager
|
||||
* @param IConfig $config
|
||||
* @param IAppManager $appManager
|
||||
* @param IGroupManager $groupManager
|
||||
* @param IUserSession $userSession
|
||||
* @param AccountManager $accountManager
|
||||
|
|
@ -81,6 +85,7 @@ class UsersController extends OCSController {
|
|||
IRequest $request,
|
||||
IUserManager $userManager,
|
||||
IConfig $config,
|
||||
IAppManager $appManager,
|
||||
IGroupManager $groupManager,
|
||||
IUserSession $userSession,
|
||||
AccountManager $accountManager,
|
||||
|
|
@ -91,6 +96,7 @@ class UsersController extends OCSController {
|
|||
|
||||
$this->userManager = $userManager;
|
||||
$this->config = $config;
|
||||
$this->appManager = $appManager;
|
||||
$this->groupManager = $groupManager;
|
||||
$this->userSession = $userSession;
|
||||
$this->accountManager = $accountManager;
|
||||
|
|
@ -309,14 +315,25 @@ class UsersController extends OCSController {
|
|||
$permittedFields = [];
|
||||
if($targetUser->getUID() === $currentLoggedInUser->getUID()) {
|
||||
// Editing self (display, email)
|
||||
$permittedFields[] = 'display';
|
||||
$permittedFields[] = AccountManager::PROPERTY_DISPLAYNAME;
|
||||
$permittedFields[] = AccountManager::PROPERTY_EMAIL;
|
||||
if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) {
|
||||
$permittedFields[] = 'display';
|
||||
$permittedFields[] = AccountManager::PROPERTY_DISPLAYNAME;
|
||||
$permittedFields[] = AccountManager::PROPERTY_EMAIL;
|
||||
}
|
||||
|
||||
$permittedFields[] = 'password';
|
||||
$permittedFields[] = AccountManager::PROPERTY_PHONE;
|
||||
$permittedFields[] = AccountManager::PROPERTY_ADDRESS;
|
||||
$permittedFields[] = AccountManager::PROPERTY_WEBSITE;
|
||||
$permittedFields[] = AccountManager::PROPERTY_TWITTER;
|
||||
|
||||
if ($this->appManager->isEnabledForUser('federatedfilesharing')) {
|
||||
$federatedFileSharing = new \OCA\FederatedFileSharing\AppInfo\Application();
|
||||
$shareProvider = $federatedFileSharing->getFederatedShareProvider();
|
||||
if ($shareProvider->isLookupServerUploadEnabled()) {
|
||||
$permittedFields[] = AccountManager::PROPERTY_PHONE;
|
||||
$permittedFields[] = AccountManager::PROPERTY_ADDRESS;
|
||||
$permittedFields[] = AccountManager::PROPERTY_WEBSITE;
|
||||
$permittedFields[] = AccountManager::PROPERTY_TWITTER;
|
||||
}
|
||||
}
|
||||
|
||||
// If admin they can edit their own quota
|
||||
if($this->groupManager->isAdmin($currentLoggedInUser->getUID())) {
|
||||
$permittedFields[] = 'quota';
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ namespace OCA\Provisioning_API\Tests\Controller;
|
|||
use Exception;
|
||||
use OC\Accounts\AccountManager;
|
||||
use OC\Group\Manager;
|
||||
use OCP\App\IAppManager;
|
||||
use OCP\Mail\IEMailTemplate;
|
||||
use OC\Settings\Mailer\NewUserMailHelper;
|
||||
use OC\SubAdmin;
|
||||
|
|
@ -58,6 +59,8 @@ class UsersControllerTest extends TestCase {
|
|||
protected $userManager;
|
||||
/** @var IConfig|PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $config;
|
||||
/** @var IAppManager|PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $appManager;
|
||||
/** @var Manager|PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $groupManager;
|
||||
/** @var IUserSession|PHPUnit_Framework_MockObject_MockObject */
|
||||
|
|
@ -66,9 +69,9 @@ class UsersControllerTest extends TestCase {
|
|||
protected $logger;
|
||||
/** @var UsersController|PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $api;
|
||||
/** @var AccountManager|PHPUnit_Framework_MockObject_MockObject */
|
||||
/** @var AccountManager|PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $accountManager;
|
||||
/** @var IRequest|PHPUnit_Framework_MockObject_MockObject */
|
||||
/** @var IRequest|PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $request;
|
||||
/** @var IFactory|PHPUnit_Framework_MockObject_MockObject */
|
||||
private $l10nFactory;
|
||||
|
|
@ -80,6 +83,7 @@ class UsersControllerTest extends TestCase {
|
|||
|
||||
$this->userManager = $this->createMock(IUserManager::class);
|
||||
$this->config = $this->createMock(IConfig::class);
|
||||
$this->appManager = $this->createMock(IAppManager::class);
|
||||
$this->groupManager = $this->createMock(Manager::class);
|
||||
$this->userSession = $this->createMock(IUserSession::class);
|
||||
$this->logger = $this->createMock(ILogger::class);
|
||||
|
|
@ -94,6 +98,7 @@ class UsersControllerTest extends TestCase {
|
|||
$this->request,
|
||||
$this->userManager,
|
||||
$this->config,
|
||||
$this->appManager,
|
||||
$this->groupManager,
|
||||
$this->userSession,
|
||||
$this->accountManager,
|
||||
|
|
@ -2647,6 +2652,7 @@ class UsersControllerTest extends TestCase {
|
|||
$this->request,
|
||||
$this->userManager,
|
||||
$this->config,
|
||||
$this->appManager,
|
||||
$this->groupManager,
|
||||
$this->userSession,
|
||||
$this->accountManager,
|
||||
|
|
@ -2707,6 +2713,7 @@ class UsersControllerTest extends TestCase {
|
|||
$this->request,
|
||||
$this->userManager,
|
||||
$this->config,
|
||||
$this->appManager,
|
||||
$this->groupManager,
|
||||
$this->userSession,
|
||||
$this->accountManager,
|
||||
|
|
|
|||
|
|
@ -78,6 +78,8 @@ class UsersController extends Controller {
|
|||
private $isEncryptionAppEnabled;
|
||||
/** @var bool contains the state of the admin recovery setting */
|
||||
private $isRestoreEnabled = false;
|
||||
/** @var IAppManager */
|
||||
private $appManager;
|
||||
/** @var IAvatarManager */
|
||||
private $avatarManager;
|
||||
/** @var AccountManager */
|
||||
|
|
@ -146,6 +148,7 @@ class UsersController extends Controller {
|
|||
$this->l10n = $l10n;
|
||||
$this->log = $log;
|
||||
$this->mailer = $mailer;
|
||||
$this->appManager = $appManager;
|
||||
$this->avatarManager = $avatarManager;
|
||||
$this->accountManager = $accountManager;
|
||||
$this->secureRandom = $secureRandom;
|
||||
|
|
@ -718,18 +721,27 @@ class UsersController extends Controller {
|
|||
);
|
||||
}
|
||||
|
||||
$data = [
|
||||
AccountManager::PROPERTY_AVATAR => ['scope' => $avatarScope],
|
||||
AccountManager::PROPERTY_DISPLAYNAME => ['value' => $displayname, 'scope' => $displaynameScope],
|
||||
AccountManager::PROPERTY_EMAIL=> ['value' => $email, 'scope' => $emailScope],
|
||||
AccountManager::PROPERTY_WEBSITE => ['value' => $website, 'scope' => $websiteScope],
|
||||
AccountManager::PROPERTY_ADDRESS => ['value' => $address, 'scope' => $addressScope],
|
||||
AccountManager::PROPERTY_PHONE => ['value' => $phone, 'scope' => $phoneScope],
|
||||
AccountManager::PROPERTY_TWITTER => ['value' => $twitter, 'scope' => $twitterScope]
|
||||
];
|
||||
|
||||
$user = $this->userSession->getUser();
|
||||
|
||||
$data = $this->accountManager->getUser($user);
|
||||
|
||||
$data[AccountManager::PROPERTY_AVATAR] = ['scope' => $avatarScope];
|
||||
if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) {
|
||||
$data[AccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope];
|
||||
$data[AccountManager::PROPERTY_EMAIL] = ['value' => $email, 'scope' => $emailScope];
|
||||
}
|
||||
|
||||
if ($this->appManager->isEnabledForUser('federatedfilesharing')) {
|
||||
$federatedFileSharing = new \OCA\FederatedFileSharing\AppInfo\Application();
|
||||
$shareProvider = $federatedFileSharing->getFederatedShareProvider();
|
||||
if ($shareProvider->isLookupServerUploadEnabled()) {
|
||||
$data[AccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope];
|
||||
$data[AccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope];
|
||||
$data[AccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope];
|
||||
$data[AccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope];
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
$this->saveUserSettings($user, $data);
|
||||
return new DataResponse(
|
||||
|
|
@ -737,15 +749,15 @@ class UsersController extends Controller {
|
|||
'status' => 'success',
|
||||
'data' => [
|
||||
'userId' => $user->getUID(),
|
||||
'avatarScope' => $avatarScope,
|
||||
'displayname' => $displayname,
|
||||
'displaynameScope' => $displaynameScope,
|
||||
'email' => $email,
|
||||
'emailScope' => $emailScope,
|
||||
'website' => $website,
|
||||
'websiteScope' => $websiteScope,
|
||||
'address' => $address,
|
||||
'addressScope' => $addressScope,
|
||||
'avatarScope' => $data[AccountManager::PROPERTY_AVATAR]['scope'],
|
||||
'displayname' => $data[AccountManager::PROPERTY_DISPLAYNAME]['value'],
|
||||
'displaynameScope' => $data[AccountManager::PROPERTY_DISPLAYNAME]['scope'],
|
||||
'email' => $data[AccountManager::PROPERTY_EMAIL]['value'],
|
||||
'emailScope' => $data[AccountManager::PROPERTY_EMAIL]['scope'],
|
||||
'website' => $data[AccountManager::PROPERTY_WEBSITE]['value'],
|
||||
'websiteScope' => $data[AccountManager::PROPERTY_WEBSITE]['scope'],
|
||||
'address' => $data[AccountManager::PROPERTY_ADDRESS]['value'],
|
||||
'addressScope' => $data[AccountManager::PROPERTY_ADDRESS]['scope'],
|
||||
'message' => (string) $this->l10n->t('Settings saved')
|
||||
]
|
||||
],
|
||||
|
|
|
|||
|
|
@ -2005,6 +2005,52 @@ class UsersControllerTest extends \Test\TestCase {
|
|||
$saveData = (!empty($email) && $validEmail) || empty($email);
|
||||
|
||||
if ($saveData) {
|
||||
$this->accountManager->expects($this->once())
|
||||
->method('getUser')
|
||||
->with($user)
|
||||
->willReturn([
|
||||
AccountManager::PROPERTY_DISPLAYNAME =>
|
||||
[
|
||||
'value' => 'Display name',
|
||||
'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY,
|
||||
'verified' => AccountManager::NOT_VERIFIED,
|
||||
],
|
||||
AccountManager::PROPERTY_ADDRESS =>
|
||||
[
|
||||
'value' => '',
|
||||
'scope' => AccountManager::VISIBILITY_PRIVATE,
|
||||
'verified' => AccountManager::NOT_VERIFIED,
|
||||
],
|
||||
AccountManager::PROPERTY_WEBSITE =>
|
||||
[
|
||||
'value' => '',
|
||||
'scope' => AccountManager::VISIBILITY_PRIVATE,
|
||||
'verified' => AccountManager::NOT_VERIFIED,
|
||||
],
|
||||
AccountManager::PROPERTY_EMAIL =>
|
||||
[
|
||||
'value' => '',
|
||||
'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY,
|
||||
'verified' => AccountManager::NOT_VERIFIED,
|
||||
],
|
||||
AccountManager::PROPERTY_AVATAR =>
|
||||
[
|
||||
'scope' => AccountManager::VISIBILITY_CONTACTS_ONLY
|
||||
],
|
||||
AccountManager::PROPERTY_PHONE =>
|
||||
[
|
||||
'value' => '',
|
||||
'scope' => AccountManager::VISIBILITY_PRIVATE,
|
||||
'verified' => AccountManager::NOT_VERIFIED,
|
||||
],
|
||||
AccountManager::PROPERTY_TWITTER =>
|
||||
[
|
||||
'value' => '',
|
||||
'scope' => AccountManager::VISIBILITY_PRIVATE,
|
||||
'verified' => AccountManager::NOT_VERIFIED,
|
||||
],
|
||||
]);
|
||||
|
||||
$controller->expects($this->once())->method('saveUserSettings');
|
||||
} else {
|
||||
$controller->expects($this->never())->method('saveUserSettings');
|
||||
|
|
|
|||
Loading…
Reference in New Issue