From 41f143f7feeb2be0da6e7b35c75bcbd514f3c48c Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 23 Jan 2020 17:04:56 -0800
Subject: [PATCH] Respect repository identities when figuring out
 authors/committers in Herald pre-commit hook rules

Summary:
Ref T13480. Currently, Herald commit hook rules use a raw address resolution query to identify the author and committer for a commit. This will get the wrong answer when the raw identity string has been explicitly bound to some non-default user (most often, it will fail to identify an author when one exists).

Instead, use the "IdentityEngine" to properly resolve identities.

Test Plan: Authored a commit as `X <y@example.com>`, a raw identity with no "natural" matches to users (e.g., no user with that email or username). Bound the identity to a particular user in Diffusion. Wrote a Herald pre-commit content rule, pushed the commit. Saw Herald recognize the correct user when evaluating rules.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20953
---
 .../herald/HeraldPreCommitContentAdapter.php  | 38 +++++++++++++++++--
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
index cee5f97419..7e56074303 100644
--- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
+++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
@@ -8,6 +8,7 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter {
   private $revision = false;
 
   private $affectedPackages;
+  private $identityCache = array();
 
   public function getAdapterContentName() {
     return pht('Commit Hook: Commit Content');
@@ -166,10 +167,39 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter {
     }
   }
 
-  private function lookupUser($author) {
-    return id(new DiffusionResolveUserQuery())
-      ->withName($author)
-      ->execute();
+  private function lookupUser($raw_identity) {
+    // See T13480. After the move to repository identities, we want to look
+    // users up in the identity table. If you push a commit which is authored
+    // by "A Duck <duck@example.org>" and that identity is bound to user
+    // "@mallard" in the identity table, Herald should see the author of the
+    // commit as "@mallard" when evaluating pre-commit content rules.
+
+    if (!array_key_exists($raw_identity, $this->identityCache)) {
+      $repository = $this->getHookEngine()->getRepository();
+      $viewer = $this->getHookEngine()->getViewer();
+
+      $identity_engine = id(new DiffusionRepositoryIdentityEngine())
+        ->setViewer($viewer);
+
+      // We must provide a "sourcePHID" when resolving identities, but don't
+      // have a legitimate one yet. Just use the repository PHID as a
+      // reasonable value. This won't actually be written to storage.
+      $source_phid = $repository->getPHID();
+      $identity_engine->setSourcePHID($source_phid);
+
+      // If the identity doesn't exist yet, we don't want to create it if
+      // we haven't seen it before. It will be created later when we actually
+      // import the commit.
+      $identity_engine->setDryRun(true);
+
+      $author_identity = $identity_engine->newResolvedIdentity($raw_identity);
+
+      $effective_phid = $author_identity->getCurrentEffectiveUserPHID();
+
+      $this->identityCache[$raw_identity] = $effective_phid;
+    }
+
+    return $this->identityCache[$raw_identity];
   }
 
   private function getCommitFields() {