Add header for attachment disposition only once
Recent refactorings have resulted in the header being added twice, this makes browsers ignore the header which removes any security gains. This changeset adds the header only once and adds integration tests ensuring the correct header in future. https://github.com/owncloud/core/issues/22577
This commit is contained in:
Lukas Reschke
parent
59acc53483
commit
fc2c5fe414
|
|
@ -114,19 +114,6 @@ class Server {
|
|||
$this->server->addPlugin(new \OCA\DAV\Connector\Sabre\FakeLockerPlugin());
|
||||
}
|
||||
|
||||
// Serve all files with an Content-Disposition of type "attachment"
|
||||
$this->server->on('beforeMethod', function (RequestInterface $requestInterface, ResponseInterface $responseInterface) {
|
||||
if ($requestInterface->getMethod() === 'GET') {
|
||||
$path = $requestInterface->getPath();
|
||||
if ($this->server->tree->nodeExists($path)) {
|
||||
$node = $this->server->tree->getNodeForPath($path);
|
||||
if (($node instanceof IFile)) {
|
||||
$responseInterface->addHeader('Content-Disposition', 'attachment');
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
// wait with registering these until auth is handled and the filesystem is setup
|
||||
$this->server->on('beforeMethod', function () {
|
||||
// custom properties plugin must be the last one
|
||||
|
|
|
|||
|
|
@ -12,6 +12,8 @@ require __DIR__ . '/../../vendor/autoload.php';
|
|||
trait WebDav {
|
||||
/** @var string*/
|
||||
private $davPath = "remote.php/webdav";
|
||||
/** @var ResponseInterface */
|
||||
private $response;
|
||||
|
||||
/**
|
||||
* @Given /^using dav path "([^"]*)"$/
|
||||
|
|
@ -104,6 +106,48 @@ trait WebDav {
|
|||
$this->downloadedContentShouldBe($content);
|
||||
}
|
||||
|
||||
/**
|
||||
* @When Downloading file :fileName
|
||||
*/
|
||||
public function downloadingFile($fileName) {
|
||||
$this->response = $this->makeDavRequest($this->currentUser, 'GET', $fileName, []);
|
||||
}
|
||||
|
||||
/**
|
||||
* @Then The following headers should be set
|
||||
*/
|
||||
public function theFollowingHeadersShouldBeSet(\Behat\Gherkin\Node\TableNode $table) {
|
||||
foreach($table->getTable() as $header) {
|
||||
$headerName = $header[0];
|
||||
$expectedHeaderValue = $header[1];
|
||||
$returnedHeader = $this->response->getHeader($headerName);
|
||||
if($returnedHeader !== $expectedHeaderValue) {
|
||||
throw new \Exception(
|
||||
sprintf(
|
||||
"Expected value '%s' for header '%s', got '%s'",
|
||||
$expectedHeaderValue,
|
||||
$headerName,
|
||||
$returnedHeader
|
||||
)
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @Then Downloaded content should start with :start
|
||||
*/
|
||||
public function downloadedContentShouldStartWith($start) {
|
||||
if(strpos($this->response->getBody()->getContents(), $start) !== 0) {
|
||||
throw new \Exception(
|
||||
sprintf(
|
||||
"Expected '%s', got '%s'",
|
||||
$start,
|
||||
$this->response->getBody()->getContents()
|
||||
)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/*Returns the elements of a propfind, $folderDepth requires 1 to see elements without children*/
|
||||
public function listFolder($user, $path, $folderDepth){
|
||||
|
|
|
|||
|
|
@ -15,7 +15,6 @@ Feature: sharing
|
|||
When Downloading file "/welcome.txt" with range "bytes=51-77"
|
||||
Then Downloaded content should be "example file for developers"
|
||||
|
||||
|
||||
Scenario: Upload forbidden if quota is 0
|
||||
Given using dav path "remote.php/webdav"
|
||||
And As an "admin"
|
||||
|
|
@ -33,9 +32,35 @@ Feature: sharing
|
|||
And Downloading last public shared file with range "bytes=51-77"
|
||||
Then Downloaded content should be "example file for developers"
|
||||
|
||||
|
||||
|
||||
|
||||
Scenario: Downloading a file on the old endpoint should serve security headers
|
||||
Given using dav path "remote.php/webdav"
|
||||
And As an "admin"
|
||||
When Downloading file "/welcome.txt"
|
||||
Then The following headers should be set
|
||||
|Content-Disposition|attachment|
|
||||
|Content-Security-Policy|default-src 'self'; script-src 'self' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; frame-src *; img-src * data: blob:; font-src 'self' data:; media-src *; connect-src *|
|
||||
|X-Content-Type-Options |nosniff|
|
||||
|X-Download-Options|noopen|
|
||||
|X-Frame-Options|Sameorigin|
|
||||
|X-Permitted-Cross-Domain-Policies|none|
|
||||
|X-Robots-Tag|none|
|
||||
|X-XSS-Protection|1; mode=block|
|
||||
And Downloaded content should start with "Welcome to your ownCloud account!"
|
||||
|
||||
Scenario: Downloading a file on the new endpoint should serve security headers
|
||||
Given using dav path "remote.php/dav/files/admin/"
|
||||
And As an "admin"
|
||||
When Downloading file "/welcome.txt"
|
||||
Then The following headers should be set
|
||||
|Content-Disposition|attachment|
|
||||
|Content-Security-Policy|default-src 'self'; script-src 'self' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; frame-src *; img-src * data: blob:; font-src 'self' data:; media-src *; connect-src *|
|
||||
|X-Content-Type-Options |nosniff|
|
||||
|X-Download-Options|noopen|
|
||||
|X-Frame-Options|Sameorigin|
|
||||
|X-Permitted-Cross-Domain-Policies|none|
|
||||
|X-Robots-Tag|none|
|
||||
|X-XSS-Protection|1; mode=block|
|
||||
And Downloaded content should start with "Welcome to your ownCloud account!"
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue