Skip to content

Commit

Permalink
Use the getRedirectUrl from OSD to generate nextUrl (opensearch-pro…
Browse files Browse the repository at this point in the history
…ject#2072) (opensearch-project#2087)

* feat: consume the get redirect url function from osd core



* feat: consume the get redirect url function from osd core



* fix: failed unit test because of an invalid mocked request



* fix: failed unit test because of an invalid mocked request



---------


(cherry picked from commit 371495b)

Signed-off-by: SuZhou-Joe <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Derek Ho <[email protected]>
  • Loading branch information
3 people authored Aug 19, 2024
1 parent ca257a3 commit 80976be
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 19 deletions.
9 changes: 6 additions & 3 deletions server/auth/types/openid/openid_auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
getExtraAuthStorageValue,
setExtraAuthStorage,
} from '../../../session/cookie_splitter';
import { getRedirectUrl } from '../../../../../../src/core/server/http';

export interface OpenIdAuthConfig {
authorizationEndpoint?: string;
Expand Down Expand Up @@ -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);
}

Expand Down
9 changes: 6 additions & 3 deletions server/auth/types/saml/saml_auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
}
Expand Down
26 changes: 14 additions & 12 deletions server/utils/next_url.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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('');
Expand All @@ -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');
Expand All @@ -45,9 +47,9 @@ describe('test composeNextUrlQueryParam', () => {
test('base, no path', () => {
expect(
composeNextUrlQueryParam(
{
url: 'http://localhost:123',
},
httpServerMock.createOpenSearchDashboardsRequest({
path: '',
}),
'xyz'
)
).toEqual('');
Expand All @@ -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');
Expand Down
9 changes: 8 additions & 1 deletion server/utils/next_url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 */
Expand Down

0 comments on commit 80976be

Please sign in to comment.