Skip to content

Commit

Permalink
[SECURITY] Flaky api keys grid (#116991)
Browse files Browse the repository at this point in the history 
* fix flaky test + adding functional test

* review + remove more jest flaky test

* fix type

* fix review/confusion

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
@XavierM @kibanamachine
XavierM and kibanamachine authored Nov 4, 2021
1 parent f7761d9 commit b0110d8
Show file tree
Hide file tree
Showing 5 changed files with 243 additions and 208 deletions.
292 changes: 84 additions & 208 deletions x-pack/plugins/security/public/management/api_keys/api_keys_grid/api_keys_grid_page.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,7 @@
* 2.0.
*/

import {
fireEvent,
render,
waitFor,
waitForElementToBeRemoved,
within,
} from '@testing-library/react';
import { render } from '@testing-library/react';
import { createMemoryHistory } from 'history';
import React from 'react';

Expand All @@ -22,60 +16,75 @@ import { Providers } from '../api_keys_management_app';
import { apiKeysAPIClientMock } from '../index.mock';
import { APIKeysGridPage } from './api_keys_grid_page';

jest.mock('@elastic/eui/lib/services/accessibility/html_id_generator', () => ({
htmlIdGenerator: () => () => `id-${Math.random()}`,
}));
/*
* Note to engineers
* we moved these 4 tests below to "x-pack/test/functional/apps/api_keys/home_page.ts":
* 1-"creates API key when submitting form, redirects back and displays base64"
* 2-"creates API key with optional expiration, redirects back and displays base64"
* 3-"deletes multiple api keys using bulk select"
* 4-"deletes api key using cta button"
* to functional tests to avoid flakyness
*/

jest.setTimeout(15000);
describe('APIKeysGridPage', () => {
// We are spying on the console.error to avoid react to throw error
// in our test "displays error when fetching API keys fails"
// since we are using EuiErrorBoundary and react will console.error any errors
const consoleWarnMock = jest.spyOn(console, 'error').mockImplementation();

const coreStart = coreMock.createStart();
const coreStart = coreMock.createStart();
const apiClientMock = apiKeysAPIClientMock.create();
const { authc } = securityMock.createSetup();

const apiClientMock = apiKeysAPIClientMock.create();
apiClientMock.checkPrivileges.mockResolvedValue({
areApiKeysEnabled: true,
canManage: true,
isAdmin: true,
});
apiClientMock.getApiKeys.mockResolvedValue({
apiKeys: [
{
creation: 1571322182082,
expiration: 1571408582082,
id: '0QQZ2m0BO2XZwgJFuWTT',
invalidated: false,
name: 'first-api-key',
realm: 'reserved',
username: 'elastic',
},
{
creation: 1571322182082,
expiration: 1571408582082,
id: 'BO2XZwgJFuWTT0QQZ2m0',
invalidated: false,
name: 'second-api-key',
realm: 'reserved',
username: 'elastic',
},
],
});
beforeEach(() => {
apiClientMock.checkPrivileges.mockClear();
apiClientMock.getApiKeys.mockClear();
coreStart.http.get.mockClear();
coreStart.http.post.mockClear();
authc.getCurrentUser.mockClear();

const authc = securityMock.createSetup().authc;
authc.getCurrentUser.mockResolvedValue(
mockAuthenticatedUser({
username: 'jdoe',
full_name: '',
email: '',
enabled: true,
roles: ['superuser'],
})
);
apiClientMock.checkPrivileges.mockResolvedValue({
areApiKeysEnabled: true,
canManage: true,
isAdmin: true,
});
apiClientMock.getApiKeys.mockResolvedValue({
apiKeys: [
{
creation: 1571322182082,
expiration: 1571408582082,
id: '0QQZ2m0BO2XZwgJFuWTT',
invalidated: false,
name: 'first-api-key',
realm: 'reserved',
username: 'elastic',
},
{
creation: 1571322182082,
expiration: 1571408582082,
id: 'BO2XZwgJFuWTT0QQZ2m0',
invalidated: false,
name: 'second-api-key',
realm: 'reserved',
username: 'elastic',
},
],
});

// FLAKY: https://github.com/elastic/kibana/issues/97085
describe.skip('APIKeysGridPage', () => {
authc.getCurrentUser.mockResolvedValue(
mockAuthenticatedUser({
username: 'jdoe',
full_name: '',
email: '',
enabled: true,
roles: ['superuser'],
})
);
});
it('loads and displays API keys', async () => {
const history = createMemoryHistory({ initialEntries: ['/'] });

const { getByText } = render(
const { findByText } = render(
<Providers services={coreStart} authc={authc} history={history}>
<APIKeysGridPage
apiKeysAPIClient={apiClientMock}
Expand All @@ -85,9 +94,14 @@ describe.skip('APIKeysGridPage', () => {
</Providers>
);

await waitForElementToBeRemoved(() => getByText(/Loading API keys/));
getByText(/first-api-key/);
getByText(/second-api-key/);
expect(await findByText(/Loading API keys/)).not.toBeInTheDocument();
await findByText(/first-api-key/);
await findByText(/second-api-key/);
});

afterAll(() => {
// Let's make sure we restore everything just in case
consoleWarnMock.mockRestore();
});

it('displays callout when API keys are disabled', async () => {
Expand All @@ -98,7 +112,7 @@ describe.skip('APIKeysGridPage', () => {
isAdmin: true,
});

const { getByText } = render(
const { findByText } = render(
<Providers services={coreStart} authc={authc} history={history}>
<APIKeysGridPage
apiKeysAPIClient={apiClientMock}
Expand All @@ -108,8 +122,8 @@ describe.skip('APIKeysGridPage', () => {
</Providers>
);

await waitForElementToBeRemoved(() => getByText(/Loading API keys/));
getByText(/API keys not enabled/);
expect(await findByText(/Loading API keys/)).not.toBeInTheDocument();
await findByText(/API keys not enabled/);
});

it('displays error when user does not have required permissions', async () => {
Expand All @@ -120,7 +134,7 @@ describe.skip('APIKeysGridPage', () => {
isAdmin: false,
});

const { getByText } = render(
const { findByText } = render(
<Providers services={coreStart} authc={authc} history={history}>
<APIKeysGridPage
apiKeysAPIClient={apiClientMock}
Expand All @@ -130,145 +144,21 @@ describe.skip('APIKeysGridPage', () => {
</Providers>
);

await waitForElementToBeRemoved(() => getByText(/Loading API keys/));
getByText(/You need permission to manage API keys/);
expect(await findByText(/Loading API keys/)).not.toBeInTheDocument();
await findByText(/You need permission to manage API keys/);
});

it('displays error when fetching API keys fails', async () => {
apiClientMock.getApiKeys.mockRejectedValueOnce({
body: { error: 'Internal Server Error', message: '', statusCode: 500 },
});
const history = createMemoryHistory({ initialEntries: ['/'] });

const { getByText } = render(
<Providers services={coreStart} authc={authc} history={history}>
<APIKeysGridPage
apiKeysAPIClient={apiClientMock}
notifications={coreStart.notifications}
history={history}
/>
</Providers>
);

await waitForElementToBeRemoved(() => getByText(/Loading API keys/));
getByText(/Could not load API keys/);
});

it('creates API key when submitting form, redirects back and displays base64', async () => {
const history = createMemoryHistory({ initialEntries: ['/create'] });
coreStart.http.get.mockResolvedValue([{ name: 'superuser' }]);
coreStart.http.post.mockResolvedValue({ id: '1D', api_key: 'AP1_K3Y' });

const { findByRole, findByDisplayValue } = render(
<Providers services={coreStart} authc={authc} history={history}>
<APIKeysGridPage
apiKeysAPIClient={apiClientMock}
notifications={coreStart.notifications}
history={history}
/>
</Providers>
);
expect(coreStart.http.get).toHaveBeenCalledWith('/api/security/role');

const dialog = await findByRole('dialog');

fireEvent.click(await findByRole('button', { name: 'Create API key' }));

const alert = await findByRole('alert');
within(alert).getByText(/Enter a name/i);

fireEvent.change(await within(dialog).findByLabelText('Name'), {
target: { value: 'Test' },
});

fireEvent.click(await findByRole('button', { name: 'Create API key' }));

await waitFor(() => {
expect(coreStart.http.post).toHaveBeenLastCalledWith('/internal/security/api_key', {
body: JSON.stringify({ name: 'Test' }),
});
expect(history.location.pathname).toBe('/');
});

await findByDisplayValue(btoa('1D:AP1_K3Y'));
});

it('creates API key with optional expiration, redirects back and displays base64', async () => {
const history = createMemoryHistory({ initialEntries: ['/create'] });
coreStart.http.get.mockResolvedValue([{ name: 'superuser' }]);
coreStart.http.post.mockResolvedValue({ id: '1D', api_key: 'AP1_K3Y' });

const { findByRole, findByDisplayValue } = render(
<Providers services={coreStart} authc={authc} history={history}>
<APIKeysGridPage
apiKeysAPIClient={apiClientMock}
notifications={coreStart.notifications}
history={history}
/>
</Providers>
);
expect(coreStart.http.get).toHaveBeenCalledWith('/api/security/role');

const dialog = await findByRole('dialog');

fireEvent.change(await within(dialog).findByLabelText('Name'), {
target: { value: 'Test' },
});

fireEvent.click(await within(dialog).findByLabelText('Expire after time'));

fireEvent.click(await findByRole('button', { name: 'Create API key' }));

const alert = await findByRole('alert');
within(alert).getByText(/Enter a valid duration or disable this option\./i);

fireEvent.change(await within(dialog).findByLabelText('Lifetime (days)'), {
target: { value: '12' },
});

fireEvent.click(await findByRole('button', { name: 'Create API key' }));

await waitFor(() => {
expect(coreStart.http.post).toHaveBeenLastCalledWith('/internal/security/api_key', {
body: JSON.stringify({ name: 'Test', expiration: '12d' }),
});
expect(history.location.pathname).toBe('/');
body: {
error: 'Internal Server Error',
message: 'Internal Server Error',
statusCode: 500,
},
});

await findByDisplayValue(btoa('1D:AP1_K3Y'));
});

it('deletes api key using cta button', async () => {
const history = createMemoryHistory({ initialEntries: ['/'] });

const { findByRole, findAllByLabelText } = render(
<Providers services={coreStart} authc={authc} history={history}>
<APIKeysGridPage
apiKeysAPIClient={apiClientMock}
notifications={coreStart.notifications}
history={history}
/>
</Providers>
);

const [deleteButton] = await findAllByLabelText(/Delete/i);
fireEvent.click(deleteButton);

const dialog = await findByRole('dialog');
fireEvent.click(await within(dialog).findByRole('button', { name: 'Delete API key' }));

await waitFor(() => {
expect(apiClientMock.invalidateApiKeys).toHaveBeenLastCalledWith(
[{ id: '0QQZ2m0BO2XZwgJFuWTT', name: 'first-api-key' }],
true
);
});
});

it('deletes multiple api keys using bulk select', async () => {
const history = createMemoryHistory({ initialEntries: ['/'] });

const { findByRole, findAllByRole } = render(
const { findByText } = render(
<Providers services={coreStart} authc={authc} history={history}>
<APIKeysGridPage
apiKeysAPIClient={apiClientMock}
Expand All @@ -278,21 +168,7 @@ describe.skip('APIKeysGridPage', () => {
</Providers>
);

const deleteCheckboxes = await findAllByRole('checkbox', { name: 'Select this row' });
deleteCheckboxes.forEach((checkbox) => fireEvent.click(checkbox));
fireEvent.click(await findByRole('button', { name: 'Delete API keys' }));

const dialog = await findByRole('dialog');
fireEvent.click(await within(dialog).findByRole('button', { name: 'Delete API keys' }));

await waitFor(() => {
expect(apiClientMock.invalidateApiKeys).toHaveBeenLastCalledWith(
[
{ id: '0QQZ2m0BO2XZwgJFuWTT', name: 'first-api-key' },
{ id: 'BO2XZwgJFuWTT0QQZ2m0', name: 'second-api-key' },
],
true
);
});
expect(await findByText(/Loading API keys/)).not.toBeInTheDocument();
await findByText(/Could not load API keys/);
});
});
3 changes: 3 additions & 0 deletions x-pack/plugins/security/public/management/api_keys/api_keys_grid/api_keys_grid_page.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ export class APIKeysGridPage extends Component<Props, State> {
{...reactRouterNavigate(this.props.history, '/create')}
fill
iconType="plusInCircleFilled"
data-test-subj="apiKeysCreatePromptButton"
>
<FormattedMessage
id="xpack.security.management.apiKeys.table.createButton"
Expand Down Expand Up @@ -207,6 +208,7 @@ export class APIKeysGridPage extends Component<Props, State> {
{...reactRouterNavigate(this.props.history, '/create')}
fill
iconType="plusInCircleFilled"
data-test-subj="apiKeysCreateTableButton"
>
<FormattedMessage
id="xpack.security.management.apiKeys.table.createButton"
Expand Down Expand Up @@ -597,6 +599,7 @@ export class APIKeysGridPage extends Component<Props, State> {
color: 'danger',
onClick: (item) =>
invalidateApiKeyPrompt([{ id: item.id, name: item.name }], this.onApiKeysInvalidated),
'data-test-subj': 'apiKeysTableDeleteAction',
},
],
},
Expand Down
Loading

0 comments on commit b0110d8

Please sign in to comment.