-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[stats] fix error when requesting extended stats by unauth users #160520
[stats] fix error when requesting extended stats by unauth users #160520
Conversation
if (isExtended) { | ||
const core = await context.core; | ||
const { asCurrentUser } = core.elasticsearch.client; | ||
const { asInternalUser } = core.elasticsearch.client; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual change: using the internal client and not the scoped one, as using the scoped client when no user is authenticated returns 401/403 error from ES.
Pinging @elastic/kibana-core (Team:Core) |
it('should return 200 for extended', async () => { | ||
const { body } = await supertestNoAuth.get('/api/stats').expect(200); | ||
expect(isUUID(body.kibana.uuid)).to.be.ok(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote a new FTR suite before discovering that there was one already (fun fact, the 401 caused by the ES client for extended stats was a "feature" not a bug). Given they don't test exactly the same things, I kept mine after discovering this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behavior is different and easy to miss. ++ to adding the new API integration test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
it('should return 200 for extended', async () => { | ||
const { body } = await supertestNoAuth.get('/api/stats').expect(200); | ||
expect(isUUID(body.kibana.uuid)).to.be.ok(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behavior is different and easy to miss. ++ to adding the new API integration test
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Summary
Fix #160385
Use the internal client instead of the scoped one for the extended stats ES requests to avoid an error with unauthenticated users (when anonymous access is allowed)