From a804f0ab939dd959923c1f4e07815bdc29649e98 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 2 Apr 2015 13:41:39 -0700
Subject: [PATCH] Make file policies for emailed files more consistent

Summary:
Fixes T7712. Currently, files sent via email get default policies, like they were dragged and dropped onto the home page.

User expectation is better aligned with giving files more restrictive policies, like they were draggged and dropped directly onto an object.

Make files sent via email have restricted default visibility. Once we identify the sender, set them as the file author. Later, the file will become visible to other users via attachment to a task, revision, etc.

Test Plan: Sent some files via email; verified they got restrictive policies, correct authorship, and appropriate object attachment.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7712

Differential Revision: https://secure.phabricator.com/D12255
---
 scripts/mail/mail_handler.php                       |  1 +
 .../PhabricatorMetaMTAMailgunReceiveController.php  |  2 +-
 .../PhabricatorMetaMTASendGridReceiveController.php |  2 +-
 .../replyhandler/PhabricatorMailReplyHandler.php    |  5 +----
 .../storage/PhabricatorMetaMTAReceivedMail.php      | 13 +++++++++++++
 5 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/scripts/mail/mail_handler.php b/scripts/mail/mail_handler.php
index 214b51ee1a..0341a0e464 100755
--- a/scripts/mail/mail_handler.php
+++ b/scripts/mail/mail_handler.php
@@ -77,6 +77,7 @@ foreach ($parser->getAttachments() as $attachment) {
     $attachment->getContent(),
     array(
       'name' => $attachment->getFilename(),
+      'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
     ));
   $attachments[] = $file->getPHID();
 }
diff --git a/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php b/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php
index d6bd298504..ce27366da3 100644
--- a/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php
+++ b/src/applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php
@@ -56,7 +56,7 @@ final class PhabricatorMetaMTAMailgunReceiveController
         $file = PhabricatorFile::newFromPHPUpload(
           $file_raw,
           array(
-            'authorPHID' => $user->getPHID(),
+            'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
           ));
         $file_phids[] = $file->getPHID();
       } catch (Exception $ex) {
diff --git a/src/applications/metamta/controller/PhabricatorMetaMTASendGridReceiveController.php b/src/applications/metamta/controller/PhabricatorMetaMTASendGridReceiveController.php
index 7df1539946..11a0184ffd 100644
--- a/src/applications/metamta/controller/PhabricatorMetaMTASendGridReceiveController.php
+++ b/src/applications/metamta/controller/PhabricatorMetaMTASendGridReceiveController.php
@@ -42,7 +42,7 @@ final class PhabricatorMetaMTASendGridReceiveController
         $file = PhabricatorFile::newFromPHPUpload(
           $file_raw,
           array(
-            'authorPHID' => $user->getPHID(),
+            'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
           ));
         $file_phids[] = $file->getPHID();
       } catch (Exception $ex) {
diff --git a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php
index 3d32f308d3..697c0622fe 100644
--- a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php
+++ b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php
@@ -327,11 +327,8 @@ abstract class PhabricatorMailReplyHandler {
       return $body;
     }
 
-    // NOTE: This is safe, but not entirely correct. Clean it up after
-    // T7712. These files have the install-default policy right now, which
-    // may or may not be permissive.
     $files = id(new PhabricatorFileQuery())
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->setViewer($this->getActor())
       ->withPHIDs($attachments)
       ->execute();
 
diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php
index c157537d59..fa5ad7f752 100644
--- a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php
+++ b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php
@@ -121,6 +121,19 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
 
       $this->setAuthorPHID($sender->getPHID());
 
+      // Now that we've identified the sender, mark them as the author of
+      // any attached files.
+      $attachments = $this->getAttachments();
+      if ($attachments) {
+        $files = id(new PhabricatorFileQuery())
+          ->setViewer(PhabricatorUser::getOmnipotentUser())
+          ->withPHIDs($attachments)
+          ->execute();
+        foreach ($files as $file) {
+          $file->setAuthorPHID($sender->getPHID())->save();
+        }
+      }
+
       $receiver->receiveMail($this, $sender);
     } catch (PhabricatorMetaMTAReceivedMailProcessingException $ex) {
       switch ($ex->getStatusCode()) {