recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Improvements to git-lfs behavior

Summary:
- (performance) Return a TTL from git-lfs-authenticate.
- (compliance) Return HTTP 401 for unauthenticated git-lfs requests.
- (future) Relax the requirement that the target be a Git repository.
- (future) Don't double up '/' in `getGitLFSURI()` for non-git repository types.

These improve the push speed of Git when using git-lfs, improve spec
compliance, and make it possible to support git-lfs for other repository types
in the future.

See Q168 for a little background.

Test Plan:
- Spin up new phorge install w/ ssh configured.
- Create a git repository, and enable git-lfs in config.
- Clone the repository over ssh, add some file content to be tracked with git-lfs.
- Track some content with LFS: `git lfs track '*.png'`.
- Add (created by `git lfs track`) and commit `.gitattributes`
- Add favorite memes
- Commit
- Push and observe that git receives a token with expiry, and caches it.
- Update some LFS content and push again. Observe git directly push to LFS with no extra SSH roundtrip.
- Clone a second copy, ensure that LFS content is downloaded, update a meme and push.
- Pull the content from the first copy and see again no additional roundtrip to get a new token.

Additionally confirm:

- All other normal git operations continue working
- Ensure that users w/o push access still can't push anything (LFS or otherwise.)

Reviewers: O1 Blessed Committers, aklapper, valerio.bozzolan

Reviewed By: O1 Blessed Committers, aklapper, valerio.bozzolan

Subscribers: aklapper, mainframe98, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D26190

amy bones 42731493 88c580da

+27 -11
+15 -6
src/applications/diffusion/controller/DiffusionServeController.php
··· 183 183 // won't prompt users who provide a username but no password otherwise. 184 184 // See T10797 for discussion. 185 185 186 - $have_user = strlen(idx($_SERVER, 'PHP_AUTH_USER', '')); 187 - $have_pass = strlen(idx($_SERVER, 'PHP_AUTH_PW', '')); 186 + $have_user = phutil_nonempty_string(idx($_SERVER, 'PHP_AUTH_USER')); 187 + $have_pass = phutil_nonempty_string(idx($_SERVER, 'PHP_AUTH_PW')); 188 + if ($this->getIsGitLFSRequest() && !($have_user && $have_pass)) { 189 + return new PhabricatorVCSResponse( 190 + 401, 191 + pht('Git-LFS Authentication required')); 192 + } 188 193 if ($have_user && $have_pass) { 189 194 $username = $_SERVER['PHP_AUTH_USER']; 190 195 $password = new PhutilOpaqueEnvelope($_SERVER['PHP_AUTH_PW']); ··· 361 366 } 362 367 } 363 368 369 + // When considering whether or not a request is mismatched for a given 370 + // repository type, there's only two considerations: whether or not the 371 + // known type matches the request vcs type, and whether it's actually a 372 + // git-lfs request. When it's a git-lfs request, then the vcs types may or 373 + // may not match (in the case of using the Mercurial git-lfs extension, for 374 + // instance), so don't throw errors for mismatched types. 364 375 $vcs_type = $repository->getVersionControlSystem(); 365 376 $req_type = $this->isVCSRequest($request); 366 - 367 - if ($vcs_type != $req_type) { 377 + if ($vcs_type != $req_type && !$this->getIsGitLFSRequest()) { 368 378 switch ($req_type) { 369 379 case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: 370 380 $result = new PhabricatorVCSResponse( ··· 758 768 } 759 769 760 770 $this->gitLFSToken = $token; 761 - 762 771 return $user; 763 772 } 764 773 ··· 1283 1292 1284 1293 unset($unguarded); 1285 1294 1286 - return $authorization; 1295 + return $authorization['header']; 1287 1296 } 1288 1297 1289 1298 private function getGitLFSRequestPath(PhabricatorRepository $repository) {
+2 -1
src/applications/diffusion/gitlfs/DiffusionGitLFSAuthenticateWorkflow.php
··· 92 92 $operation); 93 93 94 94 $headers = array( 95 - 'authorization' => $authorization, 95 + 'authorization' => $authorization['header'], 96 96 ); 97 97 98 98 $result = array( 99 99 'header' => $headers, 100 100 'href' => $lfs_uri, 101 + 'expires_in' => $authorization['ttl'], 101 102 ); 102 103 $result = phutil_json_encode($result); 103 104
+6 -3
src/applications/diffusion/gitlfs/DiffusionGitLFSTemporaryTokenType.php
··· 24 24 $lfs_pass = Filesystem::readRandomCharacters(32); 25 25 $lfs_hash = PhabricatorHash::weakDigest($lfs_pass); 26 26 27 - $ttl = PhabricatorTime::getNow() + phutil_units('1 day in seconds'); 27 + $ttl = phutil_units('1 day in seconds'); 28 28 29 29 $token = id(new PhabricatorAuthTemporaryToken()) 30 30 ->setTokenResource($repository->getPHID()) ··· 32 32 ->setTokenCode($lfs_hash) 33 33 ->setUserPHID($viewer->getPHID()) 34 34 ->setTemporaryTokenProperty('lfs.operation', $operation) 35 - ->setTokenExpires($ttl) 35 + ->setTokenExpires(PhabricatorTime::getNow() + $ttl) 36 36 ->save(); 37 37 38 38 $authorization_header = base64_encode($lfs_user.':'.$lfs_pass); 39 - return 'Basic '.$authorization_header; 39 + return array( 40 + 'header' => 'Basic '.$authorization_header, 41 + 'ttl' => $ttl, 42 + ); 40 43 } 41 44 42 45 }
+4 -1
src/applications/repository/storage/PhabricatorRepository.php
··· 1490 1490 1491 1491 $uri = $this->getRawHTTPCloneURIObject(); 1492 1492 $uri = (string)$uri; 1493 - $uri = $uri.'/'.$path; 1493 + if ($uri[-1] !== '/') { 1494 + $uri .= '/'; 1495 + } 1496 + $uri .= $path; 1494 1497 1495 1498 return $uri; 1496 1499 }