Skip to content

Commit

Permalink
[Workplace Search] Refactor oauth redirect to persist state params fr…
Browse files Browse the repository at this point in the history 
…om plugin (#90067) (#90228)

* Add routes

Also adds validation for the Kibana way of handling query params

* Add route for oauth params

* Add logic to save oauth redirect query params

* Refactor source added template to keep all logic in logic file

* Add tests for component and logic

* Add optional param to interface

Atlassian flows may also send back an oauth_verifier param that we’ll need. This was added to the server validation, but I forgot to add it to the interface

* Remove failing test

This was not needed for coverage and it appears that the helper doesn’t validate query params so removing it

* Remove index_permissions from account params

* Rename variable

* Update param syntax

* Update account route test

* Refactor params
  • Loading branch information
@scottybollinger
scottybollinger authored Feb 3, 2021
1 parent 8360eab commit 128d84f
Show file tree
Hide file tree
Showing 7 changed files with 319 additions and 89 deletions.
2 changes: 2 additions & 0 deletions x-pack/plugins/enterprise_search/public/applications/workplace_search/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,8 @@ export const GITHUB_LINK_TITLE = i18n.translate(

export const CUSTOM_SERVICE_TYPE = 'custom';

export const WORKPLACE_SEARCH_URL_PREFIX = '/app/enterprise_search/workplace_search';

export const DOCUMENTATION_LINK_TITLE = i18n.translate(
'xpack.enterpriseSearch.workplaceSearch.sources.documentation',
{
Expand Down
95 changes: 86 additions & 9 deletions ...ons/workplace_search/views/content_sources/components/add_source/add_source_logic.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,24 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { LogicMounter, mockFlashMessageHelpers, mockHttpValues } from '../../../../../__mocks__';
import {
LogicMounter,
mockFlashMessageHelpers,
mockHttpValues,
mockKibanaValues,
} from '../../../../../__mocks__';

import { AppLogic } from '../../../../app_logic';
jest.mock('../../../../app_logic', () => ({
AppLogic: { values: { isOrganization: true } },
}));

import { SourcesLogic } from '../../sources_logic';

import { nextTick } from '@kbn/test/jest';

import { CustomSource } from '../../../../types';
import { SOURCES_PATH, getSourcesPath } from '../../../../routes';

import { sourceConfigData } from '../../../../__mocks__/content_sources.mock';

Expand All @@ -28,6 +36,7 @@ import {
describe('AddSourceLogic', () => {
const { mount } = new LogicMounter(AddSourceLogic);
const { http } = mockHttpValues;
const { navigateToUrl } = mockKibanaValues;
const { clearFlashMessages, flashAPIErrors } = mockFlashMessageHelpers;

const defaultValues = {
Expand Down Expand Up @@ -264,6 +273,55 @@ describe('AddSourceLogic', () => {
});
});

describe('saveSourceParams', () => {
const params = {
code: 'code123',
state: '"{"state": "foo"}"',
session_state: 'session123',
};

const queryString =
'code=code123&state=%22%7B%22state%22%3A%20%22foo%22%7D%22&session_state=session123';

const response = { serviceName: 'name', indexPermissions: false, serviceType: 'zendesk' };

beforeEach(() => {
SourcesLogic.mount();
});

it('sends params to server and calls correct methods', async () => {
const setAddedSourceSpy = jest.spyOn(SourcesLogic.actions, 'setAddedSource');
const { serviceName, indexPermissions, serviceType } = response;
http.get.mockReturnValue(Promise.resolve(response));
AddSourceLogic.actions.saveSourceParams(queryString);
expect(http.get).toHaveBeenCalledWith('/api/workplace_search/sources/create', {
query: {
...params,
kibana_host: '',
},
});

await nextTick();

expect(setAddedSourceSpy).toHaveBeenCalledWith(serviceName, indexPermissions, serviceType);
expect(navigateToUrl).toHaveBeenCalledWith(
getSourcesPath(SOURCES_PATH, AppLogic.values.isOrganization)
);
});

it('handles error', async () => {
http.get.mockReturnValue(Promise.reject('this is an error'));

AddSourceLogic.actions.saveSourceParams(queryString);
await nextTick();

expect(flashAPIErrors).toHaveBeenCalledWith('this is an error');
expect(navigateToUrl).toHaveBeenCalledWith(
getSourcesPath(SOURCES_PATH, AppLogic.values.isOrganization)
);
});
});

describe('organization context', () => {
describe('getSourceConfigData', () => {
it('calls API and sets values', async () => {
Expand Down Expand Up @@ -301,22 +359,37 @@ describe('AddSourceLogic', () => {

AddSourceLogic.actions.getSourceConnectData('github', successCallback);

const query = {
index_permissions: false,
kibana_host: '',
};

expect(clearFlashMessages).toHaveBeenCalled();
expect(AddSourceLogic.values.buttonLoading).toEqual(true);
expect(http.get).toHaveBeenCalledWith('/api/workplace_search/org/sources/github/prepare');
expect(http.get).toHaveBeenCalledWith(
'/api/workplace_search/org/sources/github/prepare',
{ query }
);
await nextTick();
expect(setSourceConnectDataSpy).toHaveBeenCalledWith(sourceConnectData);
expect(successCallback).toHaveBeenCalledWith(sourceConnectData.oauthUrl);
expect(setButtonNotLoadingSpy).toHaveBeenCalled();
});

it('appends query params', () => {
it('passes query params', () => {
AddSourceLogic.actions.setSourceSubdomainValue('subdomain');
AddSourceLogic.actions.setSourceIndexPermissionsValue(true);
AddSourceLogic.actions.getSourceConnectData('github', successCallback);

const query = {
index_permissions: true,
kibana_host: '',
subdomain: 'subdomain',
};

expect(http.get).toHaveBeenCalledWith(
'/api/workplace_search/org/sources/github/prepare?subdomain=subdomain&index_permissions=true'
'/api/workplace_search/org/sources/github/prepare',
{ query }
);
});

Expand Down Expand Up @@ -413,7 +486,7 @@ describe('AddSourceLogic', () => {
http.put
).toHaveBeenCalledWith(
`/api/workplace_search/org/settings/connectors/${sourceConfigData.serviceType}`,
{ body: JSON.stringify({ params }) }
{ body: JSON.stringify(params) }
);

await nextTick();
Expand All @@ -436,7 +509,7 @@ describe('AddSourceLogic', () => {
};

expect(http.post).toHaveBeenCalledWith('/api/workplace_search/org/settings/connectors', {
body: JSON.stringify({ params: createParams }),
body: JSON.stringify(createParams),
});
});

Expand Down Expand Up @@ -515,11 +588,15 @@ describe('AddSourceLogic', () => {
});

it('getSourceConnectData', () => {
const query = {
kibana_host: '',
};

AddSourceLogic.actions.getSourceConnectData('github', jest.fn());

expect(http.get).toHaveBeenCalledWith(
'/api/workplace_search/account/sources/github/prepare'
);
expect(
http.get
).toHaveBeenCalledWith('/api/workplace_search/account/sources/github/prepare', { query });
});

it('getSourceReConnectData', () => {
Expand Down
63 changes: 55 additions & 8 deletions ...ications/workplace_search/views/content_sources/components/add_source/add_source_logic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,15 @@ import { keys, pickBy } from 'lodash';

import { kea, MakeLogicType } from 'kea';

import { Search } from 'history';

import { i18n } from '@kbn/i18n';

import { HttpFetchQuery } from 'src/core/public';

import { HttpLogic } from '../../../../../shared/http';
import { KibanaLogic } from '../../../../../shared/kibana';
import { parseQueryParams } from '../../../../../shared/query_params';

import {
flashAPIErrors,
Expand All @@ -19,9 +25,11 @@ import {
} from '../../../../../shared/flash_messages';

import { staticSourceData } from '../../source_data';
import { CUSTOM_SERVICE_TYPE } from '../../../../constants';
import { SOURCES_PATH, getSourcesPath } from '../../../../routes';
import { CUSTOM_SERVICE_TYPE, WORKPLACE_SEARCH_URL_PREFIX } from '../../../../constants';

import { AppLogic } from '../../../../app_logic';
import { SourcesLogic } from '../../sources_logic';
import { CustomSource } from '../../../../types';

export interface AddSourceProps {
Expand All @@ -42,6 +50,13 @@ export enum AddSourceSteps {
ReAuthenticateStep = 'ReAuthenticate',
}

export interface OauthParams {
code: string;
state: string;
session_state: string;
oauth_verifier?: string;
}

export interface AddSourceActions {
initializeAddSource: (addSourceProps: AddSourceProps) => { addSourceProps: AddSourceProps };
setAddSourceProps: ({
Expand Down Expand Up @@ -75,6 +90,7 @@ export interface AddSourceActions {
isUpdating: boolean,
successCallback?: () => void
): { isUpdating: boolean; successCallback?(): void };
saveSourceParams(search: Search): { search: Search };
getSourceConfigData(serviceType: string): { serviceType: string };
getSourceConnectData(
serviceType: string,
Expand Down Expand Up @@ -141,6 +157,15 @@ interface PreContentSourceResponse {
githubOrganizations: string[];
}

/**
* Workplace Search needs to know the host for the redirect. As of yet, we do not
* have access to this in Kibana. We parse it from the browser and pass it as a param.
*/
const {
location: { href },
} = window;
const kibanaHost = href.substr(0, href.indexOf(WORKPLACE_SEARCH_URL_PREFIX));

export const AddSourceLogic = kea<MakeLogicType<AddSourceValues, AddSourceActions>>({
path: ['enterprise_search', 'workplace_search', 'add_source_logic'],
actions: {
Expand Down Expand Up @@ -173,6 +198,7 @@ export const AddSourceLogic = kea<MakeLogicType<AddSourceValues, AddSourceAction
isUpdating,
successCallback,
}),
saveSourceParams: (search: Search) => ({ search }),
createContentSource: (
serviceType: string,
successCallback: () => void,
Expand Down Expand Up @@ -356,14 +382,15 @@ export const AddSourceLogic = kea<MakeLogicType<AddSourceValues, AddSourceAction
? `/api/workplace_search/org/sources/${serviceType}/prepare`
: `/api/workplace_search/account/sources/${serviceType}/prepare`;

const params = new URLSearchParams();
if (subdomain) params.append('subdomain', subdomain);
if (indexPermissions) params.append('index_permissions', indexPermissions.toString());
const hasParams = params.has('subdomain') || params.has('index_permissions');
const paramsString = hasParams ? `?${params}` : '';
const query = {
kibana_host: kibanaHost,
} as HttpFetchQuery;

if (isOrganization) query.index_permissions = indexPermissions;
if (subdomain) query.subdomain = subdomain;

try {
const response = await HttpLogic.values.http.get(`${route}${paramsString}`);
const response = await HttpLogic.values.http.get(route, { query });
actions.setSourceConnectData(response);
successCallback(response.oauthUrl);
} catch (e) {
Expand Down Expand Up @@ -426,7 +453,7 @@ export const AddSourceLogic = kea<MakeLogicType<AddSourceValues, AddSourceAction

try {
const response = await http(route, {
body: JSON.stringify({ params }),
body: JSON.stringify(params),
});
if (successCallback) successCallback();
if (isUpdating) {
Expand All @@ -446,6 +473,26 @@ export const AddSourceLogic = kea<MakeLogicType<AddSourceValues, AddSourceAction
actions.setButtonNotLoading();
}
},
saveSourceParams: async ({ search }) => {
const { http } = HttpLogic.values;
const { isOrganization } = AppLogic.values;
const { navigateToUrl } = KibanaLogic.values;
const { setAddedSource } = SourcesLogic.actions;
const params = (parseQueryParams(search) as unknown) as OauthParams;
const query = { ...params, kibana_host: kibanaHost };
const route = '/api/workplace_search/sources/create';

try {
const response = await http.get(route, { query });

const { serviceName, indexPermissions, serviceType } = response;
setAddedSource(serviceName, indexPermissions, serviceType);
} catch (e) {
flashAPIErrors(e);
} finally {
navigateToUrl(getSourcesPath(SOURCES_PATH, isOrganization));
}
},
createContentSource: async ({ serviceType, successCallback, errorCallback }) => {
clearFlashMessages();
const { isOrganization } = AppLogic.values;
Expand Down
35 changes: 8 additions & 27 deletions ...blic/applications/workplace_search/views/content_sources/components/source_added.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,49 +6,30 @@

import '../../../../__mocks__/shallow_useeffect.mock';

import { setMockValues, setMockActions, mockFlashMessageHelpers } from '../../../../__mocks__';
import { setMockActions } from '../../../../__mocks__';

import React from 'react';
import { shallow } from 'enzyme';

import { Redirect, useLocation } from 'react-router-dom';
import { useLocation } from 'react-router-dom';

import { Loading } from '../../../../shared/loading';

import { SourceAdded } from './source_added';

describe('SourceAdded', () => {
const { setErrorMessage } = mockFlashMessageHelpers;
const setAddedSource = jest.fn();
const saveSourceParams = jest.fn();

beforeEach(() => {
setMockActions({ setAddedSource });
setMockValues({ isOrganization: true });
setMockActions({ saveSourceParams });
});

it('renders', () => {
const search = '?name=foo&serviceType=custom&indexPermissions=false';
(useLocation as jest.Mock).mockImplementationOnce(() => ({ search }));
const wrapper = shallow(<SourceAdded />);

expect(wrapper.find(Redirect)).toHaveLength(1);
expect(setAddedSource).toHaveBeenCalled();
});

describe('hasError', () => {
it('passes default error to server', () => {
const search = '?name=foo&hasError=true&serviceType=custom&indexPermissions=false';
(useLocation as jest.Mock).mockImplementationOnce(() => ({ search }));
shallow(<SourceAdded />);

expect(setErrorMessage).toHaveBeenCalledWith('foo failed to connect.');
});

it('passes custom error to server', () => {
const search =
'?name=foo&hasError=true&serviceType=custom&indexPermissions=false&errorMessages[]=custom error';
(useLocation as jest.Mock).mockImplementationOnce(() => ({ search }));
shallow(<SourceAdded />);

expect(setErrorMessage).toHaveBeenCalledWith('custom error');
});
expect(wrapper.find(Loading)).toHaveLength(1);
expect(saveSourceParams).toHaveBeenCalled();
});
});
Loading

0 comments on commit 128d84f

Please sign in to comment.