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

[BUG] There should be a seperate configuration option to enable restapi: permissions #2571

Closed
peternied opened this issue Mar 20, 2023 · 7 comments · Fixed by #2605
Closed
Assignees
Labels
bug Something isn't working CCI College Contributor Initiative good first issue These are recommended starting points for newcomers looking to make their first contributions. triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. v2.7.0

Comments

@peternied
Copy link
Member

What is the bug?
When #2411 was added to the security codebase, it expands the way that customers can manage their OpenSearch cluster - yay! However, for existing customers this expands the exposure on their security cluster beyond what it was in previous releases.

This functionality should be opt-in via a setting in the security configuration and default to 'disabled' to prevent this exposure.

Do you have any additional context?
The security configuration has to be modified on the disk on a bootstrapping node or using the 'super admin' certificate workflow making the configuration a safe place for this default value.

@peternied peternied added bug Something isn't working untriaged Require the attention of the repository maintainers and may need to be prioritized labels Mar 20, 2023
@stephen-crawford stephen-crawford added v2.7.0 triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. good first issue These are recommended starting points for newcomers looking to make their first contributions. CCI College Contributor Initiative and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels Mar 20, 2023
@stephen-crawford
Copy link
Contributor

[Triage] This seems like a good call. Tagging as good first issue. @cwperks could you add a little bit of a getting started for a first time contributor.

@peternied
Copy link
Member Author

FYI @willyborankin @reta

@cwperks
Copy link
Member

cwperks commented Mar 20, 2023

New Settings are generally set in ConfigConstants (See example: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/support/ConfigConstants.java#L250).

From the plugin architecture of OpenSearch, plugins can extend OpenSearch to add additional settings to be placed in opensearch.yml and those settings are defined in getSettings() of the plugin. For the security plugin, that is here: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java#L886-L1069

Settings defined in getSettings() typically provide a default and for this issue that would be false (meaning disabled).

The last part is to use that settings around here: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java#L120-L124 (and maybe other places to) to evaluate the permission if the setting is enabled. Note that Settings are not presently available in that class, but you should be able to add them in the class' constructor.

@aruzhannurman
Copy link

I would like to work on this bug

@stephen-crawford
Copy link
Contributor

Hi @aruzhannurman, thank you for volunteering. I went ahead and assigned this to you. Let me know if you have any questions or need any help.

@willyborankin
Copy link
Collaborator

willyborankin commented Apr 4, 2023

@scrawfor99 and @aruzhannurman oops I did it already, sorry didn't add a comment my bad here is PR: #2605

@stephen-crawford
Copy link
Contributor

Hi @willyborankin, no worries. Thank you for following up. I will reassign this to you. I appreciate you following up so quickly so that @aruzhannurman does not end up duplicating work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CCI College Contributor Initiative good first issue These are recommended starting points for newcomers looking to make their first contributions. triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. v2.7.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants