BookStackApp / BookStack

A platform to create documentation/wiki content built with PHP & Laravel
https://www.bookstackapp.com/
MIT License
15.47k stars 1.94k forks source link

test_frame_src_csp_header_set fails on customized ALLOWED_IFRAME_SOURCES #5068

Closed mueller-contria closed 4 months ago

mueller-contria commented 5 months ago

Describe the Bug

test_frame_src_csp_header_set() (in tests/SecurityHeaderTest.php) tests the CspHeader->frame-src against a fixed string, identical to the default-setting for ALLOWED_IFRAME_SOURCES. If we customize ALLOWED_IFRAME_SOURCES in our .env, the test fails.

Comparable Environment Variables are defined in the phpunit.xml, but unfortunately not ALLOWED_IFRAME_SOURCES

I will provide a Pull-Request for phpunit.xml soon.

Steps to Reproduce

  1. Add a line like ALLOWED_IFRAME_SOURCES="https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com https://*.google.com" to your .env-file
  2. Execute vendor/bin/phpunit --filter test_frame_src_csp_header_set
  3. The test fails
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual
    @@ @@
    -'frame-src 'self' https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com'
    +'frame-src 'self' https://*.draw.io https://*.youtube.com https://*.youtube-nocookie.com https://*.vimeo.com https://*.google.com'

Expected Behaviour

The test should pass.

The tests should not rely on the ALLOWED_IFRAME_SOURCES-value set in .env. Instead, phpunit.xml should define the value of this env-var.

Screenshots or Additional Context

No response

Browser Details

CLI / PHPUnit

Exact BookStack Version

v24.03-dev

ssddanbrown commented 4 months ago

Closed with merge of #5069, thanks again @mueller-contria!