Make comments on account log entries private

Tool admins, checkusers will be able to see all account logs. Users will be able to see their own account log, even when their account is not active.
enwikipedia-acc · Dec 31, 2024 · 683bb0f · 683bb0f
1 parent f6cad71
commit 683bb0f
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 8 deletions.
diff --git a/includes/Helpers/IrcNotificationHelper.php b/includes/Helpers/IrcNotificationHelper.php
@@ -163,11 +163,10 @@ public function userApproved(User $user)
      * send a deactivated notification
      *
      * @param User   $user
-     * @param string $reason The reason the user has been deactivated
      */
-    public function userDeactivated(User $user, $reason)
+    public function userDeactivated(User $user)
     {
-        $this->send("{$user->getUsername()} deactivated by " . $this->currentUser->getUsername() . " ($reason)");
+        $this->send("{$user->getUsername()} deactivated by " . $this->currentUser->getUsername());
     }
 
     /**

diff --git a/includes/Helpers/LogHelper.php b/includes/Helpers/LogHelper.php
@@ -437,7 +437,12 @@ private static function getObjectDescription(
      *
      * @throws Exception
      */
-    public static function prepareLogsForTemplate(array $logs, PdoDatabase $database, SiteConfiguration $configuration): array
+    public static function prepareLogsForTemplate(
+        array $logs,
+        PdoDatabase $database,
+        SiteConfiguration $configuration,
+        ISecurityManager $securityManager
+    ): array
     {
         $userIds = array();
 
@@ -462,6 +467,22 @@ public static function prepareLogsForTemplate(array $logs, PdoDatabase $database
 
         $logData = array();
 
+        $currentUser = User::getCurrent($database);
+        $allowAccountLogSelf = false;
+        $allowAccountLog = false;
+
+        if ($securityManager->allows('UserData', 'accountLog', $currentUser) === ISecurityManager::ALLOWED) {
+            $allowAccountLog = true;
+        }
+        if ($securityManager->allows('UserData', 'accountLogSelf', $currentUser) === ISecurityManager::ALLOWED) {
+            $allowAccountLogSelf = true;
+        }
+
+        $protectedLogActions = [
+            'RequestedReactivation',
+            'DeactivatedUser',
+        ];
+
         foreach ($logs as $logEntry) {
             $objectDescription = self::getObjectDescription($logEntry->getObjectId(), $logEntry->getObjectType(),
                 $database, $configuration);
@@ -515,11 +536,24 @@ public static function prepareLogsForTemplate(array $logs, PdoDatabase $database
 
                 case 'JobCompleted':
                     break;
+
                 default:
                     $comment = $logEntry->getComment();
                     break;
             }
 
+            if (in_array($logEntry->getAction(), $protectedLogActions) && $logEntry->getObjectType() === 'User') {
+                if ($allowAccountLog) {
+                    // do nothing, allowed to see all account logs
+                }
+                else if ($allowAccountLogSelf && $currentUser->getId() === $logEntry->getObjectId()) {
+                    // do nothing, allowed to see own account log
+                }
+                else {
+                    $comment = null;
+                }
+            }
+
             $logData[] = array(
                 'timestamp'         => $logEntry->getTimestamp(),
                 'userid'            => $logEntry->getUser(),

diff --git a/includes/Pages/PageJobQueue.php b/includes/Pages/PageJobQueue.php
@@ -180,7 +180,7 @@ protected function view()
             $this->assign('log', array());
         }
         else {
-            list($users, $logData) = LogHelper::prepareLogsForTemplate($logs, $database, $this->getSiteConfiguration());
+            list($users, $logData) = LogHelper::prepareLogsForTemplate($logs, $database, $this->getSiteConfiguration(), $this->getSecurityManager());
 
             $this->assign("log", $logData);
             $this->assign("users", $users);

diff --git a/includes/Pages/PageLog.php b/includes/Pages/PageLog.php
@@ -66,7 +66,7 @@ protected function main()
         /** @var Log[] $logs */
         $logs = $logSearch->getRecordCount($count)->fetch();
 
-        list($users, $logData) = LogHelper::prepareLogsForTemplate($logs, $database, $this->getSiteConfiguration());
+        list($users, $logData) = LogHelper::prepareLogsForTemplate($logs, $database, $this->getSiteConfiguration(), $this->getSecurityManager());
 
         $this->setupPageData($count, array('filterUser' => $filterUser, 'filterAction' => $filterAction, 'filterObjectType' => $filterObjectType, 'filterObjectId' => $filterObjectId));
 

diff --git a/includes/Pages/PageUserManagement.php b/includes/Pages/PageUserManagement.php
@@ -278,7 +278,7 @@ protected function deactivate()
             $user->save();
             Logger::deactivatedUser($database, $user, $reason);
 
-            $this->getNotificationHelper()->userDeactivated($user, $reason);
+            $this->getNotificationHelper()->userDeactivated($user);
             SessionAlert::quick('Deactivated user ' . htmlentities($user->getUsername(), ENT_COMPAT, 'UTF-8'));
 
             // send email

diff --git a/includes/Pages/Statistics/StatsUsers.php b/includes/Pages/Statistics/StatsUsers.php
@@ -132,7 +132,7 @@ protected function detail()
             $this->assign('accountlog', array());
         }
         else {
-            list($users, $logData) = LogHelper::prepareLogsForTemplate($logs, $database, $this->getSiteConfiguration());
+            list($users, $logData) = LogHelper::prepareLogsForTemplate($logs, $database, $this->getSiteConfiguration(), $this->getSecurityManager());
 
             $this->assign("accountlog", $logData);
             $this->assign("users", $users);

diff --git a/includes/Security/RoleConfiguration.php b/includes/Security/RoleConfiguration.php
@@ -144,6 +144,9 @@ final class RoleConfiguration extends RoleConfigurationBase
             PageUserReactivate::class => array(
                 self::MAIN => self::ACCESS_ALLOW,
             ),
+            'UserData'                => array(
+                'accountLogSelf' => self::ACCESS_ALLOW,
+            ),
         ),
         'user'              => array(
             /*
@@ -311,6 +314,9 @@ final class RoleConfiguration extends RoleConfigurationBase
             PageDomainManagement::class          => array(
                 'edit'     => self::ACCESS_ALLOW,
             ),
+            'UserData'                           => array(
+                'accountLog' => self::ACCESS_ALLOW,
+            ),
         ),
         'checkuser'         => array(
             '_description'            => 'A user with CheckUser access',
@@ -335,6 +341,9 @@ final class RoleConfiguration extends RoleConfigurationBase
             'BanVisibility'             => array(
                 'checkuser' => self::ACCESS_ALLOW,
             ),
+            'UserData'                           => array(
+                'accountLog' => self::ACCESS_ALLOW,
+            ),
         ),
         'steward'         => array(
             '_description'  => 'A user with Steward access',