Use external script for the OIDC Implicit flow handler page. (#44866)

elastic · Sep 5, 2019 · 1b76070 · 1b76070
1 parent 861a1e2
commit 1b76070
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 32 deletions.
diff --git a/x-pack/legacy/plugins/security/server/routes/api/v1/authenticate.js b/x-pack/legacy/plugins/security/server/routes/api/v1/authenticate.js
@@ -12,6 +12,12 @@ import { KibanaRequest } from '../../../../../../../../src/core/server';
 import { createCSPRuleString } from '../../../../../../../../src/legacy/server/csp';
 
 export function initAuthenticateApi({ authc: { login, logout }, config }, server) {
+  function prepareCustomResourceResponse(response, contentType) {
+    return response
+      .header('cache-control', 'private, no-cache, no-store')
+      .header('content-security-policy', createCSPRuleString(server.config().get('csp.rules')))
+      .type(contentType);
+  }
 
   server.route({
     method: 'POST',
@@ -93,22 +99,36 @@ export function initAuthenticateApi({ authc: { login, logout }, config }, server
     path: '/api/security/v1/oidc/implicit',
     config: { auth: false },
     async handler(request, h) {
-      const legacyConfig = server.config();
-      const basePath = legacyConfig.get('server.basePath');
-
-      const cspRulesHeader = createCSPRuleString(legacyConfig.get('csp.rules'));
-      return h.response(`
-        <!DOCTYPE html>
-        <title>Kibana OpenID Connect Login</title>
-        <script>
+      return prepareCustomResourceResponse(
+        h.response(`
+          <!DOCTYPE html>
+          <title>Kibana OpenID Connect Login</title>
+          <script src="${server.config().get('server.basePath')}/api/security/v1/oidc/implicit.js"></script>
+        `),
+        'text/html'
+      );
+    }
+  });
+
+  /**
+   * The route that accompanies `/api/security/v1/oidc/implicit` and renders a JavaScript snippet
+   * that extracts fragment part from the URL and send it to the `/api/security/v1/oidc` route.
+   * We need this separate endpoint because of default CSP policy that forbids inline scripts.
+   */
+  server.route({
+    method: 'GET',
+    path: '/api/security/v1/oidc/implicit.js',
+    config: { auth: false },
+    async handler(request, h) {
+      return prepareCustomResourceResponse(
+        h.response(`
           window.location.replace(
-            '${basePath}/api/security/v1/oidc?authenticationResponseURI=' + encodeURIComponent(window.location.href)
+            '${server.config().get('server.basePath')}/api/security/v1/oidc?authenticationResponseURI=' + 
+              encodeURIComponent(window.location.href)
           );
-        </script>
-      `)
-        .header('cache-control', 'private, no-cache, no-store')
-        .header('content-security-policy', cspRulesHeader)
-        .type('text/html');
+        `),
+        'text/javascript'
+      );
     }
   });
 

diff --git a/x-pack/test/oidc_api_integration/apis/implicit_flow/oidc_auth.ts b/x-pack/test/oidc_api_integration/apis/implicit_flow/oidc_auth.ts
@@ -7,12 +7,14 @@
 import expect from '@kbn/expect';
 import { JSDOM } from 'jsdom';
 import request, { Cookie } from 'request';
+import { format as formatURL } from 'url';
 import { createTokens, getStateAndNonce } from '../../fixtures/oidc_tools';
 import { FtrProviderContext } from '../../ftr_provider_context';
 
 // eslint-disable-next-line import/no-default-export
 export default function({ getService }: FtrProviderContext) {
   const supertest = getService('supertestWithoutAuth');
+  const config = getService('config');
 
   describe('OpenID Connect Implicit Flow authentication', () => {
     describe('finishing handshake', () => {
@@ -31,22 +33,30 @@ export default function({ getService }: FtrProviderContext) {
       it('should return an HTML page that will parse URL fragment', async () => {
         const response = await supertest.get('/api/security/v1/oidc/implicit').expect(200);
         const dom = new JSDOM(response.text, {
+          url: formatURL({ ...config.get('servers.kibana'), auth: false }),
           runScripts: 'dangerously',
+          resources: 'usable',
           beforeParse(window) {
             // JSDOM doesn't support changing of `window.location` and throws an exception if script
-            // tries to do that and we have to workaround this behaviour.
-            Object.defineProperty(window, 'location', {
-              value: {
-                href:
-                  'https://kibana.com/api/security/v1/oidc/implicit#token=some_token&access_token=some_access_token',
-                replace(newLocation: string) {
-                  this.href = newLocation;
+            // tries to do that and we have to workaround this behaviour. We also need to wait until our
+            // script is loaded and executed, __isScriptExecuted__ is used exactly for that.
+            (window as Record<string, any>).__isScriptExecuted__ = new Promise(resolve => {
+              Object.defineProperty(window, 'location', {
+                value: {
+                  href:
+                    'https://kibana.com/api/security/v1/oidc/implicit#token=some_token&access_token=some_access_token',
+                  replace(newLocation: string) {
+                    this.href = newLocation;
+                    resolve();
+                  },
                 },
-              },
+              });
             });
           },
         });
 
+        await (dom.window as Record<string, any>).__isScriptExecuted__;
+
         // Check that proxy page is returned with proper headers.
         expect(response.headers['content-type']).to.be('text/html; charset=utf-8');
         expect(response.headers['cache-control']).to.be('private, no-cache, no-store');

diff --git a/x-pack/test/oidc_api_integration/implicit_flow.config.ts b/x-pack/test/oidc_api_integration/implicit_flow.config.ts
@@ -20,17 +20,20 @@ export default async function({ readConfigFile }: FtrConfigProviderContext) {
 
     esTestCluster: {
       ...oidcAPITestsConfig.get('esTestCluster'),
-      serverArgs: oidcAPITestsConfig.get('esTestCluster.serverArgs').map((arg: string) => {
-        if (arg.startsWith('xpack.security.authc.realms.oidc.oidc1.rp.response_type')) {
-          return 'xpack.security.authc.realms.oidc.oidc1.rp.response_type=id_token token';
-        }
+      serverArgs: oidcAPITestsConfig
+        .get('esTestCluster.serverArgs')
+        .reduce((serverArgs: string[], arg: string) => {
+          // We should change `response_type` to `id_token token` and get rid of unnecessary `token_endpoint`.
+          if (arg.startsWith('xpack.security.authc.realms.oidc.oidc1.rp.response_type')) {
+            serverArgs.push(
+              'xpack.security.authc.realms.oidc.oidc1.rp.response_type=id_token token'
+            );
+          } else if (!arg.startsWith('xpack.security.authc.realms.oidc.oidc1.op.token_endpoint')) {
+            serverArgs.push(arg);
+          }
 
-        if (arg.startsWith('xpack.security.authc.realms.oidc.oidc1.op.token_endpoint')) {
-          return 'xpack.security.authc.realms.oidc.oidc1.op.token_endpoint=should_not_be_used';
-        }
-
-        return arg;
-      }),
+          return serverArgs;
+        }, []),
     },
   };
 }