Merge branch 'main' into proxyMultiAuth

opensearch-project · Aug 16, 2024 · 679cde0 · 679cde0
2 parents 8014138 + e3da9b4
commit 679cde0
Show file tree

Hide file tree

Showing 10 changed files with 121 additions and 42 deletions.
diff --git 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(
           <AccountNavButton

diff --git a/public/apps/account/account-nav-button.scss b/public/apps/account/account-nav-button.scss
@@ -0,0 +1,9 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+// stylelint-disable-next-line @osd/stylelint/no_modifying_global_selectors
+.euiButtonEmpty.accountNavButton {
+  border: 0;
+}
diff --git a/public/apps/account/account-nav-button.tsx b/public/apps/account/account-nav-button.tsx
@@ -36,6 +36,7 @@ import { LogoutButton } from './log-out-button';
 import { resolveTenantName } from '../configuration/utils/tenant-utils';
 import { getShouldShowTenantPopup, setShouldShowTenantPopup } from '../../utils/storage-utils';
 import { getDashboardsInfo } from '../../utils/dashboards-info-utils';
+import './account-nav-button.scss';
 
 export function AccountNavButton(props: {
   coreStart: CoreStart;
@@ -73,7 +74,7 @@ export function AccountNavButton(props: {
     const fetchData = async () => {
       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: {
       />
     </div>
   );
-  return (
-    <EuiHeaderSectionItemButton id="user-icon-btn">
+
+  const isPlacedInLeftNav = props.coreStart.uiSettings.get('home:useNewHomePage');
+
+  // ToDo: Add aria-label and tooltip when isPlacedInLeftNav is true
+  const innerElement = isPlacedInLeftNav ? (
+    <EuiButtonEmpty
+      size="xs"
+      flush="both"
+      className="accountNavButton"
+      aria-expanded={isPopoverOpen}
+      aria-haspopup="true"
+    >
+      <EuiAvatar name={username} size="s" />
+    </EuiButtonEmpty>
+  ) : (
+    <EuiAvatar name={username} size="m" />
+  );
+
+  const popover = (
+    <>
       <EuiPopover
         data-test-subj="account-popover"
         id="actionsMenu"
-        button={<EuiAvatar name={username} />}
+        anchorPosition={isPlacedInLeftNav ? 'rightDown' : undefined}
+        button={innerElement}
         isOpen={isPopoverOpen}
         closePopover={() => {
           setPopoverOpen(false);
@@ -185,6 +205,14 @@ export function AccountNavButton(props: {
         <EuiContextMenuPanel>{contextMenuPanel}</EuiContextMenuPanel>
       </EuiPopover>
       {modal}
+    </>
+  );
+
+  return isPlacedInLeftNav ? (
+    popover
+  ) : (
+    <EuiHeaderSectionItemButton id="user-icon-btn" size="l">
+      {popover}
     </EuiHeaderSectionItemButton>
   );
 }

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
@@ -3,12 +3,14 @@
 exports[`Account navigation button renders 1`] = `
 <EuiHeaderSectionItemButton
   id="user-icon-btn"
+  size="l"
 >
   <EuiPopover
     anchorPosition="downCenter"
     button={
       <EuiAvatar
         name="user1"
+        size="m"
       />
     }
     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],
+            }
+          }
         />
       </div>
     </EuiContextMenuPanel>

diff --git 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
@@ -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(
       <AccountNavButton
         coreStart={mockCoreStart}
@@ -133,9 +134,7 @@ describe('Account navigation button', () => {
 });
 
 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
@@ -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
@@ -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
@@ -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
@@ -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 */