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

Add how to configure Content Security Policy (CSP) rules in OpenSearch Dashboards dynamically #6291

Merged
merged 14 commits into from
Mar 19, 2024

Conversation

tianleh
Copy link
Member

@tianleh tianleh commented Jan 29, 2024

Description

This PR is to add instructions to configure CSP rules in OSD.

Issues Resolved

Closes #6139

Checklist

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@Naarcha-AWS Naarcha-AWS self-assigned this Jan 30, 2024
@Naarcha-AWS Naarcha-AWS added v2.12.0 2 - In progress Issue/PR: The issue or PR is in progress. labels Jan 30, 2024
@hdhalter hdhalter added v-TBD and removed 2 - In progress Issue/PR: The issue or PR is in progress. v2.12.0 labels Feb 1, 2024
@tianleh tianleh removed the v-TBD label Mar 13, 2024
@hdhalter hdhalter added the release-notes PR: Include this PR in the automated release notes label Mar 13, 2024
@tianleh tianleh marked this pull request as ready for review March 14, 2024 01:07
@tianleh
Copy link
Member Author

tianleh commented Mar 14, 2024

@tianleh
Copy link
Member Author

tianleh commented Mar 14, 2024

Hi, @hdhalter and team, please help take a look.

cc @seraphjiang

@tianleh tianleh changed the title configure Content Security Policy (CSP) rules in OpenSearch Dashboards configure Content Security Policy (CSP) rules in OpenSearch Dashboards dynamically Mar 14, 2024
@hdhalter hdhalter added the 4 - Doc review PR: Doc review in progress label Mar 14, 2024
@hdhalter
Copy link
Contributor

Added @vagimeli as a reviewer since it involves Dashboards.

@tianleh
Copy link
Member Author

tianleh commented Mar 18, 2024

@tianleh Doc review is complete. Please provide me with the link to the README file, and I'll add it where needed. Once you've accepted my edits, I'll move this into editorial review and then merge the PR. If my edits chage technical accuracy, please provide alternative text in your comments, and I'll make the change. Thank you, @vagimeli

@vagimeli Thanks for the review! I just accepted your edits.

Below are the links to the two readme files.

This is for the application configuration https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/application_config/README.md

This is for the csp handler https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/csp_handler/README.md

tianleh added 5 commits March 18, 2024 23:11
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
@tianleh
Copy link
Member Author

tianleh commented Mar 18, 2024

Accidentally flushed your changes. Let me re-apply them. @vagimeli

Signed-off-by: Tianle Huang <[email protected]>
@tianleh
Copy link
Member Author

tianleh commented Mar 18, 2024

@vagimeli please take a look again

Signed-off-by: Melissa Vagi <[email protected]>

Signed-off-by: Melissa Vagi <[email protected]>
@vagimeli vagimeli added 5 - Editorial review PR: Editorial review in progress and removed 4 - Doc review PR: Doc review in progress labels Mar 19, 2024
@vagimeli
Copy link
Contributor

@vagimeli please take a look again

@tianleh Thank you for the links. I've reviewed and moved the PR into the editorial review queue. Once the editor (@natebower) reviews, I'll incorporate any edits. If the editor has technical-related questions, I'll reach out to you as needed. Once the PR is approved and finalized, I'll merge it. Cheers, @vagimeli

Copy link
Collaborator

@natebower natebower left a comment

Choose a reason for hiding this comment

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

@tianleh @vagimeli Please see my comments and changes and let me know if you have any questions. Thanks!

@@ -0,0 +1,50 @@
---
layout: default
title: Content Security Policy (CSP) rules dynamic configuration
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this match H1? Or "Dynamic configuration of CSP rules"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. I missed that change.

_dashboards/csp/csp-dynamic-configuration.md Outdated Show resolved Hide resolved
_dashboards/csp/csp-dynamic-configuration.md Outdated Show resolved Hide resolved
_dashboards/csp/csp-dynamic-configuration.md Outdated Show resolved Hide resolved
_dashboards/csp/csp-dynamic-configuration.md Outdated Show resolved Hide resolved

## Precedence

Dynamic configurations override YAML configurations, except for empty CSP rules. To prevent `clickjacking`, a `frame-ancestors: self` directive is automatically added to YAML-defined rules that lack it.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is "that lack it" necessary here (it reads slightly awkwardly), or could the sentence just end at "rules"? Or maybe "when necessary" instead?

@tianleh
Copy link
Member Author

tianleh commented Mar 19, 2024

@tianleh Thank you for the links. I've reviewed and moved the PR into the editorial review queue. Once the editor (@natebower) reviews, I'll incorporate any edits. If the editor has technical-related questions, I'll reach out to you as needed. Once the PR is approved and finalized, I'll merge it. Cheers, @vagimeli

Hi, @vagimeli

I have gone through @natebower 's comments. They look like all about editorial aspects instead of technical aspects. Let me know if there is any action item at my side or you will take care of it.

vagimeli and others added 4 commits March 19, 2024 14:17
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Melissa Vagi <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Melissa Vagi <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Melissa Vagi <[email protected]>
Co-authored-by: Nathan Bower <[email protected]>
Signed-off-by: Melissa Vagi <[email protected]>
@vagimeli
Copy link
Contributor

@tianleh Thank you for the links. I've reviewed and moved the PR into the editorial review queue. Once the editor (@natebower) reviews, I'll incorporate any edits. If the editor has technical-related questions, I'll reach out to you as needed. Once the PR is approved and finalized, I'll merge it. Cheers, @vagimeli

Hi, @vagimeli

I have gone through @natebower 's comments. They look like all about editorial aspects instead of technical aspects. Let me know if there is any action item at my side or you will take care of it.

@tianleh I've handled editorial.

Signed-off-by: Melissa Vagi <[email protected]>

Signed-off-by: Melissa Vagi <[email protected]>
Copy link
Contributor

@vagimeli vagimeli left a comment

Choose a reason for hiding this comment

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

Doc and editorial reviews completed

@vagimeli vagimeli added 3 - Done Issue is done/complete and removed 5 - Editorial review PR: Editorial review in progress labels Mar 19, 2024
@vagimeli vagimeli merged commit 2593496 into opensearch-project:main Mar 19, 2024
3 of 4 checks passed
@hdhalter hdhalter changed the title configure Content Security Policy (CSP) rules in OpenSearch Dashboards dynamically Add how to configure Content Security Policy (CSP) rules in OpenSearch Dashboards dynamically Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Done Issue is done/complete release-notes PR: Include this PR in the automated release notes v2.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DOC] Add docs about configuring Content Security Policy (CSP) rules in OpenSearch Dashboards
5 participants