Use serverBasePath provided by the core instead of the one provided…

… via legacy shim.
elastic · Oct 8, 2019 · 2b88f7b · 2b88f7b
1 parent a361167
commit 2b88f7b
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 20 deletions.
diff --git a/x-pack/legacy/plugins/security/index.js b/x-pack/legacy/plugins/security/index.js
@@ -138,7 +138,6 @@ export const security = (kibana) => new kibana.Plugin({
         server.plugins.kibana.systemApi
       ),
       cspRules: createCSPRuleString(config.get('csp.rules')),
-      serverBasePath: config.get('server.basePath'),
     });
 
     const plugin = this;

diff --git a/x-pack/plugins/security/server/authentication/providers/saml.test.ts b/x-pack/plugins/security/server/authentication/providers/saml.test.ts
@@ -527,7 +527,7 @@ describe('SAMLAuthenticationProvider', () => {
 
       expect(authenticationResult.redirected()).toBe(true);
       expect(authenticationResult.redirectURL).toBe(
-        '/base-path/api/security/saml/capture-url-fragment'
+        '/mock-server-basepath/api/security/saml/capture-url-fragment'
       );
       expect(authenticationResult.state).toEqual({ redirectURL: '/base-path/s/foo/some-path' });
     });
@@ -746,7 +746,7 @@ describe('SAMLAuthenticationProvider', () => {
 
       expect(authenticationResult.redirected()).toBe(true);
       expect(authenticationResult.redirectURL).toBe(
-        '/base-path/api/security/saml/capture-url-fragment'
+        '/mock-server-basepath/api/security/saml/capture-url-fragment'
       );
       expect(authenticationResult.state).toEqual({ redirectURL: '/base-path/s/foo/some-path' });
     });
@@ -819,7 +819,7 @@ describe('SAMLAuthenticationProvider', () => {
 
       expect(authenticationResult.redirected()).toBe(true);
       expect(authenticationResult.redirectURL).toBe(
-        '/base-path/api/security/saml/capture-url-fragment'
+        '/mock-server-basepath/api/security/saml/capture-url-fragment'
       );
       expect(authenticationResult.state).toEqual({ redirectURL: '/base-path/s/foo/some-path' });
     });

diff --git a/x-pack/plugins/security/server/authentication/providers/saml.ts b/x-pack/plugins/security/server/authentication/providers/saml.ts
@@ -571,10 +571,9 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider {
       return this.authenticateViaHandshake(request, '');
     }
 
-    // We should use `serverBasePath` here as soon as it's provided by the core. Using request scoped
-    // base path doesn't hurt, but isn't consistent with the rest of API routes.
-    return AuthenticationResult.redirectTo(`${basePath}/api/security/saml/capture-url-fragment`, {
-      state: { redirectURL },
-    });
+    return AuthenticationResult.redirectTo(
+      `${this.options.basePath.serverBasePath}/api/security/saml/capture-url-fragment`,
+      { state: { redirectURL } }
+    );
   }
 }
diff --git a/x-pack/plugins/security/server/plugin.ts b/x-pack/plugins/security/server/plugin.ts
@@ -27,7 +27,6 @@ export interface LegacyAPI {
   xpackInfo: Pick<XPackInfo, 'isAvailable' | 'feature'>;
   isSystemAPIRequest: (request: KibanaRequest) => boolean;
   cspRules: string;
-  serverBasePath?: string;
 }
 
 /**

diff --git a/x-pack/plugins/security/server/routes/authentication.test.ts b/x-pack/plugins/security/server/routes/authentication.test.ts
@@ -31,8 +31,7 @@ describe('SAML authentication routes', () => {
       logger: loggingServiceMock.create().get(),
       config: { authc: { providers: ['saml'] } } as ConfigType,
       authc,
-      getLegacyAPI: () =>
-        ({ cspRules: 'test-csp-rule', serverBasePath: '/test-base-path' } as LegacyAPI),
+      getLegacyAPI: () => ({ cspRules: 'test-csp-rule' } as LegacyAPI),
     });
   });
 
@@ -44,8 +43,7 @@ describe('SAML authentication routes', () => {
       logger: loggingServiceMock.create().get(),
       config: { authc: { providers: ['basic'] } } as ConfigType,
       authc: authenticationMock.create(),
-      getLegacyAPI: () =>
-        ({ cspRules: 'test-csp-rule', serverBasePath: '/test-base-path' } as LegacyAPI),
+      getLegacyAPI: () => ({ cspRules: 'test-csp-rule' } as LegacyAPI),
     });
 
     const samlRoutePathPredicate = ([{ path }]: [{ path: string }, any]) =>

diff --git a/x-pack/plugins/security/server/routes/authentication.ts b/x-pack/plugins/security/server/routes/authentication.ts
@@ -17,7 +17,13 @@ export function defineAuthenticationRoutes(params: RouteDefinitionParams) {
 /**
  * Defines routes required for SAML authentication.
  */
-function defineSAMLRoutes({ router, logger, authc, getLegacyAPI }: RouteDefinitionParams) {
+function defineSAMLRoutes({
+  router,
+  logger,
+  authc,
+  getLegacyAPI,
+  basePath,
+}: RouteDefinitionParams) {
   function createCustomResourceResponse(body: string, contentType: string) {
     return {
       body,
@@ -37,15 +43,14 @@ function defineSAMLRoutes({ router, logger, authc, getLegacyAPI }: RouteDefiniti
       options: { authRequired: false },
     },
     (context, request, response) => {
-      const serverBasePath = getLegacyAPI().serverBasePath || '';
       // We're also preventing `favicon.ico` request since it can cause new SAML handshake.
       return response.custom(
         createCustomResourceResponse(
           `
           <!DOCTYPE html>
           <title>Kibana SAML Login</title>
           <link rel="icon" href="data:,">
-          <script src="${serverBasePath}/api/security/saml/capture-url-fragment.js"></script>
+          <script src="${basePath.serverBasePath}/api/security/saml/capture-url-fragment.js"></script>
         `,
           'text/html'
         )
@@ -60,12 +65,11 @@ function defineSAMLRoutes({ router, logger, authc, getLegacyAPI }: RouteDefiniti
       options: { authRequired: false },
     },
     (context, request, response) => {
-      const serverBasePath = getLegacyAPI().serverBasePath || '';
       return response.custom(
         createCustomResourceResponse(
           `
           window.location.replace(
-            '${serverBasePath}/api/security/saml/start?redirectURLFragment=' + encodeURIComponent(window.location.hash)
+            '${basePath.serverBasePath}/api/security/saml/start?redirectURLFragment=' + encodeURIComponent(window.location.hash)
           );
         `,
           'text/javascript'
@@ -120,7 +124,7 @@ function defineSAMLRoutes({ router, logger, authc, getLegacyAPI }: RouteDefiniti
       async (context, request, response) => {
         try {
           if (path === '/api/security/v1/saml') {
-            const serverBasePath = getLegacyAPI().serverBasePath || '';
+            const serverBasePath = basePath.serverBasePath;
             logger.warn(
               `The "${serverBasePath}${path}" URL is deprecated and will stop working in the next major version, please use "${serverBasePath}/api/security/saml/callback" URL instead.`,
               { tags: ['deprecation'] }