Page MenuHomePhabricator

D20262.id.diff
No OneTemporary

D20262.id.diff

diff --git a/src/applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php b/src/applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php
--- a/src/applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php
+++ b/src/applications/auth/query/PhabricatorAuthTemporaryTokenQuery.php
@@ -10,6 +10,9 @@
private $expired;
private $tokenCodes;
+ private $tokenExpiresMin;
+ private $tokenExpiresMax;
+
public function withIDs(array $ids) {
$this->ids = $ids;
return $this;
@@ -40,6 +43,12 @@
return $this;
}
+ public function withTokenExpiresBetween($min, $max) {
+ $this->tokenExpiresMin = $min;
+ $this->tokenExpiresMax = $max;
+ return $this;
+ }
+
public function newResultObject() {
return new PhabricatorAuthTemporaryToken();
}
@@ -100,6 +109,20 @@
$this->userPHIDs);
}
+ if ($this->tokenExpiresMin !== null) {
+ $where[] = qsprintf(
+ $conn,
+ 'tokenExpires >= %d',
+ $this->tokenExpiresMin);
+ }
+
+ if ($this->tokenExpiresMax !== null) {
+ $where[] = qsprintf(
+ $conn,
+ 'tokenExpires <= %d',
+ $this->tokenExpiresMax);
+ }
+
return $where;
}
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
@@ -692,13 +692,12 @@
}
$lfs_pass = $password->openEnvelope();
- $lfs_hash = PhabricatorHash::weakDigest($lfs_pass);
$token = id(new PhabricatorAuthTemporaryTokenQuery())
->setViewer($omnipotent_viewer)
->withTokenResources(array($repository->getPHID()))
->withTokenTypes(array(DiffusionGitLFSTemporaryTokenType::TOKENTYPE))
- ->withTokenCodes(array($lfs_hash))
+ ->withTokenCodes(array($lfs_pass))
->withExpired(false)
->executeOne();
if (!$token) {
@@ -1092,9 +1091,6 @@
}
} else if ($want_upload) {
if (!$authorization) {
- // Here, we could reuse the existing authorization if we have one,
- // but it's a little simpler to just generate a new one
- // unconditionally.
$authorization = $this->newGitLFSHTTPAuthorization(
$repository,
$viewer,
diff --git a/src/applications/diffusion/gitlfs/DiffusionGitLFSTemporaryTokenType.php b/src/applications/diffusion/gitlfs/DiffusionGitLFSTemporaryTokenType.php
--- a/src/applications/diffusion/gitlfs/DiffusionGitLFSTemporaryTokenType.php
+++ b/src/applications/diffusion/gitlfs/DiffusionGitLFSTemporaryTokenType.php
@@ -20,20 +20,54 @@
PhabricatorUser $viewer,
$operation) {
+ $now = PhabricatorTime::getNow();
+
+ // When we generate a token, this is how long it's good for.
+ $token_window = phutil_units('25 hours in seconds');
+
+ // We'll reuse existing tokens if they still have at least this much
+ // lifetime left.
+ $reuse_window = phutil_units('24 hours in seconds');
+
+ // See PHI1123. If we already have a valid token with a reasonable amount
+ // of remaining life, just reuse it so we don't clog up the table with
+ // a lot of extra rows if there's a high LFS request rate.
+ $existing_tokens = id(new PhabricatorAuthTemporaryTokenQuery())
+ ->setViewer($viewer)
+ ->withTokenResources(array($repository->getPHID()))
+ ->withTokenTypes(array(self::TOKENTYPE))
+ ->withUserPHIDs(array($viewer->getPHID()))
+ ->withTokenExpiresBetween($now + $reuse_window, null)
+ ->execute();
+
+ $auth_token = null;
+ foreach ($existing_tokens as $token) {
+ // We can only reuse an existing token if it authorizes the same
+ // operation.
+ if ($token->getTemporaryTokenProperty('lfs.operation') !== $operation) {
+ continue;
+ }
+
+ // This token is good to reuse.
+ $auth_token = $token;
+ break;
+ }
+
+ if (!$auth_token) {
+ $ttl = $now + $token_window;
+
+ $auth_token = id(new PhabricatorAuthTemporaryToken())
+ ->setTokenResource($repository->getPHID())
+ ->setTokenType(self::TOKENTYPE)
+ ->setTokenCode(Filesystem::readRandomCharacters(32))
+ ->setUserPHID($viewer->getPHID())
+ ->setTemporaryTokenProperty('lfs.operation', $operation)
+ ->setTokenExpires($now + $token_window)
+ ->save();
+ }
+
$lfs_user = self::HTTP_USERNAME;
- $lfs_pass = Filesystem::readRandomCharacters(32);
- $lfs_hash = PhabricatorHash::weakDigest($lfs_pass);
-
- $ttl = PhabricatorTime::getNow() + phutil_units('1 day in seconds');
-
- $token = id(new PhabricatorAuthTemporaryToken())
- ->setTokenResource($repository->getPHID())
- ->setTokenType(self::TOKENTYPE)
- ->setTokenCode($lfs_hash)
- ->setUserPHID($viewer->getPHID())
- ->setTemporaryTokenProperty('lfs.operation', $operation)
- ->setTokenExpires($ttl)
- ->save();
+ $lfs_pass = $auth_token->getTokenCode();
$authorization_header = base64_encode($lfs_user.':'.$lfs_pass);
return 'Basic '.$authorization_header;

File Metadata

Mime Type
text/plain
Expires
Mon, Mar 17, 8:18 AM (2 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7684255
Default Alt Text
D20262.id.diff (5 KB)

Event Timeline