Skip to content

Commit c02aafb

Browse files
committed
fix(restrictions): Apply the app config overwrite to the typed getters
AppConfigOverwrite only overrode the deprecated getValue(). Server code has since moved to the typed getters, so the overwrite that lateSetupRestrictions() installs to force `shareapi_only_share_with_group_members` when "Hide other accounts from guests" is enabled was silently ignored by them. Most notably OC\Collaboration\Collaborators\UserPlugin reads the option with getValueString(), so the share recipient autocompletion offered guests every account of the instance instead of only the members of their own groups. OCA\Guests\Config::isSharingRestrictedToGroup() reads it with getValueBool() and was equally unaffected by the overwrite. Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
1 parent 2e92437 commit c02aafb

3 files changed

Lines changed: 79 additions & 0 deletions

File tree

lib/AppConfigOverwrite.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,22 @@ public function getValue($app, $key, $default = '') {
3030

3131
return parent::getValue($app, $key, $default);
3232
}
33+
34+
#[\Override]
35+
public function getValueString(string $app, string $key, string $default = '', bool $lazy = false): string {
36+
if (isset($this->overWrite[$app]) && isset($this->overWrite[$app][$key])) {
37+
return $this->overWrite[$app][$key];
38+
}
39+
40+
return parent::getValueString($app, $key, $default, $lazy);
41+
}
42+
43+
#[\Override]
44+
public function getValueBool(string $app, string $key, bool $default = false, bool $lazy = false): bool {
45+
if (isset($this->overWrite[$app]) && isset($this->overWrite[$app][$key])) {
46+
return in_array(strtolower($this->overWrite[$app][$key]), ['1', 'true', 'yes', 'on'], true);
47+
}
48+
49+
return parent::getValueBool($app, $key, $default, $lazy);
50+
}
3351
}

tests/stub.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,12 @@ public function __construct(
133133
*/
134134
public function getValue($app, $key, $default = '') {
135135
}
136+
137+
public function getValueString(string $app, string $key, string $default = '', bool $lazy = false): string {
138+
}
139+
140+
public function getValueBool(string $app, string $key, bool $default = false, bool $lazy = false): bool {
141+
}
136142
}
137143
}
138144

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Guests\Test\Unit;
10+
11+
use OCA\Guests\AppConfigOverwrite;
12+
use OCP\Server;
13+
use Test\TestCase;
14+
15+
/**
16+
* @group DB
17+
*/
18+
class AppConfigOverwriteTest extends TestCase {
19+
private const KEY = 'shareapi_only_share_with_group_members';
20+
21+
private ?AppConfigOverwrite $appConfig = null;
22+
23+
protected function setUp(): void {
24+
parent::setUp();
25+
26+
$this->appConfig = Server::get(AppConfigOverwrite::class);
27+
$this->appConfig->setOverwrite(['core' => [self::KEY => 'yes']]);
28+
}
29+
30+
protected function tearDown(): void {
31+
$this->appConfig->setOverwrite([]);
32+
33+
parent::tearDown();
34+
}
35+
36+
/**
37+
* The server reads the overwritten values through several getters:
38+
* IConfig::getAppValue() ends up in getValue(), the share recipient
39+
* autocompletion uses getValueString() and OCA\Guests\Config uses
40+
* getValueBool(). All of them have to see the overwrite.
41+
*/
42+
public function testOverwriteIsAppliedToAllGetters(): void {
43+
$this->assertSame('yes', $this->appConfig->getValue('core', self::KEY, 'no'));
44+
$this->assertSame('yes', $this->appConfig->getValueString('core', self::KEY, 'no'));
45+
$this->assertTrue($this->appConfig->getValueBool('core', self::KEY));
46+
}
47+
48+
public function testWithoutOverwriteTheStoredValueIsUsed(): void {
49+
$this->appConfig->setOverwrite([]);
50+
51+
$this->assertSame('no', $this->appConfig->getValue('core', self::KEY, 'no'));
52+
$this->assertSame('no', $this->appConfig->getValueString('core', self::KEY, 'no'));
53+
$this->assertFalse($this->appConfig->getValueBool('core', self::KEY));
54+
}
55+
}

0 commit comments

Comments
 (0)