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

Updated secure_headers gem to 3.0.0 #6908

Merged
merged 1 commit into from
Feb 24, 2016

Conversation

skateman
Copy link
Member

Overriding the connect-src CSP is just a partial solution and after all our WebSocket connections are proxied through :443, it should be updated.

@skateman
Copy link
Member Author

@martinpovolny please review

@skateman
Copy link
Member Author

cc @chrisarcand because of b42ddd6

@@ -4,7 +4,7 @@
config.x_content_type_options = "nosniff"
config.x_xss_protection = "1; mode=block"
config.csp = {
:enforce => true,
Copy link
Member

Choose a reason for hiding this comment

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

NOTE: this defaults to true, so it is not necessary

Copy link
Member

Choose a reason for hiding this comment

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

Confused; Based on this you seem to be changing this value, as well as the config rename. Is this intended?

Copy link
Member

Choose a reason for hiding this comment

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

@chrisarcand thanks - I was wrong. the migration guide said to s/enforce/report_only/

this was intentional

@kbrock
Copy link
Member

kbrock commented Feb 24, 2016

👍 LGTM. lets merge when 🍏

@miq-bot
Copy link
Member

miq-bot commented Feb 24, 2016

<github_pr_commenter_batch />Some comments on commit skateman@9d3dc46

@miq-bot
Copy link
Member

miq-bot commented Feb 24, 2016

Checked commit skateman@9d3dc46 with ruby 2.2.3, rubocop 0.34.2, and haml-lint 0.13.0
4 files checked, 0 offenses detected
Everything looks good. 👍

@chessbyte
Copy link
Member

@chrisarcand Are you 👍 to merge?

@chrisarcand
Copy link
Member

Yeah, looks like that option was intended. 👍 :shipit:

chessbyte added a commit that referenced this pull request Feb 24, 2016
Updated secure_headers gem to 3.0.0
@chessbyte chessbyte merged commit 2a6b0ac into ManageIQ:master Feb 24, 2016
@chessbyte chessbyte added this to the Sprint 37 Ending Mar 7, 2016 milestone Feb 24, 2016
@skateman skateman deleted the secureheaders-3 branch February 24, 2016 15:22
@kbrock
Copy link
Member

kbrock commented Feb 25, 2016

thanks all

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

Successfully merging this pull request may close these issues.

5 participants