Improve error handling during index expressions resolving #126018
Conversation
The `InvalidIndexNameException` and `IllegalArgumentException` exceptions were wrapped in a `ElasticsearchSecurityException`, which returns HTTP `403` status. These exceptions can be raised during index expression resolving, due to an invalid user input, and should result in HTTP `400` response. This PR changes exception handling to avoid wrapping them in the `ElasticsearchSecurityException`.
slobodanadamovic
added
>non-issue
:Security/Security
...ugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java
Show resolved
Hide resolved
| indicesAndAliasesResolver.resolve(action, request, projectMetadata, authorizedIndices) | ||
| ), | ||
| e -> { | ||
| if (e instanceof InvalidIndexNameException) { |
There was a problem hiding this comment.
Another exception I've considered was InvalidArgumentException but decided to skip it since it's not guaranteed that it's caused by the user's invalid input (sometimes it hides internal errors). I think we need clearer error handling overall or to be very specific which exceptions we consider client errors (e.g. analyze exception messages) .
There was a problem hiding this comment.
Yeah, InvalidArgumentException feels too broad to catch here...
WDYT about adding InvalidSelectorException (extend InvalidArgumentException) and throw that in getByKeyOrThrow and also throw either InvalidSelectorException or InvalidIndexNameException inside ensureNoSelectorsProvided? I think that would cover a decent amount of other cases.
There was a problem hiding this comment.
That's a good suggestion. Using a dedicated exception is a way to go. I'll adjust that.
There was a problem hiding this comment.
I've pushed 601d8f7, which introduces InvalidSelectorException and UnsupportedSelectorException. Let me know what you think.
…ve-error-handling # Conflicts: # x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java
|
Pinging @elastic/es-security (Team:Security) |
💔 Backport failed
You can use sqren/backport to manually backport by running |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
…6018) The `InvalidIndexNameException` exception was wrapped in a `ElasticsearchSecurityException`, which returns HTTP `403` status. This exception (along with newly introduced `InvalidSelectorException` and `UnsupportedSelectorException`) can be raised during index expression resolving due to an invalid user input and should result in HTTP `400` response instead. This PR changes exception handling to avoid wrapping them in the `ElasticsearchSecurityException`. (cherry picked from commit 0b09506) # Conflicts: # x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java # x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java
) (#126373) * Improve error handling during index expressions resolving (#126018) The `InvalidIndexNameException` exception was wrapped in a `ElasticsearchSecurityException`, which returns HTTP `403` status. This exception (along with newly introduced `InvalidSelectorException` and `UnsupportedSelectorException`) can be raised during index expression resolving due to an invalid user input and should result in HTTP `400` response instead. This PR changes exception handling to avoid wrapping them in the `ElasticsearchSecurityException`. (cherry picked from commit 0b09506) # Conflicts: # x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java # x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <[email protected]>
3 participants
The
InvalidIndexNameExceptionexception was wrapped in aElasticsearchSecurityException, which returns HTTP403status.This exception (along with newly introduced
InvalidSelectorExceptionandUnsupportedSelectorException) can be raised during index expression resolving due to an invalid user input and should result in HTTP400response instead.This PR changes exception handling to avoid wrapping them in the
ElasticsearchSecurityException.