[ML] prefer secondary authorization header for data[feed|frame] authz #54121

benwtrent · 2020-03-24T19:00:58Z

Secondary authorization headers are to be used to facilitate Kibana spaces support + ML jobs/datafeeds.

Now on PUT/Update/Preview datafeed, and PUT data frame analytics the secondary authorization is preferred over the primary (if provided).

closes #53801

benwtrent · 2020-03-24T19:01:37Z

...c/main/java/org/elasticsearch/xpack/core/security/authc/support/SecondaryAuthentication.java

@@ -24,7 +24,7 @@
 */
 public class SecondaryAuthentication {

-    private static final String THREAD_CTX_KEY = "_xpack_security_secondary_authc";
+    public static final String THREAD_CTX_KEY = "_xpack_security_secondary_authc";


@tvernum I made this public so that it is accessible to the ML plugin

benwtrent · 2020-03-24T19:05:39Z

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/AuthHeadersExtractor.java

+        if (secondaryAuth != null) {
+            headers.put(AuthenticationField.AUTHENTICATION_KEY, secondaryAuth);
+        }
+        return headers;


@tvernum We are storing _xpack_security_secondary_authc as the primary auth for some of our tasks. This way we "prefer" the secondary auth if it was provided.

Is simply replacing _xpack_security_authentication with the contents of _xpack_security_secondary_authc acceptable? Or are there technical limitations/details that I am missing?

Following the test you did yesterday this looks like it works as intended now.

This isn't how I expected you would do it...

I had presumed that you would activate the secondary authentication so that it became the only authentication and then just do what you've always done.

Something like:

Runnable body = () -> { final Map<String, String> headers = threadPool.getThreadContext().getHeaders(); datafeedConfigProvider.updateDatefeedConfig( request.getUpdate().getId(), request.getUpdate(), headers, jobConfigProvider::validateDatafeedJob, ActionListener.wrap( updatedConfig -> listener.onResponse(new PutDatafeedAction.Response(updatedConfig)), listener::onFailure)); }; SecondaryAuthentication secondaryAuth = SecondaryAuthentication.readFromContext(securityContext); if (secondaryAuth != null) { body = secondaryAuth.wrap(body); } body.run();

We could definitely, reduce some of that boilerplate and move it into SecondaryAuthentication if it would help, but the above is how I expected secondary auth to be used.

@tvernum we need the headers to be respected by _search and apply the roles that are stored in that context.

Do the other APIs respect secondary auth headers?

Another pain point is BWC. As a 'new' datafeed created with secondary auth headers could be used on an 'old' node. Then security would know nothing of the secondary auth concept and the datafeed would fail.

Ah, I think I understand. secondaryAuth.wrap changes the stored headers so that the secondary auth header becomes the Authorization header.

Seems more idiomatic than iterating a map :). Let experiment and see if I can get it to work this way.

benwtrent · 2020-03-24T19:12:08Z

@elasticmachine update branch

elasticmachine · 2020-03-24T19:12:43Z

Pinging @elastic/ml-core (:ml)

droberts195 · 2020-03-27T13:30:32Z

docs/reference/ml/anomaly-detection/apis/update-datafeed.asciidoc

@@ -35,6 +35,12 @@ IMPORTANT: When {es} {security-features} are enabled, your {dfeed} remembers
 which roles the user who updated it had at the time of update and runs the query
 using those same roles.

+
+--
+NOTE: It is possible that secondary authorization headers are supplied in the


Since "secondary authorization headers" are an ES thing, not a global standard, I think we should have a paragraph somewhere in the security docs that says the header name is es-secondary-authorization and the format is the same as for basic auth, and then "secondary authorization headers" in this sentence and the equivalent one in the other 3 files should be a link to that.

Maybe the new paragraph should go at the bottom of https://www.elastic.co/guide/en/elasticsearch/reference/current/http-clients.html?

droberts195

The Java code LGTM now

I think the docs side needs more work though - something along the lines of what I said in https://github.com/elastic/elasticsearch/pull/54121/files#r399264654

@lcawl or @szabosteve please could you advise on where it would be best to explain the secondary authentication header concept?

droberts195 · 2020-03-31T17:06:21Z

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/AuthHeadersExtractor.java

+        if (secondaryAuth != null) {
+            headers.put(AuthenticationField.AUTHENTICATION_KEY, secondaryAuth);
+        }
+        return headers;


Following the test you did yesterday this looks like it works as intended now.

droberts195 · 2020-03-31T17:07:38Z

...ulti-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsRestIT.java

+        String jobId = "secondary-privs-put-job";
+        createJob(jobId, "airline.keyword");
+        String datafeedId = "datafeed-" + jobId;
+        // Primary auth header does not have accesss, but secondary auth does


…secondary-auth-header

…elastic#54121) Secondary authorization headers are to be used to facilitate Kibana spaces support + ML jobs/datafeeds. Now on PUT/Update/Preview datafeed, and PUT data frame analytics the secondary authorization is preferred over the primary (if provided). closes elastic#53801

… authz (#54121) (#54645) * [ML] prefer secondary authorization header for data[feed|frame] authz (#54121) Secondary authorization headers are to be used to facilitate Kibana spaces support + ML jobs/datafeeds. Now on PUT/Update/Preview datafeed, and PUT data frame analytics the secondary authorization is preferred over the primary (if provided). closes #53801 * fixing for backport

[ML] prefer secondary authorization header for data[feed|frame] authz

30cfb12

benwtrent requested a review from droberts195 March 24, 2020 19:00

benwtrent commented Mar 24, 2020

View reviewed changes

Merge branch 'master' into feature/ml-allow-secondary-auth-header

740e666

benwtrent added :ml Machine learning >non-issue v8.0.0 labels Mar 24, 2020

benwtrent added the v7.8.0 label Mar 25, 2020

droberts195 reviewed Mar 27, 2020

View reviewed changes

benwtrent added 2 commits March 30, 2020 15:29

removing secondary auth header when storing and preferring it as primary

d06ae05

Merge branch 'master' into feature/ml-allow-secondary-auth-header

f821505

droberts195 approved these changes Mar 31, 2020

View reviewed changes

benwtrent and others added 5 commits March 31, 2020 14:24

Update DatafeedJobsRestIT.java

3ed4cff

Merge remote-tracking branch 'upstream/master' into feature/ml-allow-…

23578d3

…secondary-auth-header

adjusting how secondary auth headers are added

4674ffa

fixing minor things

4b93bd5

reverting unnecessary change

b7be96d

benwtrent merged commit 1d24960 into elastic:master Apr 2, 2020

benwtrent deleted the feature/ml-allow-secondary-auth-header branch April 2, 2020 14:10

benwtrent mentioned this pull request Apr 2, 2020

[7.x] [ML] prefer secondary authorization header for data[feed|frame] authz (#54121) #54645

Merged

jgowdyelastic mentioned this pull request Apr 23, 2020

[ML] Integration with Kibana Spaces elastic/kibana#64172

Closed

26 tasks

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

droberts195 mentioned this pull request May 12, 2022

[ML] Prefer secondary authentication headers when creating/updating transforms #86731

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] prefer secondary authorization header for data[feed|frame] authz #54121

[ML] prefer secondary authorization header for data[feed|frame] authz #54121

benwtrent commented Mar 24, 2020

benwtrent Mar 24, 2020 •

edited

Loading

benwtrent Mar 24, 2020

droberts195 Mar 31, 2020

tvernum Apr 2, 2020

benwtrent Apr 2, 2020

benwtrent Apr 2, 2020

benwtrent commented Mar 24, 2020

elasticmachine commented Mar 24, 2020

droberts195 Mar 27, 2020

droberts195 Mar 27, 2020

droberts195 left a comment

droberts195 Mar 31, 2020

droberts195 Mar 31, 2020

[ML] prefer secondary authorization header for data[feed|frame] authz #54121

[ML] prefer secondary authorization header for data[feed|frame] authz #54121

Conversation

benwtrent commented Mar 24, 2020

benwtrent Mar 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benwtrent commented Mar 24, 2020

elasticmachine commented Mar 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

droberts195 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benwtrent Mar 24, 2020 •

edited

Loading