Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 1.3] Add the tenant into the short URL once the short URL is resolved (#1462) #1516

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ export const ERROR_MISSING_ROLE_PATH = '/missing-role';

export const MAX_INTEGER = 2147483647;

export const GLOBAL_TENANT_SYMBOL = '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file has some of the changes from #1184 can we include all of them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've only included the ones that were required. We can backport #1184 to 1.x and 1.3 if required.

export const PRIVATE_TENANT_SYMBOL = '__user__';
export const DEFAULT_TENANT = 'default';
export const GLOBAL_TENANT_RENDERING_TEXT = 'Global';
export const PRIVATE_TENANT_RENDERING_TEXT = 'Private';
export const globalTenantName = 'global_tenant';

export enum AuthType {
BASIC = 'basicauth',
OPEN_ID = 'openid',
Expand All @@ -48,3 +55,7 @@ export function isValidResourceName(resourceName: string): boolean {
// see: https://javascript.info/regexp-unicode
return !/[\p{C}%]/u.test(resourceName) && resourceName.length > 0;
}

export function isGlobalTenant(selectedTenant: string | null) {
return selectedTenant !== null && selectedTenant === GLOBAL_TENANT_SYMBOL;
}
88 changes: 88 additions & 0 deletions server/multitenancy/tenant_resolver.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
36 changes: 32 additions & 4 deletions server/multitenancy/tenant_resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@
*/

import { isEmpty, findKey, cloneDeep } from 'lodash';
import { OpenSearchDashboardsRequest } from '../../../../src/core/server';
import { OpenSearchDashboardsRequest } from 'opensearch-dashboards/server';
import { ResponseObject } from '@hapi/hapi';
import { SecuritySessionCookie } from '../session/security_cookie';
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';
import { SecurityPluginConfigType } from '..';

const PRIVATE_TENANT_SYMBOL: string = '__user__';
const GLOBAL_TENANT_SYMBOL: string = '';

export const PRIVATE_TENANTS: string[] = [PRIVATE_TENANT_SYMBOL, 'private'];
export const GLOBAL_TENANTS: string[] = ['global', GLOBAL_TENANT_SYMBOL];
/**
Expand Down Expand Up @@ -170,3 +172,29 @@ 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;
}
10 changes: 10 additions & 0 deletions server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import { first } from 'rxjs/operators';
import { Observable } from 'rxjs';
import { ResponseObject } from '@hapi/hapi';
import {
PluginInitializerContext,
CoreSetup,
Expand Down Expand Up @@ -43,6 +44,7 @@ import { getAuthenticationHandler } from './auth/auth_handler_factory';
import { setupMultitenantRoutes } from './multitenancy/routes';
import { defineAuthTypeRoutes } from './routes/auth_type_routes';
import { createMigrationOpenSearchClient } from '../../../src/core/server/saved_objects/migrations/core';
import { addTenantParameterToResolvedShortLink } from './multitenancy/tenant_resolver';

export interface SecurityPluginRequestContext {
logger: Logger;
Expand Down Expand Up @@ -118,6 +120,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);
Expand Down