From 5f2d167b297f1feaf096b60bc958e963ff1b0689 Mon Sep 17 00:00:00 2001 From: Deepak Devarakonda Date: Mon, 22 Jan 2024 13:06:28 +0530 Subject: [PATCH 1/8] Fix for 1627 Signed-off-by: Deepak Devarakonda --- server/multitenancy/tenant_resolver.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/multitenancy/tenant_resolver.ts b/server/multitenancy/tenant_resolver.ts index 58fa241d1..3f1229150 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,16 @@ 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')) { + // 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; } } From c75e1113d50f38f73b3e197a0f7693c6b707f4cb Mon Sep 17 00:00:00 2001 From: Deepak Devarakonda Date: Wed, 31 Jan 2024 16:23:16 +0530 Subject: [PATCH 2/8] Address PR comments Signed-off-by: Deepak Devarakonda --- server/auth/types/saml/saml_auth.ts | 5 ++++- server/multitenancy/tenant_resolver.ts | 6 +++++- test/cypress/e2e/saml/saml_auth_test.spec.js | 15 ++++++++++++++- test/cypress/support/commands.js | 14 +++++++++++++- test/cypress/support/constants.js | 4 ++++ test/cypress/support/index.d.ts | 9 +++++++++ 6 files changed, 49 insertions(+), 4 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 9c7223445..883ab3deb 100755 --- a/server/multitenancy/tenant_resolver.ts +++ b/server/multitenancy/tenant_resolver.ts @@ -213,7 +213,11 @@ export function addTenantParameterToResolvedShortLink(request: OpenSearchDashboa const responsePath = rawResponse.headers.location as string; // Make sure the request really should redirect - if (rawResponse.headers.location && !responsePath.includes('security_tenant')) { + 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) { diff --git a/test/cypress/e2e/saml/saml_auth_test.spec.js b/test/cypress/e2e/saml/saml_auth_test.spec.js index 733f65e52..f555518a4 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,17 @@ 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 => { + const gotoUrl = `http://localhost:5601/goto/${response['urlId']}?security_tenant=global`; + cy.visit(gotoUrl, { + failOnStatusCode: false, + }) + + cy.getCookie('security_authentication').should('exist'); + }); + }); }); diff --git a/test/cypress/support/commands.js b/test/cypress/support/commands.js index 6a258af67..f37245238 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 : `${Cypress.env('dashboardsUrl')}${DASHBOARDS_API.SHORTEN_URL}`, + method: 'POST', + body: data, + headers: { 'securitytenant' : tenant, 'osd-xsrf': 'osd-fetch'} + } + ).then((response) => {return response.body}) + +}); diff --git a/test/cypress/support/constants.js b/test/cypress/support/constants.js index 95cb4da04..93b46ad97 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..4408eef92 100644 --- a/test/cypress/support/index.d.ts +++ b/test/cypress/support/index.d.ts @@ -52,4 +52,13 @@ declare namespace Cypress { */ createRoleMapping(roleID: string, rolemappingJson: string): Chainable; } + + interface Chainable { + /** + * Download Sample HTTP Logs data into the required tenant + * @example + * cy.downloadSampleLogs('global') + */ + shortenUrl(data: object, tenant: string): Chainable; + } } From bc2ba8ea28980bc6cb1832d169f218664025f8b4 Mon Sep 17 00:00:00 2001 From: Deepak Devarakonda Date: Wed, 31 Jan 2024 16:33:55 +0530 Subject: [PATCH 3/8] Update function documentation Signed-off-by: Deepak Devarakonda --- test/cypress/support/index.d.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/cypress/support/index.d.ts b/test/cypress/support/index.d.ts index 4408eef92..311be9923 100644 --- a/test/cypress/support/index.d.ts +++ b/test/cypress/support/index.d.ts @@ -55,9 +55,12 @@ declare namespace Cypress { interface Chainable { /** - * Download Sample HTTP Logs data into the required tenant - * @example - * cy.downloadSampleLogs('global') + * 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; } From 6874f7feac5a1332597c9140dc81c94a444967e2 Mon Sep 17 00:00:00 2001 From: Deepak Devarakonda Date: Wed, 31 Jan 2024 16:59:58 +0530 Subject: [PATCH 4/8] hope IT pass now Signed-off-by: Deepak Devarakonda --- test/cypress/support/commands.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cypress/support/commands.js b/test/cypress/support/commands.js index f37245238..980855276 100644 --- a/test/cypress/support/commands.js +++ b/test/cypress/support/commands.js @@ -94,7 +94,7 @@ Cypress.Commands.add('loginWithSamlMultiauth', () => { Cypress.Commands.add('shortenUrl', (data, tenant) => { cy.request( { - url : `${Cypress.env('dashboardsUrl')}${DASHBOARDS_API.SHORTEN_URL}`, + url : `${DASHBOARDS_API.SHORTEN_URL}`, method: 'POST', body: data, headers: { 'securitytenant' : tenant, 'osd-xsrf': 'osd-fetch'} From d126ece199d48106519378e6a5cb3d997b3c9692 Mon Sep 17 00:00:00 2001 From: Deepak Devarakonda Date: Wed, 31 Jan 2024 17:16:38 +0530 Subject: [PATCH 5/8] hope IT pass now : take 2 Signed-off-by: Deepak Devarakonda --- test/cypress/e2e/saml/saml_auth_test.spec.js | 6 +++--- test/cypress/support/commands.js | 17 ++++++++--------- test/cypress/support/constants.js | 6 +++--- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/test/cypress/e2e/saml/saml_auth_test.spec.js b/test/cypress/e2e/saml/saml_auth_test.spec.js index f555518a4..6440df47f 100644 --- a/test/cypress/e2e/saml/saml_auth_test.spec.js +++ b/test/cypress/e2e/saml/saml_auth_test.spec.js @@ -125,11 +125,11 @@ describe('Log in via SAML', () => { it('Login to Dashboard with Goto URL', () => { localStorage.setItem('home:newThemeModal:show', 'false'); - cy.shortenUrl(SHORTEN_URL_DATA, 'global').then(response => { - const gotoUrl = `http://localhost:5601/goto/${response['urlId']}?security_tenant=global`; + cy.shortenUrl(SHORTEN_URL_DATA, 'global').then((response) => { + const gotoUrl = `http://localhost:5601/goto/${response.urlId}?security_tenant=global`; cy.visit(gotoUrl, { failOnStatusCode: false, - }) + }); cy.getCookie('security_authentication').should('exist'); }); diff --git a/test/cypress/support/commands.js b/test/cypress/support/commands.js index 980855276..599cc8f76 100644 --- a/test/cypress/support/commands.js +++ b/test/cypress/support/commands.js @@ -92,13 +92,12 @@ Cypress.Commands.add('loginWithSamlMultiauth', () => { }); Cypress.Commands.add('shortenUrl', (data, tenant) => { - cy.request( - { - url : `${DASHBOARDS_API.SHORTEN_URL}`, - method: 'POST', - body: data, - headers: { 'securitytenant' : tenant, 'osd-xsrf': 'osd-fetch'} - } - ).then((response) => {return response.body}) - + cy.request({ + url: `${DASHBOARDS_API.SHORTEN_URL}`, + method: 'POST', + body: data, + headers: { securitytenant: tenant, 'osd-xsrf': 'osd-fetch' }, + }).then((response) => { + return response.body; + }); }); diff --git a/test/cypress/support/constants.js b/test/cypress/support/constants.js index 93b46ad97..b357659c6 100644 --- a/test/cypress/support/constants.js +++ b/test/cypress/support/constants.js @@ -41,6 +41,6 @@ export const SEC_API = { 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"} + SHORTEN_URL: '/api/shorten_url', +}; +export const SHORTEN_URL_DATA = { url: '/app/home#/tutorial_directory' }; From ec2b623c4676476ffeae3b16c65e15ba0a96c89c Mon Sep 17 00:00:00 2001 From: Deepak Devarakonda Date: Thu, 1 Feb 2024 18:27:00 +0530 Subject: [PATCH 6/8] hope IT pass now : take 3 Signed-off-by: Deepak Devarakonda --- test/cypress/support/commands.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/cypress/support/commands.js b/test/cypress/support/commands.js index 599cc8f76..29c565062 100644 --- a/test/cypress/support/commands.js +++ b/test/cypress/support/commands.js @@ -93,11 +93,12 @@ Cypress.Commands.add('loginWithSamlMultiauth', () => { Cypress.Commands.add('shortenUrl', (data, tenant) => { cy.request({ - url: `${DASHBOARDS_API.SHORTEN_URL}`, + 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; }); }); From 988f220bbebb7204f9678d727f3d7d24f809e655 Mon Sep 17 00:00:00 2001 From: Deepak Devarakonda Date: Thu, 1 Feb 2024 23:37:12 +0530 Subject: [PATCH 7/8] All tests passgit add testgit add test Signed-off-by: Deepak Devarakonda --- test/cypress/e2e/saml/saml_auth_test.spec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/cypress/e2e/saml/saml_auth_test.spec.js b/test/cypress/e2e/saml/saml_auth_test.spec.js index 6440df47f..0a6e0a3be 100644 --- a/test/cypress/e2e/saml/saml_auth_test.spec.js +++ b/test/cypress/e2e/saml/saml_auth_test.spec.js @@ -129,9 +129,10 @@ describe('Log in via SAML', () => { const gotoUrl = `http://localhost:5601/goto/${response.urlId}?security_tenant=global`; cy.visit(gotoUrl, { failOnStatusCode: false, + }).then(() => { + samlLogin(); + cy.getCookie('security_authentication').should('exist'); }); - - cy.getCookie('security_authentication').should('exist'); }); }); }); From 0bca55ac1e30f67c858fd517b28bd210b222eae7 Mon Sep 17 00:00:00 2001 From: Deepak Devarakonda Date: Fri, 2 Feb 2024 22:04:53 +0530 Subject: [PATCH 8/8] pls pass mate Signed-off-by: Deepak Devarakonda --- test/cypress/e2e/saml/saml_auth_test.spec.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/cypress/e2e/saml/saml_auth_test.spec.js b/test/cypress/e2e/saml/saml_auth_test.spec.js index 0a6e0a3be..b8f6a134f 100644 --- a/test/cypress/e2e/saml/saml_auth_test.spec.js +++ b/test/cypress/e2e/saml/saml_auth_test.spec.js @@ -124,12 +124,12 @@ describe('Log in via SAML', () => { it('Login to Dashboard with Goto URL', () => { localStorage.setItem('home:newThemeModal:show', 'false'); - cy.shortenUrl(SHORTEN_URL_DATA, 'global').then((response) => { - const gotoUrl = `http://localhost:5601/goto/${response.urlId}?security_tenant=global`; - cy.visit(gotoUrl, { - failOnStatusCode: false, - }).then(() => { + // 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'); });