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

6.4.0-SNAPSHOT fails get all aliases with security enabled and no indices exist #31516

Closed
dakrone opened this issue Jun 21, 2018 · 7 comments
Closed
Assignees
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates

Comments

@dakrone
Copy link
Member

dakrone commented Jun 21, 2018

Due to: #31308

There is an issue with security when retrieving all aliases and no indices exist.

Reproduction:

  • Use a 6.4.0-SNAPSHOT build with xpack.security.enabled: true in elasticsearch.yml
  • Configure a file realm user with superuser role
  • Start elasticsearch and start an x-pack trial
  • Then attempt to retrieve all aliases:
~/elasticsearch-6.4.0-SNAPSHOT λ curl -XGET -u"foo:bar" "localhost:9200/_aliases"
{}
~/elasticsearch-6.4.0-SNAPSHOT λ curl -XPOST "localhost:9200/_xpack/license/start_trial?acknowledge=true"
{"acknowledged":true,"trial_was_started":true,"type":"trial"}
~/elasticsearch-6.4.0-SNAPSHOT λ curl -XGET -u"foo:bar" "localhost:9200/_aliases"
{"error":{"root_cause":[{"type":"index_not_found_exception","reason":"no such index","index_uuid":"_na_","index":"_all"}],"type":"index_not_found_exception","reason":"no such index","index_uuid":"_na_","index":"_all"},"status":404}

With 38b1a5f reverted, the correct behavior can be seen:

~/elasticsearch-6.4.0-SNAPSHOT λ curl -XGET -u"foo:bar" "localhost:9200/_aliases"
{}
~/elasticsearch-6.4.0-SNAPSHOT λ curl -XPOST "localhost:9200/_xpack/license/start_trial?acknowledge=true"
{"acknowledged":true,"trial_was_started":true,"type":"trial"}
~/elasticsearch-6.4.0-SNAPSHOT λ curl -XGET -u"foo:bar" "localhost:9200/_aliases"
{}
@dakrone dakrone added >bug :Data Management/Indices APIs APIs to create and manage indices and templates v7.0.0 v6.4.0 labels Jun 21, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@javanna
Copy link
Member

javanna commented Jun 21, 2018

@dakrone are you sure that you get the same behaviour with get mappings? I wouldn't expect that.

There are limitations when security is enabled, that are documented in our docs: "While creating or retrieving aliases by providing wildcard expressions for alias names, if there are no existing authorized aliases that match the wildcard expression provided an IndexNotFoundException is returned.". The get aliases API is subject to this.

Before #31308 the get alias API didn't require indices:admin/aliases/get privileges when called without specifying indices and aliases, as it would internally be converted to an indices:admin/get (get index API) action. I find this a bug, as any alias was returned, while going through the get alias API we would also look at which aliases the user is authorized for and we replace them in the request. Whenever replacing aliases (not specifying aliases gets converted to _all which with security enabled means "all of the aliases that the user has access to"), the problem described above comes up, that when there are no aliases in the cluster the security plugin will throw a 404 instead of returning an empty response.

In hindsight, this is the behaviour that we should have always seen from the get alias API, in fact before removing RestGetAllAliasesAction there used to be a difference between doing GET /_alias/_all compared to GET /_alias which makes the behaviour inconsistent. Users would expect the same behaviour as they refer to the _alias endpoint as the get alias API.

This could be seen as a regression, but my personal opinion is that it was a bugfix and this behaviour, although not desirable, is known and documented, it was a mistake that the API didn't behave consistently depending on the provided parameters.

We can look as a follow-up at possibly improving this overall behaviour of the get alias API, similar to what we did in 5.0 for indices (we used to have the same limitation around indices but that was fixed), I think we have a way now to remove this limitation for the get alias API in 7.0.

@dakrone
Copy link
Member Author

dakrone commented Jun 21, 2018

@dakrone are you sure that you get the same behaviour with get mappings? I wouldn't expect that.

Just tested this and it does appear to only affect aliases, I'll change the title/body

This could be seen as a regression, but my personal opinion is that it was a bugfix and this behaviour, although not desirable, is known and documented, it was a mistake that the API didn't behave consistently depending on the provided parameters.

I think this needs to go into the migration documentation and be marked as a breaking change then, since it did break something (Kibana). I can see how it can be considered a bugfix, but at the same time I think we should strive to be as consistent as possible for a non-security versus security installation

We can look as a follow-up at possibly improving this overall behaviour of the get alias API, similar to what we did in 5.0 for indices, I think we have a way now to remove this limitation for the get alias API in 7.0.

👍

@dakrone dakrone changed the title 6.4.0-SNAPSHOT fails get all mappings/aliases with security enabled and no indices exist 6.4.0-SNAPSHOT fails get all aliases with security enabled and no indices exist Jun 21, 2018
@javanna
Copy link
Member

javanna commented Jun 22, 2018

@jaymode what's your take on this?

I am open to reverting the change on 6.x and working on master to possibly remove the limitation completely. Otherwise we can keep the change in 6.x and document the different behaviour starting from 6.4 as Lee suggested, it's more consistent but breaking and undesired.

@javanna
Copy link
Member

javanna commented Jun 22, 2018

I looked at removing the get alias limitations with security installed on master, and it is not as easy as it was with indices, the reason being that we don't support alias exclusions, hence there is no safe way to send a request through making sure that no aliases will be returned. In fact if there are no matching authorized aliases, we can't replace the aliases with an empty list of aliases as that would mean all aliases once the request is executed by Elasticsearch, which would lead to returning unauthorized aliases. With indices we replace them with an exclusion expression that always resolves to no indices, but given that we don't support aliases exclusions, it is not possible to do the same with aliases.

@jaymode
Copy link
Member

jaymode commented Jun 22, 2018

I'm personally in favor of reverting so we don't having a breaking API change in a minor. I don't have an idea right now on how we could fix this; I'll need to look at it some more.

@javanna
Copy link
Member

javanna commented Jun 28, 2018

#31308 was reverted on master to prevent any breakage in a minor version. We are looking at removing the get aliases limitations on master.

@javanna javanna closed this as completed Jun 28, 2018
javanna added a commit to javanna/elasticsearch that referenced this issue Jul 11, 2018
Resolving wildcards in aliases expression is challenging as we may end
up with no aliases to replace the original expression with, but if we
replace with an empty array that means _all which is quite the opposite.
Now that we support and serialize the original requested aliases,
whenever aliases are replaced we will be able to know what was
initially requested. MetaData#findAliases can then be updated to not
return anything in case it gets empty aliases, but the original aliases
were not empty. That means that empty aliases are interpreted as _all
only if they were originally requested that way.

Relates to elastic#31516
javanna added a commit that referenced this issue Jul 20, 2018
Resolving wildcards in aliases expression is challenging as we may end
up with no aliases to replace the original expression with, but if we
replace with an empty array that means _all which is quite the opposite.
Now that we support and serialize the original requested aliases,
whenever aliases are replaced we will be able to know what was
initially requested. `MetaData#findAliases` can then be updated to not
return anything in case it gets empty aliases, but the original aliases
were not empty. That means that empty aliases are interpreted as _all
only if they were originally requested that way.

Relates to #31516
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates
Projects
None yet
Development

No branches or pull requests

4 participants