From 78e44f8dd6792b5e010af3609473aa932f4b8bf0 Mon Sep 17 00:00:00 2001 From: Deepak Devarakonda <80896069+devardee@users.noreply.github.com> Date: Tue, 6 Feb 2024 02:16:35 +0530 Subject: [PATCH] Fixes Short URL redirection for SAML login (#1744) Signed-off-by: Deepak Devarakonda Co-authored-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> Co-authored-by: Darshit Chanpura <35282393+DarshitChanpura@users.noreply.github.com> (cherry picked from commit 4d7e5f39e046cf2e67492965685c1e077156c76b) --- server/auth/types/saml/saml_auth.ts | 5 ++++- server/multitenancy/tenant_resolver.ts | 16 ++++++++++------ test/cypress/e2e/saml/saml_auth_test.spec.js | 16 +++++++++++++++- test/cypress/support/commands.js | 14 +++++++++++++- test/cypress/support/constants.js | 4 ++++ test/cypress/support/index.d.ts | 12 ++++++++++++ 6 files changed, 58 insertions(+), 9 deletions(-) diff --git a/server/auth/types/saml/saml_auth.ts b/server/auth/types/saml/saml_auth.ts index 50801784a..23c1ba9c2 100644 --- a/server/auth/types/saml/saml_auth.ts +++ b/server/auth/types/saml/saml_auth.ts @@ -59,9 +59,12 @@ export class SamlAuthentication extends AuthenticationType { } private generateNextUrl(request: OpenSearchDashboardsRequest): string { - const path = + let path = this.coreSetup.http.basePath.serverBasePath + (request.url.pathname || '/app/opensearch-dashboards'); + if (request.url.search) { + path += request.url.search; + } return escape(path); } diff --git a/server/multitenancy/tenant_resolver.ts b/server/multitenancy/tenant_resolver.ts index 8fa7a77f3..883ab3deb 100755 --- a/server/multitenancy/tenant_resolver.ts +++ b/server/multitenancy/tenant_resolver.ts @@ -13,13 +13,13 @@ * permissions and limitations under the License. */ -import { isEmpty, findKey, cloneDeep } from 'lodash'; +import { cloneDeep, findKey, isEmpty } from 'lodash'; import { OpenSearchDashboardsRequest } from 'opensearch-dashboards/server'; import { ResponseObject } from '@hapi/hapi'; import { modifyUrl } from '@osd/std'; import { SecuritySessionCookie } from '../session/security_cookie'; import { SecurityPluginConfigType } from '..'; -import { GLOBAL_TENANT_SYMBOL, PRIVATE_TENANT_SYMBOL, globalTenantName } from '../../common'; +import { GLOBAL_TENANT_SYMBOL, globalTenantName, PRIVATE_TENANT_SYMBOL } from '../../common'; import { ensureRawRequest } from '../../../../src/core/server/http/router'; import { GOTO_PREFIX } from '../../../../src/plugins/share/common/short_url_routes'; @@ -210,16 +210,20 @@ export function addTenantParameterToResolvedShortLink(request: OpenSearchDashboa if (request.url.pathname.startsWith(`${GOTO_PREFIX}/`)) { const rawRequest = ensureRawRequest(request); const rawResponse = rawRequest.response as ResponseObject; + const responsePath = rawResponse.headers.location as string; // Make sure the request really should redirect - if (rawResponse.headers.location) { - const modifiedUrl = modifyUrl(rawResponse.headers.location as string, (parts) => { + if ( + rawResponse.headers.location && + !responsePath.includes('security_tenant') && + request.headers.securitytenant + ) { + // Mutating the headers toolkit.next({headers: ...}) logs a warning about headers being overwritten + rawResponse.headers.location = modifyUrl(responsePath, (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; } } diff --git a/test/cypress/e2e/saml/saml_auth_test.spec.js b/test/cypress/e2e/saml/saml_auth_test.spec.js index 733f65e52..b8f6a134f 100644 --- a/test/cypress/e2e/saml/saml_auth_test.spec.js +++ b/test/cypress/e2e/saml/saml_auth_test.spec.js @@ -18,7 +18,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { ALL_ACCESS_ROLE } from '../../support/constants'; +import { ALL_ACCESS_ROLE, SHORTEN_URL_DATA } from '../../support/constants'; import samlUserRoleMapping from '../../fixtures/saml/samlUserRoleMappiing.json'; @@ -121,4 +121,18 @@ describe('Log in via SAML', () => { cy.get('#tenantName').should('have.text', 'Private'); }); + + it('Login to Dashboard with Goto URL', () => { + localStorage.setItem('home:newThemeModal:show', 'false'); + cy.shortenUrl(SHORTEN_URL_DATA, 'global').then((response) => { + // We need to explicitly clear cookies, + // since the Shorten URL api is return's set-cookie header for admin user. + cy.clearCookies().then(() => { + const gotoUrl = `http://localhost:5601/goto/${response.urlId}?security_tenant=global`; + cy.visit(gotoUrl); + samlLogin(); + cy.getCookie('security_authentication').should('exist'); + }); + }); + }); }); diff --git a/test/cypress/support/commands.js b/test/cypress/support/commands.js index 6a258af67..29c565062 100644 --- a/test/cypress/support/commands.js +++ b/test/cypress/support/commands.js @@ -18,7 +18,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { SEC_API, ADMIN_AUTH } from './constants'; +import { SEC_API, ADMIN_AUTH, DASHBOARDS_API } from './constants'; /** * Overwrite request command to support authentication similar to visit. @@ -90,3 +90,15 @@ Cypress.Commands.add('loginWithSamlMultiauth', () => { cy.get('input[id=userName]').should('be.visible'); cy.get('button[id=btn-sign-in]').should('be.visible').click(); }); + +Cypress.Commands.add('shortenUrl', (data, tenant) => { + cy.request({ + url: `http://localhost:5601${DASHBOARDS_API.SHORTEN_URL}`, + method: 'POST', + body: data, + headers: { securitytenant: tenant, 'osd-xsrf': 'osd-fetch' }, + }).then((response) => { + expect(response.status).to.eq(200); + return response.body; + }); +}); diff --git a/test/cypress/support/constants.js b/test/cypress/support/constants.js index 95cb4da04..b357659c6 100644 --- a/test/cypress/support/constants.js +++ b/test/cypress/support/constants.js @@ -40,3 +40,7 @@ export const SEC_API = { ROLE_BASE: `${SEC_API_PREFIX}/roles`, ROLE_MAPPING_BASE: `${SEC_API_PREFIX}/rolesmapping`, }; +export const DASHBOARDS_API = { + SHORTEN_URL: '/api/shorten_url', +}; +export const SHORTEN_URL_DATA = { url: '/app/home#/tutorial_directory' }; diff --git a/test/cypress/support/index.d.ts b/test/cypress/support/index.d.ts index 61362a8c4..311be9923 100644 --- a/test/cypress/support/index.d.ts +++ b/test/cypress/support/index.d.ts @@ -52,4 +52,16 @@ declare namespace Cypress { */ createRoleMapping(roleID: string, rolemappingJson: string): Chainable; } + + interface Chainable { + /** + * Generate a UUID for the passed URL and store it in the tenant index. + * @example : + * cy.shortenUrl({url: "/app/home#/tutorial_directory"}, 'global') + * + * @param data - The Object which contains the url. + * @param tenant - The tenant index which will store the UUID + */ + shortenUrl(data: object, tenant: string): Chainable; + } }