fix(config)!: remove own_index default roles mapping#6147
fix(config)!: remove own_index default roles mapping#6147robertpaschedag wants to merge 2 commits into
Conversation
The own_index role mapping granted all users full access to an index matching their username. Remove this permissive default to improve security posture. BREAKING CHANGE: users relying on the default own_index roles mapping will no longer have automatic access to their eponymous index. Administrators must explicitly configure this mapping if needed. Signed-off-by: Robert Paschedag <robert.paschedag@sap.com>
PR Reviewer Guide 🔍(Review updated until commit f823fff)Here are some key observations to aid the review process:
|
|
Thank you for this PR @robertpaschedag . This is also something I'd like to do, but I think it should be done on a major release given that it could be considered a Breaking Change. Is there a workaround that would be suitable for your setup before a major release? Edit: On second thought, I would be ok with removing this in a minor version. @nibix thoughts? |
@cwperks I think, the only true workaround would be to pass a copy of the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6147 +/- ##
==========================================
+ Coverage 74.92% 75.00% +0.08%
==========================================
Files 453 453
Lines 29111 29111
Branches 4386 4386
==========================================
+ Hits 21810 21835 +25
+ Misses 5275 5253 -22
+ Partials 2026 2023 -3 🚀 New features to boost your workflow:
|
|
Persistent review updated to latest commit f823fff |
|
I think it is totally fine to remove this role mapping in a minor version, for the following reasons:
We should however document this in the CHANGELOG.md file to provide minimum awareness just in case people observe unexpected changes. |
@nibix the CHANGELOG process has been supplanted by a groomed git log + LLMs to help craft the release notes text. For 3.7.0 release there will be a release notes PR which we can edit from there. |
|
should I update the branch again (merge latest code) or is this something you will take care of? Thanks. |
no need, that failing check is flaky |
Description
Remove the
own_indexdefault roles mapping fromconfig/roles_mapping.yml.own_indexroles mapping maps all users (*) to a role that grants full access to an index named like the username. This is an overly permissive default that weakens the security posture out of the box.own_indexexists; administrators must explicitly configure this mapping if needed.We want to use the opensearch-k8s-operator to setup our clusters. The overall defaults (
rolesandroles_mapping) are very good (and we want to keep them). But with additional authentication backends (like OIDC / SAML / LDAP) this would allow any authenticated user to potentially create an index with their name (which is not what we want in a production cluster).Still...the old behaviour can be enabled again by providing this roles mapping manually again (if this is needed). As I do not know, if truly users exist, that rely on this roles mapping....this could be a BREAKING CHANGE.
This
own_indexroles mapping makes perfectly sense for some "demo / gym / playground" cluster to "play around".Issues Resolved
N/A
Testing
No new testing required — this is a configuration-only change removing a default mapping. Existing tests pass.
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.