From 066d90606147bf2d8fda68dd016b8ee1e5c6a53b Mon Sep 17 00:00:00 2001 From: Peter Ringelmann Date: Mon, 3 Aug 2026 09:10:03 +0200 Subject: [PATCH] fix(provisioning_api): read newUser.sendEmail as a boolean Signed-off-by: Peter Ringelmann --- .../lib/Controller/UsersController.php | 2 +- .../tests/Controller/UsersControllerTest.php | 61 +++++++++++++++++++ build/psalm-baseline.xml | 1 - core/Command/User/Add.php | 2 +- tests/Core/Command/User/AddTest.php | 4 +- tests/lib/AppConfigIntegrationTest.php | 28 +++++++++ 6 files changed, 93 insertions(+), 5 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 3f3b60af19460..8cd8cc00a2021 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -599,7 +599,7 @@ public function addUser( // Send new user mail only if a mail is set if ($email !== '') { $newUser->setSystemEMailAddress($email); - if ($this->config->getAppValue('core', 'newUser.sendEmail', 'yes') === 'yes') { + if ($this->appConfig->getValueBool('core', 'newUser.sendEmail', true)) { try { $emailTemplate = $this->newUserMailHelper->generateTemplate($newUser, $generatePasswordResetToken); $this->newUserMailHelper->sendMail($newUser, $emailTemplate); diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index cd0f7033896a4..56e8ff0a8fd7c 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -738,6 +738,67 @@ public function testAddUserSuccessfulGeneratePassword(): void { )); } + /** + * `newUser.sendEmail` has to be read as a boolean. It is stored as an untyped + * 'yes'/'no' string on instances created before Nextcloud 33 and as a typed + * boolean once the account settings toggle has been used, so comparing it to + * the string 'yes' silently skipped the mail on upgraded instances. + */ + #[\PHPUnit\Framework\Attributes\DataProvider('dataAddUserWelcomeMail')] + public function testAddUserSendsWelcomeMailWhenEnabled(bool $enabled): void { + $this->appConfig + ->expects($this->atLeastOnce()) + ->method('getValueBool') + ->with('core', 'newUser.sendEmail', true) + ->willReturn($enabled); + + $newUser = $this->createMock(IUser::class); + $newUser->expects($this->once()) + ->method('setSystemEMailAddress') + ->with('foo@bar.com'); + $this->userManager + ->expects($this->once()) + ->method('userExists') + ->with('NewUser') + ->willReturn(false); + $this->userManager + ->expects($this->once()) + ->method('createUser') + ->willReturn($newUser); + $loggedInUser = $this->createMock(IUser::class); + $loggedInUser + ->method('getUID') + ->willReturn('adminUser'); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->willReturn($loggedInUser); + $this->groupManager + ->expects($this->once()) + ->method('isAdmin') + ->with('adminUser') + ->willReturn(true); + + $emailTemplate = $this->createMock(IEMailTemplate::class); + $this->newUserMailHelper + ->expects($enabled ? $this->once() : $this->never()) + ->method('generateTemplate') + ->willReturn($emailTemplate); + $this->newUserMailHelper + ->expects($enabled ? $this->once() : $this->never()) + ->method('sendMail') + ->with($newUser, $emailTemplate); + + $this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', 'foo@bar.com'); + } + + public static function dataAddUserWelcomeMail(): array { + return [ + 'enabled' => [true], + 'disabled' => [false], + ]; + } + public function testAddUserSuccessfulLowercaseEmail(): void { $this->userManager ->expects($this->once()) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index bc1dfda9aca42..112c03141356f 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2269,7 +2269,6 @@ - diff --git a/core/Command/User/Add.php b/core/Command/User/Add.php index 35a4dca5cc10e..2255cd1740ed0 100644 --- a/core/Command/User/Add.php +++ b/core/Command/User/Add.php @@ -182,7 +182,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $user->setSystemEMailAddress($email); - if ($this->appConfig->getValueString('core', 'newUser.sendEmail', 'yes') === 'yes') { + if ($this->appConfig->getValueBool('core', 'newUser.sendEmail', true)) { try { $this->mailHelper->sendMail($user, $this->mailHelper->generateTemplate($user, true)); $output->writeln('Welcome email sent to ' . $email); diff --git a/tests/Core/Command/User/AddTest.php b/tests/Core/Command/User/AddTest.php index 98d11b0214c11..59ce5ca0f908c 100644 --- a/tests/Core/Command/User/AddTest.php +++ b/tests/Core/Command/User/AddTest.php @@ -100,8 +100,8 @@ public function testAddEmail( $this->userManager->method('createUser') ->willReturn($this->user); - $this->appConfig->method('getValueString') - ->willReturn($shouldSendEmail ? 'yes' : 'no'); + $this->appConfig->method('getValueBool') + ->willReturn($shouldSendEmail); $this->mailHelper->method('generateTemplate') ->willReturn(static::createMock(IEMailTemplate::class)); diff --git a/tests/lib/AppConfigIntegrationTest.php b/tests/lib/AppConfigIntegrationTest.php index cd314fb9ee19c..19a04f7234ae0 100644 --- a/tests/lib/AppConfigIntegrationTest.php +++ b/tests/lib/AppConfigIntegrationTest.php @@ -493,6 +493,34 @@ public function testGetValueBool(): void { $this->assertSame(true, $config->getValueBool('typed', 'bool')); } + /** + * Untyped values predate the typed config API and are still in the database of + * every upgraded instance, so they have to keep resolving to a boolean. The + * deprecated setter is used on purpose, as it is the only way to write a value + * without a type. + */ + #[\PHPUnit\Framework\Attributes\DataProvider('dataUntypedBool')] + public function testGetValueBoolOnUntypedValue(string $stored, bool $expected): void { + /** @var AppConfig $config */ + $config = $this->generateAppConfig(); + $config->setValue('feed', 'untyped-bool', $stored); + + $this->assertSame($expected, $config->getValueBool('feed', 'untyped-bool')); + } + + public static function dataUntypedBool(): array { + return [ + 'yes' => ['yes', true], + 'no' => ['no', false], + 'true' => ['true', true], + 'false' => ['false', false], + 'on' => ['on', true], + '1' => ['1', true], + '0' => ['0', false], + 'empty' => ['', false], + ]; + } + public function testGetValueBoolOnUnknownAppReturnsDefault(): void { $config = $this->generateAppConfig(); $this->assertSame(false, $config->getValueBool('typed-1', 'bool', false));