Fix some base URL bugs #5
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Removed the |
|
Thank you @ausi. |
leofeyer
pushed a commit
that referenced
this pull request
Jun 23, 2022
leofeyer
added a commit
that referenced
this pull request
Jun 24, 2022
Description ----------- - Commits ------- 18f27e2 Get rid of the base tag 50c2f91 Also adjust the AssetListener e70e6e2 Update the UPGRADE.md file 1db8ad0 Remove the File::getFullPath() method again a6c2d2a Add the base tag in the front end again 7de8805 Automatically add and remove `{{env::path}}` to relative links in Tin… 6a5804c Use Environment::get('path') in the legacy code 6b5f635 Add the BasePathPrefixer service 0e1562c Use the asset() function in the code.html.twig template be5de2c Apply suggestions from code review 8f051c7 Fix the tests 8023081 Use requestUri instead of request a8bab4d Fix the Controller::addToUrl() method 11a79b5 Fix a leftover cfebc2a Prevent the "creation of dynamic property" warning fbf057a Use "env::base_path" instead of "env::path" ae3483a Use "requestUri" instead of "indexFreeRequest" c287ec7 Add the base_path_prefixer to the "file" insert tag 757f9bf Do not prefix the URL in redirect exceptions a5e1ee4 Remove two redundant test methods 29bcd8c Fix some base URL bugs (see #5) dde9686 Use app.request.basePath instead of the asset() function 8ff5f08 Fix the tests 66a6b6e Use the assets.context instead of a custom service 38a4610 Use the asset() method in the code.html.twig template 9b72d6b Fix the tests beefd78 Also prefix the image URL and the redirect URL ee7e745 Add the Validator::isRelativeUrl() method f76053c Use "contao.assets.files_context" for file URLs 154fb12 Fix the URL in the file insert tag a94dc2f Remove the deprecation and add the "title" field again 3832d24 Revert the ce_player.html5 changes Co-authored-by: Martin Auswöger <martin@auswoeger.com>
leofeyer
added a commit
that referenced
this pull request
Jan 26, 2023
Description ----------- Closes contao#5712 Commits ------- 7a58d3d allow the teaser to be null (and therefore "not in the model", see #5… a33800b CS Co-authored-by: Leo Feyer <1192057+leofeyer@users.noreply.github.com>
leofeyer
added a commit
that referenced
this pull request
Jan 17, 2024
Description ----------- This introduces the ability to define Content Security Policies for the front end. There will be new settings for each website root: <img src="https://github.com/contao/contao/assets/4970961/abf4bad8-f025-4cb0-be82-c3088c68c1fc" width="898"> Within the _Content Security Policy_ field you can define your policies - with the exakt same syntax as the `Content-Security-Policy` header. For example `default-src 'self'` will tell the browser to only accept resources from the same domain and not allow any inline styles or scripts at all (at least not without nonces). Once a policy for `default-src`, `script-src` or `style-src` is set [1], Contao will automatically generate nonces for the respective `<script>` and `<style>` elements, at least in supported templates. If you have any custom templates with these tags, then you will need to adjust your template to include the nonce, which is pretty simple: ```twig <script nonce="{{ contao_csp_nonce('script-src') }}"> ``` ```php <script nonce="<?= $this->nonce('script-src') ?>"> ``` In this PR the `nonce` attribute is added conditionally everywhere, i.e. the attribute will not be set, if no nonce was necessary and thus no nonce was generated. So there will not be empty `nonce` attributes in the HTML code (though it would be fine). ## Allowing Specific Sources Dynamically Some content elements like the video, YouTube and Vimeo players dynamically allow their respective sources. This can be done directly from within the templates: ```twig {% do add_csp_source('frame-src', iframe_attrs.src) %} ``` ```php <?php $this->addCspSource('frame-src', $this->src); ?> ``` So even if you set the policies `frame-src 'self'; media-src 'none'` these content elements will still work as they dynamically allow their specific sources. ## Reporting In the CSP spec you also have the possibility to set a reporting URL. If the browser detects a CSP violation it will then automatically `POST` a report to that URL. Reporting needs to be specifically enabled in the website root. These reports will then also show up in the system log: <img src="https://github.com/contao/contao/assets/4970961/3ce826f7-48e3-4586-b28a-bdbabda9a765" width="879"> The system log entry will tell you which policy was violated and on which line. The detailed information will contain the original URL of the violation. This is of course a public API endpoint where anyone or anything can send any data. As such this needs to be handled with care. This PR uses the `ContentSecurityPolicyController` of `nelmio/security-bundle` which has the ability to filter out "noise". But instead of using that controller directly this PR implements its own controller and injects the service of Nelmio's controller. Our route always requires a valid page ID and always checks whether the current page's root has reporting enabled. If not, it will respond with 404 - otherwise it will forward the request to the `ContentSecurityPolicyController` of `nelmio/security-bundle`. ## Twig Components This PR introduces two new Twig components: `_style` and `_script`. When using them you do not have to think about having to check for nonces as the component does this automatically for you. However, I find that using these components makes the templates less readable - as there will also be no syntax highlighting by your editor anymore, since it won't know that the content of the component block is within a `<script>` or `<style>` tag. ## Thoughts, Decisions, Caveats Initially I thought may be Contao should set some sensible defaults automatically and also automatically allow the `staticFiles` and `staticPlugins` domains. However, this approach can be difficult and flawed. Contao cannot know in advance what kind of sources are hosted on its current and any of the static domains, thus we cannot set any `-src` policies (we would have to set all of them). It needs to be solely up to the administrator to set appropriate policies. One exception could be iframe sources, which could be automatically added (see above). However, allowing specific sources dynamically also has a caveat. Let's take the YouTube content element for example: on the page where such a content element is added, the Content-Security-Policy will automatically be the following [2]: ``` frame-src https://www.youtube.com/embed/abc123 ``` However, this will also mean that _no other iframes_ will be allowed on the same page, whereas without this content element they would be. We could change the helper functions to not add the directive if no existing (or `default-src` directive) is already present. Unfortunately the `CSPBuilder` has no such API at the moment. Further comments about the code will be down below in threads. PR is still a draft due to missing tests. ## Footnotes * [1]: ~If you _only_ set a `default-src` policy no nonces will be generated currently unfortunately due to an oversight in `paragonie/csp-builder` (see paragonie/csp-builder#65 not relevant after overhaul * [2]: ~Actually it will be only `child-src` without legacy browser support enabled (see paragonie/csp-builder#76 not relevant after overhaul Commits ------- 6a34219 add CSP 967f1ae fix line endings 6b36350 merge with 5.x 014c8e6 cs fix a008dce Merge remote-tracking branch 'origin/5.x' into csp 07de412 update dependencies 2473309 introduce script component 4ce5d5d mvp b211d7f add style component 0ec6738 remove unnecessary scopes 9b7dbc4 move templates to new location 9bdbf32 Merge remote-tracking branch 'origin/5.x' into csp 420bf35 add accidental removal 0fa2265 improve the implementation 613368c remove parameter definition 7c86310 remove getNonce method 7432de8 Merge remote-tracking branch 'origin/5.x' into csp fe45eed validate CSP a7466d6 fix toolbar a0f8ef9 fix toolbar yet again c9505eb consolidate CspRuntime and fix absolute source paths 99ac12a fix reporting 24e56f1 only add nonce if necessary 9029d31 remove unrelated change 1274da8 code style 87017e5 add tests 05f7141 fix service name ae8faa5 revert changes to stylesheet component 067dfce satisfy PHP stan ee4a542 update test db52a54 fix ci 4a52f50 increase nelmio/security-bundle requirements 2767dd3 Merge remote-tracking branch 'origin/5.x' into csp 2e312ca update csp-builder and security-bundle requirements, satisfy PHP stan 9131551 merge with 5.x bda5f21 overhaul 67cd1a6 fix test f365461 merge with 5.x 2598948 fix accidental öö 0fc5c61 Update core-bundle/contao/templates/twig/component/_script.html.twig ba71e55 fix _style.html.twig 13715f0 do not use any legacy CPS headers ccf927d change exception message af9a9db fix test 4510a92 move to private function 13b3c2a allow to enable report logging in website root 8356ac4 remove remnant config c727221 add comment 4cffecf fix prefer lowest 6e8538e code style ad61e35 fix functional tests differently 8d0bd31 remove remnant a0f2e74 fix another remnant 0ceea3f restore previous response context finalisation b7cae39 merge with 5.x a9fdea8 use textarea for csp field 20a5e7f Apply suggestions from code review 7056144 move addCspSource to bottom 76082c7 merge with 5.x e90cb34 do not call nonce statically 482cebd do not print line number if none given 4b83528 code style 35e1f6e Reuse HtmlAttributes (#5) dceaf2b Merge remote-tracking branch 'origin/5.x' into csp 9d2459b fix test dbac1d6 implement fallback logic for addSource 43ce805 add more tests f745ea2 fix accidental change 1874f78 set default-src 'self' as default policy b885a67 fix errors in inline generation a9f7b3b add Model annotations 40608d7 Merge remote-tracking branch 'origin/5.x' into csp 23a441d change translation 335d277 Merge remote-tracking branch 'origin/5.x' into csp 4fdd33f remove _script and _style components 27d5517 Merge remote-tracking branch 'origin/5.x' into csp e0c27eb remove comma 5f2fcb3 Fix the templates and the CI chain 0503587 Adjust the palettes b1766b6 Merge remote-tracking branch 'origin/5.x' into csp 25ad7c7 make tl_page.csp mandatory 8f892e0 always use 'document-uri' and always set an URL 284ed0c fix test Co-authored-by: Toflar <yanick.witschi@terminal42.ch> Co-authored-by: leofeyer <1192057+leofeyer@users.noreply.github.com>
leofeyer
pushed a commit
that referenced
this pull request
May 29, 2024
…ontao#7248) Description ----------- If a `tl_article` record still contains `keywords` from previous Contao versions and you reference this article in an `article` content element, the following warning will appear in the back end: ``` ErrorException: Warning: Undefined global variable $TL_KEYWORDS contao#24 /vendor/contao/core-bundle/src/Resources/contao/modules/ModuleArticle.php(207): Contao\ModuleArticle::compile contao#23 /vendor/contao/core-bundle/src/Resources/contao/modules/Module.php(214): Contao\Module::generate contao#22 /vendor/contao/core-bundle/src/Resources/contao/modules/ModuleArticle.php(70): Contao\ModuleArticle::generate contao#21 /vendor/contao/core-bundle/src/Resources/contao/library/Contao/Controller.php(550): Contao\Controller::getArticle contao#20 /vendor/contao/core-bundle/src/Resources/contao/elements/ContentArticle.php(30): Contao\ContentArticle::generate contao#19 /vendor/contao/core-bundle/src/Resources/contao/library/Contao/Controller.php(622): Contao\Controller::getContentElement contao#18 /var/cache/prod/contao/dca/tl_content.php(318): tl_content::addCteType contao#17 /vendor/contao/core-bundle/src/Resources/contao/drivers/DC_Table.php(4717): Contao\DC_Table::parentView contao#16 /vendor/contao/core-bundle/src/Resources/contao/drivers/DC_Table.php(313): Contao\DC_Table::showAll contao#15 /vendor/contao/core-bundle/src/Resources/contao/classes/Backend.php(667): Contao\Backend::getBackendModule contao#14 /vendor/contao/core-bundle/src/Resources/contao/controllers/BackendMain.php(168): Contao\BackendMain::run #13 /vendor/contao/core-bundle/src/Controller/BackendController.php(49): Contao\CoreBundle\Controller\BackendController::mainAction #12 /vendor/symfony/http-kernel/HttpKernel.php(163): Symfony\Component\HttpKernel\HttpKernel::handleRaw #11 /vendor/symfony/http-kernel/HttpKernel.php(75): Symfony\Component\HttpKernel\HttpKernel::handle #10 /vendor/symfony/http-kernel/Kernel.php(202): Symfony\Component\HttpKernel\Kernel::handle #9 /vendor/symfony/http-kernel/HttpCache/SubRequestHandler.php(86): Symfony\Component\HttpKernel\HttpCache\SubRequestHandler::handle #8 /vendor/symfony/http-kernel/HttpCache/HttpCache.php(481): Symfony\Component\HttpKernel\HttpCache\HttpCache::forward #7 /vendor/symfony/framework-bundle/HttpCache/HttpCache.php(73): Symfony\Bundle\FrameworkBundle\HttpCache\HttpCache::forward #6 /vendor/symfony/http-kernel/HttpCache/HttpCache.php(454): Symfony\Component\HttpKernel\HttpCache\HttpCache::fetch #5 /vendor/contao/manager-bundle/src/HttpKernel/ContaoCache.php(66): Contao\ManagerBundle\HttpKernel\ContaoCache::fetch #4 /vendor/symfony/http-kernel/HttpCache/HttpCache.php(348): Symfony\Component\HttpKernel\HttpCache\HttpCache::lookup #3 /vendor/symfony/http-kernel/HttpCache/HttpCache.php(226): Symfony\Component\HttpKernel\HttpCache\HttpCache::handle #2 /vendor/friendsofsymfony/http-cache/src/SymfonyCache/EventDispatchingHttpCache.php(96): Contao\ManagerBundle\HttpKernel\ContaoCache::handle #1 /web/index.php(44): require #0 /web/app.php(13): null ``` This is because `$GLOBALS['TL_KEYWORDS']` is only initialised in the front end within `PageRegular::prepare()`. This PR simply ignores it if not set. Commits ------- e54dc2b fix warning when legacy keywords are present
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See https://github.com/contao/contao/pull/4851/files
Not sure if they are all related to this very PR though.