Skip to content

Commit

Permalink
Merge pull request #38642 from nextcloud/chore/appframework/drop-empt…
Browse files Browse the repository at this point in the history 
…ycontentsecuritypolicy-allowinlinescript

chore(appframework)!: Drop \OCP\AppFramework\Http\EmptyContentSecurityPolicy::allowInlineScript
  • Loading branch information
@szaimen
szaimen authored Jun 12, 2023
2 parents 83faba5 + 08a3f37 commit 63bf207
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 61 deletions.
19 changes: 1 addition & 18 deletions lib/public/AppFramework/Http/EmptyContentSecurityPolicy.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@
* @since 9.0.0
*/
class EmptyContentSecurityPolicy {
/** @var bool Whether inline JS snippets are allowed */
protected $inlineScriptAllowed = null;
/** @var string Whether JS nonces should be used */
protected $useJsNonce = null;
/** @var bool Whether strict-dynamic should be used */
Expand Down Expand Up @@ -83,18 +81,6 @@ class EmptyContentSecurityPolicy {
/** @var array Locations to report violations to */
protected $reportTo = null;

/**
* Whether inline JavaScript snippets are allowed or forbidden
* @param bool $state
* @return $this
* @since 8.1.0
* @deprecated 10.0 CSP tokens are now used
*/
public function allowInlineScript($state = false) {
$this->inlineScriptAllowed = $state;
return $this;
}

/**
* @param bool $state
* @return EmptyContentSecurityPolicy
Expand Down Expand Up @@ -447,7 +433,7 @@ public function buildPolicy() {
$policy .= "base-uri 'none';";
$policy .= "manifest-src 'self';";

if (!empty($this->allowedScriptDomains) || $this->inlineScriptAllowed || $this->evalScriptAllowed) {
if (!empty($this->allowedScriptDomains) || $this->evalScriptAllowed) {
$policy .= 'script-src ';
if (is_string($this->useJsNonce)) {
if ($this->strictDynamicAllowed) {
Expand All @@ -464,9 +450,6 @@ public function buildPolicy() {
if (is_array($this->allowedScriptDomains)) {
$policy .= implode(' ', $this->allowedScriptDomains);
}
if ($this->inlineScriptAllowed) {
$policy .= ' \'unsafe-inline\'';
}
if ($this->evalScriptAllowed) {
$policy .= ' \'unsafe-eval\'';
}
Expand Down
18 changes: 1 addition & 17 deletions tests/lib/AppFramework/Http/ContentSecurityPolicyTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,25 +68,9 @@ public function testGetPolicyDisallowScriptDomainMultipleStacked() {
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyScriptAllowInline() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'self' 'unsafe-inline';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self' data:;connect-src 'self';media-src 'self';frame-ancestors 'self';form-action 'self'";

$this->contentSecurityPolicy->allowInlineScript(true);
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyScriptAllowInlineWithDomain() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'self' www.owncloud.com 'unsafe-inline';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self' data:;connect-src 'self';media-src 'self';frame-ancestors 'self';form-action 'self'";

$this->contentSecurityPolicy->addAllowedScriptDomain('www.owncloud.com');
$this->contentSecurityPolicy->allowInlineScript(true);
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyScriptDisallowInlineAndEval() {
public function testGetPolicyScriptDisallowEval() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'self';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self' data:;connect-src 'self';media-src 'self';frame-ancestors 'self';form-action 'self'";

$this->contentSecurityPolicy->allowInlineScript(false);
$this->contentSecurityPolicy->allowEvalScript(false);
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}
Expand Down
20 changes: 2 additions & 18 deletions tests/lib/AppFramework/Http/EmptyContentSecurityPolicyTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,25 +68,9 @@ public function testGetPolicyDisallowScriptDomainMultipleStacked() {
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyScriptAllowInline() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'unsafe-inline';frame-ancestors 'none'";
public function testGetPolicyScriptAllowEval() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'unsafe-eval';frame-ancestors 'none'";

$this->contentSecurityPolicy->allowInlineScript(true);
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyScriptAllowInlineWithDomain() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src www.owncloud.com 'unsafe-inline';frame-ancestors 'none'";

$this->contentSecurityPolicy->addAllowedScriptDomain('www.owncloud.com');
$this->contentSecurityPolicy->allowInlineScript(true);
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyScriptAllowInlineAndEval() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'unsafe-inline' 'unsafe-eval';frame-ancestors 'none'";

$this->contentSecurityPolicy->allowInlineScript(true);
$this->contentSecurityPolicy->allowEvalScript(true);
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}
Expand Down
2 changes: 0 additions & 2 deletions tests/lib/AppFramework/Http/ResponseTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ public function testOverwriteCsp() {
'Content-Security-Policy' => "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'self' 'unsafe-inline';style-src 'self' 'unsafe-inline';img-src 'self';font-src 'self' data:;connect-src 'self';media-src 'self'",
];
$policy = new Http\ContentSecurityPolicy();
$policy->allowInlineScript(true);

$this->childResponse->setContentSecurityPolicy($policy);
$headers = $this->childResponse->getHeaders();
Expand All @@ -77,7 +76,6 @@ public function testOverwriteCsp() {

public function testGetCsp() {
$policy = new Http\ContentSecurityPolicy();
$policy->allowInlineScript(true);

$this->childResponse->setContentSecurityPolicy($policy);
$this->assertEquals($policy, $this->childResponse->getContentSecurityPolicy());
Expand Down
8 changes: 2 additions & 6 deletions tests/lib/Security/CSP/ContentSecurityPolicyManagerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ public function testGetDefaultPolicyWithPolicies() {
$policy = new \OCP\AppFramework\Http\ContentSecurityPolicy();
$policy->addAllowedFontDomain('example.com');
$policy->addAllowedImageDomain('example.org');
$policy->allowInlineScript(true);
$policy->allowEvalScript(true);
$this->contentSecurityPolicyManager->addDefaultPolicy($policy);
$policy = new \OCP\AppFramework\Http\EmptyContentSecurityPolicy();
Expand All @@ -66,7 +65,6 @@ public function testGetDefaultPolicyWithPolicies() {
$this->contentSecurityPolicyManager->addDefaultPolicy($policy);

$expected = new \OC\Security\CSP\ContentSecurityPolicy();
$expected->allowInlineScript(true);
$expected->allowEvalScript(true);
$expected->addAllowedFontDomain('mydomain.com');
$expected->addAllowedFontDomain('example.com');
Expand All @@ -75,7 +73,7 @@ public function testGetDefaultPolicyWithPolicies() {
$expected->addAllowedImageDomain('anotherdomain.de');
$expected->addAllowedImageDomain('example.org');
$expected->addAllowedChildSrcDomain('childdomain');
$expectedStringPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'self' 'unsafe-inline' 'unsafe-eval';style-src 'self' 'unsafe-inline';img-src 'self' data: blob: anotherdomain.de example.org;font-src 'self' data: mydomain.com example.com anotherFontDomain;connect-src 'self';media-src 'self';child-src childdomain;frame-ancestors 'self';form-action 'self' thirdDomain";
$expectedStringPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'self' 'unsafe-eval';style-src 'self' 'unsafe-inline';img-src 'self' data: blob: anotherdomain.de example.org;font-src 'self' data: mydomain.com example.com anotherFontDomain;connect-src 'self';media-src 'self';child-src childdomain;frame-ancestors 'self';form-action 'self' thirdDomain";

$this->assertEquals($expected, $this->contentSecurityPolicyManager->getDefaultPolicy());
$this->assertSame($expectedStringPolicy, $this->contentSecurityPolicyManager->getDefaultPolicy()->buildPolicy());
Expand All @@ -96,7 +94,6 @@ public function testGetDefaultPolicyWithPoliciesViaEvent() {
$policy = new \OCP\AppFramework\Http\ContentSecurityPolicy();
$policy->addAllowedFontDomain('example.com');
$policy->addAllowedImageDomain('example.org');
$policy->allowInlineScript(true);
$policy->allowEvalScript(false);
$e->addPolicy($policy);
});
Expand All @@ -110,7 +107,6 @@ public function testGetDefaultPolicyWithPoliciesViaEvent() {
});

$expected = new \OC\Security\CSP\ContentSecurityPolicy();
$expected->allowInlineScript(true);
$expected->allowEvalScript(true);
$expected->addAllowedFontDomain('mydomain.com');
$expected->addAllowedFontDomain('example.com');
Expand All @@ -120,7 +116,7 @@ public function testGetDefaultPolicyWithPoliciesViaEvent() {
$expected->addAllowedChildSrcDomain('childdomain');
$expected->addAllowedFormActionDomain('thirdDomain');
$expected->useStrictDynamic(true);
$expectedStringPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'self' 'unsafe-inline' 'unsafe-eval';style-src 'self' 'unsafe-inline';img-src 'self' data: blob: anotherdomain.de example.org;font-src 'self' data: mydomain.com example.com anotherFontDomain;connect-src 'self';media-src 'self';child-src childdomain;frame-ancestors 'self';form-action 'self' thirdDomain";
$expectedStringPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'self' 'unsafe-eval';style-src 'self' 'unsafe-inline';img-src 'self' data: blob: anotherdomain.de example.org;font-src 'self' data: mydomain.com example.com anotherFontDomain;connect-src 'self';media-src 'self';child-src childdomain;frame-ancestors 'self';form-action 'self' thirdDomain";

$this->assertEquals($expected, $this->contentSecurityPolicyManager->getDefaultPolicy());
$this->assertSame($expectedStringPolicy, $this->contentSecurityPolicyManager->getDefaultPolicy()->buildPolicy());
Expand Down

0 comments on commit 63bf207

Please sign in to comment.