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

…105698) * 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: 30d8b1d#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. Co-authored-by: Scotty Bollinger <scotty.bollinger@elastic.co>
elastic · Jul 15, 2021 · 66da660 · 66da660
1 parent 1696afe
commit 66da660
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 25 deletions.
diff --git a/...ons/workplace_search/views/content_sources/components/add_source/add_source_logic.test.ts b/...ons/workplace_search/views/content_sources/components/add_source/add_source_logic.test.ts
@@ -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,36 +361,35 @@ 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)
           );
         });
       });
 
       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');

diff --git a/...ications/workplace_search/views/content_sources/components/add_source/add_source_logic.ts b/...ications/workplace_search/views/content_sources/components/add_source/add_source_logic.ts
@@ -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));

diff --git a/...blic/applications/workplace_search/views/content_sources/components/source_added.test.tsx b/...blic/applications/workplace_search/views/content_sources/components/source_added.test.tsx
@@ -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 />);
 

diff --git a/...ch/public/applications/workplace_search/views/content_sources/components/source_added.tsx b/...ch/public/applications/workplace_search/views/content_sources/components/source_added.tsx
@@ -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 (

diff --git a/x-pack/plugins/enterprise_search/server/routes/workplace_search/sources.ts b/x-pack/plugins/enterprise_search/server/routes/workplace_search/sources.ts
@@ -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()),
         }),
       },
     },