Don't require separate privilege for internal detail of put pipeline#60106
Conversation
Putting an ingest pipeline used to require that the user calling it had permission to get nodes info as well as permission to manage ingest. This was due to an internal implementaton detail that was not visible to the end user. This change alters the behaviour so that a user with the manage_pipeline cluster privilege can put an ingest pipeline regardless of whether they have the separate privilege to get nodes info. The internal implementation detail now runs as the internal _xpack user when security is enabled. Fixes elastic#60041
There is no test suite that specifically tests ingest with different levels of security privileges, so I used the ML with security suite. There were 3 tests of the inference ingest processor that were excluded from this suite because none of the users we were testing with had permission to create an ingest pipeline. Previously they would have needed both the ingest_admin and monitor roles to do this. After the core change in this PR they just require ingest_admin. It could be argued that adding ingest_admin to the ml_admin user used by the tests is dodgy as we could then miss the situation where ingest_admin was required to use all ML functionality. However, this is extremely unlikely and the benefit of adding this role to the ml_admin user is that then we can test that using ML inference in ingest works with a sensible set of user roles - previously we were not testing ML inference in ingest with a privilege level below superuser.
|
Pinging @elastic/es-security (:Security/Authorization) |
|
Pinging @elastic/es-core-features (:Core/Features/Ingest) |
|
Pinging @elastic/ml-core (:ml) |
|
Note regarding the testing: As far as I can see there is no test suite that specifically tests ingest with different levels of security privileges, so I used the ML with security suite. There were 3 tests of the inference ingest processor that were excluded from this suite because none of the users we were testing with had permission to create an ingest pipeline. Previously they would have needed both the ingest_admin and monitor roles to do this. After the core change in this PR they just require ingest_admin. It could be argued that adding ingest_admin to the ml_admin user used by the tests is dodgy as we could then miss the situation where ingest_admin was required to use all ML functionality. However, this is extremely unlikely and the benefit of adding this role to the ml_admin user is that then we can test that using ML inference in ingest works with a sensible set of user roles - previously we were not testing ML inference in ingest with a privilege level below superuser. |
danhermann
left a comment
There was a problem hiding this comment.
This LGTM but I would like someone from @elastic/es-security to look it over to ensure there are no subtleties involved in changing the user for the nodes info call.
ywangd
left a comment
There was a problem hiding this comment.
LGTM. I think it makes sense to add a new ingest origin given the current and potential future need.
A minor comment (feel free to ignore) is that we have some inconsistency in handlilng origins from the core: The tasks origin and the new one are differently referenced in AuthorizationUtils, while persistent_tasks is duplicated in ClientHelper. Either way is fine with me, but consistency would be great.
@ywangd I did actually tidy that up in my first local version of this change, but then thought better of it because the transport client still exists in 7.x and removing public constants from classes that are available in the transport client could break people's code. How about I merge this PR as-is, then do another small PR just for master that removes |
This is a great point and thanks for explaining the history side of things. Yes I agree to merge it as is and fix it later for |
Putting an ingest pipeline used to require that the user calling it had permission to get nodes info as well as permission to manage ingest. This was due to an internal implementaton detail that was not visible to the end user. This change alters the behaviour so that a user with the manage_pipeline cluster privilege can put an ingest pipeline regardless of whether they have the separate privilege to get nodes info. The internal implementation detail now runs as the internal _xpack user when security is enabled. Backport of elastic#60106
Putting an ingest pipeline used to require that the user calling it had permission to get nodes info as well as permission to manage ingest. This was due to an internal implementaton detail that was not visible to the end user. This change alters the behaviour so that a user with the manage_pipeline cluster privilege can put an ingest pipeline regardless of whether they have the separate privilege to get nodes info. The internal implementation detail now runs as the internal _xpack user when security is enabled. Backport of elastic#60106
This is done in #60192 |
…60190) Putting an ingest pipeline used to require that the user calling it had permission to get nodes info as well as permission to manage ingest. This was due to an internal implementaton detail that was not visible to the end user. This change alters the behaviour so that a user with the manage_pipeline cluster privilege can put an ingest pipeline regardless of whether they have the separate privilege to get nodes info. The internal implementation detail now runs as the internal _xpack user when security is enabled. Backport of #60106
…60191) Putting an ingest pipeline used to require that the user calling it had permission to get nodes info as well as permission to manage ingest. This was due to an internal implementaton detail that was not visible to the end user. This change alters the behaviour so that a user with the manage_pipeline cluster privilege can put an ingest pipeline regardless of whether they have the separate privilege to get nodes info. The internal implementation detail now runs as the internal _xpack user when security is enabled. Backport of #60106
Putting an ingest pipeline used to require that the user calling
it had permission to get nodes info as well as permission to
manage ingest. This was due to an internal implementaton detail
that was not visible to the end user.
This change alters the behaviour so that a user with the
manage_pipeline cluster privilege can put an ingest pipeline
regardless of whether they have the separate privilege to get
nodes info. The internal implementation detail now runs as
the internal _xpack user when security is enabled.
Fixes #60041