Even more changes.

elastic · Jun 12, 2020 · 3ab2b0e · 3ab2b0e
1 parent 99272f9
commit 3ab2b0e
Show file tree

Hide file tree

Showing 20 changed files with 375 additions and 324 deletions.
diff --git a/x-pack/plugins/security/public/authentication/authentication_service.ts b/x-pack/plugins/security/public/authentication/authentication_service.ts
@@ -13,6 +13,7 @@ import { loginApp } from './login';
 import { logoutApp } from './logout';
 import { loggedOutApp } from './logged_out';
 import { overwrittenSessionApp } from './overwritten_session';
+import { captureURLApp } from './capture_url';
 
 interface SetupParams {
   application: ApplicationSetup;
@@ -48,6 +49,7 @@ export class AuthenticationService {
         .apiKeysEnabled;
 
     accessAgreementApp.create({ application, getStartServices });
+    captureURLApp.create({ application, http });
     loginApp.create({ application, config, getStartServices, http });
     logoutApp.create({ application, http });
     loggedOutApp.create({ application, getStartServices, http });

diff --git a/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.ts b/x-pack/plugins/security/public/authentication/capture_url/capture_url_app.ts
@@ -0,0 +1,51 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { CoreSetup, HttpSetup } from 'src/core/public';
+import { SessionInfo } from '../../../common/types';
+
+interface CreateDeps {
+  application: CoreSetup['application'];
+  http: HttpSetup;
+}
+
+export const captureURLApp = Object.freeze({
+  id: 'security_capture_url',
+  create({ application, http }: CreateDeps) {
+    http.anonymousPaths.register('/internal/security/capture-url');
+    application.register({
+      id: this.id,
+      title: '',
+      chromeless: true,
+      appRoute: '/internal/security/capture-url',
+      async mount() {
+        try {
+          const { provider } = await http.get<SessionInfo>('/internal/security/session');
+          if (!provider) {
+            throw new Error('Cannot retrieve current provider.');
+          }
+
+          const { location } = await http.post<{ location: string }>(
+            '/internal/security/login_with',
+            {
+              body: JSON.stringify({
+                providerType: provider.type,
+                providerName: provider.name,
+                currentURL: window.location.href,
+              }),
+            }
+          );
+
+          window.location.href = location;
+        } catch (err) {
+          console.error(err);
+        }
+
+        return () => {};
+      },
+    });
+  },
+});
diff --git a/x-pack/plugins/security/public/authentication/capture_url/index.ts b/x-pack/plugins/security/public/authentication/capture_url/index.ts
@@ -0,0 +1,7 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+export { captureURLApp } from './capture_url_app';
diff --git a/x-pack/plugins/security/server/authentication/providers/oidc.ts b/x-pack/plugins/security/server/authentication/providers/oidc.ts
@@ -32,7 +32,7 @@ export enum OIDCLogin {
  * Describes the parameters that are required by the provider to process the initial login request.
  */
 export type ProviderLoginAttempt =
-  | { type: OIDCLogin.LoginInitiatedByUser; redirectURLPath: string }
+  | { type: OIDCLogin.LoginInitiatedByUser; redirectURL: string }
   | {
       type: OIDCLogin.LoginWithImplicitFlow | OIDCLogin.LoginWithAuthorizationCodeFlow;
       authenticationResponseURI: string;
@@ -58,7 +58,7 @@ interface ProviderState extends Partial<TokenPair> {
   /**
    * URL to redirect user to after successful OpenID Connect handshake.
    */
-  nextURL?: string;
+  redirectURL?: string;
 
   /**
    * The name of the OpenID Connect realm that was used to establish session.
@@ -143,11 +143,7 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider {
 
     if (attempt.type === OIDCLogin.LoginInitiatedByUser) {
       this.logger.debug(`Login has been initiated by a user.`);
-      return this.initiateOIDCAuthentication(
-        request,
-        { realm: this.realm },
-        attempt.redirectURLPath
-      );
+      return this.initiateOIDCAuthentication(request, { realm: this.realm }, attempt.redirectURL);
     }
 
     if (attempt.type === OIDCLogin.LoginWithImplicitFlow) {
@@ -200,7 +196,7 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider {
     // We might already have a state and nonce generated by Elasticsearch (from an unfinished authentication in
     // another tab)
     return authenticationResult.notHandled() && canStartNewSession(request)
-      ? await this.initiateOIDCAuthentication(request, { realm: this.realm })
+      ? await this.captureRedirectURL(request)
       : authenticationResult;
   }
 
@@ -231,8 +227,11 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider {
     // If it is an authentication response and the users' session state doesn't contain all the necessary information,
     // then something unexpected happened and we should fail because Elasticsearch won't be able to validate the
     // response.
-    const { nonce: stateNonce = '', state: stateOIDCState = '', nextURL: stateRedirectURL = '' } =
-      sessionState || {};
+    const {
+      nonce: stateNonce = '',
+      state: stateOIDCState = '',
+      redirectURL: stateRedirectURL = '',
+    } = sessionState || {};
     if (!stateNonce || !stateOIDCState || !stateRedirectURL) {
       const message =
         'Response session state does not have corresponding state or nonce parameters or redirect URL.';
@@ -272,13 +271,12 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider {
    *
    * @param request Request instance.
    * @param params OIDC authentication parameters.
-   * @param [redirectURLPath] Optional URL user is supposed to be redirected to after successful
-   * login. If not provided the URL of the specified request is used.
+   * @param redirectURL URL user is supposed to be redirected to after successful login.
    */
   private async initiateOIDCAuthentication(
     request: KibanaRequest,
     params: { realm: string } | { iss: string; login_hint?: string },
-    redirectURLPath = `${this.options.basePath.get(request)}${request.url.path}`
+    redirectURL: string
   ) {
     this.logger.debug('Trying to initiate OpenID Connect authentication.');
 
@@ -295,7 +293,7 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider {
       return AuthenticationResult.redirectTo(
         redirect,
         // Store the state and nonce parameters in the session state of the user
-        { state: { state, nonce, nextURL: redirectURLPath, realm: this.realm } }
+        { state: { state, nonce, redirectURL, realm: this.realm } }
       );
     } catch (err) {
       this.logger.debug(`Failed to initiate OpenID Connect authentication: ${err.message}`);
@@ -367,7 +365,7 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider {
         this.logger.debug(
           'Both elasticsearch access and refresh tokens are expired. Re-initiating OpenID Connect authentication.'
         );
-        return this.initiateOIDCAuthentication(request, { realm: this.realm });
+        return this.captureRedirectURL(request);
       }
 
       return AuthenticationResult.failed(
@@ -449,4 +447,19 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider {
   public getHTTPAuthenticationScheme() {
     return 'bearer';
   }
+
+  /**
+   * Tries to capture full redirect URL (both path and fragment) and initiate OIDC handshake.
+   * @param request Request instance.
+   */
+  private captureRedirectURL(request: KibanaRequest) {
+    return AuthenticationResult.redirectTo(
+      `${
+        this.options.basePath.serverBasePath
+      }/internal/security/capture-url?next=${encodeURIComponent(
+        `${this.options.basePath.get(request)}${request.url.path}`
+      )}`,
+      { state: { realm: this.realm } }
+    );
+  }
 }
diff --git a/x-pack/plugins/security/server/authentication/providers/saml.ts b/x-pack/plugins/security/server/authentication/providers/saml.ts
@@ -5,7 +5,6 @@
  */
 
 import Boom from 'boom';
-import { ByteSizeValue } from '@kbn/config-schema';
 import { KibanaRequest } from '../../../../../../src/core/server';
 import { AuthenticationResult } from '../authentication_result';
 import { DeauthenticationResult } from '../deauthentication_result';
@@ -27,6 +26,7 @@ interface ProviderState extends Partial<TokenPair> {
    * Unique identifier of the SAML request initiated the handshake.
    */
   requestId?: string;
+
   /**
    * Stores path component of the URL only or in a combination with URL fragment that was used to
    * initiate SAML handshake and where we should redirect user after successful authentication.
@@ -58,7 +58,7 @@ export enum SAMLLogin {
  * Describes the parameters that are required by the provider to process the initial login request.
  */
 type ProviderLoginAttempt =
-  | { type: SAMLLogin.LoginInitiatedByUser; redirectURLPath?: string; redirectURLFragment?: string }
+  | { type: SAMLLogin.LoginInitiatedByUser; redirectURL: string }
   | { type: SAMLLogin.LoginWithSAMLResponse; samlResponse: string };
 
 /**
@@ -93,27 +93,17 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider {
    */
   private readonly realm: string;
 
-  /**
-   * Maximum size of the URL we store in the session during SAML handshake.
-   */
-  private readonly maxRedirectURLSize: ByteSizeValue;
-
   constructor(
     protected readonly options: Readonly<AuthenticationProviderOptions>,
-    samlOptions?: Readonly<{ realm?: string; maxRedirectURLSize?: ByteSizeValue }>
+    samlOptions?: Readonly<{ realm?: string }>
   ) {
     super(options);
 
     if (!samlOptions || !samlOptions.realm) {
       throw new Error('Realm name must be specified');
     }
 
-    if (!samlOptions.maxRedirectURLSize) {
-      throw new Error('Maximum redirect URL size must be specified');
-    }
-
     this.realm = samlOptions.realm;
-    this.maxRedirectURLSize = samlOptions.maxRedirectURLSize;
   }
 
   /**
@@ -138,14 +128,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider {
     }
 
     if (attempt.type === SAMLLogin.LoginInitiatedByUser) {
-      const redirectURLPath = attempt.redirectURLPath || state?.redirectURL;
-      if (!redirectURLPath) {
-        const message = 'State or login attempt does not include URL path to redirect to.';
-        this.logger.debug(message);
-        return AuthenticationResult.failed(Boom.badRequest(message));
-      }
-
-      return this.captureRedirectURL(request, redirectURLPath, attempt.redirectURLFragment);
+      return this.authenticateViaHandshake(request, attempt.redirectURL);
     }
 
     const { samlResponse } = attempt;
@@ -579,52 +562,15 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider {
   /**
    * Tries to capture full redirect URL (both path and fragment) and initiate SAML handshake.
    * @param request Request instance.
-   * @param [redirectURLPath] Optional URL path user is supposed to be redirected to after successful
-   * login. If not provided the URL path of the specified request is used.
-   * @param [redirectURLFragment] Optional URL fragment of the URL user is supposed to be redirected
-   * to after successful login. If not provided user will be redirected to the client-side page that
-   * will grab it and redirect user back to Kibana to initiate SAML handshake.
    */
-  private captureRedirectURL(
-    request: KibanaRequest,
-    redirectURLPath = `${this.options.basePath.get(request)}${request.url.path}`,
-    redirectURLFragment?: string
-  ) {
-    // If the size of the path already exceeds the maximum allowed size of the URL to store in the
-    // session there is no reason to try to capture URL fragment and we start handshake immediately.
-    // In this case user will be redirected to the Kibana home/root after successful login.
-    let redirectURLSize = new ByteSizeValue(Buffer.byteLength(redirectURLPath));
-    if (this.maxRedirectURLSize.isLessThan(redirectURLSize)) {
-      this.logger.warn(
-        `Max URL path size should not exceed ${this.maxRedirectURLSize.toString()} but it was ${redirectURLSize.toString()}. URL is not captured.`
-      );
-      return this.authenticateViaHandshake(request, '');
-    }
-
-    // If URL fragment wasn't specified at all, let's try to capture it.
-    if (redirectURLFragment === undefined) {
-      return AuthenticationResult.redirectTo(
-        `${this.options.basePath.serverBasePath}/internal/security/saml/capture-url-fragment`,
-        { state: { redirectURL: redirectURLPath, realm: this.realm } }
-      );
-    }
-
-    if (redirectURLFragment.length > 0 && !redirectURLFragment.startsWith('#')) {
-      this.logger.warn('Redirect URL fragment does not start with `#`.');
-      redirectURLFragment = `#${redirectURLFragment}`;
-    }
-
-    let redirectURL = `${redirectURLPath}${redirectURLFragment}`;
-    redirectURLSize = new ByteSizeValue(Buffer.byteLength(redirectURL));
-    if (this.maxRedirectURLSize.isLessThan(redirectURLSize)) {
-      this.logger.warn(
-        `Max URL size should not exceed ${this.maxRedirectURLSize.toString()} but it was ${redirectURLSize.toString()}. Only URL path is captured.`
-      );
-      redirectURL = redirectURLPath;
-    } else {
-      this.logger.debug('Captured redirect URL.');
-    }
-
-    return this.authenticateViaHandshake(request, redirectURL);
+  private captureRedirectURL(request: KibanaRequest) {
+    return AuthenticationResult.redirectTo(
+      `${
+        this.options.basePath.serverBasePath
+      }/internal/security/capture-url?next=${encodeURIComponent(
+        `${this.options.basePath.get(request)}${request.url.path}`
+      )}`,
+      { state: { realm: this.realm } }
+    );
   }
 }
diff --git a/x-pack/plugins/security/server/routes/authentication/common.ts b/x-pack/plugins/security/server/routes/authentication/common.ts
@@ -84,18 +84,12 @@ export function defineCommonRoutes({
   }
 
   function getLoginAttemptForProviderType(providerType: string, redirectURL: string) {
-    const [redirectURLPath] = redirectURL.split('#');
-    const redirectURLFragment =
-      redirectURL.length > redirectURLPath.length
-        ? redirectURL.substring(redirectURLPath.length)
-        : '';
-
     if (providerType === SAMLAuthenticationProvider.type) {
-      return { type: SAMLLogin.LoginInitiatedByUser, redirectURLPath, redirectURLFragment };
+      return { type: SAMLLogin.LoginInitiatedByUser, redirectURL };
     }
 
     if (providerType === OIDCAuthenticationProvider.type) {
-      return { type: OIDCLogin.LoginInitiatedByUser, redirectURLPath };
+      return { type: OIDCLogin.LoginInitiatedByUser, redirectURL };
     }
 
     return undefined;

diff --git a/x-pack/plugins/security/server/routes/authentication/index.ts b/x-pack/plugins/security/server/routes/authentication/index.ts
@@ -4,15 +4,13 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { defineSessionRoutes } from './session';
 import { defineSAMLRoutes } from './saml';
 import { defineBasicRoutes } from './basic';
 import { defineCommonRoutes } from './common';
 import { defineOIDCRoutes } from './oidc';
 import { RouteDefinitionParams } from '..';
 
 export function defineAuthenticationRoutes(params: RouteDefinitionParams) {
-  defineSessionRoutes(params);
   defineCommonRoutes(params);
 
   if (params.authc.isProviderTypeEnabled('basic') || params.authc.isProviderTypeEnabled('token')) {