From 9ba4f24e93048a070c5890c0d0fc8f6004b6d835 Mon Sep 17 00:00:00 2001
From: vrana <jakubv@fb.com>
Date: Sun, 15 Jan 2012 01:07:56 -0800
Subject: [PATCH] Send 403 for admin pages without being admin

Summary: I've also moved the response generation for 404 from
##AphrontDefaultApplicationConfiguration## to ##buildResponseString()##

Test Plan:
Visit /
Visit /mail/
Visit /x/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, vrana

Differential Revision: https://secure.phabricator.com/D1406
---
 src/__phutil_library_map__.php                |  4 +-
 ...AphrontDefaultApplicationConfiguration.php | 16 -------
 .../default/configuration/__init__.php        |  1 -
 .../response/403/Aphront403Response.php       | 42 +++++++++++++++++++
 src/aphront/response/403/__init__.php         | 14 +++++++
 .../response/404/Aphront404Response.php       | 15 +++++--
 src/aphront/response/404/__init__.php         |  4 +-
 .../controller/base/PhabricatorController.php |  4 +-
 .../base/controller/base/__init__.php         |  2 +-
 .../PhabricatorCountdownDeleteController.php  |  4 +-
 .../countdown/controller/delete/__init__.php  |  1 +
 .../PhabricatorCountdownEditController.php    |  4 +-
 .../countdown/controller/edit/__init__.php    |  1 +
 .../PhabricatorFileAltViewController.php      |  4 +-
 .../files/controller/altview/__init__.php     |  1 +
 15 files changed, 86 insertions(+), 31 deletions(-)
 create mode 100644 src/aphront/response/403/Aphront403Response.php
 create mode 100644 src/aphront/response/403/__init__.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 34e8f179ca..0d89225cc1 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -10,6 +10,7 @@ phutil_register_library_map(array(
   array(
     'Aphront304Response' => 'aphront/response/304',
     'Aphront400Response' => 'aphront/response/400',
+    'Aphront403Response' => 'aphront/response/403',
     'Aphront404Response' => 'aphront/response/404',
     'AphrontAjaxResponse' => 'aphront/response/ajax',
     'AphrontApplicationConfiguration' => 'aphront/applicationconfiguration',
@@ -785,7 +786,8 @@ phutil_register_library_map(array(
   array(
     'Aphront304Response' => 'AphrontResponse',
     'Aphront400Response' => 'AphrontResponse',
-    'Aphront404Response' => 'AphrontResponse',
+    'Aphront403Response' => 'AphrontWebpageResponse',
+    'Aphront404Response' => 'AphrontWebpageResponse',
     'AphrontAjaxResponse' => 'AphrontResponse',
     'AphrontAttachedFileView' => 'AphrontView',
     'AphrontCSRFException' => 'AphrontException',
diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php
index 1586eebe11..bda20448dd 100644
--- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php
+++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php
@@ -466,22 +466,6 @@ class AphrontDefaultApplicationConfiguration
               'redirect' => $response->getURI(),
             ));
       }
-    } else if ($response instanceof Aphront404Response) {
-
-      $failure = new AphrontRequestFailureView();
-      $failure->setHeader('404 Not Found');
-      $failure->appendChild(
-        '<p>The page you requested was not found.</p>');
-
-      $view = new PhabricatorStandardPageView();
-      $view->setTitle('404 Not Found');
-      $view->setRequest($this->getRequest());
-      $view->appendChild($failure);
-
-      $response = new AphrontWebpageResponse();
-      $response->setContent($view->render());
-      $response->setHTTPResponseCode(404);
-      return $response;
     }
 
     return $response;
diff --git a/src/aphront/default/configuration/__init__.php b/src/aphront/default/configuration/__init__.php
index 9e2f375d92..287d42a3d2 100644
--- a/src/aphront/default/configuration/__init__.php
+++ b/src/aphront/default/configuration/__init__.php
@@ -17,7 +17,6 @@ phutil_require_module('phabricator', 'applications/people/storage/user');
 phutil_require_module('phabricator', 'infrastructure/env');
 phutil_require_module('phabricator', 'view/control/table');
 phutil_require_module('phabricator', 'view/dialog');
-phutil_require_module('phabricator', 'view/page/failure');
 phutil_require_module('phabricator', 'view/page/standard');
 
 phutil_require_module('phutil', 'error');
diff --git a/src/aphront/response/403/Aphront403Response.php b/src/aphront/response/403/Aphront403Response.php
new file mode 100644
index 0000000000..7227b22bfb
--- /dev/null
+++ b/src/aphront/response/403/Aphront403Response.php
@@ -0,0 +1,42 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * @group aphront
+ */
+class Aphront403Response extends AphrontWebpageResponse {
+
+  public function getHTTPResponseCode() {
+    return 403;
+  }
+
+  public function buildResponseString() {
+    $failure = new AphrontRequestFailureView();
+    $failure->setHeader('403 Forbidden');
+    $failure->appendChild(
+      '<p>You do not have privileges to access the requested page.</p>');
+
+    $view = new PhabricatorStandardPageView();
+    $view->setTitle('403 Forbidden');
+    $view->setRequest($this->getRequest());
+    $view->appendChild($failure);
+
+    return $view->render();
+  }
+
+}
diff --git a/src/aphront/response/403/__init__.php b/src/aphront/response/403/__init__.php
new file mode 100644
index 0000000000..aaaba162b2
--- /dev/null
+++ b/src/aphront/response/403/__init__.php
@@ -0,0 +1,14 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'aphront/response/webpage');
+phutil_require_module('phabricator', 'view/page/failure');
+phutil_require_module('phabricator', 'view/page/standard');
+
+
+phutil_require_source('Aphront403Response.php');
diff --git a/src/aphront/response/404/Aphront404Response.php b/src/aphront/response/404/Aphront404Response.php
index deaa8e42b0..a87686adb0 100644
--- a/src/aphront/response/404/Aphront404Response.php
+++ b/src/aphront/response/404/Aphront404Response.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,14 +19,23 @@
 /**
  * @group aphront
  */
-class Aphront404Response extends AphrontResponse {
+class Aphront404Response extends AphrontWebpageResponse {
 
   public function getHTTPResponseCode() {
     return 404;
   }
 
   public function buildResponseString() {
-    return '404 Not Found';
+    $failure = new AphrontRequestFailureView();
+    $failure->setHeader('404 Not Found');
+    $failure->appendChild('<p>The page you requested was not found.</p>');
+
+    $view = new PhabricatorStandardPageView();
+    $view->setTitle('404 Not Found');
+    $view->setRequest($this->getRequest());
+    $view->appendChild($failure);
+
+    return $view->render();
   }
 
 }
diff --git a/src/aphront/response/404/__init__.php b/src/aphront/response/404/__init__.php
index 149447909b..9ca287fea1 100644
--- a/src/aphront/response/404/__init__.php
+++ b/src/aphront/response/404/__init__.php
@@ -6,7 +6,9 @@
 
 
 
-phutil_require_module('phabricator', 'aphront/response/base');
+phutil_require_module('phabricator', 'aphront/response/webpage');
+phutil_require_module('phabricator', 'view/page/failure');
+phutil_require_module('phabricator', 'view/page/standard');
 
 
 phutil_require_source('Aphront404Response.php');
diff --git a/src/applications/base/controller/base/PhabricatorController.php b/src/applications/base/controller/base/PhabricatorController.php
index 4a46e0eea0..27f08fa3f8 100644
--- a/src/applications/base/controller/base/PhabricatorController.php
+++ b/src/applications/base/controller/base/PhabricatorController.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -83,7 +83,7 @@ abstract class PhabricatorController extends AphrontController {
     }
 
     if ($this->shouldRequireAdmin() && !$user->getIsAdmin()) {
-      return new Aphront404Response();
+      return new Aphront403Response();
     }
 
   }
diff --git a/src/applications/base/controller/base/__init__.php b/src/applications/base/controller/base/__init__.php
index 4273a1a029..71078ccc60 100644
--- a/src/applications/base/controller/base/__init__.php
+++ b/src/applications/base/controller/base/__init__.php
@@ -8,7 +8,7 @@
 
 phutil_require_module('phabricator', 'aphront/console/core');
 phutil_require_module('phabricator', 'aphront/controller');
-phutil_require_module('phabricator', 'aphront/response/404');
+phutil_require_module('phabricator', 'aphront/response/403');
 phutil_require_module('phabricator', 'aphront/response/webpage');
 phutil_require_module('phabricator', 'applications/people/storage/user');
 phutil_require_module('phabricator', 'infrastructure/env');
diff --git a/src/applications/countdown/controller/delete/PhabricatorCountdownDeleteController.php b/src/applications/countdown/controller/delete/PhabricatorCountdownDeleteController.php
index d2c46721a6..97e6963609 100644
--- a/src/applications/countdown/controller/delete/PhabricatorCountdownDeleteController.php
+++ b/src/applications/countdown/controller/delete/PhabricatorCountdownDeleteController.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -35,7 +35,7 @@ class PhabricatorCountdownDeleteController
 
     if (($timer->getAuthorPHID() !== $user->getPHID())
         && $user->getIsAdmin() === false) {
-      return new Aphront404Response();
+      return new Aphront403Response();
     }
 
     if ($request->isFormPost()) {
diff --git a/src/applications/countdown/controller/delete/__init__.php b/src/applications/countdown/controller/delete/__init__.php
index 55062778dc..95bb57fe15 100644
--- a/src/applications/countdown/controller/delete/__init__.php
+++ b/src/applications/countdown/controller/delete/__init__.php
@@ -6,6 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'aphront/response/403');
 phutil_require_module('phabricator', 'aphront/response/404');
 phutil_require_module('phabricator', 'aphront/response/dialog');
 phutil_require_module('phabricator', 'aphront/response/redirect');
diff --git a/src/applications/countdown/controller/edit/PhabricatorCountdownEditController.php b/src/applications/countdown/controller/edit/PhabricatorCountdownEditController.php
index 50dbcf9693..854e395aae 100644
--- a/src/applications/countdown/controller/edit/PhabricatorCountdownEditController.php
+++ b/src/applications/countdown/controller/edit/PhabricatorCountdownEditController.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,7 +39,7 @@ class PhabricatorCountdownEditController
 
       if (($timer->getAuthorPHID() != $user->getPHID())
           && $user->getIsAdmin() == false) {
-        return new Aphront404Response();
+        return new Aphront403Response();
       }
 
       $action_label = 'Update Timer';
diff --git a/src/applications/countdown/controller/edit/__init__.php b/src/applications/countdown/controller/edit/__init__.php
index 4f2100659c..18e5e4627d 100644
--- a/src/applications/countdown/controller/edit/__init__.php
+++ b/src/applications/countdown/controller/edit/__init__.php
@@ -6,6 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'aphront/response/403');
 phutil_require_module('phabricator', 'aphront/response/404');
 phutil_require_module('phabricator', 'aphront/response/redirect');
 phutil_require_module('phabricator', 'applications/countdown/controller/base');
diff --git a/src/applications/files/controller/altview/PhabricatorFileAltViewController.php b/src/applications/files/controller/altview/PhabricatorFileAltViewController.php
index 01b042cdb2..37e9104b33 100644
--- a/src/applications/files/controller/altview/PhabricatorFileAltViewController.php
+++ b/src/applications/files/controller/altview/PhabricatorFileAltViewController.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -52,7 +52,7 @@ class PhabricatorFileAltViewController extends PhabricatorFileController {
     }
 
     if (!$file->validateSecretKey($this->key)) {
-      return new Aphront404Response();
+      return new Aphront403Response();
     }
 
     // It's safe to bypass view restrictions because we know we are being served
diff --git a/src/applications/files/controller/altview/__init__.php b/src/applications/files/controller/altview/__init__.php
index a447af1b27..02d23e0bfc 100644
--- a/src/applications/files/controller/altview/__init__.php
+++ b/src/applications/files/controller/altview/__init__.php
@@ -7,6 +7,7 @@
 
 
 phutil_require_module('phabricator', 'aphront/response/400');
+phutil_require_module('phabricator', 'aphront/response/403');
 phutil_require_module('phabricator', 'aphront/response/404');
 phutil_require_module('phabricator', 'aphront/response/file');
 phutil_require_module('phabricator', 'applications/files/controller/base');