Skip to content

Commit

Permalink
Ensure all API Keys have a defined name (#175721)
Browse files Browse the repository at this point in the history
## Summary

Resolves #173890.

API Keys created in the 7.x era did not require a `name`
(https://www.elastic.co/guide/en/elasticsearch/reference/7.5/security-api-create-api-key.html#security-api-create-api-key-request-body).
The `name` is no longer an optional field, but our UIs have come to
assume that a `name` will always be available. This updates our `GET
/internal/security/api_key` API to ensure that a `name` will always
exist.

(cherry picked from commit 707b423)
  • Loading branch information
legrego committed Jan 26, 2024
1 parent 5ebb15e commit 81be5fc
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 3 deletions.
40 changes: 38 additions & 2 deletions x-pack/plugins/security/server/routes/api_keys/get.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,44 @@ describe('Get API Keys route', () => {
);

expect(response.status).toBe(200);
expect(response.payload.apiKeys).toContainEqual({ id: '123', invalidated: false });
expect(response.payload.apiKeys).not.toContainEqual({ id: '456', invalidated: true });
expect(response.payload.apiKeys).toContainEqual({ id: '123', name: '', invalidated: false });
expect(response.payload.apiKeys).not.toContainEqual({ id: '456', name: '', invalidated: true });
});

it('should substitute an empty string for keys with `null` names', async () => {
esClientMock.asCurrentUser.security.getApiKey.mockRestore();
esClientMock.asCurrentUser.security.getApiKey.mockResponse({
api_keys: [
{ id: 'with_name', name: 'foo', invalidated: false },
{ id: 'undefined_name', invalidated: false },
{ id: 'null_name', name: null, invalidated: false },
],
} as any);

const response = await routeHandler(
mockContext,
httpServerMock.createKibanaRequest(),
kibanaResponseFactory
);

expect(response.status).toBe(200);
expect(response.payload.apiKeys).toEqual([
{
id: 'with_name',
name: 'foo',
invalidated: false,
},
{
id: 'undefined_name',
name: '',
invalidated: false,
},
{
id: 'null_name',
name: '',
invalidated: false,
},
]);
});

it('should return `404` if API keys are disabled', async () => {
Expand Down
9 changes: 8 additions & 1 deletion x-pack/plugins/security/server/routes/api_keys/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,14 @@ export function defineGetApiKeysRoutes({
owner: !clusterPrivileges.manage_api_key && !clusterPrivileges.read_security,
});

const validKeys = apiResponse.api_keys.filter(({ invalidated }) => !invalidated);
const validKeys = apiResponse.api_keys
.filter(({ invalidated }) => !invalidated)
.map((key) => {
if (!key.name) {
key.name = '';
}
return key;
});

return response.ok<GetAPIKeysResult>({
body: {
Expand Down

0 comments on commit 81be5fc

Please sign in to comment.