Enroll Kibana API uses Service Accounts #76370

jkakavas · 2021-08-11T16:15:14Z

This commit changes the Enroll Kibana API to create and return
a token for this service account, instead of setting and returning the
password of the kibana_system built-in user.

This change introduces a Service Account for Kibana to use when authenticating to Elasticsearch. The Service Account with `kibana-system` service name under the `elastic` namespace, uses the same RoleDescriptor as the existing kibana_system built-in user and is its functional equivalent when it comes to AuthZ. It also changes the Enroll Kibana API to create and return a token for this service account, instead of setting and returning the password of the kibana_system built-in user.

elasticmachine · 2021-08-11T16:15:17Z

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

jkakavas · 2021-08-11T16:16:54Z

...est-high-level/src/main/java/org/elasticsearch/client/security/KibanaEnrollmentResponse.java

@@ -18,21 +18,21 @@

 public final class KibanaEnrollmentResponse {

-    private SecureString password;
+    private SecureString token;


@azasypkin would Kibana want to know the token name in addition to the value ? ( https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-create-service-token.html )

A explicit token name might be convenient for kibana, but the token value itself also has the token name embedded. The token value takes the format of \0\1\0\1elastic/kibana-system/$tokenName:$tokenSecret and is base64 encoded.

@azasypkin would Kibana want to know the token name in addition to the value ? ( https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-create-service-token.html )

Yeah, I think it'd be better to have one (to log it least).

From what I can gather we only need the service account token to authenticate so don't think we need the name at all.

Right, we don't need it in the code, but I thought it'd be beneficial to log it anyway, to know which Kibana triggered generation of which token, for troubleshooting purposes.

To be honest, for consistency, it might make sense to mirror the create service token API anyways:

{ "token": { "name": "token1", "value": "AAEAAWVsYXN0aWM...vZmxlZXQtc2VydmVyL3Rva2VuMTo3TFdaSDZ" } }

Learn once, use anywhere, etc.

coo, I'll make the change to return the name too, thanks for weighing in folks

...rc/test/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccountsTests.java

lockewritesdocs · 2021-08-11T19:17:16Z

@jkakavas, I updated the service accounts usage section in 0180b16 to include the elastic/kibana-system service account.

…unt-enroll-api

jkakavas · 2021-08-12T18:11:59Z

I split the introduction of the kibana-system service account to #76449 so that we can backport that to 7.15 and I will adjust this PR to simply add the relevant changes to the enrollment API. Sorry for the confusion, I think it will be clearer this way.

…unt-enroll-api

BigPandaToo

I added a couple of comments regarding tests, otherwise looks good.

BigPandaToo · 2021-08-13T14:59:12Z

...high-level/src/test/java/org/elasticsearch/client/documentation/SecurityDocumentationIT.java

@@ -2875,7 +2875,7 @@ public void onFailure(Exception e) {
        }
    }

-    @AwaitsFix(bugUrl = "Determine behavior for keystores with multiple keys")
+    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/75097")


Is it really blocked by this issue? I think it shouldn't be unless we use truststore containing multiple CA certs

True, we should have re-enabled this test as part of #73807. Will take care of it on Monday

Actually, testEnrollNode and testEnrollKibana here are already doing what we need, I believe

No, not really. EnrollIT means to test the HLRC behavior where the *DocumentationIT means to test that our documentation snippets are accurate and work

BigPandaToo · 2021-08-13T15:01:28Z

...high-level/src/test/java/org/elasticsearch/client/documentation/SecurityDocumentationIT.java

@@ -2918,7 +2918,7 @@ public void onFailure(Exception e) {
        }
    }

-    @AwaitsFix(bugUrl = "Determine behavior for keystores with multiple keys")
+    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/75097")


Same as above ^^

ywangd

LGTM

...est-high-level/src/main/java/org/elasticsearch/client/security/KibanaEnrollmentResponse.java

...igh-level/src/test/java/org/elasticsearch/client/security/KibanaErnollmentResponseTests.java

docs/java-rest/high-level/security/enroll_kibana.asciidoc

x-pack/docs/en/rest-api/security/enroll-kibana.asciidoc

...java/org/elasticsearch/xpack/security/action/enrollment/TransportKibanaEnrollmentAction.java

azasypkin · 2021-08-16T07:28:12Z

Looks good to me, @thomheymann since you have a WIP PR where we interact with the Enrollment API already can you please check if this change works well for you as well?

thomheymann · 2021-08-16T08:38:35Z

Looks good to me, @thomheymann since you have a WIP PR where we interact with the Enrollment API already can you please check if this change works well for you as well?

Thanks for ping. Yeh, happy to get this breaking change in before release. Shouldn't be too much work to adapt the existing PR. Very exciting!

- nits - removed unecessary parsing from KibanaEnrollmentResponse - added token name in the response - update docs

jkakavas · 2021-08-16T14:43:32Z

@ywangd thanks for the comments 🙏 , I think I addressed all the feedback 👍
@thomheymann I added the token name and updated the response as suggested, thanks, please take another look !
@lockewritesdocs can you please do a quick round of the doc changes ?
@BigPandaToo do you want to have another round too ?

jkakavas · 2021-08-16T14:47:19Z

...igh-level/src/test/java/org/elasticsearch/client/security/KibanaEnrollmentResponseTests.java

+
+import static org.hamcrest.Matchers.equalTo;
+
+public class KibanaEnrollmentResponseTests extends ESTestCase {


github fails to show this clearly but this was renamed from KibanaErnollmentResponseTests to fix the typo, hence the new file

…unt-enroll-api

BigPandaToo

LGTM

lockewritesdocs · 2021-08-16T20:44:43Z

@lockewritesdocs can you please do a quick round of the doc changes ?

@jkakavas -- done in 4dce318 👍

lockewritesdocs

👍 with the changes in 4dce318

jkakavas · 2021-08-16T21:26:59Z

Adam, it’s actually the value of the token (so <2> ) that is used to Authenticate as the service account. <1> is just the name of the token (returned for informational purposes). Can you please add a suggestion based on that ?

…

On Monday, August 16, 2021, Adam Locke ***@***.***> wrote: ***@***.**** approved this pull request. 👍 with the changes in 4dce318 <4dce318> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#76370 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACOOCKF2ZW53IQACFXWD7UDT5F2GRANCNFSM5B642DIQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email> .

jkakavas · 2021-08-17T07:14:42Z

@elasticmachine update branch

jkakavas · 2021-08-17T08:36:45Z

@elasticmachine test this please

thomheymann

Looks great, thanks!

jkakavas added 2 commits August 11, 2021 18:54

minor updates

39d6b92

jkakavas added >enhancement :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v7.15.0 labels Aug 11, 2021

jkakavas requested review from azasypkin, albertzaharovits and BigPandaToo August 11, 2021 16:15

elasticmachine added the Team:Security Meta label for security team label Aug 11, 2021

elasticsearchmachine added the v8.0.0 label Aug 11, 2021

jkakavas commented Aug 11, 2021

View reviewed changes

...rc/test/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccountsTests.java Outdated Show resolved Hide resolved

jkakavas mentioned this pull request Aug 11, 2021

Adds a kibana-system service account #73738

Closed

jkakavas and others added 3 commits August 11, 2021 22:04

fix tests

d23a85b

unused import

29cf973

Update service accounts overview to include kibana-system

0180b16

jkakavas added 2 commits August 11, 2021 22:54

fix more tests

2e9c380

Merge remote-tracking branch 'origin/master' into kibana-service-acco…

7f204e7

…unt-enroll-api

jkakavas requested review from ywangd and removed request for albertzaharovits and BigPandaToo August 12, 2021 18:12

jkakavas mentioned this pull request Aug 13, 2021

Add kibana-system service account #76449

Merged

jkakavas added 2 commits August 13, 2021 17:03

Merge remote-tracking branch 'origin/master' into kibana-service-acco…

b9280b2

…unt-enroll-api

update service name

52a3ac4

jkakavas removed the v7.15.0 label Aug 13, 2021

jkakavas added this to Implementation Milestone 4 (August 13th) in Security On by Default Implementation Tracking Aug 13, 2021

BigPandaToo reviewed Aug 13, 2021

View reviewed changes

re-enable enrollment documentation tests

89d0f38

ywangd approved these changes Aug 16, 2021

View reviewed changes

azasypkin requested a review from thomheymann August 16, 2021 07:28

Address feedback

8a3e89f

- nits - removed unecessary parsing from KibanaEnrollmentResponse - added token name in the response - update docs

jkakavas requested a review from BigPandaToo August 16, 2021 14:43

update var name

cd73a19

jkakavas commented Aug 16, 2021

View reviewed changes

jkakavas requested a review from lockewritesdocs August 16, 2021 14:48

Merge remote-tracking branch 'origin/master' into kibana-service-acco…

ed7e2fe

…unt-enroll-api

BigPandaToo approved these changes Aug 16, 2021

View reviewed changes

Fix links, streamline grammar, and other editorial changes

4dce318

lockewritesdocs approved these changes Aug 16, 2021

View reviewed changes

Clarify the value of the token used to authenticate

cca510c

Merge branch 'master' into kibana-service-account-enroll-api

3ab69c9

thomheymann approved these changes Aug 17, 2021

View reviewed changes

jkakavas merged commit a596848 into elastic:master Aug 17, 2021

Security On by Default Implementation Tracking automation moved this from Implementation Milestone 4 (August 13th) to Completed Aug 17, 2021

jakelandis added v8.0.0-alpha2 and removed v8.0.0 labels Sep 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enroll Kibana API uses Service Accounts #76370

Enroll Kibana API uses Service Accounts #76370

jkakavas commented Aug 11, 2021 •

edited

elasticmachine commented Aug 11, 2021

jkakavas Aug 11, 2021

ywangd Aug 12, 2021

azasypkin Aug 16, 2021

thomheymann Aug 16, 2021

azasypkin Aug 16, 2021

thomheymann Aug 16, 2021

jkakavas Aug 16, 2021

lockewritesdocs commented Aug 11, 2021

jkakavas commented Aug 12, 2021

BigPandaToo left a comment

BigPandaToo Aug 13, 2021

jkakavas Aug 13, 2021

BigPandaToo Aug 16, 2021

jkakavas Aug 16, 2021

BigPandaToo Aug 13, 2021

ywangd left a comment

azasypkin commented Aug 16, 2021

thomheymann commented Aug 16, 2021

jkakavas commented Aug 16, 2021 •

edited

jkakavas Aug 16, 2021

BigPandaToo left a comment

lockewritesdocs commented Aug 16, 2021

lockewritesdocs left a comment

jkakavas commented Aug 16, 2021 via email

jkakavas commented Aug 17, 2021

jkakavas commented Aug 17, 2021

thomheymann left a comment


		import static org.hamcrest.Matchers.equalTo;

		public class KibanaEnrollmentResponseTests extends ESTestCase {

Enroll Kibana API uses Service Accounts #76370

Enroll Kibana API uses Service Accounts #76370

Conversation

jkakavas commented Aug 11, 2021 • edited

elasticmachine commented Aug 11, 2021

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

lockewritesdocs commented Aug 11, 2021

jkakavas commented Aug 12, 2021

BigPandaToo 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

ywangd left a comment

Choose a reason for hiding this comment

azasypkin commented Aug 16, 2021

thomheymann commented Aug 16, 2021

jkakavas commented Aug 16, 2021 • edited

Choose a reason for hiding this comment

BigPandaToo left a comment

Choose a reason for hiding this comment

lockewritesdocs commented Aug 16, 2021

lockewritesdocs left a comment

Choose a reason for hiding this comment

jkakavas commented Aug 16, 2021 via email

jkakavas commented Aug 17, 2021

jkakavas commented Aug 17, 2021

thomheymann left a comment

Choose a reason for hiding this comment

jkakavas commented Aug 11, 2021 •

edited

jkakavas commented Aug 16, 2021 •

edited