[ML] prefer secondary auth headers on data frame analytics _explain #63281
Conversation
|
Pinging @elastic/ml-core (:ml) |
benwtrent
force-pushed
the
feature/ml-use-secondary-headers-on-explain
branch
from
05da590 to
f8d27a8
Compare
| ); | ||
| if (licenseState.isSecurityEnabled()) { | ||
| useSecondaryAuthIfAvailable(this.securityContext, () -> { | ||
| // Use secondary auth headers for the request always |
There was a problem hiding this comment.
What does "always" mean in this context? We're in the conditional branch, that's why I'm asking.
There was a problem hiding this comment.
I think it means in preference to headers that might already be stored on the data frame analytics config, if it came from the config index rather than being supplied in the explain request. The comment should say something like this.
So this means that if Alice tries to explain a pre-existing data frame analytics job created by Bob then that explanation will use Alice's credentials to access the source indices instead of Bob's. Before this change it would have used Bob's, which is superficially analogous to what what would have happened if Alice had started the job that Bob created. However, in the case of Alice starting Bob's job the outputs would go to the destination indices, which presumably Bob owns. If Bob has chosen not to give access to Alice then Alice cannot see the results. In the case of explain, Alice gets to see the results directly in the JSON response, so it makes more sense to use her credentials to access the source indices.
...rc/main/java/org/elasticsearch/xpack/ml/action/TransportExplainDataFrameAnalyticsAction.java
Show resolved
Hide resolved
...avaRestTest/java/org/elasticsearch/xpack/ml/integration/ExplainDataFrameAnalyticsRestIT.java
Outdated
Show resolved
Hide resolved
|
|
||
| private static String basicAuth(String user) { | ||
| return UsernamePasswordToken.basicAuthHeaderValue(user, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING); | ||
| } |
There was a problem hiding this comment.
nit: new line after end of method
...avaRestTest/java/org/elasticsearch/xpack/ml/integration/ExplainDataFrameAnalyticsRestIT.java
Show resolved
Hide resolved
...rc/main/java/org/elasticsearch/xpack/ml/action/TransportExplainDataFrameAnalyticsAction.java
Show resolved
Hide resolved
...rc/main/java/org/elasticsearch/xpack/ml/action/TransportExplainDataFrameAnalyticsAction.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Przemysław Witek <przemyslaw.witek@elastic.co>
…lastic#63281) We should prefer secondary auth headers when calling _explain
|
I believe this PR has caused an incompatibility with Kibana which is being tracked here. I am not familiar enough with the code to understand if this issue is something that the ML team needs to address in the Kibana repository, or it's something that needs to be addressed here. Either way, I have also raised in the machine-learning channel. It's pretty important that we resolve this quickly, as this incompatibility will prevent us from branching for 7.10. |
|
This is being resolved on the Kibana side here. |
We should prefer secondary auth headers when calling
_explain