ALLOW-ALL option is too permissive, should be configurable

Created on 19 November 2022, about 2 years ago
Updated 1 March 2023, over 1 year ago

Problem/Motivation

Steps to reproduce

Select ALLOW-ALL option, observe header is gone from your site

Proposed resolution

Allow specifying a substring/path that can be used to limit when the X-Frame-Options header is removed

Remaining tasks

User interface changes

Add a form field similar to ALLOW-FROM uri option.

API changes

Data model changes

Add a config param to store field, use it.

Feature request
Status

Needs review

Version

1.2

Component

Code

Created by

🇨🇦Canada ehossack

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇨🇦Canada efrainh

    Thanks for your request and your code contribution ehossack.
    Updating ticket so the community help us testing it.

  • 🇨🇦Canada efrainh

    Hi @ehossack, I was testing the new feature, and it looks great, but I have a few suggestions:

    1. The field 'allowed-embedding-substr' should be a textarea, for the cases where we need to have more than one path.
    2. Using str_contains to check the path might lead to allow a whole group of pages that we might not want, for example if we configure /blog, it will also allow all the blog articles that have the URL like /blog/article1 and /blog/article2. I understand that in your case you will have all the pages with the path /embedded, but for other sites that might be different. We could think of accepting tokens like and *.

    What do you think?

Production build 0.71.5 2024