diff --git a/public/apps/account/account-app.tsx b/public/apps/account/account-app.tsx index 879eb5a9..0f973a1b 100644 --- a/public/apps/account/account-app.tsx +++ b/public/apps/account/account-app.tsx @@ -107,9 +107,10 @@ export async function setupTopNavButton(coreStart: CoreStart, config: ClientConf setShouldShowTenantPopup(shouldShowTenantPopup); - coreStart.chrome.navControls.registerRight({ - // Pin to rightmost, since newsfeed plugin is using 1000, here needs a number > 1000 - order: 2000, + const isPlacedInLeftNav = coreStart.uiSettings.get('home:useNewHomePage'); + + coreStart.chrome.navControls[isPlacedInLeftNav ? 'registerLeftBottom' : 'registerRight']({ + order: isPlacedInLeftNav ? 10000 : 2000, mount: (element: HTMLElement) => { ReactDOM.render( { try { setIsMultiTenancyEnabled( - (await getDashboardsInfo(props.coreStart.http)).multitenancy_enabled + Boolean((await getDashboardsInfo(props.coreStart.http)).multitenancy_enabled) ); } catch (e) { // TODO: switch to better error display. @@ -167,12 +168,31 @@ export function AccountNavButton(props: { /> ); - return ( - + + const isPlacedInLeftNav = props.coreStart.uiSettings.get('home:useNewHomePage'); + + // ToDo: Add aria-label and tooltip when isPlacedInLeftNav is true + const innerElement = isPlacedInLeftNav ? ( + + + + ) : ( + + ); + + const popover = ( + <> } + anchorPosition={isPlacedInLeftNav ? 'rightDown' : undefined} + button={innerElement} isOpen={isPopoverOpen} closePopover={() => { setPopoverOpen(false); @@ -185,6 +205,14 @@ export function AccountNavButton(props: { {contextMenuPanel} {modal} + + ); + + return isPlacedInLeftNav ? ( + popover + ) : ( + + {popover} ); } diff --git a/public/apps/account/test/__snapshots__/account-nav-button.test.tsx.snap b/public/apps/account/test/__snapshots__/account-nav-button.test.tsx.snap index 39b9e332..41ec95fb 100644 --- a/public/apps/account/test/__snapshots__/account-nav-button.test.tsx.snap +++ b/public/apps/account/test/__snapshots__/account-nav-button.test.tsx.snap @@ -3,12 +3,14 @@ exports[`Account navigation button renders 1`] = ` } closePopover={[Function]} @@ -111,7 +113,34 @@ exports[`Account navigation button renders 1`] = ` margin="xs" /> } - http={1} + http={ + Object { + "addLoadingCountSource": [MockFunction], + "anonymousPaths": Object { + "isAnonymous": [MockFunction], + "register": [MockFunction], + }, + "basePath": BasePath { + "basePath": "", + "clientBasePath": "", + "get": [Function], + "getBasePath": [Function], + "prepend": [Function], + "remove": [Function], + "serverBasePath": "", + }, + "delete": [MockFunction], + "fetch": [MockFunction], + "get": [MockFunction], + "getLoadingCount$": [MockFunction], + "head": [MockFunction], + "intercept": [MockFunction], + "options": [MockFunction], + "patch": [MockFunction], + "post": [MockFunction], + "put": [MockFunction], + } + } /> diff --git a/public/apps/account/test/account-app.test.tsx b/public/apps/account/test/account-app.test.tsx index 3c7e1a74..ef0b4469 100644 --- a/public/apps/account/test/account-app.test.tsx +++ b/public/apps/account/test/account-app.test.tsx @@ -19,6 +19,8 @@ import { fetchAccountInfoSafe } from '../utils'; import { fetchCurrentAuthType } from '../../../utils/logout-utils'; import { fetchCurrentTenant } from '../../configuration/utils/tenant-utils'; import { getDashboardsInfoSafe } from '../../../utils/dashboards-info-utils'; +import { CoreStart } from 'opensearch-dashboards/public'; +import { coreMock } from '../../../../../../src/core/public/mocks'; jest.mock('../../../utils/storage-utils', () => ({ getShouldShowTenantPopup: jest.fn(), @@ -44,13 +46,7 @@ jest.mock('../../configuration/utils/tenant-utils', () => ({ })); describe('Account app', () => { - const mockCoreStart = { - chrome: { - navControls: { - registerRight: jest.fn(), - }, - }, - }; + const mockCoreStart: CoreStart = coreMock.createStart(); const mockConfig = { multitenancy: { diff --git a/public/apps/account/test/account-nav-button.test.tsx b/public/apps/account/test/account-nav-button.test.tsx index cdc6a00a..3e3821c8 100644 --- a/public/apps/account/test/account-nav-button.test.tsx +++ b/public/apps/account/test/account-nav-button.test.tsx @@ -19,6 +19,8 @@ import { AccountNavButton, reloadAfterTenantSwitch } from '../account-nav-button import { getShouldShowTenantPopup } from '../../../utils/storage-utils'; import { getDashboardsInfo } from '../../../utils/dashboards-info-utils'; import { render, fireEvent } from '@testing-library/react'; +import { coreMock } from '../../../../../../src/core/public/mocks'; +import { CoreStart } from 'opensearch-dashboards/public'; jest.mock('../../../utils/storage-utils', () => ({ getShouldShowTenantPopup: jest.fn(), @@ -38,9 +40,7 @@ const mockDashboardsInfo = { }; describe('Account navigation button', () => { - const mockCoreStart = { - http: 1, - }; + const mockCoreStart: CoreStart = coreMock.createStart(); const config = { multitenancy: { @@ -66,6 +66,7 @@ describe('Account navigation button', () => { (getDashboardsInfo as jest.Mock).mockImplementation(() => { return mockDashboardsInfo; }); + component = shallow( { }); describe('Account navigation button, multitenancy disabled', () => { - const mockCoreStart = { - http: 1, - }; + const mockCoreStart: CoreStart = coreMock.createStart(); const config = { multitenancy: { @@ -178,9 +177,11 @@ describe('Account navigation button, multitenancy disabled', () => { }); describe('Shows tenant info when multitenancy enabled, and hides it if disabled', () => { + const mockCoreStart: CoreStart = coreMock.createStart(); + test('Renders "switch-tenants" and "tenant-name" when multi-tenancy is enabled', () => { const props = { - coreStart: {}, + coreStart: mockCoreStart, isInternalUser: true, username: 'example_user', tenant: 'example_tenant', @@ -213,7 +214,7 @@ describe('Shows tenant info when multitenancy enabled, and hides it if disabled' test('Does not render "switch-tenants" and "tenant-name" when multi-tenancy is disabled', () => { const props = { - coreStart: {}, + coreStart: mockCoreStart, isInternalUser: true, username: 'example_user', tenant: 'example_tenant', diff --git a/server/auth/types/openid/openid_auth.ts b/server/auth/types/openid/openid_auth.ts index b1f69bd6..46c0f23a 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 1a58efb1..e7a2311d 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 83cb0e7d..56f4b074 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 6b439f87..9cc47adb 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 */