[stable19] Fix account data visibility after disabling public addressbook upload

.drone.yml

@@ -1178,6 +1178,31 @@ trigger:
     - pull_request
     - push
 
+---
+kind: pipeline
+name: integration-contacts-menu
+
+steps:
+- name: submodules
+  image: docker:git
+  commands:
+    - git submodule update --init
+- name: integration-contacts-menu
+  image: nextcloudci/integration-php7.3:integration-php7.3-2
+  commands:
+    - bash tests/drone-run-integration-tests.sh || exit 0
+    - ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
+    - cd build/integration
+    - ./run.sh features/contacts-menu.feature
+
+trigger:
+  branch:
+    - master
+    - stable*
+  event:
+    - pull_request
+    - push
+
 ---
 kind: pipeline
 name: integration-favorites

apps/settings/css/settings.scss

@@ -372,6 +372,18 @@ select {
 						font-weight: bold;
 					}
 				}
+
+				&.disabled {
+					/* !important is needed to override important opacity of
+					 * ".popovermenu li > .menuitem.active" */
+					opacity: .5 !important;
+
+					cursor: default;
+
+					span {
+						cursor: default;
+					}
+				}
 			}
 		}
 	}

apps/settings/js/federationscopemenu.js

@@ -117,6 +117,17 @@
 					break;
 			}
 
+			var lookupServerUploadEnabled = $('#lookupServerUploadEnabled').val();
+			if (!lookupServerUploadEnabled && !this._scopes[2].active) {
+				this._scopes[2].hidden = true
+			} else if (!lookupServerUploadEnabled && this._scopes[2].active) {
+				this._scopes[2].hidden = false
+				this._scopes[2].disabled = true
+			} else {
+				this._scopes[2].hidden = false
+				this._scopes[2].disabled = false
+			}
+
 			this.render();
 			this.$el.removeClass('hidden');
 

apps/settings/js/federationsettingsview.js

@@ -43,7 +43,14 @@
 				if (!scopeOnly) {
 					self._config.set(field, $('#' + field).val());
 				}
-				self._config.set(field + 'Scope', $('#' + field + 'scope').val());
+				// A scope could have been stored as null due to a previous bug.
+				// Null values should be kept as such until the user explicitly
+				// sets the right value, but they will be returned as empty
+				// strings in the template (which would overwrite the null value
+				// if sent). Due to this an extra class is needed to
+				// differentiate them.
+				var initialScopeValue = $('#' + field + 'scope').hasClass('corrupted-scope-value') ? undefined : $('#' + field + 'scope').val();
+				self._config.set(field + 'Scope', initialScopeValue);
 
 				// Set inputs whenever model values change
 				if (!scopeOnly) {
@@ -219,6 +226,12 @@
 					$icon.addClass('icon-link');
 					$icon.removeClass('hidden');
 					break;
+				case '':
+				case null:
+				case undefined:
+					$icon.addClass('icon-error');
+					$icon.removeClass('hidden');
+					break;
 			}
 		}
 	});

apps/settings/js/templates.js

@@ -1,29 +1,65 @@
 (function() {
   var template = Handlebars.template, templates = OC.Settings.Templates = OC.Settings.Templates || {};
 templates['federationscopemenu'] = template({"1":function(container,depth0,helpers,partials,data) {
+    var stack1, lookupProperty = container.lookupProperty || function(parent, propertyName) {
+        if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
+          return parent[propertyName];
+        }
+        return undefined
+    };
+
+  return ((stack1 = lookupProperty(helpers,"unless").call(depth0 != null ? depth0 : (container.nullContext || {}),(depth0 != null ? lookupProperty(depth0,"hidden") : depth0),{"name":"unless","hash":{},"fn":container.program(2, data, 0),"inverse":container.noop,"data":data,"loc":{"start":{"line":3,"column":2},"end":{"line":25,"column":13}}})) != null ? stack1 : "");
+},"2":function(container,depth0,helpers,partials,data) {
     var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=container.hooks.helperMissing, alias3="function", alias4=container.escapeExpression, lookupProperty = container.lookupProperty || function(parent, propertyName) {
         if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
           return parent[propertyName];
         }
         return undefined
     };
 
-  return "		<li tabindex=\"0\">\n			<a href=\"#\" class=\"menuitem action action-"
-    + alias4(((helper = (helper = lookupProperty(helpers,"name") || (depth0 != null ? lookupProperty(depth0,"name") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"name","hash":{},"data":data,"loc":{"start":{"line":4,"column":45},"end":{"line":4,"column":53}}}) : helper)))
-    + " permanent "
-    + ((stack1 = lookupProperty(helpers,"if").call(alias1,(depth0 != null ? lookupProperty(depth0,"active") : depth0),{"name":"if","hash":{},"fn":container.program(2, data, 0),"inverse":container.noop,"data":data,"loc":{"start":{"line":4,"column":64},"end":{"line":4,"column":91}}})) != null ? stack1 : "")
-    + "\" data-action=\""
-    + alias4(((helper = (helper = lookupProperty(helpers,"name") || (depth0 != null ? lookupProperty(depth0,"name") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"name","hash":{},"data":data,"loc":{"start":{"line":4,"column":106},"end":{"line":4,"column":114}}}) : helper)))
-    + "\">\n"
-    + ((stack1 = lookupProperty(helpers,"if").call(alias1,(depth0 != null ? lookupProperty(depth0,"iconClass") : depth0),{"name":"if","hash":{},"fn":container.program(4, data, 0),"inverse":container.program(6, data, 0),"data":data,"loc":{"start":{"line":5,"column":4},"end":{"line":9,"column":11}}})) != null ? stack1 : "")
+  return "		<li tabindex=\"0\">\n"
+    + ((stack1 = lookupProperty(helpers,"if").call(alias1,(depth0 != null ? lookupProperty(depth0,"disabled") : depth0),{"name":"if","hash":{},"fn":container.program(3, data, 0),"inverse":container.program(6, data, 0),"data":data,"loc":{"start":{"line":5,"column":3},"end":{"line":9,"column":10}}})) != null ? stack1 : "")
+    + ((stack1 = lookupProperty(helpers,"if").call(alias1,(depth0 != null ? lookupProperty(depth0,"iconClass") : depth0),{"name":"if","hash":{},"fn":container.program(8, data, 0),"inverse":container.program(10, data, 0),"data":data,"loc":{"start":{"line":10,"column":4},"end":{"line":14,"column":11}}})) != null ? stack1 : "")
     + "				<p>\n					<strong class=\"menuitem-text\">"
-    + alias4(((helper = (helper = lookupProperty(helpers,"displayName") || (depth0 != null ? lookupProperty(depth0,"displayName") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"displayName","hash":{},"data":data,"loc":{"start":{"line":11,"column":35},"end":{"line":11,"column":50}}}) : helper)))
+    + alias4(((helper = (helper = lookupProperty(helpers,"displayName") || (depth0 != null ? lookupProperty(depth0,"displayName") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"displayName","hash":{},"data":data,"loc":{"start":{"line":16,"column":35},"end":{"line":16,"column":50}}}) : helper)))
     + "</strong><br>\n					<span class=\"menuitem-text-detail\">"
-    + alias4(((helper = (helper = lookupProperty(helpers,"tooltip") || (depth0 != null ? lookupProperty(depth0,"tooltip") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"tooltip","hash":{},"data":data,"loc":{"start":{"line":12,"column":40},"end":{"line":12,"column":51}}}) : helper)))
-    + "</span>\n				</p>\n			</a>\n		</li>\n";
-},"2":function(container,depth0,helpers,partials,data) {
-    return "active";
+    + alias4(((helper = (helper = lookupProperty(helpers,"tooltip") || (depth0 != null ? lookupProperty(depth0,"tooltip") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"tooltip","hash":{},"data":data,"loc":{"start":{"line":17,"column":40},"end":{"line":17,"column":51}}}) : helper)))
+    + "</span>\n				</p>\n"
+    + ((stack1 = lookupProperty(helpers,"if").call(alias1,(depth0 != null ? lookupProperty(depth0,"disabled") : depth0),{"name":"if","hash":{},"fn":container.program(12, data, 0),"inverse":container.program(14, data, 0),"data":data,"loc":{"start":{"line":19,"column":3},"end":{"line":23,"column":10}}})) != null ? stack1 : "")
+    + "		</li>\n";
+},"3":function(container,depth0,helpers,partials,data) {
+    var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=container.hooks.helperMissing, alias3="function", alias4=container.escapeExpression, lookupProperty = container.lookupProperty || function(parent, propertyName) {
+        if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
+          return parent[propertyName];
+        }
+        return undefined
+    };
+
+  return "			<div class=\"menuitem action action-"
+    + alias4(((helper = (helper = lookupProperty(helpers,"name") || (depth0 != null ? lookupProperty(depth0,"name") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"name","hash":{},"data":data,"loc":{"start":{"line":6,"column":38},"end":{"line":6,"column":46}}}) : helper)))
+    + " permanent "
+    + ((stack1 = lookupProperty(helpers,"if").call(alias1,(depth0 != null ? lookupProperty(depth0,"active") : depth0),{"name":"if","hash":{},"fn":container.program(4, data, 0),"inverse":container.noop,"data":data,"loc":{"start":{"line":6,"column":57},"end":{"line":6,"column":84}}})) != null ? stack1 : "")
+    + " disabled\" data-action=\""
+    + alias4(((helper = (helper = lookupProperty(helpers,"name") || (depth0 != null ? lookupProperty(depth0,"name") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"name","hash":{},"data":data,"loc":{"start":{"line":6,"column":108},"end":{"line":6,"column":116}}}) : helper)))
+    + "\">\n";
 },"4":function(container,depth0,helpers,partials,data) {
+    return "active";
+},"6":function(container,depth0,helpers,partials,data) {
+    var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=container.hooks.helperMissing, alias3="function", alias4=container.escapeExpression, lookupProperty = container.lookupProperty || function(parent, propertyName) {
+        if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
+          return parent[propertyName];
+        }
+        return undefined
+    };
+
+  return "			<a href=\"#\" class=\"menuitem action action-"
+    + alias4(((helper = (helper = lookupProperty(helpers,"name") || (depth0 != null ? lookupProperty(depth0,"name") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"name","hash":{},"data":data,"loc":{"start":{"line":8,"column":45},"end":{"line":8,"column":53}}}) : helper)))
+    + " permanent "
+    + ((stack1 = lookupProperty(helpers,"if").call(alias1,(depth0 != null ? lookupProperty(depth0,"active") : depth0),{"name":"if","hash":{},"fn":container.program(4, data, 0),"inverse":container.noop,"data":data,"loc":{"start":{"line":8,"column":64},"end":{"line":8,"column":91}}})) != null ? stack1 : "")
+    + "\" data-action=\""
+    + alias4(((helper = (helper = lookupProperty(helpers,"name") || (depth0 != null ? lookupProperty(depth0,"name") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"name","hash":{},"data":data,"loc":{"start":{"line":8,"column":106},"end":{"line":8,"column":114}}}) : helper)))
+    + "\">\n";
+},"8":function(container,depth0,helpers,partials,data) {
     var helper, lookupProperty = container.lookupProperty || function(parent, propertyName) {
         if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
           return parent[propertyName];
@@ -32,10 +68,14 @@ templates['federationscopemenu'] = template({"1":function(container,depth0,helpe
     };
 
   return "					<span class=\"icon "
-    + container.escapeExpression(((helper = (helper = lookupProperty(helpers,"iconClass") || (depth0 != null ? lookupProperty(depth0,"iconClass") : depth0)) != null ? helper : container.hooks.helperMissing),(typeof helper === "function" ? helper.call(depth0 != null ? depth0 : (container.nullContext || {}),{"name":"iconClass","hash":{},"data":data,"loc":{"start":{"line":6,"column":23},"end":{"line":6,"column":36}}}) : helper)))
+    + container.escapeExpression(((helper = (helper = lookupProperty(helpers,"iconClass") || (depth0 != null ? lookupProperty(depth0,"iconClass") : depth0)) != null ? helper : container.hooks.helperMissing),(typeof helper === "function" ? helper.call(depth0 != null ? depth0 : (container.nullContext || {}),{"name":"iconClass","hash":{},"data":data,"loc":{"start":{"line":11,"column":23},"end":{"line":11,"column":36}}}) : helper)))
     + "\"></span>\n";
-},"6":function(container,depth0,helpers,partials,data) {
+},"10":function(container,depth0,helpers,partials,data) {
     return "					<span class=\"no-icon\"></span>\n";
+},"12":function(container,depth0,helpers,partials,data) {
+    return "			</div>\n";
+},"14":function(container,depth0,helpers,partials,data) {
+    return "			</a>\n";
 },"compiler":[8,">= 4.3.0"],"main":function(container,depth0,helpers,partials,data) {
     var stack1, lookupProperty = container.lookupProperty || function(parent, propertyName) {
         if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
@@ -45,7 +85,7 @@ templates['federationscopemenu'] = template({"1":function(container,depth0,helpe
     };
 
   return "<ul>\n"
-    + ((stack1 = lookupProperty(helpers,"each").call(depth0 != null ? depth0 : (container.nullContext || {}),(depth0 != null ? lookupProperty(depth0,"items") : depth0),{"name":"each","hash":{},"fn":container.program(1, data, 0),"inverse":container.noop,"data":data,"loc":{"start":{"line":2,"column":1},"end":{"line":16,"column":10}}})) != null ? stack1 : "")
+    + ((stack1 = lookupProperty(helpers,"each").call(depth0 != null ? depth0 : (container.nullContext || {}),(depth0 != null ? lookupProperty(depth0,"items") : depth0),{"name":"each","hash":{},"fn":container.program(1, data, 0),"inverse":container.noop,"data":data,"loc":{"start":{"line":2,"column":1},"end":{"line":26,"column":10}}})) != null ? stack1 : "")
     + "</ul>\n";
 },"useData":true});
 })();

apps/settings/js/templates/federationscopemenu.handlebars

@@ -1,7 +1,12 @@
 <ul>
 	{{#each items}}
+		{{#unless hidden}}
 		<li tabindex="0">
+			{{#if disabled}}
+			<div class="menuitem action action-{{name}} permanent {{#if active}}active{{/if}} disabled" data-action="{{name}}">
+			{{else}}
 			<a href="#" class="menuitem action action-{{name}} permanent {{#if active}}active{{/if}}" data-action="{{name}}">
+			{{/if}}
 				{{#if iconClass}}
 					<span class="icon {{iconClass}}"></span>
 				{{else}}
@@ -11,7 +16,12 @@
 					<strong class="menuitem-text">{{displayName}}</strong><br>
 					<span class="menuitem-text-detail">{{tooltip}}</span>
 				</p>
+			{{#if disabled}}
+			</div>
+			{{else}}
 			</a>
+			{{/if}}
 		</li>
+		{{/unless}}
 	{{/each}}
 </ul>

apps/settings/lib/Controller/UsersController.php

@@ -38,6 +38,7 @@
 use OC\Security\IdentityProof\Manager;
 use OCA\Settings\BackgroundJobs\VerifyUserData;
 use OCA\User_LDAP\User_Proxy;
+use OCP\Accounts\IAccountManager;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\DataResponse;
@@ -343,7 +344,7 @@ public function setUserSettings($avatarScope,
 									$twitter,
 									$twitterScope
 	) {
-		$email = strtolower($email);
+		$email = !is_null($email) ? strtolower($email) : $email;
 		if (!empty($email) && !$this->mailer->validateMailAddress($email)) {
 			return new DataResponse(
 				[
@@ -357,19 +358,51 @@ public function setUserSettings($avatarScope,
 		}
 		$user = $this->userSession->getUser();
 		$data = $this->accountManager->getUser($user);
-		$data[AccountManager::PROPERTY_AVATAR] = ['scope' => $avatarScope];
+		if (!is_null($avatarScope)) {
+			$data[IAccountManager::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 (!is_null($displayname)) {
+				$data[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $displayname;
+			}
+			if (!is_null($displaynameScope)) {
+				$data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $displaynameScope;
+			}
+			if (!is_null($email)) {
+				$data[IAccountManager::PROPERTY_EMAIL]['value'] = $email;
+			}
+			if (!is_null($emailScope)) {
+				$data[IAccountManager::PROPERTY_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];
+				if (!is_null($website)) {
+					$data[IAccountManager::PROPERTY_WEBSITE]['value'] = $website;
+				}
+				if (!is_null($websiteScope)) {
+					$data[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope;
+				}
+				if (!is_null($address)) {
+					$data[IAccountManager::PROPERTY_ADDRESS]['value'] = $address;
+				}
+				if (!is_null($addressScope)) {
+					$data[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope;
+				}
+				if (!is_null($phone)) {
+					$data[IAccountManager::PROPERTY_PHONE]['value'] = $phone;
+				}
+				if (!is_null($phoneScope)) {
+					$data[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope;
+				}
+				if (!is_null($twitter)) {
+					$data[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter;
+				}
+				if (!is_null($twitterScope)) {
+					$data[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope;
+				}
 			}
 		}
 		try {
@@ -379,15 +412,15 @@ public function setUserSettings($avatarScope,
 					'status' => 'success',
 					'data' => [
 						'userId' => $user->getUID(),
-						'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'],
+						'avatarScope' => $data[IAccountManager::PROPERTY_AVATAR]['scope'],
+						'displayname' => $data[IAccountManager::PROPERTY_DISPLAYNAME]['value'],
+						'displaynameScope' => $data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'],
+						'email' => $data[IAccountManager::PROPERTY_EMAIL]['value'],
+						'emailScope' => $data[IAccountManager::PROPERTY_EMAIL]['scope'],
+						'website' => $data[IAccountManager::PROPERTY_WEBSITE]['value'],
+						'websiteScope' => $data[IAccountManager::PROPERTY_WEBSITE]['scope'],
+						'address' => $data[IAccountManager::PROPERTY_ADDRESS]['value'],
+						'addressScope' => $data[IAccountManager::PROPERTY_ADDRESS]['scope'],
 						'message' => $this->l10n->t('Settings saved')
 					]
 				],