From 5938d768d6ba6a76bd7f9f417adaa1077cad159d Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 6 Apr 2016 16:19:03 -0700
Subject: [PATCH] Don't dead-end users with out-of-date links to files

Summary: Ref T10262. Instead of dumping an unhelpful 403 "ACCESS DENIED" page on users, explain the most likely cause of the issue and give them a link to return to the file detail page to learn more or get an up-to-date link.

Test Plan: Hit both errors, had a lovely experience with the helpful dialog text.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10262

Differential Revision: https://secure.phabricator.com/D15650
---
 .../PhabricatorFileDataController.php         | 38 +++++++++++++++----
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/src/applications/files/controller/PhabricatorFileDataController.php b/src/applications/files/controller/PhabricatorFileDataController.php
index fc0f2114e9..e7ff2827e5 100644
--- a/src/applications/files/controller/PhabricatorFileDataController.php
+++ b/src/applications/files/controller/PhabricatorFileDataController.php
@@ -119,22 +119,46 @@ final class PhabricatorFileDataController extends PhabricatorFileController {
       return new Aphront404Response();
     }
 
+    // We may be on the CDN domain, so we need to use a fully-qualified URI
+    // here to make sure we end up back on the main domain.
+    $info_uri = PhabricatorEnv::getURI($file->getInfoURI());
+
+
     if (!$file->validateSecretKey($this->key)) {
-      return new Aphront403Response();
+      $dialog = $this->newDialog()
+        ->setTitle(pht('Invalid Authorization'))
+        ->appendParagraph(
+          pht(
+            'The link you followed to access this file is no longer '.
+            'valid. The visibility of the file may have changed after '.
+            'the link was generated.'))
+        ->appendParagraph(
+          pht(
+            'You can continue to the file detail page to get more '.
+            'information and attempt to access the file.'))
+        ->addCancelButton($info_uri, pht('Continue'));
+
+      return id(new AphrontDialogResponse())
+        ->setDialog($dialog)
+        ->setHTTPResponseCode(404);
     }
 
     if ($file->getIsPartial()) {
-      // We may be on the CDN domain, so we need to use a fully-qualified URI
-      // here to make sure we end up back on the main domain.
-      $info_uri = PhabricatorEnv::getURI($file->getInfoURI());
-
-      return $this->newDialog()
+      $dialog = $this->newDialog()
         ->setTitle(pht('Partial Upload'))
         ->appendParagraph(
           pht(
             'This file has only been partially uploaded. It must be '.
             'uploaded completely before you can download it.'))
-        ->addCancelButton($info_uri);
+        ->appendParagraph(
+          pht(
+            'You can continue to the file detail page to monitor the '.
+            'upload progress of the file.'))
+        ->addCancelButton($info_uri, pht('Continue'));
+
+      return id(new AphrontDialogResponse())
+        ->setDialog($dialog)
+        ->setHTTPResponseCode(404);
     }
 
     $this->file = $file;