From 492f496618d7a87a44d97a5fcecb82947077fef7 Mon Sep 17 00:00:00 2001 From: Sujith Vadakkepat Date: Mon, 10 Feb 2020 19:35:53 -0800 Subject: [PATCH 1/2] Fix headers whitelist to throw missing role error --- index.js | 12 ++++++------ lib/auth/types/AuthType.js | 7 ++++--- lib/auth/types/basicauth/BasicAuth.js | 4 ++-- lib/auth/types/jwt/Jwt.js | 5 ++--- lib/auth/types/openid/OpenId.js | 5 ++--- lib/auth/types/proxycache/ProxyCache.js | 5 ++--- lib/auth/types/saml/Saml.js | 5 ++--- public/apps/customerror/customerror.js | 7 ++++--- 8 files changed, 24 insertions(+), 26 deletions(-) diff --git a/index.js b/index.js index 22b1da0c8..9739ba8e3 100644 --- a/index.js +++ b/index.js @@ -281,7 +281,7 @@ export default function (kibana) { }, async init(server, options) { - const legacyEsConfig = await server.newPlatform.setup.core.elasticsearch.legacy.config$.pipe(first()).toPromise(); + const legacyEsConfig = await server.newPlatform.setup.core.elasticsearch.legacy.config$.pipe(first()).toPromise(); APP_ROOT = ''; API_ROOT = `${APP_ROOT}/api/v1`; const config = server.config(); @@ -370,20 +370,20 @@ export default function (kibana) { if (authType == 'openid') { let OpenId = require('./lib/auth/types/openid/OpenId'); - authClass = new OpenId(pluginRoot, server, this, APP_ROOT, API_ROOT); + authClass = new OpenId(pluginRoot, server, this, APP_ROOT, API_ROOT, legacyEsConfig); } else if (authType == 'basicauth') { let BasicAuth = require('./lib/auth/types/basicauth/BasicAuth'); - authClass = new BasicAuth(pluginRoot, server, this, APP_ROOT, API_ROOT); + authClass = new BasicAuth(pluginRoot, server, this, APP_ROOT, API_ROOT, legacyEsConfig); } else if (authType == 'jwt') { let Jwt = require('./lib/auth/types/jwt/Jwt'); - authClass = new Jwt(pluginRoot, server, this, APP_ROOT, API_ROOT); + authClass = new Jwt(pluginRoot, server, this, APP_ROOT, API_ROOT, legacyEsConfig); this.status.yellow("Security copy JWT params registered."); } else if (authType == 'saml') { let Saml = require('./lib/auth/types/saml/Saml'); - authClass = new Saml(pluginRoot, server, this, APP_ROOT, API_ROOT); + authClass = new Saml(pluginRoot, server, this, APP_ROOT, API_ROOT, legacyEsConfig); } else if (authType == 'proxycache') { let ProxyCache = require('./lib/auth/types/proxycache/ProxyCache'); - authClass = new ProxyCache(pluginRoot, server, this, APP_ROOT, API_ROOT); + authClass = new ProxyCache(pluginRoot, server, this, APP_ROOT, API_ROOT, legacyEsConfig); } if (authClass) { diff --git a/lib/auth/types/AuthType.js b/lib/auth/types/AuthType.js index 78cb9fa3c..033bd496d 100644 --- a/lib/auth/types/AuthType.js +++ b/lib/auth/types/AuthType.js @@ -29,7 +29,7 @@ * permissions and limitations under the License. */ -import { assign } from 'lodash'; +import { assign, union } from 'lodash'; import Boom from 'boom'; import InvalidSessionError from "../errors/invalid_session_error"; import SessionExpiredError from "../errors/session_expired_error"; @@ -37,12 +37,13 @@ import filterAuthHeaders from '../filter_auth_headers'; export default class AuthType { - constructor(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT) { + constructor(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT, esConfig) { this.pluginRoot = pluginRoot; this.server = server; this.kbnServer = kbnServer; this.APP_ROOT = APP_ROOT; this.API_ROOT = API_ROOT; + this.esConfig = esConfig; this.config = server.config(); this.basePath = this.config.get('server.basePath'); @@ -84,7 +85,7 @@ export default class AuthType { * Do not use headers here that have an effect on which user is logged in. * @type {string[]} */ - this.allowedAdditionalAuthHeaders = ['security_impersonate_as']; + this.allowedAdditionalAuthHeaders = union(['security_impersonate_as'], esConfig.requestHeadersWhitelist); /** * This is a workaround for keeping track of what caused hapi-auth-cookie's validateFunc to fail. diff --git a/lib/auth/types/basicauth/BasicAuth.js b/lib/auth/types/basicauth/BasicAuth.js index e5e3ce4b8..89bea0645 100644 --- a/lib/auth/types/basicauth/BasicAuth.js +++ b/lib/auth/types/basicauth/BasicAuth.js @@ -34,8 +34,8 @@ import MissingRoleError from "../../errors/missing_role_error"; export default class BasicAuth extends AuthType { - constructor(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT) { - super(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT); + constructor(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT, esConfig) { + super(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT, esConfig); /** * The authType is saved in the auth cookie for later reference * @type {string} diff --git a/lib/auth/types/jwt/Jwt.js b/lib/auth/types/jwt/Jwt.js index 72f42e2db..73ee85b76 100644 --- a/lib/auth/types/jwt/Jwt.js +++ b/lib/auth/types/jwt/Jwt.js @@ -37,9 +37,8 @@ import MissingRoleError from "../../errors/missing_role_error"; export default class Jwt extends AuthType { - constructor(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT) { - - super(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT); + constructor(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT, esConfig) { + super(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT, esConfig); /** * The authType is saved in the auth cookie for later reference diff --git a/lib/auth/types/openid/OpenId.js b/lib/auth/types/openid/OpenId.js index 362f2ed2b..c5145d0a1 100644 --- a/lib/auth/types/openid/OpenId.js +++ b/lib/auth/types/openid/OpenId.js @@ -39,9 +39,8 @@ const fs = require('fs'); export default class OpenId extends AuthType { - constructor(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT) { - - super(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT); + constructor(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT, esConfig) { + super(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT, esConfig); /** * The authType is saved in the auth cookie for later reference diff --git a/lib/auth/types/proxycache/ProxyCache.js b/lib/auth/types/proxycache/ProxyCache.js index 7fe63ef34..4002df50c 100644 --- a/lib/auth/types/proxycache/ProxyCache.js +++ b/lib/auth/types/proxycache/ProxyCache.js @@ -38,9 +38,8 @@ import {parseLoginEndpoint} from "./parse_login_endpoint"; export default class ProxyCache extends AuthType { - constructor(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT) { - - super(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT); + constructor(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT, esConfig) { + super(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT, esConfig); /** * The authType is saved in the auth cookie for later reference diff --git a/lib/auth/types/saml/Saml.js b/lib/auth/types/saml/Saml.js index e407228fd..1400c8615 100644 --- a/lib/auth/types/saml/Saml.js +++ b/lib/auth/types/saml/Saml.js @@ -36,9 +36,8 @@ import MissingRoleError from "../../errors/missing_role_error"; export default class Saml extends AuthType { - constructor(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT) { - - super(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT); + constructor(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT, esConfig) { + super(pluginRoot, server, kbnServer, APP_ROOT, API_ROOT, esConfig); /** * The authType is saved in the auth cookie for later reference diff --git a/public/apps/customerror/customerror.js b/public/apps/customerror/customerror.js index 57f9698a9..7e6d67f31 100644 --- a/public/apps/customerror/customerror.js +++ b/public/apps/customerror/customerror.js @@ -36,8 +36,9 @@ import 'ui/autoload/styles'; */ import 'plugins/opendistro_security/apps/customerror/customerror.less'; import PageController from './page_controller'; +import template from 'plugins/opendistro_security/apps/customerror/customerror.html'; chrome -.setVisible(false) -.setRootTemplate(require('plugins/opendistro_security/apps/customerror/customerror.html')) -.setRootController('ui', PageController); + .setVisible(false) + .setRootTemplate(template) + .setRootController('ui', PageController); From f41b24c6d409b73a77c8230e323392c6faadad0b Mon Sep 17 00:00:00 2001 From: Sujith Vadakkepat Date: Tue, 11 Feb 2020 17:20:33 -0800 Subject: [PATCH 2/2] Add tests for AuthType --- .babelrc | 7 +++++++ package.json | 2 +- tests/AuthType.test.js | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 .babelrc create mode 100644 tests/AuthType.test.js diff --git a/.babelrc b/.babelrc new file mode 100644 index 000000000..c06dd8dfc --- /dev/null +++ b/.babelrc @@ -0,0 +1,7 @@ +{ + "env": { + "test": { + "presets": [["@babel/preset-env"]] + } + } +} diff --git a/package.json b/package.json index ce8821e21..5949a197d 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "start": "plugin-helpers start", "test:server": "plugin-helpers test:server", "test:browser": "./node_modules/.bin/jest --clearCache && ./node_modules/.bin/jest --config ./tests/jest.config.js", - "test:jest": "../../kibana/node_modules/.bin/jest --config ./test/jest.config.js", + "test:jest": "./node_modules/.bin/jest --config tests/jest.config.js", "build": "plugin-helpers build" }, "dependencies": { diff --git a/tests/AuthType.test.js b/tests/AuthType.test.js new file mode 100644 index 000000000..7e02b1f17 --- /dev/null +++ b/tests/AuthType.test.js @@ -0,0 +1,35 @@ +import AuthType from "../lib/auth/types/AuthType"; + +const mockServer = { + config: () => { + return { + get: () => { + return null; + } + } + } +} + +describe('AuthType tests', () => { + it('should contain only security_impersonate_as when no additional headers are passed', () => { + // act + var authType = new AuthType(null, mockServer, null, null, null, {}); + // assert + expect(authType.allowedAdditionalAuthHeaders).toHaveLength(1); + expect(authType.allowedAdditionalAuthHeaders).toContain("security_impersonate_as"); + }); + + it('should add whitelisted headers when present', () => { + // arrange + const mockEsConfig = { + requestHeadersWhitelist: ["test-header-1", "test-header-2"] + } + // act + var authType = new AuthType(null, mockServer, null, null, null, mockEsConfig); + // assert + expect(authType.allowedAdditionalAuthHeaders).toHaveLength(3); + expect(authType.allowedAdditionalAuthHeaders).toContain("security_impersonate_as"); + expect(authType.allowedAdditionalAuthHeaders).toContain("test-header-1"); + expect(authType.allowedAdditionalAuthHeaders).toContain("test-header-2") + }); +});