Remove header for LG TV apps

[noahm]

Problem

Fixes #125

Solution

• Updated nginx config to exclude the X-Frame-Options header when responding to clients sending WebOS in their user agent.
• I have manually tested and confirmed the fix with my own TV.

PR Status

• [x] Code finished and ready to be reviewed/tested
• [x] The fix/enhancement were manually tested (if applicable)

Automatic tests

Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)

[tituspijean]

I actually now experience this issue too, thanks for this PR! I will try it out by Thursday. 🤞

[tituspijean]

Using if in NGINX is generally discouraged due to poorer performance, so it got me looking into the issue. Could you have a look to this forum thread? https://forum.jellyfin.org/t-solved-webos-error-code-27

It looks like specifying without the if block more_set_headers X-Frame-Options : "ALLOW-FROM https://__DOMAIN____PATH__"; would work? Could you try it?

[noahm]

Ah, interesting. I'll try that and report back.

[noahm]

Using if in NGINX is generally discouraged due to poorer performance

Do you have any documentation for this? I did some searches and couldn't come up with anything. (Perhaps this was true of nginx many years ago, but is it still true today?)

It looks like specifying without the if block more_set_headers X-Frame-Options : "ALLOW-FROM https://__DOMAIN____PATH__"; would work? Could you try it?

I looked into this a bit further and the ALLOW-FROM directive of X-Frame-Origins is obsolete and not respected by any browser shipped in many years. This means it's essentially equivalent to simply omitting the X-Frame-Options header entirely.

The successor and modern alternative is the Content-Security-Policy header and its frame-ancestors directive. I tested with several different variants of that and all of them broke the LG app, but in a different way than before. With the current X-Frame-Options present, an "Unable To Load" error screen is presented with error code 27. With the CSP frame-ancestors set instead, the app appears to boot, but it gets stuck on a blank dark grey screen without any content or error message at all.

In summary I think the only two functioning, reasonable options are either:

1. Simply remove the X-Frame-Options header from jellyfin responses unconditionally, accepting this allows the possibility someone can iframe-embed your jellyfin server into other places on the web.
2. Attempt to preserve existing security posture at the cost of some performance by remove the header conditionally with the if directive I started with

I'll happily update this PR and remove the conditional check, going with option 1 there if you think that's better over all.

[tituspijean]

So, it was actually a bad remembrance on my part, the old NGINX wiki mentions that "if is evil", but not due to performance: https://github.com/nginxinc/nginx-wiki/blob/836ecd605a1b9861fb608e848336bca9b8640b54/source/start/topics/depth/ifisevil.rst

Either we remove the header with more_set_headers X-Frame-Options : ""; or we implement what you suggested in this PR. I'll ping other packagers, I actually have no idea what's best or worst.

[noahm]

Wow, that's a fascinating hole to dive down. I read through everything and the other explanation blog post linked at the end and I think the use case I've proposed will work as long as a second if directive isn't added in the future?

I'm also curious, is this advice still relevant? The page seems to have been taken down and there's no mention of the dangers in the official docs on the if statement.

[tituspijean]

Indeed, it has been removed, and the performance impact I imagined should be minimal. As for safety, let's minimize whose clients we remove the header from, so let's go with your code. :)