Page MenuHomePhorge
Files F2893128

No OneTemporary
Actions

View Options

diff --git a/src/applications/base/controller/Phabricator404Controller.php b/src/applications/base/controller/Phabricator404Controller.php
index 1aa785a040..e47c51fd4b 100644
--- a/src/applications/base/controller/Phabricator404Controller.php
+++ b/src/applications/base/controller/Phabricator404Controller.php
@@ -1,9 +1,38 @@
<?php
final class Phabricator404Controller extends PhabricatorController {
+ public function shouldRequireLogin() {
+
+ // NOTE: See T2102 for discussion. When a logged-out user requests a page,
+ // we give them a login form and set a `next_uri` cookie so we send them
+ // back to the page they requested after they login. However, some browsers
+ // or extensions request resources which may not exist (like
+ // "apple-touch-icon.png" and "humans.txt") and these requests may overwrite
+ // the stored "next_uri" after the login page loads. Our options for dealing
+ // with this are all bad:
+ //
+ // 1. We can't put the URI in the form because some login methods (OAuth2)
+ // issue redirects to third-party sites. After T1536 we might be able
+ // to.
+ // 2. We could set the cookie only if it doesn't exist, but then a user who
+ // declines to login will end up in the wrong place if they later do
+ // login.
+ // 3. We can blacklist all the resources browsers request, but this is a
+ // mess.
+ // 4. We can just allow users to access the 404 page without login, so
+ // requesting bad URIs doesn't set the cookie.
+ //
+ // This implements (4). The main downside is that an attacker can now detect
+ // if a URI is routable (i.e., some application is installed) by testing for
+ // 404 vs login. If possible, we should implement T1536 in such a way that
+ // we can pass the next URI through the login process.
+
+ return false;
+ }
+
public function processRequest() {
return new Aphront404Response();
}
}

File Metadata

Mime Type
text/x-diff
Expires
Sun, Jan 19, 17:55 (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1127173
Default Alt Text
(1 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0