From 371495b680e13ec53561d44fbdfe8e67f347955d Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 15 Aug 2024 23:23:23 +0800 Subject: [PATCH] Use the `getRedirectUrl` from OSD to generate nextUrl (#2072) * feat: consume the get redirect url function from osd core Signed-off-by: SuZhou-Joe * feat: consume the get redirect url function from osd core Signed-off-by: SuZhou-Joe * fix: failed unit test because of an invalid mocked request Signed-off-by: SuZhou-Joe * fix: failed unit test because of an invalid mocked request Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe --- server/auth/types/openid/openid_auth.ts | 9 ++++++--- server/auth/types/saml/saml_auth.ts | 9 ++++++--- server/utils/next_url.test.ts | 26 +++++++++++++------------ server/utils/next_url.ts | 9 ++++++++- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/server/auth/types/openid/openid_auth.ts b/server/auth/types/openid/openid_auth.ts index b1f69bd64..46c0f23a4 100644 --- a/server/auth/types/openid/openid_auth.ts +++ b/server/auth/types/openid/openid_auth.ts @@ -46,6 +46,7 @@ import { getExtraAuthStorageValue, setExtraAuthStorage, } from '../../../session/cookie_splitter'; +import { getRedirectUrl } from '../../../../../../src/core/server/http'; export interface OpenIdAuthConfig { authorizationEndpoint?: string; @@ -127,9 +128,11 @@ export class OpenIdAuthentication extends AuthenticationType { } private generateNextUrl(request: OpenSearchDashboardsRequest): string { - const path = - this.coreSetup.http.basePath.serverBasePath + - (request.url.pathname || '/app/opensearch-dashboards'); + const path = getRedirectUrl({ + request, + basePath: this.coreSetup.http.basePath.serverBasePath, + nextUrl: request.url.pathname || '/app/opensearch-dashboards', + }); return escape(path); } diff --git a/server/auth/types/saml/saml_auth.ts b/server/auth/types/saml/saml_auth.ts index 1a58efb1a..e7a2311db 100644 --- a/server/auth/types/saml/saml_auth.ts +++ b/server/auth/types/saml/saml_auth.ts @@ -41,6 +41,7 @@ import { getExtraAuthStorageValue, ExtraAuthStorageOptions, } from '../../../session/cookie_splitter'; +import { getRedirectUrl } from '../../../../../../src/core/server/http'; export class SamlAuthentication extends AuthenticationType { public static readonly AUTH_HEADER_NAME = 'authorization'; @@ -59,9 +60,11 @@ export class SamlAuthentication extends AuthenticationType { } private generateNextUrl(request: OpenSearchDashboardsRequest): string { - let path = - this.coreSetup.http.basePath.serverBasePath + - (request.url.pathname || '/app/opensearch-dashboards'); + let path = getRedirectUrl({ + request, + basePath: this.coreSetup.http.basePath.serverBasePath, + nextUrl: request.url.pathname || '/app/opensearch-dashboards', + }); if (request.url.search) { path += request.url.search; } diff --git a/server/utils/next_url.test.ts b/server/utils/next_url.test.ts index 83cb0e7d5..56f4b0741 100644 --- a/server/utils/next_url.test.ts +++ b/server/utils/next_url.test.ts @@ -18,14 +18,16 @@ import { validateNextUrl, INVALID_NEXT_URL_PARAMETER_MESSAGE, } from './next_url'; +import { httpServerMock } from '../../../../src/core/server/mocks'; describe('test composeNextUrlQueryParam', () => { + httpServerMock.createOpenSearchDashboardsRequest(); test('no base, no path', () => { expect( composeNextUrlQueryParam( - { - url: 'http://localhost:123', - }, + httpServerMock.createOpenSearchDashboardsRequest({ + path: '', + }), '' ) ).toEqual(''); @@ -34,9 +36,9 @@ describe('test composeNextUrlQueryParam', () => { test('no base, path', () => { expect( composeNextUrlQueryParam( - { - url: 'http://localhost:123/alpha/major/foxtrot', - }, + httpServerMock.createOpenSearchDashboardsRequest({ + path: '/alpha/major/foxtrot', + }), '' ) ).toEqual('nextUrl=%2Falpha%2Fmajor%2Ffoxtrot'); @@ -45,9 +47,9 @@ describe('test composeNextUrlQueryParam', () => { test('base, no path', () => { expect( composeNextUrlQueryParam( - { - url: 'http://localhost:123', - }, + httpServerMock.createOpenSearchDashboardsRequest({ + path: '', + }), 'xyz' ) ).toEqual(''); @@ -56,9 +58,9 @@ describe('test composeNextUrlQueryParam', () => { test('base, path', () => { expect( composeNextUrlQueryParam( - { - url: 'http://localhost:123/alpha/major/foxtrot', - }, + httpServerMock.createOpenSearchDashboardsRequest({ + path: '/alpha/major/foxtrot', + }), 'xyz' ) ).toEqual('nextUrl=xyz%2Falpha%2Fmajor%2Ffoxtrot'); diff --git a/server/utils/next_url.ts b/server/utils/next_url.ts index 6b439f872..9cc47adbd 100644 --- a/server/utils/next_url.ts +++ b/server/utils/next_url.ts @@ -17,6 +17,7 @@ import { parse } from 'url'; import { ParsedUrlQuery } from 'querystring'; import { OpenSearchDashboardsRequest } from 'opensearch-dashboards/server'; import { encodeUriQuery } from '../../../../src/plugins/opensearch_dashboards_utils/common/url/encode_uri_query'; +import { getRedirectUrl } from '../../../../src/core/server/http'; export function composeNextUrlQueryParam( request: OpenSearchDashboardsRequest, @@ -28,7 +29,13 @@ export function composeNextUrlQueryParam( const nextUrl = parsedUrl?.path; if (!!nextUrl && nextUrl !== '/') { - return `nextUrl=${encodeUriQuery(basePath + nextUrl)}`; + return `nextUrl=${encodeUriQuery( + getRedirectUrl({ + request, + basePath, + nextUrl, + }) + )}`; } } catch (error) { /* Ignore errors from parsing */