diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php index 30e90db826..3e8d0a28dc 100644 --- a/src/applications/diffusion/controller/DiffusionServeController.php +++ b/src/applications/diffusion/controller/DiffusionServeController.php @@ -183,8 +183,13 @@ final class DiffusionServeController extends DiffusionController { // won't prompt users who provide a username but no password otherwise. // See T10797 for discussion. - $have_user = strlen(idx($_SERVER, 'PHP_AUTH_USER', '')); - $have_pass = strlen(idx($_SERVER, 'PHP_AUTH_PW', '')); + $have_user = phutil_nonempty_string(idx($_SERVER, 'PHP_AUTH_USER')); + $have_pass = phutil_nonempty_string(idx($_SERVER, 'PHP_AUTH_PW')); + if ($this->getIsGitLFSRequest() && !($have_user && $have_pass)) { + return new PhabricatorVCSResponse( + 401, + pht('Git-LFS Authentication required')); + } if ($have_user && $have_pass) { $username = $_SERVER['PHP_AUTH_USER']; $password = new PhutilOpaqueEnvelope($_SERVER['PHP_AUTH_PW']); @@ -361,10 +366,15 @@ final class DiffusionServeController extends DiffusionController { } } + // When considering whether or not a request is mismatched for a given + // repository type, there's only two considerations: whether or not the + // known type matches the request vcs type, and whether it's actually a + // git-lfs request. When it's a git-lfs request, then the vcs types may or + // may not match (in the case of using the Mercurial git-lfs extension, for + // instance), so don't throw errors for mismatched types. $vcs_type = $repository->getVersionControlSystem(); $req_type = $this->isVCSRequest($request); - - if ($vcs_type != $req_type) { + if ($vcs_type != $req_type && !$this->getIsGitLFSRequest()) { switch ($req_type) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: $result = new PhabricatorVCSResponse( @@ -758,7 +768,6 @@ final class DiffusionServeController extends DiffusionController { } $this->gitLFSToken = $token; - return $user; } @@ -1283,7 +1292,7 @@ final class DiffusionServeController extends DiffusionController { unset($unguarded); - return $authorization; + return $authorization['header']; } private function getGitLFSRequestPath(PhabricatorRepository $repository) { diff --git a/src/applications/diffusion/gitlfs/DiffusionGitLFSAuthenticateWorkflow.php b/src/applications/diffusion/gitlfs/DiffusionGitLFSAuthenticateWorkflow.php index 56297f4d8b..a1fa1dd5ba 100644 --- a/src/applications/diffusion/gitlfs/DiffusionGitLFSAuthenticateWorkflow.php +++ b/src/applications/diffusion/gitlfs/DiffusionGitLFSAuthenticateWorkflow.php @@ -92,12 +92,13 @@ final class DiffusionGitLFSAuthenticateWorkflow $operation); $headers = array( - 'authorization' => $authorization, + 'authorization' => $authorization['header'], ); $result = array( 'header' => $headers, 'href' => $lfs_uri, + 'expires_in' => $authorization['ttl'], ); $result = phutil_json_encode($result); diff --git a/src/applications/diffusion/gitlfs/DiffusionGitLFSTemporaryTokenType.php b/src/applications/diffusion/gitlfs/DiffusionGitLFSTemporaryTokenType.php index e5425b07e1..f35b91a5ac 100644 --- a/src/applications/diffusion/gitlfs/DiffusionGitLFSTemporaryTokenType.php +++ b/src/applications/diffusion/gitlfs/DiffusionGitLFSTemporaryTokenType.php @@ -24,7 +24,7 @@ final class DiffusionGitLFSTemporaryTokenType $lfs_pass = Filesystem::readRandomCharacters(32); $lfs_hash = PhabricatorHash::weakDigest($lfs_pass); - $ttl = PhabricatorTime::getNow() + phutil_units('1 day in seconds'); + $ttl = phutil_units('1 day in seconds'); $token = id(new PhabricatorAuthTemporaryToken()) ->setTokenResource($repository->getPHID()) @@ -32,11 +32,14 @@ final class DiffusionGitLFSTemporaryTokenType ->setTokenCode($lfs_hash) ->setUserPHID($viewer->getPHID()) ->setTemporaryTokenProperty('lfs.operation', $operation) - ->setTokenExpires($ttl) + ->setTokenExpires(PhabricatorTime::getNow() + $ttl) ->save(); $authorization_header = base64_encode($lfs_user.':'.$lfs_pass); - return 'Basic '.$authorization_header; + return array( + 'header' => 'Basic '.$authorization_header, + 'ttl' => $ttl, + ); } } diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 5997e80f7d..8a639a8f58 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -1490,7 +1490,10 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO $uri = $this->getRawHTTPCloneURIObject(); $uri = (string)$uri; - $uri = $uri.'/'.$path; + if ($uri[-1] !== '/') { + $uri .= '/'; + } + $uri .= $path; return $uri; }