fix(settings): hide settings that requires auth for logged-out user (#…

…982)

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<AppLayoutProps> = ({ 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<AppLayoutProps> = ({ 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]);

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,
 };

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,19 +61,33 @@ 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';
 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<SettingsProps> = (_) => {
   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 */
-                <Trans i18nKey={c.descConstruct.key} children={c.descConstruct.parts} />
-              ),
-            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 */
+                  <Trans i18nKey={c.descConstruct.key} children={c.descConstruct.parts} />
+                ),
+              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();

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) => {

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<AppRoutesProps> = (_) => {
-  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 (
     <LastLocationProvider>

src/app/utils/useFeatureLevel.ts

@@ -45,7 +45,7 @@ export function useFeatureLevel() {
   const subRef = React.useRef<Subscription>();
   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]);

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;
+};

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: () => <Text>Automated Analysis Config Component</Text>,
+    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('<Settings/>', () => {
     expect(tree.toJSON()).toMatchSnapshot();
   });
 
+  it('should not show setting that requires authentication when the user is logged out', async () => {
+    const { user } = renderWithServiceContextAndRouter(<Settings />);
+
+    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(<Settings />);