maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[Workplace Search] Fix Chrome issues with GitHub sources (#105680)

Browse source 
* Fix route validation

This param is not always required. Was already fixed for org version but the personal dashboard came later and was not fixed.

Original fix for org:
30d8b1dfa8 (diff-07f094b2a4719e8511f003d8e278a77cd6b808d11b14d1c528705f9b259c328fR373)

* Fix route to account for private github route

Previously had the org route hard-coded

* Move the logic for parsing the query params to template

Because the useEffect call comes after the initial render, the chrome flashes. We originally got around this by hiding the chrome always because in non-github scenarios, this worked fine.

However, because the oauth plugin sends the state in the quert params and uses the same URL, we need to parse that to determine whether this is an org or accoutn route. We now do that logic in the template and set the chrome before calling the useEffect.

We still need to pass both the parsed params and the original quert string because the redirect passes that string to the next view.
This commit is contained in:
Scotty Bollinger 2021-07-14 18:44:49 -05:00 committed by GitHub
parent f0b44d7ef9
commit 0f3e4f415f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 34 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
x-pack/plugins/enterprise_search/public/applications/workplace_search/views/content_sources/components/add_source/add_source_logic.test.ts
View file

@ -289,6 +289,7 @@ describe('AddSourceLogic', () => {
describe('saveSourceParams', () => {
const params = {
code: 'code123',
session_state: 'session_state123',
state:
'{"action":"create","context":"organization","service_type":"gmail","csrf_token":"token==","index_permissions":false}',
};
@ -306,7 +307,7 @@ describe('AddSourceLogic', () => {
const setAddedSourceSpy = jest.spyOn(SourcesLogic.actions, 'setAddedSource');
const { serviceName, indexPermissions, serviceType } = response;
http.get.mockReturnValue(Promise.resolve(response));
AddSourceLogic.actions.saveSourceParams(queryString);
AddSourceLogic.actions.saveSourceParams(queryString, params, true);
expect(http.get).toHaveBeenCalledWith('/api/workplace_search/sources/create', {
query: {
...params,
@ -324,7 +325,7 @@ describe('AddSourceLogic', () => {
const accountQueryString =
'?state=%7B%22action%22:%22create%22,%22context%22:%22account%22,%22service_type%22:%22gmail%22,%22csrf_token%22:%22token%3D%3D%22,%22index_permissions%22:false%7D&code=code';
AddSourceLogic.actions.saveSourceParams(accountQueryString);
AddSourceLogic.actions.saveSourceParams(accountQueryString, params, false);
await nextTick();
@ -345,7 +346,7 @@ describe('AddSourceLogic', () => {
preContentSourceId,
})
);
AddSourceLogic.actions.saveSourceParams(queryString);
AddSourceLogic.actions.saveSourceParams(queryString, params, true);
expect(http.get).toHaveBeenCalledWith('/api/workplace_search/sources/create', {
query: {
...params,
@ -360,28 +361,27 @@ describe('AddSourceLogic', () => {
});
describe('Github error edge case', () => {
const GITHUB_ERROR =
'The redirect_uri MUST match the registered callback URL for this application.';
const errorParams = { ...params, error_description: GITHUB_ERROR };
const getGithubQueryString = (context: 'organization' | 'account') =>
`?error=redirect_uri_mismatch&error_description=The+redirect_uri+MUST+match+the+registered+callback+URL+for+this+application.&error_uri=https%3A%2F%2Fdocs.github.com%2Fapps%2Fmanaging-oauth-apps%2Ftroubleshooting-authorization-request-errors%2F%23redirect-uri-mismatch&state=%7B%22action%22%3A%22create%22%2C%22context%22%3A%22${context}%22%2C%22service_type%22%3A%22github%22%2C%22csrf_token%22%3A%22TOKEN%3D%3D%22%2C%22index_permissions%22%3Afalse%7D`;
it('handles "organization" redirect and displays error', () => {
const githubQueryString = getGithubQueryString('organization');
AddSourceLogic.actions.saveSourceParams(githubQueryString);
AddSourceLogic.actions.saveSourceParams(githubQueryString, errorParams, true);
expect(navigateToUrl).toHaveBeenCalledWith('/');
expect(setErrorMessage).toHaveBeenCalledWith(
'The redirect_uri MUST match the registered callback URL for this application.'
);
expect(setErrorMessage).toHaveBeenCalledWith(GITHUB_ERROR);
});
it('handles "account" redirect and displays error', () => {
const githubQueryString = getGithubQueryString('account');
AddSourceLogic.actions.saveSourceParams(githubQueryString);
AddSourceLogic.actions.saveSourceParams(githubQueryString, errorParams, false);
expect(navigateToUrl).toHaveBeenCalledWith(PERSONAL_SOURCES_PATH);
expect(setErrorMessage).toHaveBeenCalledWith(
PERSONAL_DASHBOARD_SOURCE_ERROR(
'The redirect_uri MUST match the registered callback URL for this application.'
)
PERSONAL_DASHBOARD_SOURCE_ERROR(GITHUB_ERROR)
);
});
});
@ -389,7 +389,7 @@ describe('AddSourceLogic', () => {
it('handles error', async () => {
http.get.mockReturnValue(Promise.reject('this is an error'));
AddSourceLogic.actions.saveSourceParams(queryString);
AddSourceLogic.actions.saveSourceParams(queryString, params, true);
await nextTick();
expect(flashAPIErrors).toHaveBeenCalledWith('this is an error');

20
x-pack/plugins/enterprise_search/public/applications/workplace_search/views/content_sources/components/add_source/add_source_logic.ts
View file

@ -20,7 +20,6 @@ import {
} from '../../../../../shared/flash_messages';
import { HttpLogic } from '../../../../../shared/http';
import { KibanaLogic } from '../../../../../shared/kibana';
import { parseQueryParams } from '../../../../../shared/query_params';
import { AppLogic } from '../../../../app_logic';
import { CUSTOM_SERVICE_TYPE, WORKPLACE_SEARCH_URL_PREFIX } from '../../../../constants';
import {
@ -95,7 +94,11 @@ export interface AddSourceActions {
isUpdating: boolean,
successCallback?: () => void
): { isUpdating: boolean; successCallback?(): void };
saveSourceParams(search: Search): { search: Search };
saveSourceParams(
search: Search,
params: OauthParams,
isOrganization: boolean
): { search: Search; params: OauthParams; isOrganization: boolean };
getSourceConfigData(serviceType: string): { serviceType: string };
getSourceConnectData(
serviceType: string,
@ -206,7 +209,11 @@ export const AddSourceLogic = kea<MakeLogicType<AddSourceValues, AddSourceAction
isUpdating,
successCallback,
}),
saveSourceParams: (search: Search) => ({ search }),
saveSourceParams: (search: Search, params: OauthParams, isOrganization: boolean) => ({
search,
params,
isOrganization,
}),
createContentSource: (
serviceType: string,
successCallback: () => void,
@ -500,15 +507,12 @@ export const AddSourceLogic = kea<MakeLogicType<AddSourceValues, AddSourceAction
actions.setButtonNotLoading();
}
},
saveSourceParams: async ({ search }) => {
saveSourceParams: async ({ search, params, isOrganization }) => {
const { http } = HttpLogic.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';
const state = JSON.parse(params.state);
const isOrganization = state.context !== 'account';
/**
There is an extreme edge case where the user is trying to connect Github as source from ent-search,
@ -539,7 +543,7 @@ export const AddSourceLogic = kea<MakeLogicType<AddSourceValues, AddSourceAction
// GitHub requires an intermediate configuration step, where we collect the repos to index.
if (hasConfigureStep && !values.oauthConfigCompleted) {
actions.setPreContentSourceId(preContentSourceId);
navigateToUrl(`${ADD_GITHUB_PATH}/configure${search}`);
navigateToUrl(getSourcesPath(`${ADD_GITHUB_PATH}/configure${search}`, isOrganization));
} else {
setAddedSource(serviceName, indexPermissions, serviceType);
navigateToUrl(getSourcesPath(SOURCES_PATH, isOrganization));

3
x-pack/plugins/enterprise_search/public/applications/workplace_search/views/content_sources/components/source_added.test.tsx
View file

@ -28,7 +28,8 @@ describe('SourceAdded', () => {
});
it('renders', () => {
const search = '?name=foo&serviceType=custom&indexPermissions=false';
const search =
'?code=1234&state=%7B%22action%22%3A%22create%22%2C%22context%22%3A%22account%22%2C%22service_type%22%3A%22github%22%2C%22csrf_token%22%3A%22TOKEN123%3D%3D%22%2C%22index_permissions%22%3Afalse%7D';
(useLocation as jest.Mock).mockImplementationOnce(() => ({ search }));
const wrapper = shallow(<SourceAdded />);

10
x-pack/plugins/enterprise_search/public/applications/workplace_search/views/content_sources/components/source_added.tsx
View file

@ -15,8 +15,9 @@ import { EuiPage, EuiPageBody } from '@elastic/eui';
import { KibanaLogic } from '../../../../shared/kibana';
import { Loading } from '../../../../shared/loading';
import { parseQueryParams } from '../../../../shared/query_params';
import { AddSourceLogic } from './add_source/add_source_logic';
import { AddSourceLogic, OauthParams } from './add_source/add_source_logic';
/**
* This component merely triggers catchs the redirect from the oauth application and initializes the saving
@ -25,14 +26,17 @@ import { AddSourceLogic } from './add_source/add_source_logic';
*/
export const SourceAdded: React.FC = () => {
const { search } = useLocation() as Location;
const params = (parseQueryParams(search) as unknown) as OauthParams;
const state = JSON.parse(params.state);
const isOrganization = state.context !== 'account';
const { setChromeIsVisible } = useValues(KibanaLogic);
const { saveSourceParams } = useActions(AddSourceLogic);
// We don't want the personal dashboard to flash the Kibana chrome, so we hide it.
setChromeIsVisible(false);
setChromeIsVisible(isOrganization);
useEffect(() => {
saveSourceParams(search);
saveSourceParams(search, params, isOrganization);
}, []);
return (

2
x-pack/plugins/enterprise_search/server/routes/workplace_search/sources.ts
View file

@ -135,7 +135,7 @@ export function registerAccountCreateSourceRoute({
login: schema.maybe(schema.string()),
password: schema.maybe(schema.string()),
organizations: schema.maybe(schema.arrayOf(schema.string())),
indexPermissions: schema.boolean(),
indexPermissions: schema.maybe(schema.boolean()),
}),
},
},