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>

apps/provisioning_api/lib/Controller/UsersController.php

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

apps/provisioning_api/tests/Controller/UsersControllerTest.php

@@ -46,9 +46,7 @@ use OC\Authentication\Token\RemoteWipe;
 use OC\Group\Manager;
 use OC\KnownUser\KnownUserService;
 use OC\SubAdmin;
-use OCA\FederatedFileSharing\FederatedShareProvider;
 use OCA\Provisioning_API\Controller\UsersController;
-use OCA\Provisioning_API\FederatedShareProviderFactory;
 use OCA\Settings\Mailer\NewUserMailHelper;
 use OCP\Accounts\IAccountManager;
 use OCP\App\IAppManager;
@@ -97,8 +95,6 @@ class UsersControllerTest extends TestCase {
 	private $l10nFactory;
 	/** @var NewUserMailHelper|MockObject */
 	private $newUserMailHelper;
-	/** @var FederatedShareProviderFactory|MockObject */
-	private $federatedShareProviderFactory;
 	/** @var ISecureRandom|MockObject */
 	private $secureRandom;
 	/** @var RemoteWipe|MockObject */
@@ -122,7 +118,6 @@ class UsersControllerTest extends TestCase {
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->l10nFactory = $this->createMock(IFactory::class);
 		$this->newUserMailHelper = $this->createMock(NewUserMailHelper::class);
-		$this->federatedShareProviderFactory = $this->createMock(FederatedShareProviderFactory::class);
 		$this->secureRandom = $this->createMock(ISecureRandom::class);
 		$this->remoteWipe = $this->createMock(RemoteWipe::class);
 		$this->knownUserService = $this->createMock(KnownUserService::class);
@@ -142,7 +137,6 @@ class UsersControllerTest extends TestCase {
 				$this->logger,
 				$this->l10nFactory,
 				$this->newUserMailHelper,
-				$this->federatedShareProviderFactory,
 				$this->secureRandom,
 				$this->remoteWipe,
 				$this->knownUserService,
@@ -407,7 +401,6 @@ class UsersControllerTest extends TestCase {
 				$this->logger,
 				$this->l10nFactory,
 				$this->newUserMailHelper,
-				$this->federatedShareProviderFactory,
 				$this->secureRandom,
 				$this->remoteWipe,
 				$this->knownUserService,
@@ -3246,7 +3239,6 @@ class UsersControllerTest extends TestCase {
 				$this->logger,
 				$this->l10nFactory,
 				$this->newUserMailHelper,
-				$this->federatedShareProviderFactory,
 				$this->secureRandom,
 				$this->remoteWipe,
 				$this->knownUserService,
@@ -3313,7 +3305,6 @@ class UsersControllerTest extends TestCase {
 				$this->logger,
 				$this->l10nFactory,
 				$this->newUserMailHelper,
-				$this->federatedShareProviderFactory,
 				$this->secureRandom,
 				$this->remoteWipe,
 				$this->knownUserService,
@@ -3638,18 +3629,13 @@ class UsersControllerTest extends TestCase {
 
 	public function dataGetEditableFields() {
 		return [
-			[false, false, []],
-			[false,  true, [
+			[false, [
 				IAccountManager::PROPERTY_PHONE,
 				IAccountManager::PROPERTY_ADDRESS,
 				IAccountManager::PROPERTY_WEBSITE,
 				IAccountManager::PROPERTY_TWITTER,
 			]],
-			[ true, false, [
-				IAccountManager::PROPERTY_DISPLAYNAME,
-				IAccountManager::PROPERTY_EMAIL,
-			]],
-			[ true,  true ,[
+			[ true, [
 				IAccountManager::PROPERTY_DISPLAYNAME,
 				IAccountManager::PROPERTY_EMAIL,
 				IAccountManager::PROPERTY_PHONE,
@@ -3664,27 +3650,15 @@ class UsersControllerTest extends TestCase {
 	 * @dataProvider dataGetEditableFields
 	 *
 	 * @param bool $allowedToChangeDisplayName
-	 * @param bool $federatedSharingEnabled
 	 * @param array $expected
 	 */
-	public function testGetEditableFields(bool $allowedToChangeDisplayName, bool $federatedSharingEnabled, array $expected) {
+	public function testGetEditableFields(bool $allowedToChangeDisplayName, array $expected) {
 		$this->config
 			->method('getSystemValue')
 			->with(
 				$this->equalTo('allow_user_to_change_display_name'),
 				$this->anything()
 			)->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);
 		$this->assertEquals($expectedResp, $this->api->getEditableFields());

apps/settings/lib/Controller/UsersController.php

@@ -46,7 +46,6 @@ use OC\KnownUser\KnownUserService;
 use OC\L10N\Factory;
 use OC\Security\IdentityProof\Manager;
 use OC\User\Manager as UserManager;
-use OCA\FederatedFileSharing\FederatedShareProvider;
 use OCA\Settings\BackgroundJobs\VerifyUserData;
 use OCA\Settings\Events\BeforeTemplateRenderedEvent;
 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_EMAIL] = ['value' => $email, 'scope' => $emailScope];
 		}
-		if ($this->appManager->isEnabledForUser('federatedfilesharing')) {
-			$shareProvider = \OC::$server->query(FederatedShareProvider::class);
-			if ($shareProvider->isLookupServerUploadEnabled()) {
-				$data[IAccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope];
-				$data[IAccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope];
-				$data[IAccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope];
-				$data[IAccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope];
-			}
-		}
+		$data[IAccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope];
+		$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 {
 			$data = $this->saveUserSettings($user, $data);
 			if ($beforeData[IAccountManager::PROPERTY_PHONE]['value'] !== $data[IAccountManager::PROPERTY_PHONE]['value']) {

apps/settings/templates/settings/personal/personal.info.php

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

apps/settings/tests/Controller/UsersControllerTest.php

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