From 7429da91d293477019a3bb3219a74c895167a70f Mon Sep 17 00:00:00 2001
From: Valerio Bozzolan <nsa+gitspam@succhia.cz>
Date: Wed, 11 Dec 2024 09:24:47 +0100
Subject: [PATCH] Repository Identity "Automatically Detected User": don't
 trust unverified emails

Summary:
Make sure that Repository Diffusion Identities "Automatically Detected User " are not created from unverified emails.

Closes T15965

Test Plan:
Find at least one identity that is assigned to nobody:

http://phorge.localhost/diffusion/identity/

(For example, you may easily find an identity of "GitHub <noreply@github.com>")

(Double check that its "Assigned To" is unset or make sure it's unset for this test)

Be evil: add *that* email in your {nav Profile > Settings > Email addresses}. So, for example add "noreply@github.com", like a rogue. The email can stay unverified.

Run this command to immediately cause an effect:

    ./bin/repository rebuild-identities --all-identities

- before this change, you can reproduce that you successfully steal that identity and you become "GitHub" or whoever
- after this change, you see that "Automatically Detected User" is unset again
- after this change, any other identity manually assigned, is still assigned to that value
- after this change, any other identity automatically assigned to verified emails, are still "Automatically Detected User"

Reviewers: O1 Blessed Committers, speck, 20after4

Reviewed By: O1 Blessed Committers, speck, 20after4

Subscribers: aklapper, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15965

Differential Revision: https://we.phorge.it/D25845
---
 .../query/DiffusionResolveUserQuery.php       |  3 ++-
 .../people/storage/PhabricatorUser.php        | 22 +++++++++++++++++++
 ...oryManagementRebuildIdentitiesWorkflow.php |  2 +-
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/src/applications/diffusion/query/DiffusionResolveUserQuery.php b/src/applications/diffusion/query/DiffusionResolveUserQuery.php
index 8ad13f660e..3d9f4054c3 100644
--- a/src/applications/diffusion/query/DiffusionResolveUserQuery.php
+++ b/src/applications/diffusion/query/DiffusionResolveUserQuery.php
@@ -93,7 +93,8 @@ final class DiffusionResolveUserQuery extends Phobject {
 
 
   private function findUserByEmailAddress($email_address) {
-    $by_email = PhabricatorUser::loadOneWithEmailAddress($email_address);
+    $by_email =
+      PhabricatorUser::loadOneWithVerifiedEmailAddress($email_address);
 
     if ($by_email) {
       return $by_email->getPHID();
diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php
index 2c9cf9f877..70c6c1e52b 100644
--- a/src/applications/people/storage/PhabricatorUser.php
+++ b/src/applications/people/storage/PhabricatorUser.php
@@ -652,6 +652,28 @@ final class PhabricatorUser
     return $this->getUsername();
   }
 
+  /**
+   * Load one user from their verified email address.
+   * @param string $address
+   * @return PhabricatorUser|null
+   */
+  public static function loadOneWithVerifiedEmailAddress($address) {
+    $email = id(new PhabricatorUserEmail())->loadOneWhere(
+      'address = %s AND isVerified = 1',
+      $address);
+    if (!$email) {
+      return null;
+    }
+    return id(new self())->loadOneWhere(
+      'phid = %s',
+      $email->getUserPHID());
+  }
+
+  /**
+   * Load one user from their potentially unverified email address.
+   * @param string $address
+   * @return PhabricatorUser|null
+   */
   public static function loadOneWithEmailAddress($address) {
     $email = id(new PhabricatorUserEmail())->loadOneWhere(
       'address = %s',
diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementRebuildIdentitiesWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementRebuildIdentitiesWorkflow.php
index 9a0f68713e..9df88ad055 100644
--- a/src/applications/repository/management/PhabricatorRepositoryManagementRebuildIdentitiesWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementRebuildIdentitiesWorkflow.php
@@ -281,7 +281,7 @@ final class PhabricatorRepositoryManagementRebuildIdentitiesWorkflow
           $user->getMonogram()));
 
       $emails = id(new PhabricatorUserEmail())->loadAllWhere(
-        'userPHID = %s',
+        'userPHID = %s AND isVerified = 1',
         $user->getPHID());
       if ($emails) {
         $this->rebuildEmails(mpull($emails, 'getAddress'));