-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Enroll Kibana API uses Service Accounts #76370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
c84c36f
39d6b92
d23a85b
29cf973
0180b16
2e9c380
7f204e7
b9280b2
52a3ac4
89d0f38
8a3e89f
cd73a19
ed7e2fe
4dce318
cca510c
3ab69c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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") | ||
|
||
| public void testNodeEnrollment() throws Exception { | ||
| RestHighLevelClient client = highLevelClient(); | ||
|
|
||
|
|
@@ -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") | ||
|
||
| public void testKibanaEnrollment() throws Exception { | ||
| RestHighLevelClient client = highLevelClient(); | ||
|
|
||
|
|
@@ -2928,10 +2928,10 @@ public void testKibanaEnrollment() throws Exception { | |
| // end::kibana-enrollment-execute | ||
|
|
||
| // tag::kibana-enrollment-response | ||
| SecureString password = response.getPassword(); // <1> | ||
| SecureString token = response.getToken(); // <1> | ||
| String httoCa = response.getHttpCa(); // <2> | ||
| // end::kibana-enrollment-response | ||
| assertThat(password.length(), equalTo(14)); | ||
| assertNotNull(token); | ||
| } | ||
|
|
||
| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:$tokenSecretand is base64 encoded.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think it'd be better to have one (to log it least).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I can gather we only need the service account token to authenticate so don't think we need the name at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, for consistency, it might make sense to mirror the create service token API anyways:
Learn once, use anywhere, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
coo, I'll make the change to return the name too, thanks for weighing in folks