Replace inline onclick with addEventListener inside .js file to be compatible with CSP (v6)

[orazioedoardo]

Thank you for your contribution to the Pi-hole Community!

Please read the comments below to help us consider your Pull Request.

We are all volunteers and completing the process outlined will help us review your commits quicker.

Please make sure you

1. Base your code and PRs against the repositories developmental branch.
2. Sign Off all commits as we enforce the DCO for all contributions
3. Sign all your commits as they must have verified signatures
4. File a pull request for any change that requires changes to our documentation at our documentation repo

---

What does this PR aim to accomplish?:

Makes the page compatible with a CSP that only allows script sourced from same origin, see pi-hole/FTL#2029.

How does this PR accomplish the above?:

Replaces inline onclick with addEventListener inside .js file.

---

By submitting this pull request, I confirm the following:

1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
2. I have commented my proposed changes within the code and I have tested my changes.
3. I am willing to help maintain this change if there are issues with it later.
4. It is compatible with the EUPL 1.2 license
5. I have squashed any insignificant commits. (git rebase)
6. I have checked that another pull request for this purpose does not exist.
7. I have considered, and confirmed that this submission will be valuable to others.
8. I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
9. I give this submission freely, and claim no ownership to its content.

---

• I have read the above and my PR is ready for review. Check this box to confirm

[DL6ER]

I'm not too familiar with CSP but for me logging out still works on latest Firefox and Chrome even when on the new FTL branch but not on this web branch (i.e., with the existing development-v6 code). Does this mean the CSP is non-effective?

[orazioedoardo]

Does this mean the CSP is non-effective?

CSP in the configuration of my other pull request doesn't allow inline onlick.

To verify, recompile FTL with the other pull request, then visit the web interface without this pull request and observe that logout doesn't work.

[DL6ER]

This is exactly what I did. The Dev Tools confirm it's working as expected.

[orazioedoardo]

Logout shouldn't work anymore (check error in console) unless you also apply this pull request, can you confirm?

[DL6ER]

Yes, it still works with FTL being on your branch and web being on develpoment-v6. Log out works and there are no errors in the Console despite these two unrelated messages after the logout succeeded:

[orazioedoardo]

Yes, it still works with FTL being on your branch and web being on develpoment-v6.

It shouldn't though. CSP should give a violation report blocking the execution. It's the reason why I opened this pull request.

[PromoFaux]

Can confirm the same behaviour as DL6ER:

orazioedoarado:csp branch of FTL with pi-hole:development-v6 branch of web:

Logout works, following unrelated errors on console

orazioedoarado:csp branch of FTL with orazioedoarado:onclick branch of web

Logout works, following unrelated errors on console

Tried both http and https

[orazioedoardo]

Just tried with the changes of my CSP branch of FTL and the onlick handler is blocked on Chrome, Firefox and Safari.

Only difference is that I'm applying the headers by proxying through Nginx but the output is the same.

Firefox shows another (unrelated) error regarding the SVG logo for which I will open a workaround PR once you are able to reproduce this PR.

[rdwebdesign]

Do you see the same if you access http://pi.hole/admin ?

[orazioedoardo]

Do you see the same if you access http://pi.hole/admin ?

It doesn't make a difference (as I expected).

[PromoFaux]

Are we saying here that one needs to be behind a reverse proxy in order to reproduce this?

[orazioedoardo]

Are we saying here that one needs to be behind a reverse proxy in order to reproduce this?

No, it's a response header handled by the browser. I just did it this way to avoid recompiling FTL. Also works for me by injecting the header via Chrome developer tools.

Are there builds of FTL I can try?

[PromoFaux]

You can find the latest builds for development-v6 here https://ftl.pi-hole.net/development-v6/

Though that might not be what you mean! When testing your branches locally, I first built it in the devcontainer, and then built a new pi-hole docker image locally with ./build.sh -l from the docker-pi-hole repo. (obviously replacing the web branch with yours, too - which I needed to adjust the dockerfile to do)

[orazioedoardo]

Though that might not be what you mean!

Yeah I meant builds of my branch of FTL which I assumed you have to test it.

[DL6ER]

Foreign branches aren't built to protect internal secrets which could otherwise be "stolen". This is a default defensive behavior with Github Actions. But I don't think this matters here, both @PromoFaux and myself pulled your changes and built FTL locally for testing. FTL sent the expected CSP headers as you can see in my dev console screenshot further up. Yet, we cannot reproduce the described behavior which makes us a bit clueless at what is going on here.

[orazioedoardo]

Looks like FTL is adding "additional_header" for static files and API requests but NOT actual pages (text/html), where CSP matters the most.

[DL6ER]

You are right, this is a bug in the webserver we embedd: CivetWeb. I've already created a bugfix PR upstream: civetweb/civetweb#1286

Thanks for pointing this out. With this fix applied, I can reproduce your observation and indeed confirm that your PR here fixes this issue.