Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14839667
D8057.id18232.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
5 KB
Referenced Files
None
Subscribers
None
D8057.id18232.diff
View Options
Index: src/aphront/AphrontRequest.php
===================================================================
--- src/aphront/AphrontRequest.php
+++ src/aphront/AphrontRequest.php
@@ -1,10 +1,9 @@
<?php
/**
+ * @task data Accessing Request Data
+ * @task cookie Managing Cookies
*
- * @task data Accessing Request Data
- *
- * @group aphront
*/
final class AphrontRequest {
@@ -297,62 +296,76 @@
unset($_COOKIE[$name]);
}
- final public function setCookie($name, $value, $expire = null) {
-
- $is_secure = false;
+ /**
+ * Get the domain which cookies should be set on for this request, or null
+ * if the request does not correspond to a valid cookie domain.
+ *
+ * @return PhutilURI|null Domain URI, or null if no valid domain exists.
+ *
+ * @task cookie
+ */
+ private function getCookieDomainURI() {
+ $host = $this->getHost();
- // If a base URI has been configured, ensure cookies are only set on that
- // domain. Also, use the URI protocol to control SSL-only cookies.
+ // If there's no base domain configured, just use whatever the request
+ // domain is. This makes setup easier, and we'll tell administrators to
+ // configure a base domain during the setup process.
$base_uri = PhabricatorEnv::getEnvConfig('phabricator.base-uri');
- if ($base_uri) {
- $alternates = PhabricatorEnv::getEnvConfig('phabricator.allowed-uris');
- $allowed_uris = array_merge(
- array($base_uri),
- $alternates);
-
- $host = $this->getHost();
-
- $match = null;
- foreach ($allowed_uris as $allowed_uri) {
- $uri = new PhutilURI($allowed_uri);
- $domain = $uri->getDomain();
- if ($host == $domain) {
- $match = $uri;
- break;
- }
- }
+ if (!strlen($base_uri)) {
+ return new PhutilURI('http://'.$host.'/');
+ }
- if ($match === null) {
- if (count($allowed_uris) > 1) {
- throw new Exception(
- pht(
- 'This Phabricator install is configured as "%s", but you are '.
- 'accessing it via "%s". Access Phabricator via the primary '.
- 'configured domain, or one of the permitted alternate '.
- 'domains: %s. Phabricator will not set cookies on other domains '.
- 'for security reasons.',
- $base_uri,
- $host,
- implode(', ', $alternates)));
- } else {
- throw new Exception(
- pht(
- 'This Phabricator install is configured as "%s", but you are '.
- 'accessing it via "%s". Acccess Phabricator via the primary '.
- 'configured domain. Phabricator will not set cookies on other '.
- 'domains for security reasons.',
- $base_uri,
- $host));
- }
+ $alternates = PhabricatorEnv::getEnvConfig('phabricator.allowed-uris');
+ $allowed_uris = array_merge(
+ array($base_uri),
+ $alternates);
+
+ foreach ($allowed_uris as $allowed_uri) {
+ $uri = new PhutilURI($allowed_uri);
+ if ($uri->getDomain() == $host) {
+ return $uri;
}
+ }
- $base_domain = $match->getDomain();
- $is_secure = ($match->getProtocol() == 'https');
- } else {
- $base_uri = new PhutilURI(PhabricatorEnv::getRequestBaseURI());
- $base_domain = $base_uri->getDomain();
+ return null;
+ }
+
+ /**
+ * Determine if security policy rules will allow cookies to be set when
+ * responding to the request.
+ *
+ * @return bool True if setCookie() will succeed. If this method returns
+ * false, setCookie() will throw.
+ *
+ * @task cookie
+ */
+ final public function canSetCookies() {
+ return (bool)$this->getCookieDomainURI();
+ }
+
+ final public function setCookie($name, $value, $expire = null) {
+
+ $is_secure = false;
+
+ $base_domain_uri = $this->getCookieDomainURI();
+ if (!$base_domain_uri) {
+ $configured_as = PhabricatorEnv::getEnvConfig('phabricator.base-uri');
+ $accessed_as = $this->getHost();
+
+ throw new Exception(
+ pht(
+ 'This Phabricator install is configured as "%s", but you are '.
+ 'using the domain name "%s" to access a page which is trying to '.
+ 'set a cookie. Acccess Phabricator on the configured primary '.
+ 'domain or a configured alternate domain. Phabricator will not '.
+ 'set cookies on other domains for security reasons.',
+ $configured_as,
+ $accessed_as));
}
+ $base_domain = $base_domain_uri->getDomain();
+ $is_secure = ($base_domain_uri->getProtocol() == 'https');
+
if ($expire === null) {
$expire = time() + (60 * 60 * 24 * 365 * 5);
}
Index: src/applications/base/controller/PhabricatorController.php
===================================================================
--- src/applications/base/controller/PhabricatorController.php
+++ src/applications/base/controller/PhabricatorController.php
@@ -50,9 +50,15 @@
$phsid = $session_engine->establishSession(
PhabricatorAuthSession::TYPE_WEB,
null);
- $request->setCookie(PhabricatorCookies::COOKIE_SESSION, $phsid);
+
+ // This may be a resource request, in which case we just don't set
+ // the cookie.
+ if ($request->canSetCookies()) {
+ $request->setCookie(PhabricatorCookies::COOKIE_SESSION, $phsid);
+ }
}
+
if (!$user->isLoggedIn()) {
$user->attachAlternateCSRFString(PhabricatorHash::digest($phsid));
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sun, Feb 2, 12:15 AM (18 h, 27 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7082699
Default Alt Text
D8057.id18232.diff (5 KB)
Attached To
Mode
D8057: Don't try to set anonymous session cookie on CDN/file domain
Attached
Detach File
Event Timeline
Log In to Comment