Merge pull request #56732 from nextcloud/techdebt/noid/migrate-settings-pii-listener-to-events

fix(settings): Migrate PII listener to IEventListener

Author: nickvergessen

apps/settings/lib/AppInfo/Application.php

@@ -85,7 +85,6 @@
 use OCP\AppFramework\Bootstrap\IBootstrap;
 use OCP\AppFramework\Bootstrap\IRegistrationContext;
 use OCP\AppFramework\IAppContainer;
-use OCP\AppFramework\QueryException;
 use OCP\Defaults;
 use OCP\Group\Events\GroupDeletedEvent;
 use OCP\Group\Events\UserAddedEvent;
@@ -94,6 +93,8 @@
 use OCP\Settings\Events\DeclarativeSettingsGetValueEvent;
 use OCP\Settings\Events\DeclarativeSettingsSetValueEvent;
 use OCP\Settings\IManager;
+use OCP\User\Events\PasswordUpdatedEvent;
+use OCP\User\Events\UserChangedEvent;
 use OCP\Util;
 
 class Application extends App implements IBootstrap {
@@ -121,6 +122,8 @@ public function register(IRegistrationContext $context): void {
 		$context->registerEventListener(UserAddedEvent::class, UserAddedToGroupActivityListener::class);
 		$context->registerEventListener(UserRemovedEvent::class, UserRemovedFromGroupActivityListener::class);
 		$context->registerEventListener(GroupDeletedEvent::class, GroupRemovedListener::class);
+		$context->registerEventListener(PasswordUpdatedEvent::class, Hooks::class);
+		$context->registerEventListener(UserChangedEvent::class, Hooks::class);
 
 		// Register Mail Provider listeners
 		$context->registerEventListener(DeclarativeSettingsGetValueEvent::class, MailProviderListener::class);
@@ -223,37 +226,5 @@ public function register(IRegistrationContext $context): void {
 	}
 
 	public function boot(IBootContext $context): void {
-		Util::connectHook('OC_User', 'post_setPassword', $this, 'onChangePassword');
-		Util::connectHook('OC_User', 'changeUser', $this, 'onChangeInfo');
-	}
-
-	/**
-	 * @param array $parameters
-	 * @throws \InvalidArgumentException
-	 * @throws \BadMethodCallException
-	 * @throws \Exception
-	 * @throws QueryException
-	 */
-	public function onChangePassword(array $parameters) {
-		/** @var Hooks $hooks */
-		$hooks = $this->getContainer()->query(Hooks::class);
-		$hooks->onChangePassword($parameters['uid']);
-	}
-
-	/**
-	 * @param array $parameters
-	 * @throws \InvalidArgumentException
-	 * @throws \BadMethodCallException
-	 * @throws \Exception
-	 * @throws QueryException
-	 */
-	public function onChangeInfo(array $parameters) {
-		if ($parameters['feature'] !== 'eMailAddress') {
-			return;
-		}
-
-		/** @var Hooks $hooks */
-		$hooks = $this->getContainer()->query(Hooks::class);
-		$hooks->onChangeEmail($parameters['user'], $parameters['old_value']);
 	}
 }

apps/settings/lib/Hooks.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -9,6 +11,8 @@
 use OCA\Settings\Activity\Provider;
 use OCP\Activity\IManager as IActivityManager;
 use OCP\Defaults;
+use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventListener;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IURLGenerator;
@@ -17,8 +21,13 @@
 use OCP\IUserSession;
 use OCP\L10N\IFactory;
 use OCP\Mail\IMailer;
+use OCP\User\Events\PasswordUpdatedEvent;
+use OCP\User\Events\UserChangedEvent;
 
-class Hooks {
+/**
+ * @template-implements IEventListener<PasswordUpdatedEvent|UserChangedEvent>
+ */
+class Hooks implements IEventListener {
 
 	public function __construct(
 		protected IActivityManager $activityManager,
@@ -33,16 +42,19 @@ public function __construct(
 	) {
 	}
 
-	/**
-	 * @param string $uid
-	 * @throws \InvalidArgumentException
-	 * @throws \BadMethodCallException
-	 * @throws \Exception
-	 */
-	public function onChangePassword($uid) {
-		$user = $this->userManager->get($uid);
+	public function handle(Event $event): void {
+		if ($event instanceof PasswordUpdatedEvent) {
+			$this->onChangePassword($event);
+		}
+		if ($event instanceof UserChangedEvent) {
+			$this->onChangeEmail($event);
+		}
+	}
 
-		if (!$user instanceof IUser || $user->getLastLogin() === 0) {
+	public function onChangePassword(PasswordUpdatedEvent $handle): void {
+		$user = $handle->getUser();
+
+		if ($user->getLastLogin() === 0) {
 			// User didn't login, so don't create activities and emails.
 			return;
 		}
@@ -89,6 +101,7 @@ public function onChangePassword($uid) {
 				'displayname' => $user->getDisplayName(),
 				'emailAddress' => $user->getEMailAddress(),
 				'instanceUrl' => $instanceUrl,
+				'event' => $handle,
 			]);
 
 			$template->setSubject($l->t('Password for %1$s changed on %2$s', [$user->getDisplayName(), $instanceName]));
@@ -105,13 +118,14 @@ public function onChangePassword($uid) {
 		}
 	}
 
-	/**
-	 * @param IUser $user
-	 * @param string|null $oldMailAddress
-	 * @throws \InvalidArgumentException
-	 * @throws \BadMethodCallException
-	 */
-	public function onChangeEmail(IUser $user, $oldMailAddress) {
+	public function onChangeEmail(UserChangedEvent $handle): void {
+		if ($handle->getFeature() !== 'eMailAddress') {
+			return;
+		}
+
+		$oldMailAddress = $handle->getOldValue();
+		$user = $handle->getUser();
+
 		if ($oldMailAddress === $user->getEMailAddress()
 			|| $user->getLastLogin() === 0) {
 			// Email didn't really change or user didn't login,

build/psalm-baseline.xml

@@ -2274,8 +2274,6 @@
       <code><![CDATA[IAppContainer]]></code>
     </DeprecatedInterface>
     <DeprecatedMethod>
-      <code><![CDATA[Util::connectHook('OC_User', 'changeUser', $this, 'onChangeInfo')]]></code>
-      <code><![CDATA[Util::connectHook('OC_User', 'post_setPassword', $this, 'onChangePassword')]]></code>
       <code><![CDATA[getConfig]]></code>
       <code><![CDATA[getCrypto]]></code>
       <code><![CDATA[getL10NFactory]]></code>