Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-add CSP, use flask-talisman #234

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Conversation

epicfaace
Copy link
Contributor

  • Re-add CSP, set unsafe-inline for CSS and unsafe-eval for JS to ensure the proofer functionality still works
  • Use flask-talisman to set CSP. This also sets a bunch of other best-practice security defaults like HSTS, etc. -- see https://github.com/GoogleCloudPlatform/flask-talisman

@akprasad
Copy link
Contributor

Just to check, have you already looked through the docs here?

https://alpinejs.dev/advanced/csp

@epicfaace
Copy link
Contributor Author

Just to check, have you already looked through the docs here?

https://alpinejs.dev/advanced/csp

Yes, I tried this, though 1) alpinejs-csp is not available through the cdn and you need to bundle it locally, and 2) when I did that, and did what the docs suggested around x-data, I still faced some other issues -- which I haven't resolved yet. Happy to push that branch up though.

@akprasad
Copy link
Contributor

akprasad commented Aug 30, 2022

Cool, overall SGTM.

Before merge, let's resolve the outstanding conflicts. I also removed the Server API so let's just use ordinary fetches instead.

@@ -8,6 +8,7 @@ import Reader from './reader';
import Proofer from './proofer';
import HamburgerButton from './hamburger-button';
import ProofingCreatePoll from './proofing-create-poll';
import SortableList from './sortable-list';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move to line 9

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, maybe my diff was out of date? lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants