mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-19 12:00:55 +01:00
Only require view permissions for read-only Git LFS requests
Summary: Ref T7789. Implement proper detection for read-only requests. Previously, we assumed every request was read/write and required lots of permissions, but we don't need "Can Push" permission if you're only cloning/fetching/pulling. Test Plan: - Set push policy to "no one". - Fetched, got clean data out of LFS. - Tried to push, got useful error. Reviewers: chad Reviewed By: chad Maniphest Tasks: T7789 Differential Revision: https://secure.phabricator.com/D15499
This commit is contained in:
parent
d5f9e49e29
commit
76f07ec80b
1 changed files with 55 additions and 8 deletions
|
@ -7,6 +7,7 @@ final class DiffusionServeController extends DiffusionController {
|
||||||
|
|
||||||
private $isGitLFSRequest;
|
private $isGitLFSRequest;
|
||||||
private $gitLFSToken;
|
private $gitLFSToken;
|
||||||
|
private $gitLFSInput;
|
||||||
|
|
||||||
public function setServiceViewer(PhabricatorUser $viewer) {
|
public function setServiceViewer(PhabricatorUser $viewer) {
|
||||||
$this->getRequest()->setUser($viewer);
|
$this->getRequest()->setUser($viewer);
|
||||||
|
@ -284,11 +285,20 @@ final class DiffusionServeController extends DiffusionController {
|
||||||
DiffusionPushCapability::CAPABILITY);
|
DiffusionPushCapability::CAPABILITY);
|
||||||
if (!$can_push) {
|
if (!$can_push) {
|
||||||
if ($viewer->isLoggedIn()) {
|
if ($viewer->isLoggedIn()) {
|
||||||
return new PhabricatorVCSResponse(
|
$error_code = 403;
|
||||||
403,
|
$error_message = pht(
|
||||||
pht(
|
'You do not have permission to push to this repository ("%s").',
|
||||||
'You do not have permission to push to this '.
|
$repository->getDisplayName());
|
||||||
'repository.'));
|
|
||||||
|
if ($this->getIsGitLFSRequest()) {
|
||||||
|
return DiffusionGitLFSResponse::newErrorResponse(
|
||||||
|
$error_code,
|
||||||
|
$error_message);
|
||||||
|
} else {
|
||||||
|
return new PhabricatorVCSResponse(
|
||||||
|
$error_code,
|
||||||
|
$error_message);
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
if ($allow_auth) {
|
if ($allow_auth) {
|
||||||
return new PhabricatorVCSResponse(
|
return new PhabricatorVCSResponse(
|
||||||
|
@ -422,7 +432,9 @@ final class DiffusionServeController extends DiffusionController {
|
||||||
|
|
||||||
// TODO: This implementation is safe by default, but very incomplete.
|
// TODO: This implementation is safe by default, but very incomplete.
|
||||||
|
|
||||||
// TODO: This doesn't get the right result for Git LFS yet.
|
if ($this->getIsGitLFSRequest()) {
|
||||||
|
return $this->isGitLFSReadOnlyRequest($repository);
|
||||||
|
}
|
||||||
|
|
||||||
switch ($repository->getVersionControlSystem()) {
|
switch ($repository->getVersionControlSystem()) {
|
||||||
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
|
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
|
||||||
|
@ -911,8 +923,7 @@ final class DiffusionServeController extends DiffusionController {
|
||||||
PhabricatorRepository $repository,
|
PhabricatorRepository $repository,
|
||||||
PhabricatorUser $viewer) {
|
PhabricatorUser $viewer) {
|
||||||
|
|
||||||
$input = PhabricatorStartup::getRawInput();
|
$input = $this->getGitLFSInput();
|
||||||
$input = phutil_json_decode($input);
|
|
||||||
|
|
||||||
$operation = idx($input, 'operation');
|
$operation = idx($input, 'operation');
|
||||||
switch ($operation) {
|
switch ($operation) {
|
||||||
|
@ -1061,6 +1072,10 @@ final class DiffusionServeController extends DiffusionController {
|
||||||
$oid));
|
$oid));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Remove the execution time limit because uploading large files may take
|
||||||
|
// a while.
|
||||||
|
set_time_limit(0);
|
||||||
|
|
||||||
$request_stream = new AphrontRequestStream();
|
$request_stream = new AphrontRequestStream();
|
||||||
$request_iterator = $request_stream->getIterator();
|
$request_iterator = $request_stream->getIterator();
|
||||||
$hashing_iterator = id(new PhutilHashingIterator($request_iterator))
|
$hashing_iterator = id(new PhutilHashingIterator($request_iterator))
|
||||||
|
@ -1133,4 +1148,36 @@ final class DiffusionServeController extends DiffusionController {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function getGitLFSInput() {
|
||||||
|
if (!$this->gitLFSInput) {
|
||||||
|
$input = PhabricatorStartup::getRawInput();
|
||||||
|
$input = phutil_json_decode($input);
|
||||||
|
$this->gitLFSInput = $input;
|
||||||
|
}
|
||||||
|
|
||||||
|
return $this->gitLFSInput;
|
||||||
|
}
|
||||||
|
|
||||||
|
private function isGitLFSReadOnlyRequest(PhabricatorRepository $repository) {
|
||||||
|
if (!$this->getIsGitLFSRequest()) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
$path = $this->getGitLFSRequestPath($repository);
|
||||||
|
|
||||||
|
if ($path === 'objects/batch') {
|
||||||
|
$input = $this->getGitLFSInput();
|
||||||
|
$operation = idx($input, 'operation');
|
||||||
|
switch ($operation) {
|
||||||
|
case 'download':
|
||||||
|
return true;
|
||||||
|
default:
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue