Make extra user profile fields always editable

The fields for phone number, address, website and twitter are now
editable regardless whether federated sharing and the lookup server
are enabled or not.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
This commit is contained in:
Vincent Petry 2021-03-23 16:59:05 +01:00
parent 278a73789e
commit 5d14fd4396
No known key found for this signature in database
GPG Key ID: E055D6A4D513575C
5 changed files with 19 additions and 82 deletions

View File

@ -50,7 +50,6 @@ use OC\Accounts\AccountManager;
use OC\Authentication\Token\RemoteWipe; use OC\Authentication\Token\RemoteWipe;
use OC\HintException; use OC\HintException;
use OC\KnownUser\KnownUserService; use OC\KnownUser\KnownUserService;
use OCA\Provisioning_API\FederatedShareProviderFactory;
use OCA\Settings\Mailer\NewUserMailHelper; use OCA\Settings\Mailer\NewUserMailHelper;
use OCP\Accounts\IAccountManager; use OCP\Accounts\IAccountManager;
use OCP\App\IAppManager; use OCP\App\IAppManager;
@ -85,8 +84,6 @@ class UsersController extends AUserData {
protected $l10nFactory; protected $l10nFactory;
/** @var NewUserMailHelper */ /** @var NewUserMailHelper */
private $newUserMailHelper; private $newUserMailHelper;
/** @var FederatedShareProviderFactory */
private $federatedShareProviderFactory;
/** @var ISecureRandom */ /** @var ISecureRandom */
private $secureRandom; private $secureRandom;
/** @var RemoteWipe */ /** @var RemoteWipe */
@ -108,7 +105,6 @@ class UsersController extends AUserData {
LoggerInterface $logger, LoggerInterface $logger,
IFactory $l10nFactory, IFactory $l10nFactory,
NewUserMailHelper $newUserMailHelper, NewUserMailHelper $newUserMailHelper,
FederatedShareProviderFactory $federatedShareProviderFactory,
ISecureRandom $secureRandom, ISecureRandom $secureRandom,
RemoteWipe $remoteWipe, RemoteWipe $remoteWipe,
KnownUserService $knownUserService, KnownUserService $knownUserService,
@ -127,7 +123,6 @@ class UsersController extends AUserData {
$this->logger = $logger; $this->logger = $logger;
$this->l10nFactory = $l10nFactory; $this->l10nFactory = $l10nFactory;
$this->newUserMailHelper = $newUserMailHelper; $this->newUserMailHelper = $newUserMailHelper;
$this->federatedShareProviderFactory = $federatedShareProviderFactory;
$this->secureRandom = $secureRandom; $this->secureRandom = $secureRandom;
$this->remoteWipe = $remoteWipe; $this->remoteWipe = $remoteWipe;
$this->knownUserService = $knownUserService; $this->knownUserService = $knownUserService;
@ -532,15 +527,10 @@ class UsersController extends AUserData {
$permittedFields[] = IAccountManager::PROPERTY_EMAIL; $permittedFields[] = IAccountManager::PROPERTY_EMAIL;
} }
if ($this->appManager->isEnabledForUser('federatedfilesharing')) { $permittedFields[] = IAccountManager::PROPERTY_PHONE;
$shareProvider = $this->federatedShareProviderFactory->get(); $permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
if ($shareProvider->isLookupServerUploadEnabled()) { $permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
$permittedFields[] = IAccountManager::PROPERTY_PHONE; $permittedFields[] = IAccountManager::PROPERTY_TWITTER;
$permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
$permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
$permittedFields[] = IAccountManager::PROPERTY_TWITTER;
}
}
return new DataResponse($permittedFields); return new DataResponse($permittedFields);
} }
@ -586,15 +576,10 @@ class UsersController extends AUserData {
$permittedFields[] = 'locale'; $permittedFields[] = 'locale';
} }
if ($this->appManager->isEnabledForUser('federatedfilesharing')) { $permittedFields[] = IAccountManager::PROPERTY_PHONE;
$shareProvider = $this->federatedShareProviderFactory->get(); $permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
if ($shareProvider->isLookupServerUploadEnabled()) { $permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
$permittedFields[] = IAccountManager::PROPERTY_PHONE; $permittedFields[] = IAccountManager::PROPERTY_TWITTER;
$permittedFields[] = IAccountManager::PROPERTY_ADDRESS;
$permittedFields[] = IAccountManager::PROPERTY_WEBSITE;
$permittedFields[] = IAccountManager::PROPERTY_TWITTER;
}
}
// If admin they can edit their own quota // If admin they can edit their own quota
if ($this->groupManager->isAdmin($currentLoggedInUser->getUID())) { if ($this->groupManager->isAdmin($currentLoggedInUser->getUID())) {

View File

@ -46,9 +46,7 @@ use OC\Authentication\Token\RemoteWipe;
use OC\Group\Manager; use OC\Group\Manager;
use OC\KnownUser\KnownUserService; use OC\KnownUser\KnownUserService;
use OC\SubAdmin; use OC\SubAdmin;
use OCA\FederatedFileSharing\FederatedShareProvider;
use OCA\Provisioning_API\Controller\UsersController; use OCA\Provisioning_API\Controller\UsersController;
use OCA\Provisioning_API\FederatedShareProviderFactory;
use OCA\Settings\Mailer\NewUserMailHelper; use OCA\Settings\Mailer\NewUserMailHelper;
use OCP\Accounts\IAccountManager; use OCP\Accounts\IAccountManager;
use OCP\App\IAppManager; use OCP\App\IAppManager;
@ -97,8 +95,6 @@ class UsersControllerTest extends TestCase {
private $l10nFactory; private $l10nFactory;
/** @var NewUserMailHelper|MockObject */ /** @var NewUserMailHelper|MockObject */
private $newUserMailHelper; private $newUserMailHelper;
/** @var FederatedShareProviderFactory|MockObject */
private $federatedShareProviderFactory;
/** @var ISecureRandom|MockObject */ /** @var ISecureRandom|MockObject */
private $secureRandom; private $secureRandom;
/** @var RemoteWipe|MockObject */ /** @var RemoteWipe|MockObject */
@ -122,7 +118,6 @@ class UsersControllerTest extends TestCase {
$this->urlGenerator = $this->createMock(IURLGenerator::class); $this->urlGenerator = $this->createMock(IURLGenerator::class);
$this->l10nFactory = $this->createMock(IFactory::class); $this->l10nFactory = $this->createMock(IFactory::class);
$this->newUserMailHelper = $this->createMock(NewUserMailHelper::class); $this->newUserMailHelper = $this->createMock(NewUserMailHelper::class);
$this->federatedShareProviderFactory = $this->createMock(FederatedShareProviderFactory::class);
$this->secureRandom = $this->createMock(ISecureRandom::class); $this->secureRandom = $this->createMock(ISecureRandom::class);
$this->remoteWipe = $this->createMock(RemoteWipe::class); $this->remoteWipe = $this->createMock(RemoteWipe::class);
$this->knownUserService = $this->createMock(KnownUserService::class); $this->knownUserService = $this->createMock(KnownUserService::class);
@ -142,7 +137,6 @@ class UsersControllerTest extends TestCase {
$this->logger, $this->logger,
$this->l10nFactory, $this->l10nFactory,
$this->newUserMailHelper, $this->newUserMailHelper,
$this->federatedShareProviderFactory,
$this->secureRandom, $this->secureRandom,
$this->remoteWipe, $this->remoteWipe,
$this->knownUserService, $this->knownUserService,
@ -407,7 +401,6 @@ class UsersControllerTest extends TestCase {
$this->logger, $this->logger,
$this->l10nFactory, $this->l10nFactory,
$this->newUserMailHelper, $this->newUserMailHelper,
$this->federatedShareProviderFactory,
$this->secureRandom, $this->secureRandom,
$this->remoteWipe, $this->remoteWipe,
$this->knownUserService, $this->knownUserService,
@ -3247,7 +3240,6 @@ class UsersControllerTest extends TestCase {
$this->logger, $this->logger,
$this->l10nFactory, $this->l10nFactory,
$this->newUserMailHelper, $this->newUserMailHelper,
$this->federatedShareProviderFactory,
$this->secureRandom, $this->secureRandom,
$this->remoteWipe, $this->remoteWipe,
$this->knownUserService, $this->knownUserService,
@ -3314,7 +3306,6 @@ class UsersControllerTest extends TestCase {
$this->logger, $this->logger,
$this->l10nFactory, $this->l10nFactory,
$this->newUserMailHelper, $this->newUserMailHelper,
$this->federatedShareProviderFactory,
$this->secureRandom, $this->secureRandom,
$this->remoteWipe, $this->remoteWipe,
$this->knownUserService, $this->knownUserService,
@ -3639,18 +3630,13 @@ class UsersControllerTest extends TestCase {
public function dataGetEditableFields() { public function dataGetEditableFields() {
return [ return [
[false, false, []], [false, [
[false, true, [
IAccountManager::PROPERTY_PHONE, IAccountManager::PROPERTY_PHONE,
IAccountManager::PROPERTY_ADDRESS, IAccountManager::PROPERTY_ADDRESS,
IAccountManager::PROPERTY_WEBSITE, IAccountManager::PROPERTY_WEBSITE,
IAccountManager::PROPERTY_TWITTER, IAccountManager::PROPERTY_TWITTER,
]], ]],
[ true, false, [ [ true, [
IAccountManager::PROPERTY_DISPLAYNAME,
IAccountManager::PROPERTY_EMAIL,
]],
[ true, true ,[
IAccountManager::PROPERTY_DISPLAYNAME, IAccountManager::PROPERTY_DISPLAYNAME,
IAccountManager::PROPERTY_EMAIL, IAccountManager::PROPERTY_EMAIL,
IAccountManager::PROPERTY_PHONE, IAccountManager::PROPERTY_PHONE,
@ -3665,27 +3651,15 @@ class UsersControllerTest extends TestCase {
* @dataProvider dataGetEditableFields * @dataProvider dataGetEditableFields
* *
* @param bool $allowedToChangeDisplayName * @param bool $allowedToChangeDisplayName
* @param bool $federatedSharingEnabled
* @param array $expected * @param array $expected
*/ */
public function testGetEditableFields(bool $allowedToChangeDisplayName, bool $federatedSharingEnabled, array $expected) { public function testGetEditableFields(bool $allowedToChangeDisplayName, array $expected) {
$this->config $this->config
->method('getSystemValue') ->method('getSystemValue')
->with( ->with(
$this->equalTo('allow_user_to_change_display_name'), $this->equalTo('allow_user_to_change_display_name'),
$this->anything() $this->anything()
)->willReturn($allowedToChangeDisplayName); )->willReturn($allowedToChangeDisplayName);
$this->appManager
->method('isEnabledForUser')
->with($this->equalTo('federatedfilesharing'))
->willReturn($federatedSharingEnabled);
$shareprovider = $this->createMock(FederatedShareProvider::class);
$shareprovider->method('isLookupServerUploadEnabled')->willReturn(true);
$this->federatedShareProviderFactory
->method('get')
->willReturn($shareprovider);
$expectedResp = new DataResponse($expected); $expectedResp = new DataResponse($expected);
$this->assertEquals($expectedResp, $this->api->getEditableFields()); $this->assertEquals($expectedResp, $this->api->getEditableFields());

View File

@ -46,7 +46,6 @@ use OC\KnownUser\KnownUserService;
use OC\L10N\Factory; use OC\L10N\Factory;
use OC\Security\IdentityProof\Manager; use OC\Security\IdentityProof\Manager;
use OC\User\Manager as UserManager; use OC\User\Manager as UserManager;
use OCA\FederatedFileSharing\FederatedShareProvider;
use OCA\Settings\BackgroundJobs\VerifyUserData; use OCA\Settings\BackgroundJobs\VerifyUserData;
use OCA\Settings\Events\BeforeTemplateRenderedEvent; use OCA\Settings\Events\BeforeTemplateRenderedEvent;
use OCA\User_LDAP\User_Proxy; use OCA\User_LDAP\User_Proxy;
@ -401,15 +400,11 @@ class UsersController extends Controller {
$data[IAccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope]; $data[IAccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope];
$data[IAccountManager::PROPERTY_EMAIL] = ['value' => $email, 'scope' => $emailScope]; $data[IAccountManager::PROPERTY_EMAIL] = ['value' => $email, 'scope' => $emailScope];
} }
if ($this->appManager->isEnabledForUser('federatedfilesharing')) { $data[IAccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope];
$shareProvider = \OC::$server->query(FederatedShareProvider::class); $data[IAccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope];
if ($shareProvider->isLookupServerUploadEnabled()) { $data[IAccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope];
$data[IAccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope]; $data[IAccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope];
$data[IAccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope];
$data[IAccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope];
$data[IAccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope];
}
}
try { try {
$data = $this->saveUserSettings($user, $data); $data = $this->saveUserSettings($user, $data);
if ($beforeData[IAccountManager::PROPERTY_PHONE]['value'] !== $data[IAccountManager::PROPERTY_PHONE]['value']) { if ($beforeData[IAccountManager::PROPERTY_PHONE]['value'] !== $data[IAccountManager::PROPERTY_PHONE]['value']) {

View File

@ -177,7 +177,6 @@ script('settings', [
<?php } ?> <?php } ?>
</form> </form>
</div> </div>
<?php if (!empty($_['phone']) || $_['lookupServerUploadEnabled']) { ?>
<div class="personal-settings-setting-box"> <div class="personal-settings-setting-box">
<form id="phoneform" class="section"> <form id="phoneform" class="section">
<h3> <h3>
@ -188,9 +187,7 @@ script('settings', [
</span> </span>
</div> </div>
</h3> </h3>
<input type="tel" id="phone" name="phone" <?php if (!$_['lookupServerUploadEnabled']) { <input type="tel" id="phone" name="phone"
print_unescaped('disabled="1"');
} ?>
value="<?php p($_['phone']) ?>" value="<?php p($_['phone']) ?>"
placeholder="<?php p($l->t('Your phone number')); ?>" placeholder="<?php p($l->t('Your phone number')); ?>"
autocomplete="on" autocapitalize="none" autocorrect="off" /> autocomplete="on" autocapitalize="none" autocorrect="off" />
@ -199,8 +196,6 @@ script('settings', [
<input type="hidden" id="phonescope" value="<?php p($_['phoneScope']) ?>"> <input type="hidden" id="phonescope" value="<?php p($_['phoneScope']) ?>">
</form> </form>
</div> </div>
<?php } ?>
<?php if (!empty($_['address']) || $_['lookupServerUploadEnabled']) { ?>
<div class="personal-settings-setting-box"> <div class="personal-settings-setting-box">
<form id="addressform" class="section"> <form id="addressform" class="section">
<h3> <h3>
@ -211,9 +206,7 @@ script('settings', [
</span> </span>
</div> </div>
</h3> </h3>
<input type="text" id="address" name="address" <?php if (!$_['lookupServerUploadEnabled']) { <input type="text" id="address" name="address"
print_unescaped('disabled="1"');
} ?>
placeholder="<?php p($l->t('Your postal address')); ?>" placeholder="<?php p($l->t('Your postal address')); ?>"
value="<?php p($_['address']) ?>" value="<?php p($_['address']) ?>"
autocomplete="on" autocapitalize="none" autocorrect="off" /> autocomplete="on" autocapitalize="none" autocorrect="off" />
@ -222,8 +215,6 @@ script('settings', [
<input type="hidden" id="addressscope" value="<?php p($_['addressScope']) ?>"> <input type="hidden" id="addressscope" value="<?php p($_['addressScope']) ?>">
</form> </form>
</div> </div>
<?php } ?>
<?php if (!empty($_['website']) || $_['lookupServerUploadEnabled']) { ?>
<div class="personal-settings-setting-box"> <div class="personal-settings-setting-box">
<form id="websiteform" class="section"> <form id="websiteform" class="section">
<h3> <h3>
@ -267,17 +258,12 @@ script('settings', [
<input type="url" name="website" id="website" value="<?php p($_['website']); ?>" <input type="url" name="website" id="website" value="<?php p($_['website']); ?>"
placeholder="<?php p($l->t('Link https://…')); ?>" placeholder="<?php p($l->t('Link https://…')); ?>"
autocomplete="on" autocapitalize="none" autocorrect="off" autocomplete="on" autocapitalize="none" autocorrect="off"
<?php if (!$_['lookupServerUploadEnabled']) {
print_unescaped('disabled="1"');
} ?>
/> />
<span class="icon-checkmark hidden"></span> <span class="icon-checkmark hidden"></span>
<span class="icon-error hidden" ></span> <span class="icon-error hidden" ></span>
<input type="hidden" id="websitescope" value="<?php p($_['websiteScope']) ?>"> <input type="hidden" id="websitescope" value="<?php p($_['websiteScope']) ?>">
</form> </form>
</div> </div>
<?php } ?>
<?php if (!empty($_['twitter']) || $_['lookupServerUploadEnabled']) { ?>
<div class="personal-settings-setting-box"> <div class="personal-settings-setting-box">
<form id="twitterform" class="section"> <form id="twitterform" class="section">
<h3> <h3>
@ -321,16 +307,12 @@ script('settings', [
<input type="text" name="twitter" id="twitter" value="<?php p($_['twitter']); ?>" <input type="text" name="twitter" id="twitter" value="<?php p($_['twitter']); ?>"
placeholder="<?php p($l->t('Twitter handle @…')); ?>" placeholder="<?php p($l->t('Twitter handle @…')); ?>"
autocomplete="on" autocapitalize="none" autocorrect="off" autocomplete="on" autocapitalize="none" autocorrect="off"
<?php if (!$_['lookupServerUploadEnabled']) {
print_unescaped('disabled="1"');
} ?>
/> />
<span class="icon-checkmark hidden"></span> <span class="icon-checkmark hidden"></span>
<span class="icon-error hidden" ></span> <span class="icon-error hidden" ></span>
<input type="hidden" id="twitterscope" value="<?php p($_['twitterScope']) ?>"> <input type="hidden" id="twitterscope" value="<?php p($_['twitterScope']) ?>">
</form> </form>
</div> </div>
<?php } ?>
</div> </div>
<div class="profile-settings-container"> <div class="profile-settings-container">

View File

@ -190,6 +190,7 @@ class UsersControllerTest extends \Test\TestCase {
public function testSetUserSettings($email, $validEmail, $expectedStatus) { public function testSetUserSettings($email, $validEmail, $expectedStatus) {
$controller = $this->getController(false, ['saveUserSettings']); $controller = $this->getController(false, ['saveUserSettings']);
$user = $this->createMock(IUser::class); $user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('johndoe');
$this->userSession->method('getUser')->willReturn($user); $this->userSession->method('getUser')->willReturn($user);