D15642.diff
No OneTemporary
Actions

Size

15 KB

Referenced Files

None

Subscribers

None

D15642.diff
View Options

	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 @@
	// <https://github.com/github/git-lfs/issues/1088>
	$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 @@
	-<?php
	-
	-final class PhabricatorFileAccessTemporaryTokenType
	- extends PhabricatorAuthTemporaryTokenType {
	-
	- const TOKENTYPE = 'file:onetime';
	-
	- public function getTokenTypeDisplayName() {
	- return pht('File Access');
	- }
	-
	- public function getTokenReadableTypeName(
	- PhabricatorAuthTemporaryToken $token) {
	- return pht('File Access Token');
	- }
	-
	-}
	diff --git a/src/applications/system/controller/PhabricatorRobotsController.php b/src/applications/system/controller/PhabricatorRobotsController.php
	--- a/src/applications/system/controller/PhabricatorRobotsController.php
	+++ b/src/applications/system/controller/PhabricatorRobotsController.php
	@@ -32,6 +32,7 @@

	return id(new AphrontPlainTextResponse())
	->setContent($content)
	- ->setCacheDurationInSeconds(phutil_units('2 hours in seconds'));
	+ ->setCacheDurationInSeconds(phutil_units('2 hours in seconds'))
	+ ->setCanCDN(true);
	}
	}

File Metadata

Mime Type: text/plain
Expires: Tue, Oct 22, 9:31 PM (3 w, 6 d ago)
Storage Engine: blob
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: 6742877
Default Alt Text: D15642.diff (15 KB)

D15642.diffNo OneTemporaryActions

D15642.diffView Options

File Metadata

Event Timeline

D15642.diff
No OneTemporary
Actions

D15642.diff
View Options