Skip to content

Fix initial security.json rbap rules #299

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.

Sign up for GitHub

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

Jump to bottom
Merged
thelabdude merged 2 commits into from
Jul 27, 2021
Merged

Fix initial security.json rbap rules #299

thelabdude merged 2 commits into from
Jul 27, 2021

Conversation

@thelabdude
Copy link
Contributor

@thelabdude thelabdude commented Jul 26, 2021 •
edited
Loading

Fixes #274 and #289

Very minor change to remove the users role from the all permission, move the all permission to the last index in the json file, and add a new rule for the /admin/zookeeper/status path needed for updates to the exporter in 8.9 (see #289 )

Manual integration-style testing required:
Create a SolrCloud running Apache Solr 8.9.0, created a collection, and then tried to index some docs as the solr user, which now fails with a 403 ~ Unauthorized. Indexing as the admin user works. The solr user can log in to the Admin UI but can only query collections.

Also verified the Prometheus exporter (also running 8.9) can now retrieve metrics when basic auth is enabled.

@HoustonPutman HoustonPutman linked an issue Jul 26, 2021 that may be closed by this pull request
Boostrapped security.json config doesn't work with the Prometheus exporter in Solr 8.9.0 due to new endpoint #289
Closed
@HoustonPutman
Copy link
Contributor

HoustonPutman commented Jul 27, 2021

+1 to the change, but we need to update the documentation to use the new rules and also explain that the default solr user will not be able to add/update/delete docs.

@HoustonPutman
Copy link
Contributor

HoustonPutman commented Jul 27, 2021

Also an addition in the changelog of the Solr Operator helm chart would be nice! You can follow the templates of the other entries. Possible change kind values are found here, fixed or security probably work best. Also if you think it's a serious security concern being addressed, you could always change artifacthub.io/containsSecurityUpdates to true.

@thelabdude
Copy link
Contributor Author

thelabdude commented Jul 27, 2021

Also an addition in the changelog of the Solr Operator helm chart would be nice! You can follow the templates of the other entries. Possible change kind values are found here, fixed or security probably work best. Also if you think it's a serious security concern being addressed, you could always change artifacthub.io/containsSecurityUpdates to true.

I don't think it's a serious security issue (willing to be convinced though) ... mainly the solr user ended up getting more access than I intended but users still have to give out the solr user's credentials, which presumes they've reviewed that account's access privileges before handing it out willy-nilly ;-)

HoustonPutman
HoustonPutman approved these changes Jul 27, 2021
View reviewed changes
Copy link
Contributor

@HoustonPutman HoustonPutman left a comment

Choose a reason for hiding this comment

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

Looks good!

@thelabdude thelabdude merged commit 43f91ea into apache:main Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@HoustonPutman HoustonPutman HoustonPutman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

2 participants

@thelabdude @HoustonPutman