Skip to content

Commit

Permalink
[App Search] Credentials: Add final Logic and server routes (elastic#…
Browse files Browse the repository at this point in the history 
…81519) (elastic#81540)

* Add fullEngineAccessChecked logic

* Add onEngineSelect logic

* [Refactor] DRY out/simplify http mocks

Note: import reorder is required in for mocks to work correctly

* Add onApiTokenChange logic

* Update flyout footer to use onApiTokenChange

* Add new POST/PUT server routes

+ some opinionated comments

* [PR feedback] tests copy, extra data tests

* [PR feedback] Reuse fullEngineAccessChecked, fix fullEngineAccessChecked being undefined vs a bool

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
@kibanamachine
Constance and kibanamachine authored Oct 26, 2020
1 parent e1b6c68 commit 9385b49
Show file tree
Hide file tree
Showing 6 changed files with 539 additions and 25 deletions.
3 changes: 2 additions & 1 deletion .../public/applications/app_search/components/credentials/credentials_flyout/footer.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe('CredentialsFlyoutFooter', () => {
};
const actions = {
hideCredentialsForm: jest.fn(),
onApiTokenChange: jest.fn(),
};

beforeEach(() => {
Expand Down Expand Up @@ -59,6 +60,6 @@ describe('CredentialsFlyoutFooter', () => {
const button = wrapper.find('[data-test-subj="APIKeyActionButton"]');
button.simulate('click');

// TODO: Expect onApiTokenChange to have been called
expect(actions.onApiTokenChange).toHaveBeenCalled();
});
});
4 changes: 2 additions & 2 deletions ...earch/public/applications/app_search/components/credentials/credentials_flyout/footer.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { i18n } from '@kbn/i18n';
import { CredentialsLogic } from '../credentials_logic';

export const CredentialsFlyoutFooter: React.FC = () => {
const { hideCredentialsForm } = useActions(CredentialsLogic);
const { hideCredentialsForm, onApiTokenChange } = useActions(CredentialsLogic);
const { activeApiTokenExists } = useValues(CredentialsLogic);

return (
Expand All @@ -33,7 +33,7 @@ export const CredentialsFlyoutFooter: React.FC = () => {
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButton
onClick={() => window.alert('submit')}
onClick={onApiTokenChange}
fill={true}
color="secondary"
iconType="check"
Expand Down
223 changes: 205 additions & 18 deletions ...se_search/public/applications/app_search/components/credentials/credentials_logic.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@

import { resetContext } from 'kea';

import { CredentialsLogic } from './credentials_logic';
import { ApiTokenTypes } from './constants';

import { mockHttpValues } from '../../../__mocks__';
jest.mock('../../../shared/http', () => ({
HttpLogic: { values: { http: { get: jest.fn(), delete: jest.fn() } } },
HttpLogic: { values: mockHttpValues },
}));
import { HttpLogic } from '../../../shared/http';
const { http } = mockHttpValues;

jest.mock('../../../shared/flash_messages', () => ({
FlashMessagesLogic: { actions: { clearFlashMessages: jest.fn() } },
setSuccessMessage: jest.fn(),
Expand All @@ -24,6 +23,17 @@ import {
flashAPIErrors,
} from '../../../shared/flash_messages';

jest.mock('../../app_logic', () => ({
AppLogic: {
selectors: { myRole: jest.fn(() => ({})) },
values: { myRole: jest.fn(() => ({})) },
},
}));
import { AppLogic } from '../../app_logic';

import { ApiTokenTypes } from './constants';
import { CredentialsLogic } from './credentials_logic';

describe('CredentialsLogic', () => {
const DEFAULT_VALUES = {
activeApiToken: {
Expand All @@ -44,6 +54,7 @@ describe('CredentialsLogic', () => {
meta: {},
nameInputBlurred: false,
shouldShowCredentialsForm: false,
fullEngineAccessChecked: false,
};

const mount = (defaults?: object) => {
Expand Down Expand Up @@ -1081,10 +1092,10 @@ describe('CredentialsLogic', () => {
mount();
jest.spyOn(CredentialsLogic.actions, 'setCredentialsData').mockImplementationOnce(() => {});
const promise = Promise.resolve({ meta, results });
(HttpLogic.values.http.get as jest.Mock).mockReturnValue(promise);
http.get.mockReturnValue(promise);

CredentialsLogic.actions.fetchCredentials(2);
expect(HttpLogic.values.http.get).toHaveBeenCalledWith('/api/app_search/credentials', {
expect(http.get).toHaveBeenCalledWith('/api/app_search/credentials', {
query: {
'page[current]': 2,
},
Expand All @@ -1096,7 +1107,7 @@ describe('CredentialsLogic', () => {
it('handles errors', async () => {
mount();
const promise = Promise.reject('An error occured');
(HttpLogic.values.http.get as jest.Mock).mockReturnValue(promise);
http.get.mockReturnValue(promise);

CredentialsLogic.actions.fetchCredentials();
try {
Expand All @@ -1114,12 +1125,10 @@ describe('CredentialsLogic', () => {
.spyOn(CredentialsLogic.actions, 'setCredentialsDetails')
.mockImplementationOnce(() => {});
const promise = Promise.resolve(credentialsDetails);
(HttpLogic.values.http.get as jest.Mock).mockReturnValue(promise);
http.get.mockReturnValue(promise);

CredentialsLogic.actions.fetchDetails();
expect(HttpLogic.values.http.get).toHaveBeenCalledWith(
'/api/app_search/credentials/details'
);
expect(http.get).toHaveBeenCalledWith('/api/app_search/credentials/details');
await promise;
expect(CredentialsLogic.actions.setCredentialsDetails).toHaveBeenCalledWith(
credentialsDetails
Expand All @@ -1129,7 +1138,7 @@ describe('CredentialsLogic', () => {
it('handles errors', async () => {
mount();
const promise = Promise.reject('An error occured');
(HttpLogic.values.http.get as jest.Mock).mockReturnValue(promise);
http.get.mockReturnValue(promise);

CredentialsLogic.actions.fetchDetails();
try {
Expand All @@ -1147,12 +1156,10 @@ describe('CredentialsLogic', () => {
mount();
jest.spyOn(CredentialsLogic.actions, 'onApiKeyDelete').mockImplementationOnce(() => {});
const promise = Promise.resolve();
(HttpLogic.values.http.delete as jest.Mock).mockReturnValue(promise);
http.delete.mockReturnValue(promise);

CredentialsLogic.actions.deleteApiKey(tokenName);
expect(HttpLogic.values.http.delete).toHaveBeenCalledWith(
`/api/app_search/credentials/${tokenName}`
);
expect(http.delete).toHaveBeenCalledWith(`/api/app_search/credentials/${tokenName}`);
await promise;
expect(CredentialsLogic.actions.onApiKeyDelete).toHaveBeenCalledWith(tokenName);
expect(setSuccessMessage).toHaveBeenCalled();
Expand All @@ -1161,7 +1168,7 @@ describe('CredentialsLogic', () => {
it('handles errors', async () => {
mount();
const promise = Promise.reject('An error occured');
(HttpLogic.values.http.delete as jest.Mock).mockReturnValue(promise);
http.delete.mockReturnValue(promise);

CredentialsLogic.actions.deleteApiKey(tokenName);
try {
Expand All @@ -1171,9 +1178,189 @@ describe('CredentialsLogic', () => {
}
});
});

describe('onApiTokenChange', () => {
it('calls a POST API endpoint that creates a new token if the active token does not exist yet', async () => {
const createdToken = {
name: 'new-key',
type: ApiTokenTypes.Admin,
};
mount({
activeApiToken: createdToken,
});
jest.spyOn(CredentialsLogic.actions, 'onApiTokenCreateSuccess');
const promise = Promise.resolve(createdToken);
http.post.mockReturnValue(promise);

CredentialsLogic.actions.onApiTokenChange();
expect(http.post).toHaveBeenCalledWith('/api/app_search/credentials', {
body: JSON.stringify(createdToken),
});
await promise;
expect(CredentialsLogic.actions.onApiTokenCreateSuccess).toHaveBeenCalledWith(createdToken);
expect(setSuccessMessage).toHaveBeenCalled();
});

it('calls a PUT endpoint that updates the active token if it already exists', async () => {
const updatedToken = {
name: 'test-key',
type: ApiTokenTypes.Private,
read: true,
write: false,
access_all_engines: false,
engines: ['engine1'],
};
mount({
activeApiToken: {
...updatedToken,
id: 'some-id',
},
});
jest.spyOn(CredentialsLogic.actions, 'onApiTokenUpdateSuccess');
const promise = Promise.resolve(updatedToken);
http.put.mockReturnValue(promise);

CredentialsLogic.actions.onApiTokenChange();
expect(http.put).toHaveBeenCalledWith('/api/app_search/credentials/test-key', {
body: JSON.stringify(updatedToken),
});
await promise;
expect(CredentialsLogic.actions.onApiTokenUpdateSuccess).toHaveBeenCalledWith(updatedToken);
expect(setSuccessMessage).toHaveBeenCalled();
});

it('handles errors', async () => {
mount();
const promise = Promise.reject('An error occured');
http.post.mockReturnValue(promise);

CredentialsLogic.actions.onApiTokenChange();
try {
await promise;
} catch {
expect(flashAPIErrors).toHaveBeenCalledWith('An error occured');
}
});

describe('token type data', () => {
it('does not send extra read/write/engine access data for admin tokens', () => {
const correctAdminToken = {
name: 'bogus-admin',
type: ApiTokenTypes.Admin,
};
const extraData = {
read: true,
write: true,
access_all_engines: true,
};
mount({ activeApiToken: { ...correctAdminToken, ...extraData } });

CredentialsLogic.actions.onApiTokenChange();
expect(http.post).toHaveBeenCalledWith('/api/app_search/credentials', {
body: JSON.stringify(correctAdminToken),
});
});

it('does not send extra read/write access data for search tokens', () => {
const correctSearchToken = {
name: 'bogus-search',
type: ApiTokenTypes.Search,
access_all_engines: false,
engines: ['some-engine'],
};
const extraData = {
read: true,
write: false,
};
mount({ activeApiToken: { ...correctSearchToken, ...extraData } });

CredentialsLogic.actions.onApiTokenChange();
expect(http.post).toHaveBeenCalledWith('/api/app_search/credentials', {
body: JSON.stringify(correctSearchToken),
});
});

// Private tokens send all data per the PUT test above.
// If that ever changes, we should capture that in another test here.
});
});

describe('onEngineSelect', () => {
it('calls addEngineName if the engine is not selected', () => {
mount({
activeApiToken: {
...DEFAULT_VALUES.activeApiToken,
engines: [],
},
});
jest.spyOn(CredentialsLogic.actions, 'addEngineName');

CredentialsLogic.actions.onEngineSelect('engine1');
expect(CredentialsLogic.actions.addEngineName).toHaveBeenCalledWith('engine1');
expect(CredentialsLogic.values.activeApiToken.engines).toEqual(['engine1']);
});

it('calls removeEngineName if the engine is already selected', () => {
mount({
activeApiToken: {
...DEFAULT_VALUES.activeApiToken,
engines: ['engine1', 'engine2'],
},
});
jest.spyOn(CredentialsLogic.actions, 'removeEngineName');

CredentialsLogic.actions.onEngineSelect('engine1');
expect(CredentialsLogic.actions.removeEngineName).toHaveBeenCalledWith('engine1');
expect(CredentialsLogic.values.activeApiToken.engines).toEqual(['engine2']);
});
});
});

describe('selectors', () => {
describe('fullEngineAccessChecked', () => {
it('should be true if active token is set to access all engines and the user can access all engines', () => {
(AppLogic.selectors.myRole as jest.Mock).mockReturnValueOnce({
canAccessAllEngines: true,
});
mount({
activeApiToken: {
...DEFAULT_VALUES.activeApiToken,
access_all_engines: true,
},
});

expect(CredentialsLogic.values.fullEngineAccessChecked).toEqual(true);
});

it('should be false if the token is not set to access all engines', () => {
(AppLogic.selectors.myRole as jest.Mock).mockReturnValueOnce({
canAccessAllEngines: true,
});
mount({
activeApiToken: {
...DEFAULT_VALUES.activeApiToken,
access_all_engines: false,
},
});

expect(CredentialsLogic.values.fullEngineAccessChecked).toEqual(false);
});

it('should be false if the user cannot acess all engines', () => {
(AppLogic.selectors.myRole as jest.Mock).mockReturnValueOnce({
canAccessAllEngines: false,
});
mount({
activeApiToken: {
...DEFAULT_VALUES.activeApiToken,
access_all_engines: true,
},
});

expect(CredentialsLogic.values.fullEngineAccessChecked).toEqual(false);
});
});

describe('activeApiTokenExists', () => {
it('should be false if the token has no id', () => {
mount({
Expand Down
Loading

0 comments on commit 9385b49

Please sign in to comment.