Create API Key on behalf of other user #52886

tvernum · 2020-02-27T12:56:09Z

This change adds a "grant API key action"

POST /_security/api_key/grant

that creates a new API key using the privileges of one user ("the
system user") to execute the action, but creates the API key with
the roles of the second user ("the end user").

This allows a system (such as Kibana) to create API keys representing
the identity and access of an authenticated user without requiring
that user to have permission to create API keys on their own.

Relates: #48716

This change adds a "grant API key action" POST /_security/api_key/grant that creates a new API key using the privileges of one user ("the system user") to execute the action, but creates the API key with the roles of the second user ("the end user"). This allows a system (such as Kibana) to create API keys representing the identity and access of an authenticated user without requiring that user to have permission to create API keys on their own.

elasticmachine · 2020-02-27T12:56:11Z

Pinging @elastic/es-security (:Security/Authentication)

tvernum · 2020-02-27T12:57:40Z

This lacks:

docs
rest spec
rest (yaml) tests
HLRC
a new privilege (grant_api_key)

but it's big enough as it is, and I want to keep it to a reviewable size.

tvernum · 2020-02-28T05:14:42Z

Ping: @peterschretlen @mikecote

jkakavas

Did a first pass but didn't look that closely into tests. Will make another pass early next week

...ugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GrantApiKeyRequest.java

jkakavas · 2020-02-28T14:37:20Z

...ty/qa/security-basic/src/test/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java

+        invalidateApiKeysForUser(END_USER);
+    }
+
+    public void testGrantApiKeyForOtherUser() throws IOException {


should we add one with access_token grant since we're at it?

I can't in this test, because basic license doesn't suppor tokens, but I can move the test elsewhere and do that if you'd like it.

Since the transport action test already covers Username+Password and AccessToken (both valid and invalid), I didn't think it was necessary to cover it again in the IT. I just wanted something integrated to test it top-to-bottom.

Testing's a tricky balance, so if it's something you think we need, I'm happy to make it happen (but it will probably means a whole new QA project for rest tests using security on trial licenses).

SGTM to leave it as is

I can't in this test, because basic license doesn't support tokens

But you can start a trial, as it's done in SecurityWithBasicLicenseIT#testWithTrialLicense , right ? If this is possible, I would also just slightly prefer a test case using tokens.

No, because a cluster can only start a trial once, and the other test does that.
If we want to test this with a non-basic license then we'll need a new QA project.

You convinced me that I was just being lazy, so I've added a new QA project, and a new test for authenticating with a token + API key expiration.

...curity/src/main/java/org/elasticsearch/xpack/security/action/TransportGrantApiKeyAction.java

...n/security/src/main/java/org/elasticsearch/xpack/security/authc/support/ApiKeyGenerator.java

mikecote · 2020-03-02T19:16:47Z

Hi @tvernum I played with this PR locally last Friday. Thank you for getting this done 🙏 From my perspective it works great and I can see what needs to be done on Kibana side without issue. As you noted, the privileges aren't given to the kibana user so I did my testing with elastic / others with manage_api_key privilege for now and it works! 👍

This test was piggy-backing on the security-in-basic QA project we had, but the grant-api-key endpoint has the ability to use tokens, which are not a basic licensed feature. This creates a new QA project for security on trial licenses and run the API key tests there

albertzaharovits · 2020-03-05T10:33:21Z

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java

+     * {@code null} authentication object.
+     */
+    public void authenticateToken(SecureString tokenString, ActionListener<Authentication> listener) {
+        if (isEnabled()) {


I believe this new method should return the new fancy 400 exceptions when the token service is not enabled (since it's not used in the authentication chain).

albertzaharovits · 2020-03-05T10:37:43Z

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java

+                        listener::onFailure
+                    ));
+                } else {
+                    listener.onResponse(null);


Also because this new method is not used in the authn chain, when a token fails decoding it's a client error.
I believe:

listener.onFailure(new IllegalArgumentException());

would do, it would return 400 (currently this falls back to 500 in ApiKeyGenerator#generateApiKey).

Good point @albertzaharovits

albertzaharovits · 2020-03-05T11:15:20Z

...src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGrantApiKeyAction.java

+        try (XContentParser parser = request.contentParser()) {
+            final GrantApiKeyRequest grantRequest = PARSER.parse(parser, null);
+            if (refresh != null) {
+                grantRequest.setRefreshPolicy(WriteRequest.RefreshPolicy.parse(refresh));


The refresh_policy should be forwarded to the inner CreateApiKeyRequest to take effect.

albertzaharovits · 2020-03-05T11:32:48Z

...src/main/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGrantApiKeyAction.java

+        public void onFailure(Exception e) {
+            RestStatus status = ExceptionsHelper.status(e);
+            if (status == RestStatus.UNAUTHORIZED) {
+                sendFailure(new ElasticsearchSecurityException("Failed to authenticate api key grant", RestStatus.FORBIDDEN, e));


I like this wrapping to translate 401 to 403!
Assuming this is the sole reason of existence for the inner class, in order to make it easier to read, I would make it more concise:

return channel -> client.execute(GrantApiKeyAction.INSTANCE, grantRequest, ActionListener.delegateResponse(new RestToXContentListener<>(channel), (l, e) -> { RestStatus status = ExceptionsHelper.status(e); if (status == RestStatus.UNAUTHORIZED) { l.onFailure(new ElasticsearchSecurityException("Failed to authenticate api key grant", RestStatus.FORBIDDEN, e)); } else { l.onFailure(e); } }));

Thanks @albertzaharovits that was a huge improvement.

albertzaharovits

LGTM overall, I spotted only minor problems, no second take is necessary after we settle those.

jkakavas

LGTM Tim

This change adds a "grant API key action" POST /_security/api_key/grant that creates a new API key using the privileges of one user ("the system user") to execute the action, but creates the API key with the roles of the second user ("the end user"). This allows a system (such as Kibana) to create API keys representing the identity and access of an authenticated user without requiring that user to have permission to create API keys on their own. This also creates a new QA project for security on trial licenses and runs the API key tests there Backport of: elastic#52886

This change adds a "grant API key action" POST /_security/api_key/grant that creates a new API key using the privileges of one user ("the system user") to execute the action, but creates the API key with the roles of the second user ("the end user"). This allows a system (such as Kibana) to create API keys representing the identity and access of an authenticated user without requiring that user to have permission to create API keys on their own. This also creates a new QA project for security on trial licenses and runs the API key tests there Backport of: #52886

tvernum added >feature :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.0.0 v7.7.0 labels Feb 27, 2020

tvernum requested review from albertzaharovits and jkakavas February 27, 2020 12:56

tvernum added 2 commits February 28, 2020 00:03

Revert formatting change

21c16ff

Revert unnecessary changes

17d292f

jkakavas reviewed Feb 28, 2020

View reviewed changes

jkakavas mentioned this pull request Mar 3, 2020

Add REST endpoint to register a new SAML SP #52936

Merged

tvernum added 2 commits March 5, 2020 11:45

Merge branch 'master' into feature/48716-api-key-other-user

24e0fcc

tvernum requested a review from jkakavas March 5, 2020 03:48

albertzaharovits reviewed Mar 5, 2020

View reviewed changes

albertzaharovits approved these changes Mar 5, 2020

View reviewed changes

jkakavas approved these changes Mar 5, 2020

View reviewed changes

mikecote mentioned this pull request Mar 9, 2020

Use kibana_system user to create API keys on behalf of other users. elastic/kibana#49398

Closed

tvernum added 4 commits March 11, 2020 22:54

Merge branch 'master' into feature/48716-api-key-other-user

dbd7726

Address feedback

036c402

Merge branch 'master' into feature/48716-api-key-other-user

8215d25

Address more feedback

04339c6

tvernum merged commit 150735c into elastic:master Mar 12, 2020

tvernum added the backport pending label Mar 12, 2020

tvernum mentioned this pull request Mar 13, 2020

Add "grant_api_key" cluster privilege #53527

Merged

albertzaharovits mentioned this pull request Mar 17, 2020

[APM] Allow kibana to collect APM telemetry in background task #52917

Merged

kobelb mentioned this pull request Mar 18, 2020

Adding authc.grantAPIKeyAsInternalUser elastic/kibana#60423

Merged

tvernum mentioned this pull request Mar 23, 2020

[Backport] Create API Key on behalf of other user #53943

Merged

tvernum removed the backport pending label Mar 23, 2020

tvernum mentioned this pull request Mar 26, 2020

Allow creation of API keys on behalf of other users #48716

Closed

This was referenced Apr 1, 2020

7.7.0 meta ticket elastic/elasticsearch-net#4525

Closed

7.7.0 meta ticket (Part 2) elastic/elasticsearch-net#4533

Closed

albertzaharovits mentioned this pull request May 28, 2020

Docs missing for the API-Key grant on behalf of others API #57305

Closed

mikecote mentioned this pull request Jul 9, 2020

Ability to clone granted API keys #59304

Open

lcawl mentioned this pull request Oct 7, 2020

[DOCS] Add grant API key API and grant_api_key privilege #63449

Merged

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create API Key on behalf of other user #52886

Create API Key on behalf of other user #52886

tvernum commented Feb 27, 2020 •

edited

Loading

elasticmachine commented Feb 27, 2020

tvernum commented Feb 27, 2020

tvernum commented Feb 28, 2020

jkakavas left a comment

jkakavas Feb 28, 2020

tvernum Mar 2, 2020

jkakavas Mar 3, 2020

albertzaharovits Mar 4, 2020

tvernum Mar 4, 2020

tvernum Mar 5, 2020

mikecote commented Mar 2, 2020

albertzaharovits Mar 5, 2020

albertzaharovits Mar 5, 2020

jkakavas Mar 5, 2020

albertzaharovits Mar 5, 2020

albertzaharovits Mar 5, 2020

tvernum Mar 12, 2020

albertzaharovits left a comment

jkakavas left a comment

Create API Key on behalf of other user #52886

Create API Key on behalf of other user #52886

Conversation

tvernum commented Feb 27, 2020 • edited Loading

elasticmachine commented Feb 27, 2020

tvernum commented Feb 27, 2020

tvernum commented Feb 28, 2020

jkakavas left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikecote commented Mar 2, 2020

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

albertzaharovits left a comment

Choose a reason for hiding this comment

jkakavas left a comment

Choose a reason for hiding this comment

tvernum commented Feb 27, 2020 •

edited

Loading