[GOVCMS-13132] Allow SAML assets in Nginx rules.#512
[GOVCMS-13132] Allow SAML assets in Nginx rules.#512barbun wants to merge 6 commits intogovCMS:3.x-developfrom
Conversation
barbun
changed the title
|
Hi @barbun, thanks for raising this question and for the PR. The way SimpleSAMLphp deploys public assets is a bit unusual. The public/assets directory in the main repository is just an empty placeholder (see: https://github.com/simplesamlphp/simplesamlphp/tree/master/public/assets). Instead of shipping assets directly, SimpleSAMLphp relies on a Composer plugin called composer-module-installer to install “asset modules” that contain the pre-built JS/CSS files. For example, the simplesamlphp-assets-base package (https://github.com/simplesamlphp/simplesamlphp-assets-base) contains the core assets. This means that in order for assets to be deployed into public/assets/, the composer-module-installer plugin must be enabled. If the plugin is disabled, no asset (JS or CSS files) will be installed. For security reasons, GovCMS explicitly disables the composer-module-installer plugin (see: https://github.com/govCMS/lagoon/blob/3.x-develop/composer.json#L44). So even if we loosen the Nginx rules as this PR proposed, the required JS and CSS for the post form still won’t exist in the file system, because the asset modules were never installed. |
|
hi @drupal-spider That's right, I should have mentioned that the below PR needs to be merged in for this rule to have a meaningful effect: My security assessment did not indicate any harm in allowing |
|
@cb-govcms The SSO work I did was based on @mingsong-hu work, and I believe that particular change was adopted from this commit: |
|
@barbun , As discussed, I created another PR (#532) to achieve the goal via a different approach, instead of govCMS/GovCMS#1723 In terms of this PR, as discussed, I would suggest narrowing down what JS files explicitly required by the HTTP-POST binding login. |
|
Hi @mingsong-hu thanks for the update. I have now updated the Nginx rules to only allow one JS file that should be sufficient for ensuring seamless flow with HTTP-POST. |
|
Thanks @barbun for your update. I wonder if we need to include other asset types as well, such as CSS and font? |
|
hi @mingsong-hu my tests showed that it isn't required to ensure the auto-submit functionality. However, it means that the intermediary page will not be styled. If the auto-submit happens quickly then the exposure to the raw page is almost un-noticeable. If I remember correctly, the current customer in question that uses HTTP-POST only had the JS file added, which was sufficient for them. |
| alias /app/vendor/simplesamlphp/simplesamlphp/public/assets/base/js/post.js; | ||
| access_log off; | ||
| expires 1h; | ||
| add_header Cache-Control "public"; |
There was a problem hiding this comment.
Mostly, it looks good to me. I wonder if we need to add the header of 'public' here. That will prevent the inheritance of all headers directives from parent blocks, such as X-Content-Type-Options, X-Frame-Options, Content-Security-Policy, etc for this JS file.
There was a problem hiding this comment.
@mingsong-hu fair point, I've removed that line now.
3 participants
Issue
Current Nginx rules are very narrow and do not allow assets from
SimpleSAMLphplibrary to be loaded. These assets are needed when e.g. using HTTP-POST binding that creates an intermediary page with a form to ensure that the form gets auto-submitted. When missing, the users get stuck on that intermediary page having to manually submit the form.Proposed solution
Open up the Nginx rules to allow
public/assets/path fromsimplesamlphplibrary. That path is meant to be public by design and does not include any php or secrets but only some fonts, js and css.