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

feat: Middleware to add Content-Security-Policy headers #289

Merged
merged 7 commits into from
Apr 12, 2023
Merged

Conversation

timmc-edx
Copy link
Contributor

This replaces edx/edx-arch-experiments#256 and removes some features. First commit is a straight copy of the Python (plus a couple of unrelated fixes), second commit contains unreviewed changes.

Merge checklist:

  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed

**Note:** This commit will fail tests, as edx-toggles has not been added to
the requirements (it would create a circular dependency) and there is still
a module reference to edx-arch-experiments. This is a straight copy of the
Python from <edx/edx-arch-experiments#256>.

Also:

- Add missing changelog entry
- Remove outdated note about pinned dependency (was unpinned in 7c9d5af)
- Remove toggle; we can't import edx-toggles, and having One More Thing to
  remember to configure is probably just too much complexity for
  deployment. The risk of "I think I turned it on, but I don't see reports"
  and then making a bad policy change as a result is worse than "oops, I
  deployed a bad policy and overwhelmed the reporting server".
- Remove validation of report-to name. Just give general guidance, and
  people can read the spec if they want to use a weird name.
- Update module reference in tests.
- Fix docstring for `CSP_REPORTING_URI`, which I had failed to update after
  copy-pasting.
Copy link
Contributor

@rgraber rgraber left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@rgraber rgraber left a comment

Choose a reason for hiding this comment

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

Still looking good

@timmc-edx
Copy link
Contributor Author

Tests were passing before the previous two commits, but now there's something badly wrong with the codecov package (https://pypi.org/project/codecov/) and CI is broken. But make test, make quality, and make docs all work locally, so I'm going to go ahead and merge.

@timmc-edx timmc-edx merged commit 0864c3c into master Apr 12, 2023
@timmc-edx timmc-edx deleted the timmc/csp branch April 12, 2023 13:27
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