Merge branch 'main' into #2704

opensearch-project · Oct 10, 2023 · 0570d1d · 0570d1d
2 parents bc100c0 + d575e00
commit 0570d1d
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 8 deletions.
diff --git a/.github/workflows/cypress-test-tenancy-disabled.yml b/.github/workflows/cypress-test-tenancy-disabled.yml
@@ -50,6 +50,7 @@ jobs:
           opensearch-version: ${{ env.OPENSEARCH_VERSION }}
           plugin-name: ${{ env.PLUGIN_NAME }}
           setup-script-name: setup
+          admin-password: admin
 
       - name: Run Dashboard with Security Dashboards Plugin
         uses: ./.github/actions/install-dashboards
@@ -73,4 +74,4 @@ jobs:
           git clone https://github.com/opensearch-project/opensearch-dashboards-functional-test.git
           cd opensearch-dashboards-functional-test
           npm install cypress --save-dev
-          yarn cypress:run-with-security --browser chrome --spec "cypress/integration/plugins/security-dashboards-plugin/inaccessible_tenancy_features.js"
+          yarn cypress:run-with-security --browser firefox --spec "cypress/integration/plugins/security-dashboards-plugin/inaccessible_tenancy_features.js"
diff --git a/.github/workflows/cypress-test.yml b/.github/workflows/cypress-test.yml
@@ -20,7 +20,7 @@ jobs:
         os: [ ubuntu-latest , windows-latest ]
     runs-on: ${{ matrix.os }}
 
-    steps:
+    steps:       
       - name: Set up JDK
         uses: actions/setup-java@v1
         with:
@@ -50,6 +50,7 @@ jobs:
           opensearch-version: ${{ env.OPENSEARCH_VERSION }}
           plugin-name: ${{ env.PLUGIN_NAME }}
           setup-script-name: setup
+          admin-password: admin
 
       - name: Run Dashboard with Security Dashboards Plugin
         uses: ./.github/actions/install-dashboards
@@ -75,6 +76,6 @@ jobs:
           git clone https://github.com/opensearch-project/opensearch-dashboards-functional-test.git
           cd opensearch-dashboards-functional-test
           npm install cypress --save-dev
-          yarn cypress:run-with-security-and-aggregation-view --browser chrome --spec "cypress/integration/plugins/security-dashboards-plugin/aggregation_view.js"
-          yarn cypress:run-with-security --browser chrome --spec "cypress/integration/plugins/security-dashboards-plugin/multi_tenancy.js"
-          yarn cypress:run-with-security --browser chrome --spec "cypress/integration/plugins/security-dashboards-plugin/default_tenant.js"
+          yarn cypress:run-with-security-and-aggregation-view --browser firefox --spec "cypress/integration/plugins/security-dashboards-plugin/aggregation_view.js"
+          yarn cypress:run-with-security --browser firefox --spec "cypress/integration/plugins/security-dashboards-plugin/multi_tenancy.js"
+          yarn cypress:run-with-security --browser firefox --spec "cypress/integration/plugins/security-dashboards-plugin/default_tenant.js"
diff --git a/.github/workflows/integration-test.yml b/.github/workflows/integration-test.yml
@@ -19,7 +19,7 @@ jobs:
     steps:
       - name: Checkout Branch
         uses: actions/checkout@v3
-
+        
       - name: Set up JDK
         uses: actions/setup-java@v1
         with:
@@ -64,6 +64,7 @@ jobs:
           opensearch-version: ${{ env.OPENSEARCH_VERSION }}
           plugin-name: ${{ env.PLUGIN_NAME }}
           setup-script-name: setup
+          admin-password: admin
 
       - id: install-dashboards
         uses: ./.github/actions/install-dashboards

diff --git a/server/session/cookie_splitter.test.ts b/server/session/cookie_splitter.test.ts
@@ -14,6 +14,7 @@
  */
 import { Request as HapiRequest, ResponseObject as HapiResponseObject } from '@hapi/hapi';
 import { httpServerMock } from '../../../../src/core/server/http/http_server.mocks';
+import { merge } from 'lodash';
 import {
   clearSplitCookies,
   getExtraAuthStorageValue,
@@ -171,4 +172,68 @@ describe('Test extra auth storage', () => {
 
     expect(unsplitValue).toEqual('abcdefghi');
   });
+
+  test('should check for cookie values updated in the same request', async () => {
+    const cookiePrefix = 'testcookie';
+    const additionalCookies = 5;
+
+    const mockRequest = httpServerMock.createRawRequest();
+
+    const extendedMockRequest = merge(mockRequest, {
+      _states: {
+        [cookiePrefix + '1']: {
+          name: cookiePrefix + '1',
+          value: 'abc',
+        },
+        [cookiePrefix + '2']: {
+          name: cookiePrefix + '2',
+          value: 'def',
+        },
+        [cookiePrefix + '3']: {
+          name: cookiePrefix + '3',
+          value: 'ghi',
+        },
+      },
+    }) as HapiRequest;
+
+    const osRequest = OpenSearchDashboardsRequest.from(extendedMockRequest);
+    const unsplitValue = unsplitCookiesIntoValue(osRequest, cookiePrefix, additionalCookies);
+
+    expect(unsplitValue).toEqual('abcdefghi');
+  });
+
+  test('should not mix cookie values updated in the same request with previous cookie values', async () => {
+    const cookiePrefix = 'testcookie';
+    const additionalCookies = 5;
+
+    const mockRequest = httpServerMock.createRawRequest({
+      state: {
+        [cookiePrefix + '1']: 'abc',
+        [cookiePrefix + '2']: 'def',
+        [cookiePrefix + '3']: 'ghi',
+      },
+    });
+
+    const extendedMockRequest = merge(mockRequest, {
+      _states: {
+        [cookiePrefix + '1']: {
+          name: cookiePrefix + '1',
+          value: 'jkl',
+        },
+        [cookiePrefix + '2']: {
+          name: cookiePrefix + '2',
+          value: 'mno',
+        },
+        [cookiePrefix + '3']: {
+          name: cookiePrefix + '3',
+          value: 'pqr',
+        },
+      },
+    }) as HapiRequest;
+
+    const osRequest = OpenSearchDashboardsRequest.from(extendedMockRequest);
+    const unsplitValue = unsplitCookiesIntoValue(osRequest, cookiePrefix, additionalCookies);
+
+    expect(unsplitValue).toEqual('jklmnopqr');
+  });
 });
diff --git a/server/session/cookie_splitter.ts b/server/session/cookie_splitter.ts
@@ -29,6 +29,15 @@ export interface ExtraAuthStorageOptions {
 
 type CookieAuthWithResponseObject = HapiRequest['cookieAuth'] & { h: HapiResponseObject };
 
+interface HapiStates {
+  [cookieName: string]: {
+    name: string;
+    value: string;
+  };
+}
+
+export type HapiRequestWithStates = HapiRequest & { _states: HapiStates };
+
 export function getExtraAuthStorageValue(
   request: OpenSearchDashboardsRequest,
   options: ExtraAuthStorageOptions
@@ -134,12 +143,23 @@ export function unsplitCookiesIntoValue(
   cookiePrefix: string,
   additionalCookies: number
 ): string {
-  const rawRequest: HapiRequest = ensureRawRequest(request);
+  const rawRequest: HapiRequestWithStates = ensureRawRequest(request) as HapiRequestWithStates;
   let fullCookieValue = '';
 
+  // We don't want to mix and match between _states and .state.
+  // If we find the first additional cookie in _states, we
+  // use _states for all subsequent additional cookies
+  const requestHasNewerCookieState = rawRequest._states && rawRequest._states[cookiePrefix + 1];
+
   for (let i = 1; i <= additionalCookies; i++) {
     const cookieName = cookiePrefix + i;
-    if (rawRequest.state[cookieName]) {
+    if (
+      requestHasNewerCookieState &&
+      rawRequest._states[cookieName] &&
+      rawRequest._states[cookieName].value
+    ) {
+      fullCookieValue = fullCookieValue + rawRequest._states[cookieName].value;
+    } else if (!requestHasNewerCookieState && rawRequest.state[cookieName]) {
       fullCookieValue = fullCookieValue + rawRequest.state[cookieName];
     }
   }