You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Grigorov <mg...@apache.org> on 2020/08/28 11:31:04 UTC
Re: [wicket] 01/01: WICKET-6821 disabled CSP
Hi,
It is not directly related to the changes in this commit but I notice that
the CSP settings is a proper citizen in WebApplication, while the new
Coop/Coep Configuration classes are members of SecuritySettings.
I like the latter better, but it is a bit too late to move CSP into
SecuritySettings too.
Anyone else being bothered by this inconsistency?
On Thu, Aug 27, 2020 at 8:50 PM <sv...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> svenmeier pushed a commit to branch WICKET-6821-disable-CSP
> in repository https://gitbox.apache.org/repos/asf/wicket.git
>
> commit f1f95dd92e6c559cf5243fdf59e1ec20821df2c0
> Author: Sven Meier <sv...@apache.org>
> AuthorDate: Thu Aug 27 19:49:18 2020 +0200
>
> WICKET-6821 disabled CSP
> ---
> .../wicket/csp/ContentSecurityPolicySettings.java | 8 ++++++++
> .../wicket/protocol/http/WebApplication.java | 6 ++++--
> .../csp/CSPSettingRequestCycleListenerTest.java | 23
> +++++++++++++++++-----
> .../head/filter/FilteringHeaderResponseTest.java | 6 ++++++
> 4 files changed, 36 insertions(+), 7 deletions(-)
>
> diff --git
> a/wicket-core/src/main/java/org/apache/wicket/csp/ContentSecurityPolicySettings.java
> b/wicket-core/src/main/java/org/apache/wicket/csp/ContentSecurityPolicySettings.java
> index a768055..7bd1bdd 100644
> ---
> a/wicket-core/src/main/java/org/apache/wicket/csp/ContentSecurityPolicySettings.java
> +++
> b/wicket-core/src/main/java/org/apache/wicket/csp/ContentSecurityPolicySettings.java
> @@ -183,4 +183,12 @@ public class ContentSecurityPolicySettings
> .add(response -> new
> CSPNonceHeaderResponseDecorator(response, this));
> application.mount(new ReportCSPViolationMapper(this));
> }
> +
> + /**
> + * Is CSP enabled.
> + */
> + public boolean isEnabled()
> + {
> + return
> configs.values().stream().anyMatch(CSPHeaderConfiguration::isSet);
> + }
> }
> diff --git
> a/wicket-core/src/main/java/org/apache/wicket/protocol/http/WebApplication.java
> b/wicket-core/src/main/java/org/apache/wicket/protocol/http/WebApplication.java
> index 22dcc71..d38cadf 100644
> ---
> a/wicket-core/src/main/java/org/apache/wicket/protocol/http/WebApplication.java
> +++
> b/wicket-core/src/main/java/org/apache/wicket/protocol/http/WebApplication.java
> @@ -760,8 +760,6 @@ public abstract class WebApplication extends
> Application
>
> getAjaxRequestTargetListeners().add(new
> AjaxEnclosureListener());
>
> - getCspSettings().enforce(this);
> -
> // Configure the app.
> configure();
> if (getConfigurationType() ==
> RuntimeConfigurationType.DEVELOPMENT)
> @@ -782,6 +780,10 @@ public abstract class WebApplication extends
> Application
> {
> super.validateInit();
>
> + if (getCspSettings().isEnabled()) {
> + getCspSettings().enforce(this);
> + }
> +
> // enable coop and coep listeners if specified in security
> settings
> CrossOriginOpenerPolicyConfiguration coopConfig =
> getSecuritySettings()
> .getCrossOriginOpenerPolicyConfiguration();
> diff --git
> a/wicket-core/src/test/java/org/apache/wicket/csp/CSPSettingRequestCycleListenerTest.java
> b/wicket-core/src/test/java/org/apache/wicket/csp/CSPSettingRequestCycleListenerTest.java
> index 9679cbc..08a4d36 100644
> ---
> a/wicket-core/src/test/java/org/apache/wicket/csp/CSPSettingRequestCycleListenerTest.java
> +++
> b/wicket-core/src/test/java/org/apache/wicket/csp/CSPSettingRequestCycleListenerTest.java
> @@ -37,21 +37,34 @@ import java.util.Set;
> import java.util.stream.Collectors;
> import java.util.stream.Stream;
>
> -import org.apache.wicket.mock.MockHomePage;
> +import org.apache.wicket.mock.MockApplication;
> import org.apache.wicket.protocol.http.WebApplication;
> import org.apache.wicket.request.cycle.RequestCycle;
> import org.apache.wicket.util.tester.DummyHomePage;
> import org.apache.wicket.util.tester.WicketTestCase;
> -import org.apache.wicket.util.tester.WicketTester;
> import org.junit.jupiter.api.Assertions;
> import org.junit.jupiter.api.Test;
>
> public class CSPSettingRequestCycleListenerTest extends WicketTestCase
> {
> - @Override
> - protected WicketTester newWicketTester(WebApplication app)
> + @Override
> + protected WebApplication newApplication()
> {
> - return new WicketTester(MockHomePage.class);
> + return new MockApplication()
> + {
> + @Override
> + protected ContentSecurityPolicySettings
> newCspSettings()
> + {
> + return new
> ContentSecurityPolicySettings(this)
> + {
> + @Override
> + public boolean isEnabled()
> + {
> + return true;
> + }
> + };
> + }
> + };
> }
>
> @Test
> diff --git
> a/wicket-core/src/test/java/org/apache/wicket/markup/head/filter/FilteringHeaderResponseTest.java
> b/wicket-core/src/test/java/org/apache/wicket/markup/head/filter/FilteringHeaderResponseTest.java
> index 8adfa94..34c6d8a 100644
> ---
> a/wicket-core/src/test/java/org/apache/wicket/markup/head/filter/FilteringHeaderResponseTest.java
> +++
> b/wicket-core/src/test/java/org/apache/wicket/markup/head/filter/FilteringHeaderResponseTest.java
> @@ -53,6 +53,12 @@ class FilteringHeaderResponseTest extends WicketTestCase
> {
> return "NONCE";
> }
> +
> + @Override
> + public boolean isEnabled()
> + {
> + return true;
> + }
> };
> }
> };
>
>
Re: [wicket] 01/01: WICKET-6821 disabled CSP
Posted by Sven Meier <sv...@meiers.net>.
Yes, I've noticed that too.
Additionally you have to subclass CSPSettings for some customizations (e.g. nonce creation), which doesn't fit with how other settings are working.
IMHO we should try to keep things consistent, even for new requirements.
Sven
Am 28. August 2020 13:31:04 MESZ schrieb Martin Grigorov <mg...@apache.org>:
>Hi,
>
>It is not directly related to the changes in this commit but I notice
>that
>the CSP settings is a proper citizen in WebApplication, while the new
>Coop/Coep Configuration classes are members of SecuritySettings.
>I like the latter better, but it is a bit too late to move CSP into
>SecuritySettings too.
>Anyone else being bothered by this inconsistency?
>
>
>On Thu, Aug 27, 2020 at 8:50 PM <sv...@apache.org> wrote:
>
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> svenmeier pushed a commit to branch WICKET-6821-disable-CSP
>> in repository https://gitbox.apache.org/repos/asf/wicket.git
>>
>> commit f1f95dd92e6c559cf5243fdf59e1ec20821df2c0
>> Author: Sven Meier <sv...@apache.org>
>> AuthorDate: Thu Aug 27 19:49:18 2020 +0200
>>
>> WICKET-6821 disabled CSP
>> ---
>> .../wicket/csp/ContentSecurityPolicySettings.java | 8 ++++++++
>> .../wicket/protocol/http/WebApplication.java | 6 ++++--
>> .../csp/CSPSettingRequestCycleListenerTest.java | 23
>> +++++++++++++++++-----
>> .../head/filter/FilteringHeaderResponseTest.java | 6 ++++++
>> 4 files changed, 36 insertions(+), 7 deletions(-)
>>
>> diff --git
>>
>a/wicket-core/src/main/java/org/apache/wicket/csp/ContentSecurityPolicySettings.java
>>
>b/wicket-core/src/main/java/org/apache/wicket/csp/ContentSecurityPolicySettings.java
>> index a768055..7bd1bdd 100644
>> ---
>>
>a/wicket-core/src/main/java/org/apache/wicket/csp/ContentSecurityPolicySettings.java
>> +++
>>
>b/wicket-core/src/main/java/org/apache/wicket/csp/ContentSecurityPolicySettings.java
>> @@ -183,4 +183,12 @@ public class ContentSecurityPolicySettings
>> .add(response -> new
>> CSPNonceHeaderResponseDecorator(response, this));
>> application.mount(new
>ReportCSPViolationMapper(this));
>> }
>> +
>> + /**
>> + * Is CSP enabled.
>> + */
>> + public boolean isEnabled()
>> + {
>> + return
>> configs.values().stream().anyMatch(CSPHeaderConfiguration::isSet);
>> + }
>> }
>> diff --git
>>
>a/wicket-core/src/main/java/org/apache/wicket/protocol/http/WebApplication.java
>>
>b/wicket-core/src/main/java/org/apache/wicket/protocol/http/WebApplication.java
>> index 22dcc71..d38cadf 100644
>> ---
>>
>a/wicket-core/src/main/java/org/apache/wicket/protocol/http/WebApplication.java
>> +++
>>
>b/wicket-core/src/main/java/org/apache/wicket/protocol/http/WebApplication.java
>> @@ -760,8 +760,6 @@ public abstract class WebApplication extends
>> Application
>>
>> getAjaxRequestTargetListeners().add(new
>> AjaxEnclosureListener());
>>
>> - getCspSettings().enforce(this);
>> -
>> // Configure the app.
>> configure();
>> if (getConfigurationType() ==
>> RuntimeConfigurationType.DEVELOPMENT)
>> @@ -782,6 +780,10 @@ public abstract class WebApplication extends
>> Application
>> {
>> super.validateInit();
>>
>> + if (getCspSettings().isEnabled()) {
>> + getCspSettings().enforce(this);
>> + }
>> +
>> // enable coop and coep listeners if specified in
>security
>> settings
>> CrossOriginOpenerPolicyConfiguration coopConfig =
>> getSecuritySettings()
>> .getCrossOriginOpenerPolicyConfiguration();
>> diff --git
>>
>a/wicket-core/src/test/java/org/apache/wicket/csp/CSPSettingRequestCycleListenerTest.java
>>
>b/wicket-core/src/test/java/org/apache/wicket/csp/CSPSettingRequestCycleListenerTest.java
>> index 9679cbc..08a4d36 100644
>> ---
>>
>a/wicket-core/src/test/java/org/apache/wicket/csp/CSPSettingRequestCycleListenerTest.java
>> +++
>>
>b/wicket-core/src/test/java/org/apache/wicket/csp/CSPSettingRequestCycleListenerTest.java
>> @@ -37,21 +37,34 @@ import java.util.Set;
>> import java.util.stream.Collectors;
>> import java.util.stream.Stream;
>>
>> -import org.apache.wicket.mock.MockHomePage;
>> +import org.apache.wicket.mock.MockApplication;
>> import org.apache.wicket.protocol.http.WebApplication;
>> import org.apache.wicket.request.cycle.RequestCycle;
>> import org.apache.wicket.util.tester.DummyHomePage;
>> import org.apache.wicket.util.tester.WicketTestCase;
>> -import org.apache.wicket.util.tester.WicketTester;
>> import org.junit.jupiter.api.Assertions;
>> import org.junit.jupiter.api.Test;
>>
>> public class CSPSettingRequestCycleListenerTest extends
>WicketTestCase
>> {
>> - @Override
>> - protected WicketTester newWicketTester(WebApplication app)
>> + @Override
>> + protected WebApplication newApplication()
>> {
>> - return new WicketTester(MockHomePage.class);
>> + return new MockApplication()
>> + {
>> + @Override
>> + protected ContentSecurityPolicySettings
>> newCspSettings()
>> + {
>> + return new
>> ContentSecurityPolicySettings(this)
>> + {
>> + @Override
>> + public boolean isEnabled()
>> + {
>> + return true;
>> + }
>> + };
>> + }
>> + };
>> }
>>
>> @Test
>> diff --git
>>
>a/wicket-core/src/test/java/org/apache/wicket/markup/head/filter/FilteringHeaderResponseTest.java
>>
>b/wicket-core/src/test/java/org/apache/wicket/markup/head/filter/FilteringHeaderResponseTest.java
>> index 8adfa94..34c6d8a 100644
>> ---
>>
>a/wicket-core/src/test/java/org/apache/wicket/markup/head/filter/FilteringHeaderResponseTest.java
>> +++
>>
>b/wicket-core/src/test/java/org/apache/wicket/markup/head/filter/FilteringHeaderResponseTest.java
>> @@ -53,6 +53,12 @@ class FilteringHeaderResponseTest extends
>WicketTestCase
>> {
>> return "NONCE";
>> }
>> +
>> + @Override
>> + public boolean isEnabled()
>> + {
>> + return true;
>> + }
>> };
>> }
>> };
>>
>>