Merge pull request #139 from sujithvm/opendistro-1.4

Remove session and request header validation check
opensearch-project · Feb 14, 2020 · fd88830 · fd88830
2 parents 67326ba + b2e6491
commit fd88830
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 16 deletions.
diff --git a/lib/auth/types/AuthType.js b/lib/auth/types/AuthType.js
@@ -80,12 +80,18 @@ export default class AuthType {
          */
         this.authHeaderName = 'authorization';
 
+        /**
+         * Elasticsearch whitelisted headers.
+         * @type {string[]}
+         */
+        this.requestHeadersWhitelist = this.esConfig.requestHeadersWhitelist;
+
         /**
          * Additional headers that should be passed as part as the authentication.
          * Do not use headers here that have an effect on which user is logged in.
          * @type {string[]}
          */
-        this.allowedAdditionalAuthHeaders = union(['security_impersonate_as'], esConfig.requestHeadersWhitelist);
+        this.allowedAdditionalAuthHeaders = ['security_impersonate_as'];
 
          /**
          * This is a workaround for keeping track of what caused hapi-auth-cookie's validateFunc to fail.
@@ -112,7 +118,7 @@ export default class AuthType {
             options: {
                 authType: this.type,
                 authHeaderName: this.authHeaderName,
-                allowedAdditionalAuthHeaders: this.allowedAdditionalAuthHeaders,
+                allowedHeaders: union(this.requestHeadersWhitelist, this.allowedAdditionalAuthHeaders),
                 authenticateFunction: this.authenticate.bind(this),
                 validateAvailableTenants: this.validateAvailableTenants
             }

diff --git a/lib/session/sessionPlugin.js b/lib/session/sessionPlugin.js
@@ -16,7 +16,7 @@ let internals = {};
 internals.config = Joi.object({
     authType: Joi.string().allow(null),
     authHeaderName: Joi.string(),
-    allowedAdditionalAuthHeaders: Joi.array().default([]),
+    allowedHeaders: Joi.array().default([]),
     authenticateFunction: Joi.func(),
     validateAvailableTenants: Joi.boolean().default(true),
     validateAvailableRoles: Joi.boolean().default(true)
@@ -42,7 +42,7 @@ const register = function (server, options) {
              */
             authenticate: async function(credentials, options = {}) {
                 try {
-                    const additionalAuthHeaders = filterAuthHeaders(request.headers, settings.allowedAdditionalAuthHeaders);
+                    const additionalAuthHeaders = filterAuthHeaders(request.headers, settings.allowedHeaders);
                     // authResponse is an object with .session and .user
                     const authResponse = await settings.authenticateFunction(credentials, options, additionalAuthHeaders);
 
@@ -58,7 +58,7 @@ const register = function (server, options) {
 
             authenticateWithHeaders: async function(headers, credentials = {}, options = {}) {
                 try {
-                    const additionalAuthHeaders = filterAuthHeaders(request.headers, settings.allowedAdditionalAuthHeaders);
+                    const additionalAuthHeaders = filterAuthHeaders(request.headers, settings.allowedHeaders);
                     let user = await server.plugins.opendistro_security.getSecurityBackend().authenticateWithHeaders(headers, credentials, additionalAuthHeaders);
                     let session = {
                         username: user.username,

diff --git a/tests/AuthType.test.js b/tests/AuthType.test.js
@@ -1,35 +1,43 @@
 import AuthType  from "../lib/auth/types/AuthType";
 
-const mockServer = {
-    config: () => { 
+class MockServer {
+    config() {
         return {
             get: () => {
                 return null;
             }
         }
-    } 
+    }
+    register(args) {
+        this.registerArgs = args;
+    }
 }
 
 describe('AuthType tests', () => {
     it('should contain only security_impersonate_as when no additional headers are passed', () => {
+        // arrange
+        var mockServer = new MockServer();
+        var authType = new AuthType(() => {}, mockServer, null, null, null, {});
         // act
-        var authType = new AuthType(null, mockServer, null, null, null, {});
+        authType.setupStorage();
         // assert
-        expect(authType.allowedAdditionalAuthHeaders).toHaveLength(1);
-        expect(authType.allowedAdditionalAuthHeaders).toContain("security_impersonate_as");
+        expect(mockServer.registerArgs.options.allowedHeaders).toHaveLength(1);
+        expect(mockServer.registerArgs.options.allowedHeaders).toContain("security_impersonate_as");
     });
 
     it('should add whitelisted headers when present', () => {
         // arrange
+        var mockServer = new MockServer();
         const mockEsConfig = {
             requestHeadersWhitelist: ["test-header-1", "test-header-2"]
         }
+        var authType = new AuthType(() => {}, mockServer, null, null, null, mockEsConfig);
         // act
-        var authType = new AuthType(null, mockServer, null, null, null, mockEsConfig);
+        authType.setupStorage();
         // 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")
+        expect(mockServer.registerArgs.options.allowedHeaders).toHaveLength(3);
+        expect(mockServer.registerArgs.options.allowedHeaders).toContain("security_impersonate_as");
+        expect(mockServer.registerArgs.options.allowedHeaders).toContain("test-header-1");
+        expect(mockServer.registerArgs.options.allowedHeaders).toContain("test-header-2");
     });
 });