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: nginx.conf updated with url sources based on cfpb analytics whitelist #818

Merged
merged 8 commits into from
Aug 1, 2024

Conversation

shindigira
Copy link
Contributor

closes #817

Changes

  • feat: nginx.conf updated with url sources based on cfpb analytics whitelist

How to test this PR

  1. Test the deployment for google analytics handling

Copy link
Contributor

@billhimmelsbach billhimmelsbach left a comment

Choose a reason for hiding this comment

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

I think you accidentally pushed things you don't want in this PR. Messaging you now.

@shindigira
Copy link
Contributor Author

I think you accidentally pushed things you don't want in this PR. Messaging you now.

Thank you for catching that.

Copy link
Contributor

@billhimmelsbach billhimmelsbach left a comment

Choose a reason for hiding this comment

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

Heyo @shindigira! Can you push this change up to the preview branch, so we can test it / confirm it with the analytics team before we move it to staging?

@shindigira
Copy link
Contributor Author

Heyo @shindigira! Can you push this change up to the preview branch, so we can test it / confirm it with the analytics team before we move it to staging?

Updated preview with this branch.

Copy link
Contributor

@billhimmelsbach billhimmelsbach left a comment

Choose a reason for hiding this comment

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

Heyo! So we confirmed this morning that things were working now, but if you take another peek at the issue, Paul just wants us to add these four domains:

"dap.digitalgov.gov",
"*.googleanalytics.com",
"*.google-analytics.com",
"*.googletagmanager.com",

We don't need all the new CSP additions from that example Paul gave us (like the font-src exceptions), we just need the 4 domains above in the same places they were in the example. Having said that, I might also keep the mouseflow.com ones, just to save us a step when he implements it soon.

Copy link
Contributor

@billhimmelsbach billhimmelsbach left a comment

Choose a reason for hiding this comment

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

I confirmed with Paul that things are looking good, so this one is ready for staging. 🎉

@shindigira shindigira merged commit f35e303 into main Aug 1, 2024
4 checks passed
@shindigira shindigira deleted the 817-csp-update branch August 1, 2024 16:19
billhimmelsbach pushed a commit that referenced this pull request Aug 1, 2024
…telist (#818)

closes #817 

## Changes
- feat: nginx.conf updated with url sources based on cfpb analytics
whitelist

## How to test this PR

1. Test the deployment for google analytics handling
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.

Content Security Policy - Update to allow certain domains
2 participants