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

Restrict ES data proxy to msearch and search #13020

Merged
merged 1 commit into from
Jul 21, 2017
Merged

Conversation

epixa
Copy link
Contributor

@epixa epixa commented Jul 21, 2017

We are only using msearch and search on this proxy, so there's no reason
for us to expose more capabilities than this. Ultimately we want to move
all functionality like this to REST API rather than being a straight up
proxy, so this will help proxy usage not expand throughout upcoming
versions until it can be removed entirely.

@@ -26,15 +26,15 @@ describe('formatMsg', function () {
data: {
statusCode: 403,
error: 'Forbidden',
message: '[security_exception] action [indices:data/read/mget] is unauthorized for user [user]'
message: '[security_exception] action [indices:data/read/msearch] is unauthorized for user [user]'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These references didn't have to be updated, but I wanted to remove all possible references to mget from client-side code to make future grepping a little easier.

We are only using msearch and search on this proxy, so there's no reason
for us to expose more capabilities than this. Ultimately we want to move
all functionality like this to REST API rather than being a straight up
proxy, so this will help proxy usage not expand throughout upcoming
versions until it can be removed entirely.
@epixa epixa added review and removed WIP Work in progress labels Jul 21, 2017
@epixa epixa requested review from tylersmalley and spalger July 21, 2017 15:17
@tylersmalley
Copy link
Contributor

Doesn't have to be part of this PR, but does this mean we could get rid of the DocSourceProvider including the data strategy?

@epixa
Copy link
Contributor Author

epixa commented Jul 21, 2017

@tylersmalley I'm almost positive we can, but I wanted to run through it with Spencer to be certain

Copy link
Contributor

@tylersmalley tylersmalley left a comment

Choose a reason for hiding this comment

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

LGTM

@epixa epixa merged commit 1b02420 into elastic:master Jul 21, 2017
@epixa epixa deleted the restrictproxy branch July 21, 2017 15:29
@tylersmalley
Copy link
Contributor

That works for me @epixa, without mget that provider will error out so it might be worth going back and cleaning up the code.

@jimgoodwin jimgoodwin added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Jul 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Plugins release_note:breaking review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v6.0.0-beta1 v6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants