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

Block non-ckan routes from inventory-proxy #4044

Closed
1 task
nickumia-reisys opened this issue Nov 3, 2022 · 2 comments
Closed
1 task

Block non-ckan routes from inventory-proxy #4044

nickumia-reisys opened this issue Nov 3, 2022 · 2 comments
Assignees

Comments

@nickumia-reisys
Copy link
Contributor

nickumia-reisys commented Nov 3, 2022

User Story

In order to reduce noisy logs, the Data.gov security team wants to prevent non-ckan traffic from hitting the ckan app and causing non-helpful errors.

Acceptance Criteria

[ACs should be clearly demoable/verifiable whenever possible. Try specifying them using BDD.]

Background

[Any helpful contextual notes or links to artifacts/evidence, if needed]

Security Considerations (required)

[Any security concerns that might be implicated in the change. "None" is OK, just be explicit here!]

Sketch

[Notes or a checklist reflecting our understanding of the selected approach]

@hkdctol hkdctol moved this to 📔 Product Backlog in data.gov team board Nov 3, 2022
@hkdctol hkdctol moved this from 📔 Product Backlog to 📟 Sprint Backlog [7] in data.gov team board Mar 16, 2023
@robert-bryson robert-bryson self-assigned this Mar 20, 2023
@robert-bryson robert-bryson moved this from 📟 Sprint Backlog [7] to 🏗 In Progress [8] in data.gov team board Mar 20, 2023
@nickumia-reisys
Copy link
Contributor Author

nickumia-reisys commented Mar 21, 2023

Since you asked yesterday @robert-bryson, it's not necessary to whitelist each full route independently. We can still gain a lot of benefit by just whitelisting the top-level routes (i.e. the "Parent" Routes minus the '/' route). We'll want / to be available, but not /*.

Edit 1: The only routes that may be problematic are the "static files" (i.e. the webassets [js/css] and any other files that have custom routes. I don't anticipate this to be a large effort, but if anyone knows differently, please advise.

Edit 2: If you are still worried about having to maintain a bunch of code in the nginx.conf, we could look into writing a python script that generates the conf file, so we don't need to edit it directly, but whether that is worth it depends on how many lines we have. If we have 20 routes to whitelist, I think it's okay. If it's 50 (for whatever reason), maybe generating it is not such a bad idea. I think we'll have less than 20 though, so 🤷

@robert-bryson
Copy link
Contributor

With this PR and a successful deploy into prod, this is done. I think this will be something of a first pass on reducing noise in New Relic. Future work to tighten the rules further may be needed.

As written this ticket targets inventory specifically. @nickumia-reisys & @FuhuXia & team as a whole, do you think this will need to be done for catalog as well?

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

No branches or pull requests

3 participants