Skip to content

Commit

Permalink
Merge pull request #324 from alexander-schranz/feature/csrf-protectio…
Browse files Browse the repository at this point in the history
…n-option

Add option to disable csrf protection
  • Loading branch information
chirimoya authored Apr 4, 2022
2 parents 8c6f977 + 8fcc4ca commit 1ab810f
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 36 deletions.
4 changes: 4 additions & 0 deletions DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ public function getConfigTreeBuilder()
$rootNode = $treeBuilder->getRootNode();

$rootNode->children()
->booleanNode('csrf_protection')
->info('Enable csrf protection for dynamic forms.')
->defaultFalse()
->end()
->scalarNode('sendinblue_api_key')->defaultValue(null)->end()
->scalarNode('mailchimp_api_key')->defaultValue(null)->end()
->scalarNode('mailchimp_subscribe_status')->defaultValue('subscribed')->end()
Expand Down
1 change: 1 addition & 0 deletions DependencyInjection/SuluFormExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ public function load(array $configs, ContainerBuilder $container): void

$mediaCollectionStrategy = $config['media_collection_strategy'] ? $config['media_collection_strategy'] : $config['media']['collection_strategy'];

$container->setParameter('sulu_form.csrf_protection', $config['csrf_protection']);
$container->setParameter('sulu_form.mail.from', $config['mail']['from']);
$container->setParameter('sulu_form.mail.to', $config['mail']['to']);
$container->setParameter('sulu_form.mail.sender', $config['mail']['sender']);
Expand Down
11 changes: 9 additions & 2 deletions Form/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,20 @@ class Builder implements BuilderInterface
*/
private $csrfTokenManager;

/**
* @var bool
*/
private $csrfProtection;

public function __construct(
RequestStack $requestStack,
FormFieldTypePool $formFieldTypePool,
TitleProviderPoolInterface $titleProviderPool,
FormRepository $formRepository,
FormFactory $formFactory,
Checksum $checksum,
CsrfTokenManagerInterface $csrfTokenManager
CsrfTokenManagerInterface $csrfTokenManager,
bool $csrfProtection = false
) {
$this->requestStack = $requestStack;
$this->formFieldTypePool = $formFieldTypePool;
Expand All @@ -87,6 +93,7 @@ public function __construct(
$this->formFactory = $formFactory;
$this->checksum = $checksum;
$this->csrfTokenManager = $csrfTokenManager;
$this->csrfProtection = $csrfProtection;
}

public function buildByRequest(Request $request): ?FormInterface
Expand Down Expand Up @@ -197,7 +204,7 @@ private function createForm(string $name, string $type, string $typeId, string $
$typeName = $this->titleProviderPool->get($type)->getTitle($typeId, $locale);

$recaptchaFields = $formEntity->getFieldsByType('recaptcha');
$csrfTokenProtection = true;
$csrfTokenProtection = $this->csrfProtection;

if (\count($recaptchaFields)) {
$csrfTokenProtection = false;
Expand Down
1 change: 1 addition & 0 deletions Resources/config/services.xml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
<argument type="service" id="form.factory" />
<argument type="service" id="sulu_form.checksum" />
<argument type="service" id="security.csrf.token_manager"/>
<argument>%sulu_form.csrf_protection%</argument>
</service>

<!-- Dynamic Form Type -->
Expand Down
50 changes: 33 additions & 17 deletions Resources/doc/csrf.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,13 @@
# CSRF Token

The csrf token is session based so it need to be loaded over
an seperate request this can be done by esi (used by the basic theme) or ajax.
In your form theme template.
The csrf token is session based, because of the sulu caching mechanism
it is needed to be load that token over a separate request (ajax).

## ESI

Add the following to your form theme to overwrite the default
behaviour of token generation or use the `@SuluForm/themes/basic.html.twig` theme.
Enable csrf protection for dynamic forms via:

```twig
{%- block csrf_token_widget -%}
{{ render_esi(controller('Sulu\\Bundle\\FormBundle\\Controller\\FormTokenController::tokenAction', {
'form': form.parent.vars.name,
'html': true,
_requestAnalyzer: false
})) }}
{% endblock %}
```yaml
sulu_form:
csrf_protection: true
```
## Ajax
Expand All @@ -43,6 +34,7 @@ A simple example for loading the csrf token over ajax looks like this:
{%- block csrf_token_widget -%}
{{ block('hidden_widget') }}
{# this is just an example it should use data attributes or something similar to read formName and fieldId #}
<script>
var formName = '{{ form.parent.vars.name }}';
var fieldId = '{{ id }}';
Expand All @@ -51,8 +43,15 @@ A simple example for loading the csrf token over ajax looks like this:
```

```js
jQuery.get('/_form/token?form=' + formName + '&html=0').done(function(data) {
jQuery('#' + fieldId).val(data);
fetch('/_form/token?form=' + formName + '&html=0', {
credentials: 'same-origin', // required for old safari versions
headers: {
'X-Requested-With': 'XMLHttpRequest',
},
}).then((response) => {
return response.text();
}).then((token) => {
document.getElementById(fieldId).value = token;
});
```

Expand Down Expand Up @@ -98,3 +97,20 @@ import CsrfToken from './components/csrf-token';
web.registerComponent('csrf-token', CsrfToken);
```

## ESI

> This solution does not work with Symfony 5.4 or later. Please use ajax loading when enabling csrf protection.

Add the following to your form theme to overwrite the default
behaviour of token generation or use the `@SuluForm/themes/basic.html.twig` theme.

```twig
{%- block csrf_token_widget -%}
{{ render_esi(controller('Sulu\\Bundle\\FormBundle\\Controller\\FormTokenController::tokenAction', {
'form': form.parent.vars.name,
'html': true,
_requestAnalyzer: false
})) }}
{% endblock %}
```
8 changes: 4 additions & 4 deletions Resources/doc/dynamic.md
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,9 @@ sulu_form:

The following things you should check when implement the dynamic form type on your website.

- Test CSRF Token on production in 2 different browser sessions
- Test media upload
- Test the notifiy email
- Test the notify email
- Test the customer email
- Test backend field errors
- Test backend general errors ( e.g. remove CSRF token value )
- Test form submit errors (e.g. use spaces in required fields should show error after submit)
- Test backend general errors ( e.g. remove CSRF token value if enabled )
- Test CSRF Token on production in 2 different browser sessions (when csrf protection enabled)
13 changes: 0 additions & 13 deletions Resources/doc/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,6 @@ Sulu\Bundle\FormBundle\SuluFormBundle::class => ['all' => true],

## Config

Activate esi for csrf token reload on cache pages
by change the following lines in `config/packages/framework.yaml`.

```yml
framework:
esi: true
fragments: true
```
Configure the default sender and receivers email address (optional):

```yml
Expand Down Expand Up @@ -92,7 +83,3 @@ Make sure you've set the correct permissions in the Sulu backend for this bundle
- [Sendinblue](sendinblue.md "Sendinblue Form Field")
- [Recaptcha](recaptcha.md "Recaptcha Form Field")
- [Dropzone](dropzone.md "Dropzone Form Field")

## Varnish

Using varnish have a look at the [CSRF](csrf.md "CSRF Token") documentation.
22 changes: 22 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
# Upgrade

## 2.4.0

### CSRF Protection disabled

By default, the csrf protection is disabled now because of caching mechanism it is required that
csrf token is loaded over ajax. Since Symfony 5.4, it is not possible todo this over ESI, as the
session cookie can not be created by a Symfony subrequest, which was already the behaviour before when
using a caching server like varnish.

If you want to enable csrf protection again it is required to configure:

```yaml
sulu_form:
csrf_protection: true
```
And important implement ajax loading of the csrf protection.
See the [CSRF Ajax documentation](Resources/doc/csrf.md) as an example.
As an alternative protection there is [HoneyPotField](Resources/doc/dynamic.md#add-honeypot-field-for-spam-protection)
or [Recaptcha](Resources/doc/recaptcha.md). Which both do not require a session.
## 2.1.1
### Builder constructor changed
Expand Down

0 comments on commit 1ab810f

Please sign in to comment.