From feafad6b92b0f9f1754aaee98369ff382080eafd Mon Sep 17 00:00:00 2001 From: Thuan Vo Date: Tue, 25 Apr 2023 00:31:45 -0400 Subject: [PATCH] fix(settings): hide settings that requires auth for logged-out user --- src/app/AppLayout/AppLayout.tsx | 13 +--- src/app/Settings/AutomatedAnalysisConfig.tsx | 1 + src/app/Settings/Settings.tsx | 74 ++++++++++--------- src/app/Settings/SettingsUtils.ts | 1 + src/app/routes.tsx | 23 +----- src/app/utils/useFeatureLevel.ts | 2 +- src/app/utils/useLogin.ts | 55 ++++++++++++++ src/test/Settings/Settings.test.tsx | 23 ++++++ .../__snapshots__/Settings.test.tsx.snap | 61 ++------------- 9 files changed, 133 insertions(+), 120 deletions(-) create mode 100644 src/app/utils/useLogin.ts diff --git a/src/app/AppLayout/AppLayout.tsx b/src/app/AppLayout/AppLayout.tsx index d0747bcc8..56cbbd464 100644 --- a/src/app/AppLayout/AppLayout.tsx +++ b/src/app/AppLayout/AppLayout.tsx @@ -95,7 +95,7 @@ import { QuestionCircleIcon, UserIcon, } from '@patternfly/react-icons'; -import * as _ from 'lodash'; +import _ from 'lodash'; import * as React from 'react'; import { useTranslation } from 'react-i18next'; import { Link, matchPath, NavLink, useHistory, useLocation } from 'react-router-dom'; @@ -104,6 +104,7 @@ import CryostatJoyride from '../Joyride/CryostatJoyride'; import { GlobalQuickStartDrawer } from '../QuickStarts/QuickStartDrawer'; import { AuthModal } from './AuthModal'; import { SslErrorModal } from './SslErrorModal'; +import { useLogin } from '@app/utils/useLogin'; interface AppLayoutProps { children: React.ReactNode; } @@ -127,7 +128,7 @@ const AppLayout: React.FC = ({ children }) => { const [showSslErrorModal, setShowSslErrorModal] = React.useState(false); const [aboutModalOpen, setAboutModalOpen] = React.useState(false); const [isNotificationDrawerExpanded, setNotificationDrawerExpanded] = React.useState(false); - const [showUserIcon, setShowUserIcon] = React.useState(false); + const showUserIcon = useLogin(); const [showUserInfoDropdown, setShowUserInfoDropdown] = React.useState(false); const [showHelpDropdown, setShowHelpDropdown] = React.useState(false); const [username, setUsername] = React.useState(''); @@ -287,14 +288,6 @@ const AppLayout: React.FC = ({ children }) => { notificationsContext.setDrawerState(true); }, [notificationsContext]); - React.useEffect(() => { - addSubscription( - serviceContext.login.getSessionState().subscribe((sessionState) => { - setShowUserIcon(sessionState === SessionState.USER_SESSION); - }) - ); - }, [serviceContext.login, serviceContext.login.getSessionState, setShowUserIcon, addSubscription]); - const handleLogout = React.useCallback(() => { addSubscription(serviceContext.login.setLoggedOut().subscribe()); }, [serviceContext.login, addSubscription]); diff --git a/src/app/Settings/AutomatedAnalysisConfig.tsx b/src/app/Settings/AutomatedAnalysisConfig.tsx index e6c1f1eb2..1ac193932 100644 --- a/src/app/Settings/AutomatedAnalysisConfig.tsx +++ b/src/app/Settings/AutomatedAnalysisConfig.tsx @@ -63,4 +63,5 @@ export const AutomatedAnalysisConfig: UserSetting = { descConstruct: 'SETTINGS.AUTOMATED_ANALYSIS_CONFIG.DESCRIPTION', content: Component, category: SettingTab.DASHBOARD, + authenticated: true, }; diff --git a/src/app/Settings/Settings.tsx b/src/app/Settings/Settings.tsx index 7a8c2a192..c9960cfdb 100644 --- a/src/app/Settings/Settings.tsx +++ b/src/app/Settings/Settings.tsx @@ -39,6 +39,7 @@ import { BreadcrumbPage } from '@app/BreadcrumbPage/BreadcrumbPage'; import { FeatureFlag } from '@app/Shared/FeatureFlag/FeatureFlag'; import { FeatureLevel } from '@app/Shared/Services/Settings.service'; +import { useLogin } from '@app/utils/useLogin'; import { cleanDataId, getActiveTab, hashCode, switchTab } from '@app/utils/utils'; import { Card, @@ -60,8 +61,9 @@ import { css } from '@patternfly/react-styles'; import * as React from 'react'; import { Trans, useTranslation } from 'react-i18next'; import { useHistory, useLocation } from 'react-router-dom'; -import { AutomatedAnalysisConfig } from './AutomatedAnalysisConfig'; +import { paramAsTab, SettingTab, tabAsParam, _TransformedUserSetting } from './SettingsUtils'; import { AutoRefresh } from './AutoRefresh'; +import { AutomatedAnalysisConfig } from './AutomatedAnalysisConfig'; import { ChartCardsConfig } from './ChartCardsConfig'; import { CredentialsStorage } from './CredentialsStorage'; import { DatetimeControl } from './DatetimeControl'; @@ -69,10 +71,23 @@ import { DeletionDialogControl } from './DeletionDialogControl'; import { FeatureLevels } from './FeatureLevels'; import { Language } from './Language'; import { NotificationControl } from './NotificationControl'; -import { paramAsTab, SettingTab, tabAsParam, _TransformedUserSetting } from './SettingsUtils'; import { Theme } from './Theme'; import { WebSocketDebounce } from './WebSocketDebounce'; +export const allSettings = [ + NotificationControl, + AutomatedAnalysisConfig, + ChartCardsConfig, + CredentialsStorage, + DeletionDialogControl, + WebSocketDebounce, + AutoRefresh, + FeatureLevels, + Language, + DatetimeControl, + Theme, +]; + interface SettingGroup { groupLabel: SettingTab; groupKey: string; @@ -92,41 +107,32 @@ export interface SettingsProps {} export const Settings: React.FC = (_) => { const [t] = useTranslation(); + const loggedIn = useLogin(); const settings = React.useMemo( () => - [ - NotificationControl, - AutomatedAnalysisConfig, - ChartCardsConfig, - CredentialsStorage, - DeletionDialogControl, - WebSocketDebounce, - AutoRefresh, - FeatureLevels, - Language, - DatetimeControl, - Theme, - ].map( - (c) => - ({ - title: t(c.titleKey) || '', - description: - typeof c.descConstruct === 'string' ? ( - t(c.descConstruct) - ) : ( - // Use children prop to avoid i18n parses body as key - /* eslint react/no-children-prop: 0 */ - - ), - element: React.createElement(c.content, null), - category: c.category, - disabled: c.disabled, - orderInGroup: c.orderInGroup || -1, - featureLevel: c.featureLevel || FeatureLevel.PRODUCTION, - } as _TransformedUserSetting) - ), - [t] + allSettings + .filter((s) => !s.authenticated || loggedIn) + .map( + (c) => + ({ + title: t(c.titleKey) || '', + description: + typeof c.descConstruct === 'string' ? ( + t(c.descConstruct) + ) : ( + // Use children prop to avoid i18n parses body as key + /* eslint react/no-children-prop: 0 */ + + ), + element: React.createElement(c.content, null), + category: c.category, + disabled: c.disabled, + orderInGroup: c.orderInGroup || -1, + featureLevel: c.featureLevel || FeatureLevel.PRODUCTION, + } as _TransformedUserSetting) + ), + [t, loggedIn] ); const history = useHistory(); diff --git a/src/app/Settings/SettingsUtils.ts b/src/app/Settings/SettingsUtils.ts index ed5bb28e9..6a35c60ce 100644 --- a/src/app/Settings/SettingsUtils.ts +++ b/src/app/Settings/SettingsUtils.ts @@ -78,6 +78,7 @@ export interface UserSetting { category: SettingTab; orderInGroup?: number; // default -1 featureLevel?: FeatureLevel; // default PRODUCTION + authenticated?: boolean; } export const selectTab = (tabKey: SettingTab) => { diff --git a/src/app/routes.tsx b/src/app/routes.tsx index cff6dd103..8cee0131f 100644 --- a/src/app/routes.tsx +++ b/src/app/routes.tsx @@ -54,13 +54,12 @@ import Rules from './Rules/Rules'; import SecurityPanel from './SecurityPanel/SecurityPanel'; import Settings from './Settings/Settings'; import { DefaultFallBack, ErrorBoundary } from './Shared/ErrorBoundary'; -import { SessionState } from './Shared/Services/Login.service'; -import { ServiceContext } from './Shared/Services/Services'; import { FeatureLevel } from './Shared/Services/Settings.service'; import CreateTarget from './Topology/Actions/CreateTarget'; import Topology from './Topology/Topology'; import { useDocumentTitle } from './utils/useDocumentTitle'; -import { useSubscriptions } from './utils/useSubscriptions'; +import { useFeatureLevel } from './utils/useFeatureLevel'; +import { useLogin } from './utils/useLogin'; import { accessibleRouteChangeHandler } from './utils/utils'; let routeFocusTimer: number; @@ -270,22 +269,8 @@ const PageNotFound = ({ title }: { title: string }) => { export interface AppRoutesProps {} const AppRoutes: React.FunctionComponent = (_) => { - const context = React.useContext(ServiceContext); - const addSubscription = useSubscriptions(); - const [loggedIn, setLoggedIn] = React.useState(false); - const [activeLevel, setActiveLevel] = React.useState(FeatureLevel.PRODUCTION); - - React.useEffect(() => { - addSubscription( - context.login - .getSessionState() - .subscribe((sessionState) => setLoggedIn(sessionState === SessionState.USER_SESSION)) - ); - }, [addSubscription, context.login, setLoggedIn]); - - React.useLayoutEffect(() => { - addSubscription(context.settings.featureLevel().subscribe((featureLevel) => setActiveLevel(featureLevel))); - }, [addSubscription, context.settings, setActiveLevel]); + const loggedIn = useLogin(); + const activeLevel = useFeatureLevel(); return ( diff --git a/src/app/utils/useFeatureLevel.ts b/src/app/utils/useFeatureLevel.ts index bb0766ca0..93407a7c2 100644 --- a/src/app/utils/useFeatureLevel.ts +++ b/src/app/utils/useFeatureLevel.ts @@ -45,7 +45,7 @@ export function useFeatureLevel() { const subRef = React.useRef(); const services = React.useContext(ServiceContext); - React.useEffect(() => { + React.useLayoutEffect(() => { subRef.current = services.settings.featureLevel().subscribe(setFeatureLevel); return () => subRef.current && subRef.current.unsubscribe(); }, [subRef, services.settings]); diff --git a/src/app/utils/useLogin.ts b/src/app/utils/useLogin.ts new file mode 100644 index 000000000..cdb863e3b --- /dev/null +++ b/src/app/utils/useLogin.ts @@ -0,0 +1,55 @@ +/* + * Copyright The Cryostat Authors + * + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or data + * (collectively the "Software"), free of charge and under any and all copyright + * rights in the Software, and any and all patent rights owned or freely + * licensable by each licensor hereunder covering either (i) the unmodified + * Software as contributed to or provided by such licensor, or (ii) the Larger + * Works (as defined below), to deal in both + * + * (a) the Software, and + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software (each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * The above copyright notice and either this complete permission notice or at + * a minimum a reference to the UPL must be included in all copies or + * substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +import { SessionState } from '@app/Shared/Services/Login.service'; +import { ServiceContext } from '@app/Shared/Services/Services'; +import * as React from 'react'; + +export const useLogin = () => { + const context = React.useContext(ServiceContext); + const [loggedIn, setLoggedIn] = React.useState(false); + + React.useEffect(() => { + const sub = context.login + .getSessionState() + .subscribe((sessionState) => setLoggedIn(sessionState === SessionState.USER_SESSION)); + + return () => sub.unsubscribe(); + }, [context.login, setLoggedIn]); + + return loggedIn; +}; diff --git a/src/test/Settings/Settings.test.tsx b/src/test/Settings/Settings.test.tsx index 656b68761..8f806f12f 100644 --- a/src/test/Settings/Settings.test.tsx +++ b/src/test/Settings/Settings.test.tsx @@ -51,6 +51,7 @@ import { renderWithServiceContextAndRouter, testT } from '../Common'; import { createMemoryHistory } from 'history'; import { Router } from 'react-router-dom'; import { UserSetting } from '@app/Settings/SettingsUtils'; +import { SessionState } from '@app/Shared/Services/Login.service'; jest.mock('@app/Settings/NotificationControl', () => ({ NotificationControl: { @@ -68,6 +69,7 @@ jest.mock('@app/Settings/AutomatedAnalysisConfig', () => ({ descConstruct: 'SETTINGS.AUTOMATED_ANALYSIS_CONFIG.DESCRIPTION', category: 'SETTINGS.CATEGORIES.DASHBOARD', content: () => Automated Analysis Config Component, + authenticated: true, } as UserSetting, })); @@ -160,6 +162,11 @@ jest.mock('@app/Settings/Theme', () => ({ })); jest.spyOn(defaultServices.settings, 'featureLevel').mockReturnValue(of(FeatureLevel.PRODUCTION)); +jest + .spyOn(defaultServices.login, 'getSessionState') + .mockReturnValueOnce(of(SessionState.NO_USER_SESSION)) // render correctly + .mockReturnValueOnce(of(SessionState.NO_USER_SESSION)) // should not show settings that require authentication when the user is logged out + .mockReturnValue(of(SessionState.USER_SESSION)); const history = createMemoryHistory({ initialEntries: ['/settings'] }); @@ -180,6 +187,22 @@ describe('', () => { expect(tree.toJSON()).toMatchSnapshot(); }); + it('should not show setting that requires authentication when the user is logged out', async () => { + const { user } = renderWithServiceContextAndRouter(); + + const dashboardTab = screen.getByRole('tab', { name: testT('SETTINGS.CATEGORIES.DASHBOARD') }); + expect(dashboardTab).toBeInTheDocument(); + expect(dashboardTab).toBeVisible(); + expect(dashboardTab.getAttribute('aria-selected')).toBe('false'); + + await user.click(dashboardTab); + + expect(dashboardTab.getAttribute('aria-selected')).toBe('true'); + + const dashboardSettings = screen.queryByText('Automated Analysis Config Component'); + expect(dashboardSettings).not.toBeInTheDocument(); + }); + // Currently, no tab is lower than PRODUCTION it.skip('should not show tabs with featureLevel lower than current', async () => { renderWithServiceContextAndRouter(); diff --git a/src/test/Settings/__snapshots__/Settings.test.tsx.snap b/src/test/Settings/__snapshots__/Settings.test.tsx.snap index 0c5040174..726e48c93 100644 --- a/src/test/Settings/__snapshots__/Settings.test.tsx.snap +++ b/src/test/Settings/__snapshots__/Settings.test.tsx.snap @@ -542,57 +542,6 @@ exports[` renders correctly 1`] = ` data-ouia-component-id="OUIA-Generated-Title-7" data-ouia-component-type="PF4/Title" data-ouia-safe={true} - > - Automated Analysis Recording Configuration - - - - - -
-
-
- - Set the recording configuration for automated analysis recordings. You may want smaller or larger values for max-age and max-size depending on how recent you want events to be recorded from the analysis. - -
-
-

- Automated Analysis Config Component -

-
- -
-
-

renders correctly 1`] = ` >

@@ -680,7 +629,7 @@ exports[` renders correctly 1`] = `

renders correctly 1`] = ` >

@@ -731,7 +680,7 @@ exports[` renders correctly 1`] = `