Skip to content

Commit f902adf

Browse files
authored
Merge pull request #25973 from nextcloud/backport/25417/stable20-fix-account-data-visibility-after-disabling-public-addressbook-upload
[stable20] Fix account data visibility after disabling public addressbook upload
2 parents c8374de + 546be0e commit f902adf

13 files changed

Lines changed: 771 additions & 104 deletions

File tree

.drone.yml

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,6 +1155,31 @@ trigger:
11551155
- pull_request
11561156
- push
11571157

1158+
---
1159+
kind: pipeline
1160+
name: integration-contacts-menu
1161+
1162+
steps:
1163+
- name: submodules
1164+
image: docker:git
1165+
commands:
1166+
- git submodule update --init
1167+
- name: integration-contacts-menu
1168+
image: nextcloudci/integration-php7.3:integration-php7.3-2
1169+
commands:
1170+
- bash tests/drone-run-integration-tests.sh || exit 0
1171+
- ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int
1172+
- cd build/integration
1173+
- ./run.sh features/contacts-menu.feature
1174+
1175+
trigger:
1176+
branch:
1177+
- master
1178+
- stable*
1179+
event:
1180+
- pull_request
1181+
- push
1182+
11581183
---
11591184
kind: pipeline
11601185
name: integration-favorites

apps/settings/css/settings.scss

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,16 @@ select {
403403
font-weight: bold;
404404
}
405405
}
406+
407+
&.disabled {
408+
opacity: .5;
409+
410+
cursor: default;
411+
412+
span {
413+
cursor: default;
414+
}
415+
}
406416
}
407417
}
408418
}

apps/settings/js/federationscopemenu.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,17 @@
117117
break;
118118
}
119119

120+
var lookupServerUploadEnabled = $('#lookupServerUploadEnabled').val();
121+
if (!lookupServerUploadEnabled && !this._scopes[2].active) {
122+
this._scopes[2].hidden = true
123+
} else if (!lookupServerUploadEnabled && this._scopes[2].active) {
124+
this._scopes[2].hidden = false
125+
this._scopes[2].disabled = true
126+
} else {
127+
this._scopes[2].hidden = false
128+
this._scopes[2].disabled = false
129+
}
130+
120131
this.render();
121132
this.$el.removeClass('hidden');
122133

apps/settings/js/federationsettingsview.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,14 @@
4343
if (!scopeOnly) {
4444
self._config.set(field, $('#' + field).val());
4545
}
46-
self._config.set(field + 'Scope', $('#' + field + 'scope').val());
46+
// A scope could have been stored as null due to a previous bug.
47+
// Null values should be kept as such until the user explicitly
48+
// sets the right value, but they will be returned as empty
49+
// strings in the template (which would overwrite the null value
50+
// if sent). Due to this an extra class is needed to
51+
// differentiate them.
52+
var initialScopeValue = $('#' + field + 'scope').hasClass('corrupted-scope-value') ? undefined : $('#' + field + 'scope').val();
53+
self._config.set(field + 'Scope', initialScopeValue);
4754

4855
// Set inputs whenever model values change
4956
if (!scopeOnly) {
@@ -219,6 +226,12 @@
219226
$icon.addClass('icon-link');
220227
$icon.removeClass('hidden');
221228
break;
229+
case '':
230+
case null:
231+
case undefined:
232+
$icon.addClass('icon-error');
233+
$icon.removeClass('hidden');
234+
break;
222235
}
223236
}
224237
});

apps/settings/js/templates.js

Lines changed: 56 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,65 @@
11
(function() {
22
var template = Handlebars.template, templates = OC.Settings.Templates = OC.Settings.Templates || {};
33
templates['federationscopemenu'] = template({"1":function(container,depth0,helpers,partials,data) {
4+
var stack1, lookupProperty = container.lookupProperty || function(parent, propertyName) {
5+
if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
6+
return parent[propertyName];
7+
}
8+
return undefined
9+
};
10+
11+
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 : "");
12+
},"2":function(container,depth0,helpers,partials,data) {
413
var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=container.hooks.helperMissing, alias3="function", alias4=container.escapeExpression, lookupProperty = container.lookupProperty || function(parent, propertyName) {
514
if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
615
return parent[propertyName];
716
}
817
return undefined
918
};
1019

11-
return " <li tabindex=\"0\">\n <a href=\"#\" class=\"menuitem action action-"
12-
+ 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)))
13-
+ " permanent "
14-
+ ((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 : "")
15-
+ "\" data-action=\""
16-
+ 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)))
17-
+ "\">\n"
18-
+ ((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 : "")
20+
return " <li tabindex=\"0\">\n"
21+
+ ((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 : "")
22+
+ ((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 : "")
1923
+ " <p>\n <strong class=\"menuitem-text\">"
20-
+ 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)))
24+
+ 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)))
2125
+ "</strong><br>\n <span class=\"menuitem-text-detail\">"
22-
+ 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)))
23-
+ "</span>\n </p>\n </a>\n </li>\n";
24-
},"2":function(container,depth0,helpers,partials,data) {
25-
return "active";
26+
+ 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)))
27+
+ "</span>\n </p>\n"
28+
+ ((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 : "")
29+
+ " </li>\n";
30+
},"3":function(container,depth0,helpers,partials,data) {
31+
var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=container.hooks.helperMissing, alias3="function", alias4=container.escapeExpression, lookupProperty = container.lookupProperty || function(parent, propertyName) {
32+
if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
33+
return parent[propertyName];
34+
}
35+
return undefined
36+
};
37+
38+
return " <div class=\"menuitem action action-"
39+
+ 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)))
40+
+ " permanent "
41+
+ ((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 : "")
42+
+ " disabled\" data-action=\""
43+
+ 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)))
44+
+ "\">\n";
2645
},"4":function(container,depth0,helpers,partials,data) {
46+
return "active";
47+
},"6":function(container,depth0,helpers,partials,data) {
48+
var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=container.hooks.helperMissing, alias3="function", alias4=container.escapeExpression, lookupProperty = container.lookupProperty || function(parent, propertyName) {
49+
if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
50+
return parent[propertyName];
51+
}
52+
return undefined
53+
};
54+
55+
return " <a href=\"#\" class=\"menuitem action action-"
56+
+ 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)))
57+
+ " permanent "
58+
+ ((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 : "")
59+
+ "\" data-action=\""
60+
+ 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)))
61+
+ "\">\n";
62+
},"8":function(container,depth0,helpers,partials,data) {
2763
var helper, lookupProperty = container.lookupProperty || function(parent, propertyName) {
2864
if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
2965
return parent[propertyName];
@@ -32,10 +68,14 @@ templates['federationscopemenu'] = template({"1":function(container,depth0,helpe
3268
};
3369

3470
return " <span class=\"icon "
35-
+ 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)))
71+
+ 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)))
3672
+ "\"></span>\n";
37-
},"6":function(container,depth0,helpers,partials,data) {
73+
},"10":function(container,depth0,helpers,partials,data) {
3874
return " <span class=\"no-icon\"></span>\n";
75+
},"12":function(container,depth0,helpers,partials,data) {
76+
return " </div>\n";
77+
},"14":function(container,depth0,helpers,partials,data) {
78+
return " </a>\n";
3979
},"compiler":[8,">= 4.3.0"],"main":function(container,depth0,helpers,partials,data) {
4080
var stack1, lookupProperty = container.lookupProperty || function(parent, propertyName) {
4181
if (Object.prototype.hasOwnProperty.call(parent, propertyName)) {
@@ -45,7 +85,7 @@ templates['federationscopemenu'] = template({"1":function(container,depth0,helpe
4585
};
4686

4787
return "<ul>\n"
48-
+ ((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 : "")
88+
+ ((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 : "")
4989
+ "</ul>\n";
5090
},"useData":true});
5191
})();

apps/settings/js/templates/federationscopemenu.handlebars

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
<ul>
22
{{#each items}}
3+
{{#unless hidden}}
34
<li tabindex="0">
5+
{{#if disabled}}
6+
<div class="menuitem action action-{{name}} permanent {{#if active}}active{{/if}} disabled" data-action="{{name}}">
7+
{{else}}
48
<a href="#" class="menuitem action action-{{name}} permanent {{#if active}}active{{/if}}" data-action="{{name}}">
9+
{{/if}}
510
{{#if iconClass}}
611
<span class="icon {{iconClass}}"></span>
712
{{else}}
@@ -11,7 +16,12 @@
1116
<strong class="menuitem-text">{{displayName}}</strong><br>
1217
<span class="menuitem-text-detail">{{tooltip}}</span>
1318
</p>
19+
{{#if disabled}}
20+
</div>
21+
{{else}}
1422
</a>
23+
{{/if}}
1524
</li>
25+
{{/unless}}
1626
{{/each}}
1727
</ul>

apps/settings/lib/Controller/UsersController.php

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
use OCA\Settings\BackgroundJobs\VerifyUserData;
4747
use OCA\Settings\Events\BeforeTemplateRenderedEvent;
4848
use OCA\User_LDAP\User_Proxy;
49+
use OCP\Accounts\IAccountManager;
4950
use OCP\App\IAppManager;
5051
use OCP\AppFramework\Controller;
5152
use OCP\AppFramework\Http\DataResponse;
@@ -360,7 +361,7 @@ public function setUserSettings($avatarScope,
360361
$twitter,
361362
$twitterScope
362363
) {
363-
$email = strtolower($email);
364+
$email = !is_null($email) ? strtolower($email) : $email;
364365
if (!empty($email) && !$this->mailer->validateMailAddress($email)) {
365366
return new DataResponse(
366367
[
@@ -374,18 +375,50 @@ public function setUserSettings($avatarScope,
374375
}
375376
$user = $this->userSession->getUser();
376377
$data = $this->accountManager->getUser($user);
377-
$data[AccountManager::PROPERTY_AVATAR] = ['scope' => $avatarScope];
378+
if (!is_null($avatarScope)) {
379+
$data[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope;
380+
}
378381
if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) {
379-
$data[AccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope];
380-
$data[AccountManager::PROPERTY_EMAIL] = ['value' => $email, 'scope' => $emailScope];
382+
if (!is_null($displayname)) {
383+
$data[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $displayname;
384+
}
385+
if (!is_null($displaynameScope)) {
386+
$data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $displaynameScope;
387+
}
388+
if (!is_null($email)) {
389+
$data[IAccountManager::PROPERTY_EMAIL]['value'] = $email;
390+
}
391+
if (!is_null($emailScope)) {
392+
$data[IAccountManager::PROPERTY_EMAIL]['scope'] = $emailScope;
393+
}
381394
}
382395
if ($this->appManager->isEnabledForUser('federatedfilesharing')) {
383396
$shareProvider = \OC::$server->query(FederatedShareProvider::class);
384397
if ($shareProvider->isLookupServerUploadEnabled()) {
385-
$data[AccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope];
386-
$data[AccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope];
387-
$data[AccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope];
388-
$data[AccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope];
398+
if (!is_null($website)) {
399+
$data[IAccountManager::PROPERTY_WEBSITE]['value'] = $website;
400+
}
401+
if (!is_null($websiteScope)) {
402+
$data[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope;
403+
}
404+
if (!is_null($address)) {
405+
$data[IAccountManager::PROPERTY_ADDRESS]['value'] = $address;
406+
}
407+
if (!is_null($addressScope)) {
408+
$data[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope;
409+
}
410+
if (!is_null($phone)) {
411+
$data[IAccountManager::PROPERTY_PHONE]['value'] = $phone;
412+
}
413+
if (!is_null($phoneScope)) {
414+
$data[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope;
415+
}
416+
if (!is_null($twitter)) {
417+
$data[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter;
418+
}
419+
if (!is_null($twitterScope)) {
420+
$data[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope;
421+
}
389422
}
390423
}
391424
try {
@@ -395,15 +428,15 @@ public function setUserSettings($avatarScope,
395428
'status' => 'success',
396429
'data' => [
397430
'userId' => $user->getUID(),
398-
'avatarScope' => $data[AccountManager::PROPERTY_AVATAR]['scope'],
399-
'displayname' => $data[AccountManager::PROPERTY_DISPLAYNAME]['value'],
400-
'displaynameScope' => $data[AccountManager::PROPERTY_DISPLAYNAME]['scope'],
401-
'email' => $data[AccountManager::PROPERTY_EMAIL]['value'],
402-
'emailScope' => $data[AccountManager::PROPERTY_EMAIL]['scope'],
403-
'website' => $data[AccountManager::PROPERTY_WEBSITE]['value'],
404-
'websiteScope' => $data[AccountManager::PROPERTY_WEBSITE]['scope'],
405-
'address' => $data[AccountManager::PROPERTY_ADDRESS]['value'],
406-
'addressScope' => $data[AccountManager::PROPERTY_ADDRESS]['scope'],
431+
'avatarScope' => $data[IAccountManager::PROPERTY_AVATAR]['scope'],
432+
'displayname' => $data[IAccountManager::PROPERTY_DISPLAYNAME]['value'],
433+
'displaynameScope' => $data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'],
434+
'email' => $data[IAccountManager::PROPERTY_EMAIL]['value'],
435+
'emailScope' => $data[IAccountManager::PROPERTY_EMAIL]['scope'],
436+
'website' => $data[IAccountManager::PROPERTY_WEBSITE]['value'],
437+
'websiteScope' => $data[IAccountManager::PROPERTY_WEBSITE]['scope'],
438+
'address' => $data[IAccountManager::PROPERTY_ADDRESS]['value'],
439+
'addressScope' => $data[IAccountManager::PROPERTY_ADDRESS]['scope'],
407440
'message' => $this->l10n->t('Settings saved')
408441
]
409442
],

0 commit comments

Comments
 (0)