Add admin authentication to dashboards api/status #2117
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2117 +/- ##
=========================================
Coverage 94.21% 94.21%
Complexity 25 25
=========================================
Files 204 204
Lines 3943 3943
Branches 29 29
=========================================
Hits 3715 3715
Misses 222 222
Partials 6 6
Continue to review full report at Codecov.
|
Signed-off-by: Zelin Hao <[email protected]>
| @@ -68,7 +68,7 @@ def url(self, path=""): | |||
| def get_service_response(self): | |||
| url = self.url("/api/status") | |||
| logging.info(f"Pinging {url}") | |||
| return requests.get(url, verify=False, auth=("kibanaserver", "kibanaserver") if self.security_enabled else None) | |||
| return requests.get(url, verify=False, auth=("admin", "admin")) | |||
There was a problem hiding this comment.
This is not correct you should not change the behavior of how to handle enabled security plugin, rather than only focus on the api/status I think.
There was a problem hiding this comment.
Also need @cliu123 to chime in on whether api/status requires admin:admin, or kibanaserver:kibanaserver is enough.
There was a problem hiding this comment.
Yes, kibanaserver:kibanaserver is good enough. Basically, any valid credential will go through authentication here. There is no authorization enforced here, so it doesn't matter what roles a user has. As long as the credential provided is valid, request goes through.
There was a problem hiding this comment.
Hey @cliu123 the kibanaserver user is commented out on default setup
https://github.com/opensearch-project/security/blob/main/securityconfig/config.yml#L71
So correct me if i'm wrong kibanaserver:kibanaserver wont work I guess, unless we enable the right role for the index?
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks all for the inputs here. Reverted the changes here on integ test.
@peterzhuamazon Please help review again.
| mock_requests_get.assert_called_once_with(mock_url_result, verify=False, auth=("kibanaserver", "kibanaserver")) | ||
| mock_requests_get.assert_called_once_with(mock_url_result, verify=False, auth=("admin", "admin")) |
Signed-off-by: Zelin Hao <[email protected]>
Signed-off-by: Zelin Hao <[email protected]>
Signed-off-by: Zelin Hao [email protected]
Description
Fixed the authentication for api/status
Issues Resolved
Part of #2112
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.