Add the tenant into the short URL once the short URL is resolved (#1462…

…) (#1485) * More information added Signed-off-by: leanneeliatra <[email protected]> * More information added Signed-off-by: leanneeliatra <[email protected]> * fixed linting errors Signed-off-by: leanneeliatra <[email protected]> * Removing Prerequisite Checks Workflow (#1456) Signed-off-by: Ryan Liang <[email protected]> Signed-off-by: leanneeliatra <[email protected]> * Removing Prerequisite Checks Workflow (#1456) Signed-off-by: Ryan Liang <[email protected]> Signed-off-by: leanneeliatra <[email protected]> * Removing Prerequisite Checks Workflow (#1456) Signed-off-by: Ryan Liang <[email protected]> Signed-off-by: Ryan Liang <[email protected]> Signed-off-by: leanneeliatra <[email protected]> * Extracting function to tenant_resolver and adding more appropriate comments. Signed-off-by: leanneeliatra <[email protected]> * lint errors fixed Signed-off-by: leanneeliatra <[email protected]> * Use version from package.json for integration tests (#1463) * Use version from package.json for integration tests Signed-off-by: Craig Perkins <[email protected]> Signed-off-by: leanneeliatra <[email protected]> * Adds 2.8 release notes (#1464) Signed-off-by: Darshit Chanpura <[email protected]> Co-authored-by: Ryan Liang <[email protected]> Signed-off-by: leanneeliatra <[email protected]> * Cleaning up comments Signed-off-by: Leanne Lacey-Byrne <[email protected]> Signed-off-by: leanneeliatra <[email protected]> * linting issues resolved Signed-off-by: leanneeliatra <[email protected]> * Removing Prerequisite Checks Workflow (#1456) Signed-off-by: Ryan Liang <[email protected]> Signed-off-by: leanneeliatra <[email protected]> * Removing Prerequisite Checks Workflow (#1456) Signed-off-by: Ryan Liang <[email protected]> Signed-off-by: Ryan Liang <[email protected]> Signed-off-by: leanneeliatra <[email protected]> * Update server/multitenancy/tenant_resolver.ts Co-authored-by: Darshit Chanpura <[email protected]> Signed-off-by: leanneeliatra <[email protected]> Signed-off-by: leanneeliatra <[email protected]> * comments addressed & linting amended Signed-off-by: leanneeliatra <[email protected]> * integration test fix following rebase Signed-off-by: leanneeliatra <[email protected]> Signed-off-by: leanneeliatra <[email protected]> --------- Signed-off-by: leanneeliatra <[email protected]> Signed-off-by: Ryan Liang <[email protected]> Signed-off-by: Ryan Liang <[email protected]> Signed-off-by: Craig Perkins <[email protected]> Signed-off-by: Darshit Chanpura <[email protected]> Signed-off-by: Leanne Lacey-Byrne <[email protected]> Signed-off-by: leanneeliatra <[email protected]> Co-authored-by: Ryan Liang <[email protected]> Co-authored-by: Craig Perkins <[email protected]> Co-authored-by: Darshit Chanpura <[email protected]> (cherry picked from commit e9f9576) Co-authored-by: leanneeliatra <[email protected]>
opensearch-project · Jun 23, 2023 · 37cddf3 · 37cddf3
1 parent a3b0761
commit 37cddf3
Show file tree

Hide file tree

Showing 3 changed files with 129 additions and 0 deletions.
diff --git a/server/multitenancy/tenant_resolver.test.ts b/server/multitenancy/tenant_resolver.test.ts
@@ -0,0 +1,88 @@
+/*
+ *   Copyright OpenSearch Contributors
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License").
+ *   You may not use this file except in compliance with the License.
+ *   A copy of the License is located at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   or in the "license" file accompanying this file. This file is distributed
+ *   on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *   express or implied. See the License for the specific language governing
+ *   permissions and limitations under the License.
+ */
+
+import { httpServerMock } from '../../../../src/core/server/mocks';
+import { OpenSearchDashboardsRequest } from '../../../../src/core/server';
+import { addTenantParameterToResolvedShortLink } from './tenant_resolver';
+import { Request, ResponseObject } from '@hapi/hapi';
+
+describe('Preserve the tenant parameter in short urls', () => {
+  it(`adds the tenant as a query parameter for goto short links`, async () => {
+    const resolvedUrl = '/url/resolved';
+    const rawRequest = httpServerMock.createRawRequest({
+      url: {
+        pathname: '/goto/123',
+      },
+      headers: {
+        securitytenant: 'dummy_tenant',
+      },
+      response: {
+        headers: {
+          location: resolvedUrl,
+        },
+      },
+    }) as Request;
+
+    const osRequest = OpenSearchDashboardsRequest.from(rawRequest);
+    addTenantParameterToResolvedShortLink(osRequest);
+
+    expect((rawRequest.response as ResponseObject).headers.location).toEqual(
+      resolvedUrl + '?security_tenant=dummy_tenant'
+    );
+  });
+
+  it(`ignores links not starting with /goto`, async () => {
+    const resolvedUrl = '/url/resolved';
+    const rawRequest = httpServerMock.createRawRequest({
+      url: {
+        pathname: '/dontgoto/123',
+      },
+      headers: {
+        securitytenant: 'dummy_tenant',
+      },
+      response: {
+        headers: {
+          location: resolvedUrl,
+        },
+      },
+    }) as Request;
+
+    const osRequest = OpenSearchDashboardsRequest.from(rawRequest);
+    addTenantParameterToResolvedShortLink(osRequest);
+
+    expect((rawRequest.response as ResponseObject).headers.location).toEqual(resolvedUrl);
+  });
+
+  it(`checks that a redirect location is present before applying the query parameter`, async () => {
+    const rawRequest = httpServerMock.createRawRequest({
+      url: {
+        pathname: '/goto/123',
+      },
+      headers: {
+        securitytenant: 'dummy_tenant',
+      },
+      response: {
+        headers: {
+          someotherheader: 'abc',
+        },
+      },
+    }) as Request;
+
+    const osRequest = OpenSearchDashboardsRequest.from(rawRequest);
+    addTenantParameterToResolvedShortLink(osRequest);
+
+    expect((rawRequest.response as ResponseObject).headers.location).toBeFalsy();
+  });
+});
diff --git a/server/multitenancy/tenant_resolver.ts b/server/multitenancy/tenant_resolver.ts
@@ -14,9 +14,14 @@
  */
 
 import { isEmpty, findKey, cloneDeep } from 'lodash';
+import { OpenSearchDashboardsRequest } from 'opensearch-dashboards/server';
+import { ResponseObject } from '@hapi/hapi';
 import { SecuritySessionCookie } from '../session/security_cookie';
 import { SecurityPluginConfigType } from '..';
 import { GLOBAL_TENANT_SYMBOL, PRIVATE_TENANT_SYMBOL, globalTenantName } from '../../common';
+import { modifyUrl } from '../../../../packages/osd-std';
+import { ensureRawRequest } from '../../../../src/core/server/http/router';
+import { GOTO_PREFIX } from '../../../../src/plugins/share/common/short_url_routes';
 
 export const PRIVATE_TENANTS: string[] = [PRIVATE_TENANT_SYMBOL, 'private'];
 export const GLOBAL_TENANTS: string[] = ['global', GLOBAL_TENANT_SYMBOL, 'Global'];
@@ -194,3 +199,29 @@ export function resolve(
 export function isValidTenant(tenant: string | undefined | null): boolean {
   return tenant !== undefined && tenant !== null;
 }
+
+/**
+ * If multitenancy is enabled & the URL entered starts with /goto,
+ * We will modify the rawResponse to add a new parameter to the URL, the security_tenant (or value for tenant when in multitenancy)
+ * With the security_tenant added, the resolved short URL now contains the security_tenant information (so the short URL retains the tenant information).
+ **/
+
+export function addTenantParameterToResolvedShortLink(request: OpenSearchDashboardsRequest) {
+  if (request.url.pathname.startsWith(`${GOTO_PREFIX}/`)) {
+    const rawRequest = ensureRawRequest(request);
+    const rawResponse = rawRequest.response as ResponseObject;
+
+    // Make sure the request really should redirect
+    if (rawResponse.headers.location) {
+      const modifiedUrl = modifyUrl(rawResponse.headers.location as string, (parts) => {
+        if (parts.query.security_tenant === undefined) {
+          parts.query.security_tenant = request.headers.securitytenant as string;
+        }
+        // Mutating the headers toolkit.next({headers: ...}) logs a warning about headers being overwritten
+      });
+      rawResponse.headers.location = modifiedUrl;
+    }
+  }
+
+  return request;
+}
diff --git a/server/plugin.ts b/server/plugin.ts
@@ -15,6 +15,7 @@
 
 import { first } from 'rxjs/operators';
 import { Observable } from 'rxjs';
+import { ResponseObject } from '@hapi/hapi';
 import {
   PluginInitializerContext,
   CoreSetup,
@@ -47,6 +48,7 @@ import { setupMultitenantRoutes } from './multitenancy/routes';
 import { defineAuthTypeRoutes } from './routes/auth_type_routes';
 import { createMigrationOpenSearchClient } from '../../../src/core/server/saved_objects/migrations/core';
 import { SecuritySavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper';
+import { addTenantParameterToResolvedShortLink } from './multitenancy/tenant_resolver';
 
 export interface SecurityPluginRequestContext {
   logger: Logger;
@@ -125,6 +127,14 @@ export class SecurityPlugin implements Plugin<SecurityPluginSetup, SecurityPlugi
     );
     core.http.registerAuth(auth.authHandler);
 
+    /* Here we check if multitenancy is enabled to ensure if it is, we insert the tenant info (security_tenant) into the resolved, short URL so the page can correctly load with the right tenant information [Fix for issue 1203](https://github.com/opensearch-project/security-dashboards-plugin/issues/1203 */
+    if (config.multitenancy?.enabled) {
+      core.http.registerOnPreResponse((request, preResponse, toolkit) => {
+        addTenantParameterToResolvedShortLink(request);
+        return toolkit.next();
+      });
+    }
+
     // Register server side APIs
     defineRoutes(router);
     defineAuthTypeRoutes(router, config);