diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2378,7 +2378,6 @@ 'PhabricatorFeedStoryPublisher' => 'applications/feed/PhabricatorFeedStoryPublisher.php', 'PhabricatorFeedStoryReference' => 'applications/feed/storage/PhabricatorFeedStoryReference.php', 'PhabricatorFile' => 'applications/files/storage/PhabricatorFile.php', - 'PhabricatorFileAccessTemporaryTokenType' => 'applications/files/temporarytoken/PhabricatorFileAccessTemporaryTokenType.php', 'PhabricatorFileBundleLoader' => 'applications/files/query/PhabricatorFileBundleLoader.php', 'PhabricatorFileChunk' => 'applications/files/storage/PhabricatorFileChunk.php', 'PhabricatorFileChunkIterator' => 'applications/files/engine/PhabricatorFileChunkIterator.php', @@ -6842,7 +6841,6 @@ 'PhabricatorPolicyInterface', 'PhabricatorDestructibleInterface', ), - 'PhabricatorFileAccessTemporaryTokenType' => 'PhabricatorAuthTemporaryTokenType', 'PhabricatorFileBundleLoader' => 'Phobject', 'PhabricatorFileChunk' => array( 'PhabricatorFileDAO', diff --git a/src/aphront/response/AphrontFileResponse.php b/src/aphront/response/AphrontFileResponse.php --- a/src/aphront/response/AphrontFileResponse.php +++ b/src/aphront/response/AphrontFileResponse.php @@ -11,7 +11,6 @@ private $rangeMin; private $rangeMax; private $allowOrigins = array(); - private $fileToken; public function addAllowOrigin($origin) { $this->allowOrigins[] = $origin; @@ -76,15 +75,6 @@ return $this; } - public function setTemporaryFileToken(PhabricatorAuthTemporaryToken $token) { - $this->fileToken = $token; - return $this; - } - - public function getTemporaryFileToken() { - return $this->fileToken; - } - public function getHeaders() { $headers = array( array('Content-Type', $this->getMimeType()), @@ -128,15 +118,4 @@ return $headers; } - public function didCompleteWrite($aborted) { - if (!$aborted) { - $token = $this->getTemporaryFileToken(); - if ($token) { - $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); - $token->delete(); - unset($unguarded); - } - } - } - } diff --git a/src/aphront/response/AphrontProxyResponse.php b/src/aphront/response/AphrontProxyResponse.php --- a/src/aphront/response/AphrontProxyResponse.php +++ b/src/aphront/response/AphrontProxyResponse.php @@ -39,6 +39,11 @@ return $this; } + public function setCanCDN($can_cdn) { + $this->getProxy()->setCanCDN($can_cdn); + return $this; + } + public function setLastModified($epoch_timestamp) { $this->getProxy()->setLastModified($epoch_timestamp); return $this; diff --git a/src/aphront/response/AphrontResponse.php b/src/aphront/response/AphrontResponse.php --- a/src/aphront/response/AphrontResponse.php +++ b/src/aphront/response/AphrontResponse.php @@ -4,6 +4,7 @@ private $request; private $cacheable = false; + private $canCDN; private $responseCode = 200; private $lastModified = null; @@ -66,6 +67,11 @@ return $this; } + public function setCanCDN($can_cdn) { + $this->canCDN = $can_cdn; + return $this; + } + public function setLastModified($epoch_timestamp) { $this->lastModified = $epoch_timestamp; return $this; @@ -186,6 +192,18 @@ public function getCacheHeaders() { $headers = array(); if ($this->cacheable) { + if ($this->canCDN) { + $headers[] = array( + 'Cache-Control', + 'public', + ); + } else { + $headers[] = array( + 'Cache-Control', + 'private', + ); + } + $headers[] = array( 'Expires', $this->formatEpochTimestampForHTTPHeader(time() + $this->cacheable), @@ -193,11 +211,7 @@ } else { $headers[] = array( 'Cache-Control', - 'private, no-cache, no-store, must-revalidate', - ); - $headers[] = array( - 'Pragma', - 'no-cache', + 'no-store', ); $headers[] = array( 'Expires', diff --git a/src/applications/celerity/controller/CelerityResourceController.php b/src/applications/celerity/controller/CelerityResourceController.php --- a/src/applications/celerity/controller/CelerityResourceController.php +++ b/src/applications/celerity/controller/CelerityResourceController.php @@ -140,6 +140,7 @@ private function makeResponseCacheable(AphrontResponse $response) { $response->setCacheDurationInSeconds(60 * 60 * 24 * 30); $response->setLastModified(time()); + $response->setCanCDN(true); return $response; } diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php --- a/src/applications/diffusion/controller/DiffusionServeController.php +++ b/src/applications/diffusion/controller/DiffusionServeController.php @@ -991,7 +991,7 @@ // $no_authorization = 'Basic '.base64_encode('none'); - $get_uri = $file->getCDNURIWithToken(); + $get_uri = $file->getCDNURI(); $actions['download'] = array( 'href' => $get_uri, 'header' => array( diff --git a/src/applications/files/controller/PhabricatorFileDataController.php b/src/applications/files/controller/PhabricatorFileDataController.php --- a/src/applications/files/controller/PhabricatorFileDataController.php +++ b/src/applications/files/controller/PhabricatorFileDataController.php @@ -4,7 +4,6 @@ private $phid; private $key; - private $token; private $file; public function shouldRequireLogin() { @@ -15,7 +14,6 @@ $viewer = $request->getViewer(); $this->phid = $request->getURIData('phid'); $this->key = $request->getURIData('key'); - $this->token = $request->getURIData('token'); $alt = PhabricatorEnv::getEnvConfig('security.alternate-file-domain'); $base_uri = PhabricatorEnv::getEnvConfig('phabricator.base-uri'); @@ -24,99 +22,40 @@ $req_domain = $request->getHost(); $main_domain = id(new PhutilURI($base_uri))->getDomain(); - $cache_response = true; - if (empty($alt) || $main_domain == $alt_domain) { - // Alternate files domain isn't configured or it's set - // to the same as the default domain - - $response = $this->loadFile($viewer); - if ($response) { - return $response; - } - $file = $this->getFile(); - - // when the file is not CDNable, don't allow cache - $cache_response = $file->getCanCDN(); + if (!strlen($alt) || $main_domain == $alt_domain) { + // No alternate domain. + $should_redirect = false; + $use_viewer = $viewer; + $is_alternate_domain = false; } else if ($req_domain != $alt_domain) { - // Alternate domain is configured but this request isn't using it + // Alternate domain, but this request is on the main domain. + $should_redirect = true; + $use_viewer = $viewer; + $is_alternate_domain = false; + } else { + // Alternate domain, and on the alternate domain. + $should_redirect = false; + $use_viewer = PhabricatorUser::getOmnipotentUser(); + $is_alternate_domain = true; + } - $response = $this->loadFile($viewer); - if ($response) { - return $response; - } - $file = $this->getFile(); + $response = $this->loadFile($use_viewer); + if ($response) { + return $response; + } - // if the user can see the file, generate a token; - // redirect to the alt domain with the token; - $token_uri = $file->getCDNURIWithToken(); - $token_uri = new PhutilURI($token_uri); - $token_uri = $this->addURIParameters($token_uri); + $file = $this->getFile(); + if ($should_redirect) { return id(new AphrontRedirectResponse()) ->setIsExternal(true) - ->setURI($token_uri); - - } else { - // We are using the alternate domain. We don't have authentication - // on this domain, so we bypass policy checks when loading the file. - - $bypass_policies = PhabricatorUser::getOmnipotentUser(); - $response = $this->loadFile($bypass_policies); - if ($response) { - return $response; - } - $file = $this->getFile(); - - $acquire_token_uri = id(new PhutilURI($file->getViewURI())) - ->setDomain($main_domain); - $acquire_token_uri = $this->addURIParameters($acquire_token_uri); - - if ($this->token) { - // validate the token, if it is valid, continue - $validated_token = $file->validateOneTimeToken($this->token); - - if (!$validated_token) { - $dialog = $this->newDialog() - ->setShortTitle(pht('Expired File')) - ->setTitle(pht('File Link Has Expired')) - ->appendParagraph( - pht( - 'The link you followed to view this file is invalid or '. - 'expired.')) - ->appendParagraph( - pht( - 'Continue to generate a new link to the file. You may be '. - 'required to log in.')) - ->addCancelButton( - $acquire_token_uri, - pht('Continue')); - - // Build an explicit response so we can respond with HTTP/403 instead - // of HTTP/200. - $response = id(new AphrontDialogResponse()) - ->setDialog($dialog) - ->setHTTPResponseCode(403); - - return $response; - } - // return the file data without cache headers - $cache_response = false; - } else if (!$file->getCanCDN()) { - // file cannot be served via cdn, and no token given - // redirect to the main domain to aquire a token - - // This is marked as an "external" URI because it is fully qualified. - return id(new AphrontRedirectResponse()) - ->setIsExternal(true) - ->setURI($acquire_token_uri); - } + ->setURI($file->getCDNURI()); } $response = new AphrontFileResponse(); - if ($cache_response) { - $response->setCacheDurationInSeconds(60 * 60 * 24 * 30); - } + $response->setCacheDurationInSeconds(60 * 60 * 24 * 30); + $response->setCanCDN($file->getCanCDN()); $begin = null; $end = null; @@ -138,10 +77,6 @@ $response->setHTTPResponseCode(206); $response->setRange($begin, ($end - 1)); } - } else if (isset($validated_token)) { - // We set this on the response, and the response deletes it after the - // transfer completes. This allows transfers to be resumed, in theory. - $response->setTemporaryFileToken($validated_token); } $is_viewable = $file->isViewableInBrowser(); @@ -150,7 +85,7 @@ if ($is_viewable && !$force_download) { $response->setMimeType($file->getViewableMimeType()); } else { - if (!$request->isHTTPPost() && !$alt_domain) { + if (!$request->isHTTPPost() && !$is_alternate_domain) { // NOTE: Require POST to download files from the primary domain. We'd // rather go full-bore and do a real CSRF check, but can't currently // authenticate users on the file domain. This should blunt any @@ -174,20 +109,6 @@ return $response; } - /** - * Add passthrough parameters to the URI so they aren't lost when we - * redirect to acquire tokens. - */ - private function addURIParameters(PhutilURI $uri) { - $request = $this->getRequest(); - - if ($request->getBool('download')) { - $uri->setQueryParam('download', 1); - } - - return $uri; - } - private function loadFile(PhabricatorUser $viewer) { $file = id(new PhabricatorFileQuery()) ->setViewer($viewer) diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -697,10 +697,10 @@ pht('You must save a file before you can generate a view URI.')); } - return $this->getCDNURI(null); + return $this->getCDNURI(); } - private function getCDNURI($token) { + public function getCDNURI() { $name = self::normalizeFileName($this->getName()); $name = phutil_escape_uri($name); @@ -720,9 +720,6 @@ $parts[] = $this->getSecretKey(); $parts[] = $this->getPHID(); - if ($token) { - $parts[] = $token; - } $parts[] = $name; $path = '/'.implode('/', $parts); @@ -737,19 +734,6 @@ } } - /** - * Get the CDN URI for this file, including a one-time-use security token. - * - */ - public function getCDNURIWithToken() { - if (!$this->getPHID()) { - throw new Exception( - pht('You must save a file before you can generate a CDN URI.')); - } - - return $this->getCDNURI($this->generateOneTimeToken()); - } - public function getInfoURI() { return '/'.$this->getMonogram(); @@ -1120,38 +1104,6 @@ return $this; } - protected function generateOneTimeToken() { - $key = Filesystem::readRandomCharacters(16); - $token_type = PhabricatorFileAccessTemporaryTokenType::TOKENTYPE; - - // Save the new secret. - $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); - $token = id(new PhabricatorAuthTemporaryToken()) - ->setTokenResource($this->getPHID()) - ->setTokenType($token_type) - ->setTokenExpires(time() + phutil_units('1 hour in seconds')) - ->setTokenCode(PhabricatorHash::digest($key)) - ->save(); - unset($unguarded); - - return $key; - } - - public function validateOneTimeToken($token_code) { - $token_type = PhabricatorFileAccessTemporaryTokenType::TOKENTYPE; - - $token = id(new PhabricatorAuthTemporaryTokenQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->withTokenResources(array($this->getPHID())) - ->withTokenTypes(array($token_type)) - ->withExpired(false) - ->withTokenCodes(array(PhabricatorHash::digest($token_code))) - ->executeOne(); - - return $token; - } - - /** * Write the policy edge between this file and some object. * diff --git a/src/applications/files/temporarytoken/PhabricatorFileAccessTemporaryTokenType.php b/src/applications/files/temporarytoken/PhabricatorFileAccessTemporaryTokenType.php deleted file mode 100644 --- a/src/applications/files/temporarytoken/PhabricatorFileAccessTemporaryTokenType.php +++ /dev/null @@ -1,17 +0,0 @@ -setContent($content) - ->setCacheDurationInSeconds(phutil_units('2 hours in seconds')); + ->setCacheDurationInSeconds(phutil_units('2 hours in seconds')) + ->setCanCDN(true); } }