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
phacility · Apr 2, 2015 · a804f0a · a804f0a
1 parent e74c386
commit a804f0a
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 6 deletions.
diff --git a/scripts/mail/mail_handler.php b/scripts/mail/mail_handler.php
@@ -77,6 +77,7 @@
     $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
@@ -56,7 +56,7 @@ public function processRequest() {
         $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
@@ -42,7 +42,7 @@ public function processRequest() {
         $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
@@ -327,11 +327,8 @@ final protected function enhanceBodyWithAttachments(
       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
@@ -121,6 +121,19 @@ public function processReceivedMail() {
 
       $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()) {